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.
August 9th
After the “list” command, “create” was the next thing missing in the CLI app.
implement and extend steps for “create an issue” scenario
As before, the steps of the “create an issue” scenario had to be implemented before the scenario could point out missing features. The first thing was to implement the step “when we create an issue” which was missing completely. This step uses a text block as step data which can be extracted from the behave context.
Implementing the step definition posed a minor technical challenge: The title and description of an issue may well contain blanks, so the start_fix function could no longer just split the arguments by blanks but had to respect quoted strings as a single argument. Luckily, Python provides that functionality with shlex.split.
The second thing to do was an extension to the output expectations: I do not want to provide an exact hash for issue IDs in the feature files, as that would require knowledge of the hash function which is an implementation detail. I do not want to write regular expressions in the feature files either, so I used “[hash]” as a placeholder.
The pexpect library which I am using to check for the program output supports regular expressions. However, I currently use that feature only if there was a “[hash]” placeholder in the expected output. Otherwise, other expected outputs like the usage string that contains “[–help]” would be interpreted as a regex and break existing tests.
With more functionality, the expected outputs will probably become more complicated and the current naive implementation won’t hold any longer. However, for now, it is sufficient and I’ll cross that bridge when I come to it.
add command “create” to CLI app
While writing the test for the “create” command, I came across the same issue with splitting by blanks that I had in Python. Instead of digging into ranges again and trying to find a solution that respects quoted strings, I went for the easy solution: I refactored the run_app helper function to have an overload that takes the vector of arguments.
This first test calls “create” for a single issue and expects a matching output. The minimal implementation required by TDD may seem disappointing: the CLI app handles the command by exactly printing the matching output, and that’s it. However, the code now does handle the create command. In addition, the unit test uses a different title and expects a different output than the behave scenario. So, to make that pass, proper handling of the “create” command is not far now.
Planning the next changes
Extending the tests for the “create” command will lead to actual domain logic for the first time: The title will have to be parsed and a matching issue ID has to be created. In addition, the requirements call for checks on the length and content of the title and issue. It’s hard or at least tedious to check those in behave, but there should be unit tests that make sure the requirements are met.
All these things do not belong in the CLI app. Instead, there will be domain objects that contain the necessary domain logic. These may include a class for issues, but also things like the algorithm used to generate issue IDs. There will also be one or several application services that are responsible for taking the “raw” strings the CLI app extracts from the command line and translate them into domain logic calls.
The upcoming scenarios require some kind of persistence which will be provided to the application services in the form of repositories. For now, I have planned to implement the persistence with a single JSON file.
With those additions, instances of the major components of a “ports and adapters” architecture will be in place, as it is often referenced in Domain-Driven Design literature:
– The domain core, containing the domain logic
– An application service, orchestrating calls to the domain core and implementing and providing ports to the required functionality
– The CLI app that uses the port provided by the application service
– A JSON-based repository that implements the repository port required by the application service
August 13th
Today, I took the first step in the direction of DDD: I created an application service that provides access to the domain logic. Every call to the domain logic ba the CLI app will go through this or a similar service.
add application service
This commit adds an application service class in a new domain library. Pure DDD experts may not like this: strictly speaking, the application service layer should reside outside of the domain layer. For now, I’ll put application service and domain classes in the same library until there are enough classes to separate them – I’m no fan of having a separate library for every class.
The application service itself should not be much of a surprise: In the last diary entry, I wrote that the logic that will have to be implemented for the create command belongs in the core domain. Therefore, the application service gets a create method and pretty much the same test we had for creating an issue we’ve seen in the CLI app.
After all, I am in the process of factoring the existing functionality out of the CLI app, so the service needs to provide the same functionality for now. With that single test, the only functionality we test for is that a call to create returns the one string we test for. Don’t worry, I’ll get to actual logic in a minute.
You may have noticed the // NOLINT and similar comments: these are needed to avoid static analyzers like clang-tidy and cppcheck to complain about the create method not being static. It could be at the moment, but there will be a point where the method needs to store issues in a repository. This is a bit planning ahead, but I’d rather have those comments in the code for a few days than making the method static now and having to rewrite all call sites when it becomes non-static.
call application_service from cli app
Now that the application service provides the “create” functionality, it is time to call it. There is nothing much surprising happening: Instead of writing a hard-coded issue ID to the output, the CLI app now calls the application service to get the issue ID. The “Issue created:” part of the output is part of the user interface and stays in the CLI app. Since the create function does not rely on the input, empty strings are passed for now.
implement issue ID prefix algorithm
To satisfy the behave test, calling the “create” command with another title has to return the correct issue ID. Therefore the unit test is extended to check for that second call to create in the application service as well.
The test could be satisfied by using if/else in the implementation of the function, but that approach would lead nowhere. We need the actual algorithm at some point, and TDD is about writing as little code as necessary, not as dumb code as we can get away with, even though the first lines might have looked like that.
For the implementation of the id prefix algorithm, I first wanted to reuse the split algorithm from the CLI app unit test. However, it does not quite fit the bill, so after copying it over there were some modifications and it looks much different now.
I think that ranges here show their strengths in terms of expressiveness. Ignoring corner cases, for now, the [requirements]({% link _pages/milestones.md %}) say “an issue ID has a special format: four blocks of 2-3 alphanumeric characters each, and a block of 7 hexadecimal characters. Blocks are separated by hyphens. The issue ID is generated by abbreviating the first four words of the title…”. So, the algorithm for the prefix is:
- split the title into words:
title | rv::split(' ') - take the first four words:
| rv::take(4) - take (at most) 3 characters from each word:
word | rv::take(3) - put everything together, separated by hyphens:
| rv::join('-')
As I wrote, corner cases like less than 4 words or words with less than 2 characters are ignored for now. There will have to be more tests to get that behavior right. There’s only one thing in the code that is not in the algorithm: the three characters in each word are transformed to lower case. Why is that?
It turns out that the behave test requires this behavior: the provided title has an upper case character, the required prefix is all lower case. The person who wrote the test (me) did implicitly assume that the ID prefix should be lower case. I asked the product owner (i.e. myself) to clarify that requirement, and he asked me (the developer) how much trouble it would be to implement the lower case version. “No trouble at all, half a line of code”, I said, so the product owner added a note about the lower case issue ID to the requirements document.
Compiling and running the tests brought a slight surprise: The application service tests were green, but suddenly the CLI app test was red: since the application service now evaluates the title, an empty string does not quite cut it anymore. The minimal effort to make the test pass again is to pass the required title to the application service. That’s sufficient to make the unit tests pass, but not the behave test: I’ll have to actually parse the title from the command line and pass it on to the application service in the next step.