Don’t stop writing code comments

TL;DR: The older I get, the more diverse projects I have seen, the more often I refrain from giving someone an absolute advice. In almost any cases, there are many aspects to consider. In times when people hardly read documentation and often only headlines, an advice like “Stop Writing Code Comments” guarantees some clicks, but isn’t absolutely for the good nor appropriate in all scenarios.

At the beginning of this week, @java tweeted this:

Pointing to this blog post Stop Writing Code Comments. Brian seems to be a big apologist of the clean code movement. I can understand this to some extends, but moved away from “keep methods as small as possible” and similar quiet some extend. Sometimes it’s just necessary and as long as the logic in a bigger method stays testable, I don’t see an issue. And apart from that, what’s dirty code anyway?

What and how to comment

But I want to address some of the methods in the Medium post above. The name of the method is indeed good. But imagine the following:

/**
 * Finds all the employees with a given status. The method will return all employees with a given status, regardless
 * when they joined the company. The returned list will be immutable, changes to the employee objects will not be tracked.
 * 
 * @param status The employment status of the employees to find. Must not be null.
 * @return A list of employees with the given status or an empty list of no employee has the given status
 * @throws IllegalArgumentException when the status is literal null
 */
List<Employee> getEmployeesByStatus(Status status) {
  ...
}

As a user of that method I know now exactly what to pass to the method and what I can expect to return. I have an idea that the list may be empty, but is never null.

The employee class is badly named, that’s nothing to fix with a comment indeed. But chose a better naming and add some meaningful comment and things look bright:

/**
 * A person that is currently employed at the company. Instances of this class are not attached to the database
 * and represents a current employee purley from the business domain and can be savey passed around services.
 *
 * For other types of employees, see {@link RitiredEmployee} or {@link FiredEmployee}
 */
public class CurrentEmployee {
}

I do actually like this part:

public void sendPromotionEmailToUsers() {
  calculatePrices();
  compareCalculatedPricesWithSalesPromotions();
  checkIfCalculatedPricesAreValid();
  sendPromotionEmail();
}

There’s nothing much to add here. But let’s focus on sendPromotionEmail. It’s intentions are pretty clear, but I think it can be improved:

/**
 * Sends an email to all users eligible for a promotion. It does not include any retry mechanism, an e-mail is considered
 * sent when the configured SMTP server acknowledged the e-mail
 * An exception is thrown when the SMTP server is not reachable.
 *
 * When no users are eligible for promotion, no emails are sent
 * @see #getEmployeesByStatus
 * @throws RuntimeException when the SMTP Server could not be contacted
 */
void sendPromotionEmail() {
}

By coincidence I found this paper on The Use Of Sub-Routines In Programmes by David Wheeler. My friend Lukas quoted it in his talk 10 Reasons Why we Love Some APIS and Why we Hate Some Others. The paper – being from 1952 – summarises things neatly:

[…] sub-routines are very useful – although not absolutely necessary – and that the prime objectives to be born in mind when constructing them are simplicity of use, correctness of codes and accuracy of description. […]

Neither small and grokable methods nor comments alone help to create an understandable software system. One needs both.

On the obsolesce of comments

Brian speaks a lot about comments that becomes invalid and outdated. In my opinion, comments just needs to be treated as deliverable artefact as well and needs to be refactored with the code being documented. It’s easy as that.

Useless comments

I wholeheartedly agree with large chunks of commented code: Remove it, please! That’s what a version control system is for.

Regarding TODO comments: I use them occasionally to remind me of things I need to come back to in the future, which would block me from more important things at the very moment.
However, one alternative solution to that is to write failing tests: Those reminds you much better of things to fix.

Title photo by Marc Schäfer on Unsplash

Filed in English posts | Comments (2) »

04-Jun-19

Separation of concerns in Spring applications

TL;DR: Don’t surprise your fellow developers.

Yesterday I learned – or realized again, don’t know – that one can add @CompenenScan to all Spring components and not only to @Configuration components:

It’s also ok to annotated methods of standard components with @Bean and turn them into weird factory components (Those being processed in “light” though, as Marten reminded us.)

I see several issues here, the dilution of concern’s being my primary. I can get a quick overview of all services of an application with my IDE looking for @Service or whatever. The same with configuration. That helps me a lot not to have to think about things.

