Contents
In my dev diary blog post series, I document the minutiae of what I am doing for my toy project Fix. The diary can also be found in smaller bites on the GitHub pages of the project. Many of the subheadings link to the commits in question. Since there did not happen much in these last two weeks, I collect the diary entries for both weeks
into this single post.
August 29th
Only a few minor fixes
fix minor review findings in title
After writing the previous diary entry on Sunday, I only fixed the two findings I had while reviewing the code. The first one is small: it gives the lambda that does the actual creation of the title object a name. It’s not as much of an improvement as I’d have liked, but it’s the best I could come up with. The alternatives did not quite cut it:
- We can not take the address of a constructor and pass it as a callable, so there has to be some indirection if I want to fully use the chain call feature (i.e.
mapin this case) - A normal function, e.g. in an anonymous namespace, does not work, since it would have to use a private constructor. Only member functions can do that.
- I could have made
make_titlea static member function, but that would only be more code and unnecessarily add that detail to the header, which feels even more awkward than the named lambda. I wish modules would already work out of the box with CMake and everything. - Not using
maphere would not make the code much more readable.
The second finding was that trimming the text and comparing lengths to check whether it has been trimmed was overkill. Checking whether the first or last character is whitespace is more readable as it directly conveys the meaning of what “not trimmed” means.
The added check for an empty text is necessary here in any case. Yes, I could have moved the check_trimmed call after the check_length call, and then it would exit early on empty strings, but making the implementation undefined behavior or not depending on the call order is a bad idea. There’s no need to cut corners here.
September 3rd
Compared to the title, the description class is really simple
add description class
There’s not much going on with the description class: it is not more than a wrapper around a std::string without any checks. Contrary to the stated requirements, it uses a narrow byte string (aka std::string) instead of a UTF-8 string I would use for Unicode. The reason is that I have not yet read up enough on how to deal with Unicode to confidently make decisions about the proper types and how to handle Unicode text.
I’ll probably switch to UTF-8 (aka. std::u8string) when I have done my homework. I’d really like to hear your thoughts about what’s the right choice here.
The development of the class has been pretty much by the book in terms of TDD. You could add the content of the test file from top to bottom line by line and would probably end up with essentially the same code.
September 8th
Start of the issue IDs, namely a new place for the generation algorithm
add issue_id class with generation algorithm
After the description class, the Issue ID was the next piece missing in the puzzle. As usual, I started with the unit test. For now, they only encompass tests for the construction and the existing algorithm for the generator function.
The algorithm is the one I previously had implemented in the application service. It does not belong there as application services should not contain proper domain logic, so the generator function is the better place for it.
Why did I call the function generate and not create like in the title class? Well, I use create merely as a replacement for constructors that can fail, since I opted for expect error reporting instead of exceptions. The generate function on the other hand does not simply create an object that contains the provided data as in the case of the title class. It uses its arguments and some domain logic to determine the actual data needed to create the issue ID.
One bit that implementing the class has revealed was that the application service did not account for the possibility of invalid input when creating an issue. application_service::create simply returns a string and I had not yet planned what would happen if the provided title was not valid.
For that reason, there still is a TODO in the function. I yet have to decide which error reporting mechanism to use in the interface of the application service. One choice could be to forward the expected error codes, another to translate them to the error reporting mechanism of choice of the CLI application, namely exceptions.
That’s it for these last two weeks. Did you spot mistakes I made? Would you have done things differently? I’d love to know what you think, please leave your comments and questions below.