Loading collection data...
Collections are a way for you to organize kata so that you can create your own training routines. Every collection you create is public and automatically sharable with other warriors. After you have added a few kata to a collection you and others can train on the kata contained within the collection.
Get started now by creating a new collection.
There's already an abundance of katas asking you to do a mapping between possible inputs and outputs.
What does
fromAustrianToLithuanian
even have to do with Austrian grades? It haspercent
as its input."—" is a dash, it can't be used before a number as in "—1". And in the Wikipedia article the uppercase "1" is a reference to a note, so facepalm.
I like the aim of this kata and I also like requirements that might occur in real life. Nice. But there are things I don't like about this kata:
Person
has two responsibilities: (1) manage a person's data and maintain the invariants and (2) read a person's data from a line of text and also be able to construct that line.When the storage of the person changes, or you want to add another storage type or you want to create a Person object without a file you would either have to modify the class or build a converter that builds that string to pass it to the constructor of
Person
.In a code review I'd strongly suggest that
Person
should only deal with one responsibility: manage a person's data and maintain the invariants.The other aspect could be handled by a different class or functions, say
personFromString(const std::string &)
andtoString(const Person& p)
.Person::print()
andPerson::writeString()
sound like they print or write a representation of that object tostd::cout
or to some file, but instead they create and return a string representation.Also, it looks like
Person::writeString()
only exists to be called byWorker::writeSplitedItems()
. NowPerson
andWorker
have the shared responsibility of writing gender-split files.Worker::writeAllItems()
andWorker::showItems()
sound different but they do nearly the same, print a representation of all persons, the first to a file, the latter tostd::cout
. I'd suggest renaming.Worker::showItems()
is the only function not concerned with reading or writing a file. Should it really be in there?Worker::writeSplitedItems()
uses a different format thanPerson::print()
and, in extension,Worker::readItems()
andWorker::writeAllItems
. IMHO that should either be made mor clear in the function name or that function should not be grouped with the others.Also, shouldn't
Worker::writeSplitedItems()
be calledWorker::writeSplitItems()
? (I'm not a native English speaker, but see Wiktionary)In conclusion: Your kata aims to train static functions, reading/writing files, and writing data classes. But it does not separate its concerns and the resulting code is therefore harder to read and maintain than it should be. And you should be more consistent with your names, e.g. use
print
forstd::cout
related things,write
for file related things.Given what the intended solution is this isn't a interesting kata by any stretch. Much more of "another attempt at shitposting a bugfix kata".
Please just unpublish this. It's not fun to anyone out there, and it teaches nothing.