I spent a considerable amount of time yesterday helping a friend debugging a weird @SpringBootTest. He wanted to exclude some services from being started and only test some. The first impression was: He tried this by specifying them on the classes attribute of @SpringBootTest. That’s not what that attribute does. That attribute is used to point the test context to configuration classes.

To include or exclude components from being started, one uses @ContextConfiguration, also from the test package. It has include and exclude.

So we added this and removed the classes from @SpringBootTest attribute. And: Nothing worked anymore. Wtf? After some digging it turned out, that those services mentioned on that attribute – while clearly named and annotated service – also configured more component scanning and the module did not have a @SpringBootApplication, so the test context couldn’t figure out about the components without being explicitly told to.

Given the fact that the project in question was just a bit older than a year, I think it’s already legacy and not in a good sense: A new developer joining that team would be have a super hard time figuring out what’s happening and why? Things are being named in one way and do something else or something in addition.

Go figure what happens next: It’s always Springs fault, of course.

Well, this time I’d say it’s quite dangerous indeed that all components contribute to the application context and not only @Configuration classes.

Imagine a library, containing some cool services. When you add this library via a custom starter, you would be hopefully aware, that the starter can add many things to your context. You could however be explicit and write down your own @Configuration and return new AwesomeLibraryService() from within a @Bean method. The AwesomeLibraryService could be annotated with an arbitrary @ComponentScan and scan the kitchen sink, the JDK and all the rest, including a blockchain mining library. Probably not what you want.

Anyway, we all are responsible developers, everyone does DDD and writes nice, clean micro services.

My impression of real world projects, especially in big cooperates, is a another. I always stumble upon the same issues:

  • Issues with dependencies
  • Incompatibile versions
  • Cargo cult: A configuration hacked together from various sources is passed on all the time

People are gonna reuse the stuff the find somehow working in projects, whether it’s good or bad.

It’s a seniors developers responsibility to make sure that things have well defined concerns. I am sure that it is impossible to prevent cargo cult, but one can make it a bit safe.

Title photo by Franck V. on Unsplash.

Filed in English posts | Comments (1) »

17-May-19

GraalVM on macOS

I have been doing some research with GraalVM and I noticed that RC14 brings an Info.plist inside it’s contents folder. That’s a nice thing to have, as it automatically registers it if you unzip it into /Library/Java/JavaVirtualMachines as a JDK 8 implementation, but a nasty surprise when using /usr/libexec/java_home -v1.8 to configure your JAVA_HOME, which now point to the GraalVM. Probably it’s not much of a problem, but I stumbled upon it as one of my favourite tools, jQAssistant suddenly stopped working.

The fix is easy and I described it already for JDK 9 ea years ago:

Just delete or rename the info.plist file:

export GRAALVM_HOME=/Library/Java/JavaVirtualMachines/graalvm-ce-1.0.0-rc14/Contents/Home
sudo mv $GRAALVM_HOME/../Info.plist $GRAALVM_HOME/../Info.plist.bak

This way you can have a standard JDK 8 having around that is selected via a standard java_home call and explicitly using GraalVM.

Filed in Apple, Java | Comments (0) »

27-Mar-19

Using Thymeleaf inline JavaScript with D3.js

I gave this talk several times now and every time I got asked how I created this visualization, clustering the genres I’m listening to by decade and country:

I just did a super spontaneous video for that. Key take away: Just use server side rendered HTML through Thymeleaf, obviously running on Spring Boot, render the content model as inlined Javascript and be good to go. Want an API later on?

Structure your Spring MVC controller and your service or repository layer in a way that makes it actually reusable.

Anyway, here’s the short clip and below the relevant code snippets:

Spring MVC controller, GenresController.java, either returning a String, denoting the template to be rendered, or the Model as just the response body:

@GetMapping(value = { "", "/" }, produces = MediaType.TEXT_HTML_VALUE)
public ModelAndView genres() {
 
	var model = genresModel();
	return new ModelAndView("genres", model);
}
 
@GetMapping(value = { "", "/" }, produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
@ResponseBody
public Map<String, Object> genresModel() {
 
	var genres = this.genreRepository.findAll(Sort.by("name").ascending());
	var allSubgrenes = genreRepository.findAllSubgrenes();
	var top10Subgenres = allSubgrenes.stream().sorted(comparingLong(Subgenre::getFrequency).reversed()).limit(10).collect(toList());
	return Map.of(
			"genres", genres,
			"subgenres", allSubgrenes,
			"top10Subgenres", top10Subgenres
	);
}

The Thymeleaf template genres.html. The standard controller above renders it when / is requested, accepting text/html.

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:layout="http://www.ultraq.net.nz/thymeleaf/layout"
      xmlns:th="http://www.thymeleaf.org"
      layout:decorate="~{layout}">
<head>
    <title th:text="#{genres}">Placeholder</title>
    <script th:inline="javascript">
        var subgenres = /*[[${subgenres}]]*/ {};
    </script>
</head>
<body>
<!-- Some more content -->
</body>
</html>

Calling this thing application.js is way too much. It’s a simple function, reading in the global subgenres variable and displays it as a nice D3.js chart.

Remember: This is also no silver bullet and I absolutely don’t have anything agains todays Javascript development. I just feel more at home near a database.

Filed in Uncategorized | Comments (11) »

31-Jan-19

Documentation as code, code as documentation

This morning, I read this kind tweet by Raumzeitfalle:

This refers to arc42 by example. There will be a second edition of this book soon, with new contributions from Gernot and new, Ralf.

My example in this book, biking2 is still up to date. I used this now several times at customers and every time, I also recommended my approach of generating the documentation as part of the build process. You see the live output here.

What makes those special are the following facts:

  • The docs are written in Asciidoctor, see: src/docs.
  • I’m using the Asciidoctor Maven plugin. That means they are an essential part of the build and aren’t left to die in a Confluence or whatever.
  • Even more, I use maven-resources-plugin to copy them into the output dir from which they are put into the final artifact.
  • While the documentation is strictly structured by the ideas of arc42, here come’s the fun parts:

    • I’m using jQAssistant to continuously verify my personal quality goals with that project: jQAssistant is a QA tool, which allows the definition and validation of project specific rules on a structural level. It is built upon the graph database Neo4j.
      jQAssistant integrates also in your build process. It first analyzes your sources and dependencies and writes every information it can discover through various plugins into an embedded Neo4j instance. That is the scan step. In the following analyze step, it verifies the data agains build-in or custom rules. You write custom rules as Cypher queries. Now the interesting fact: Those concept and rules can be written in Asciidoctor as well. This one, concepts_structure.adoc, declares my concept of a config and support package. Those concepts are executed in Neo4j and add labels to certain nodes. The label is then used in this rule (structure.adoc):

      MATCH (a:Main:Artifact)
MATCH (a) -[:CONTAINS]-> (p1:Package) -[:DEPENDS_ON]-> (p2:Package) <-[:CONTAINS]- (a)
WHERE not p1:Config
  and not (p1) -[:CONTAINS]-> (p2)
  and not p2:Support
  and not p1.fqn = 'ac.simons.biking2.summary'
RETURN p1, p2

      “Give me all the packages of the main artifact but the config and the summary package that depend on other packages that are not contained in the package itself.” If that query returns an entry, the rule is violated. I use that rule to enforce horizontal, domain specific slices.

      Now, the very same, executable rule becomes part of my documentation (see building block view) by just including them. Isn’t that great?

    • I also include specific information from my class files in the docs. Asciidoctor allows including dedicated part of all text files. For example, I use package-info.java files like this directly in the docs: “bikes”. I did this to a much bigger extend in this repo, find the linked articles there. I love Asciidoctor.
    • Last but not least, I use Spring REST docs in my unit tests. Spring REST docs combines hand-written documentation written with Asciidoctor and auto-generated snippets produced with Spring MVC Test. A simple unit test like this gets instrumented by a call to document like show in this block. This describes expectations about parameters and returned values. If they aren’t there in the request or response, the test fails. So one cannot change the api without adapting documentation. You might have guess: The generated Asciidoctor snippet as included again in the final docs, you find it here.

I started working on the documentation in early 2016, after a great workshop with Peter Hruschka and Gernot Starke. This is now 2 years ago and the tooling just got better and better.

Whether you are writing a monolithic application like my example, micro services or desktop applications. There’s no reason not to document your application.

Here are some more interesting projects, that have similar goals:

Also, as an added bonus, there’s DocGist, that creates rendered Asciidoctor files for you to share. Thanks Michael for the tip.

Filed in English posts | Comments (5) »

05-Dec-18