Holger's Blog

If you work on your branch you run into the situation that you would like to push your changes to the remote repository. CI will then pick up your changes and run the linting and code quality checks on it. Afterwards, you will see whether you improved the quality. But perhaps there are some new violations that crept into the code? Happens to all of us!

I usually like to see and check if any new issues might come up on CI. This lets me improve them before I push. The checks I run locally depend on the kind of work that I do. Lately that’s a lot of Ruby on Rails again — which is great. I love that framework and the language. To grade my code, I use Rubocop, Reek and Flay. If you run their respective commands on your repository, they will check the whole code base. This might be ok, if you didn’t have any issues before. Since I join teams, these days, that work on legacy projects it is rare that there are no problems with the code. If I run the commands just so, I will get a long list and couldn’t possibly see the issues that I introduced through my changes. Lucikly, there is Git and some “command line foo” that can help us here:

git fetch && git diff-tree -r --no-commit-id --name-only master@\{u\} head | xargs ls -1 2>/dev/null | grep '\.rb$' | xargs rubocop

This command will fetch the current state from the remote and diff your branch/changes to master branch. It then runs rubocop on these changes. In my ~/.aliases.local file I added three lines for all three linters.

# Code Quality
alias rubocop-changes="git fetch && git diff-tree -r --no-commit-id --name-only master@\{u\} head | xargs ls -1 2>/dev/null | grep '\.rb$' | xargs rubocop"
alias reek-changes="git fetch && git diff-tree -r --no-commit-id --name-only master@\{u\} head | xargs ls -1 2>/dev/null | grep '\.rb$' | xargs reek"
alias flay-changes="git fetch && git diff-tree -r --no-commit-id --name-only master@\{u\} head | xargs ls -1 2>/dev/null | grep '\.rb$' | xargs flay"

I am still working on a way to just call one command and have all thee commands run. That doesn’t yet work. Probably because of exit-code reasons, when one linter finds issues.

These simple commands offer a convenient way to find local issues and correct them before pushing to CI.

A few weeks ago I wrote about deleting your tests. Yesterday I received the weekly email from Avdi Grimm, where he touches on this subject.

Some premises about my relationship with unit testing:

I like test-driven development. I like driving out individual object design small, isolated (e.g. no database) unit tests. I think of these unit tests as a design aid, full stop. Any help they provide with preventing regressions is gravy. I treat unit tests as disposable. Once they have served their purpose as design aids, I will only keep them around so long as they aren’t getting in the way. These four premises are strongly interconnected. Take away #1 (test first), and tests are no longer a design aid and none of the other points are valid. Take away #2 (isolation) and we get into integration test territory where there’s a much higher possibility of tests having regression-prevention value. Take away #3 (design focus) and the other points lose their justification. Take away #4 (disposability) and I spend all my time updating tests broken code changes.

This makes it easy for me to find myself at cross purposes with others in discussions about unit testing, because often they come into the conversation not sharing my premises. For instance, if your focus in unit testing is on preventing regressions, you might well decide isolated unit tests are more trouble than they are worth. I can’t really argue with that.

A recent conversation on this topic inspired the thought from the driving-design perspective, maybe unit tests are really just a crutch for languages that don’t have a sufficiently strong REPL experience. While I don’t think this is strictly true, I think the perspective shines a useful light on what we’re really trying to accomplish with unit tests. I almost think that what I really want out of unit tests is the ability to fiddle with live objects in a REPL until they do what I want, and then save that REPL session directly to a test for convenience in flagging API changes.

That conversation also spawned the idea of immutable unit tests: unit tests that can only be deleted, never updated. A bit like TCR. I wonder if this would place some helpful incentives on the test-writing process.

You should subscribe to his newsletter, if you haven’t yet.

Yesterday I closed with this idea:

Spot places where knowledge about something does not belong.

What do I mean by that? Sometimes I come across some code that does not read right. I will use a pseudo code example to illustrate:

class Foo
  def initialize(bar_service)
    @bar_service = bar_service
  end

  def quux
    if @bar_service.greeting == "hello"
      @bar_service.greet("goodbye")
    else
      @bar_service.greet("hello")
    end
  end
end

class BarService
  attr_accessor :greeting
  def greet(message)
    @greeting = message
  end
end

What bothers me with this code? The method quux has too much knowledge about how the @bar_service works. Foo.quux knows that the @bar_service has an instance variable called greeting and at least one specific value it might have ("hello"). It also knows two values that the greet() method might be called with. Now it happens that this knowledge about how the greeting and greet work, is also spread into other parts of the application. What happens if you need to change something about the greet() method? You have to find every place in your application and update it to reflect the new changes. This isn’t good.

There are places like this inside many applications. You might need some practice to spot them, but with some practice it becomes easier. For this example I would like to suggest to move all knowledge about how greeting and greet work inside the BarService. Start with the conditional, like this:

class Foo
  def initialize(bar_service)
    @bar_service = bar_service
  end

  def quux
    @bar_service.greet
  end
end

class BarService
  attr_accessor :greeting
  def greet
    if @greeting == "hello"
      @greeting = "goodbye"
    else
      @greeting = "hello"
    end
  end
end

Now we are free to change the internals of the greet method. We could add a third option or change it completely. The class Foo does not need to change at all. It continues to call greet as if nothing has happened.

One of my overarching topics is testing. A refactoring like this should be covered with tests. Not only do you need tests for the Foo class, but also for how BarService.greet works. And for every part of the app that interacts with either.

Tomorrow we’ll look at another way to do a refactoring.

Yesterday, we had the first part of this series on quick wins and simple steps to improve your code quality. It was about naming — specifically variables and method names.

Two things were not 100% right in these examples.

The first error

You might have noticed that my loop examples were written in Ruby code. Yet the method name doSomething was written in camelCase. This is unusual for Ruby code where developers tend to use snake_case for method names. I did not lint that email. Hence no robot told me about my error. I believe it is a good example of the benefit of automatic linting. This error would have been found. If you read the code and were put off by this naming scheme, you even experienced why conventions and rules are necessary: Because coding by the rules helps developers to focus on the semantics of the code, not the syntax.

The second problem

Do you remember that I wrote about JavaScript loops and gave the classic for loop example? I complained about the i variable and that it should be called iterator. Perhaps you did not like this idea and my example? Let me take a step back for a second. When naming variables and method names, you have to make sure they “speak.” The names should indicate their meaning and make it easier for another developer to understand the semantics of your code. Yet, when you are fully aware of the problem domain you code deals with, it can happen that you try to be too specific. If you are, you tend to use long, verbose names for variables and methods. An example could be this:

A common value for the allowed length of method names is 30 chars. The above example could be broken into two methods.

The next possible quick win might be to think about the existence of these methods inside the Article class. Verifying and fixing meta tags surely does not need to be done inside this class. If you want to follow the Single Responsibility Principle, you should make sure that the Article class does not have a reason to change, if you decide to change something about verifying and fixing meta tags. Rather the Article class might change when you decide that an article should have a mandatory sub-headline.

Back to the iterator. This name could pass the test for too specific. Only, when i refers to a variable that is declared and initialized outside of the scope of the loop, does it make sense to choose a different name. Or doesn’t it? As always it depends. The classic for loop is taught in almost every book on JavaScript, and it is easily identifiable as to what it is. But there might be reasons to deviate from that, as I indicated above.

Conclusion

To summarize:

1. Choose good names. 😎
2. Spot places where knowledge about something does not belong.

Tomorrow we’ll look at another example for 2 and a practical idea of what to do about it.

My work as a consultant offers me the opportunity to accompany a team for a certain amount of time. I join them, we work together, and then I leave again. Our time together gets extended, sometimes. This model has the benefit that I get to know a lot of people and teams — and how they work.

Do you know the Pareto principle? It’s also called the power law distribution or the 80/20 rule. A simple explanation would be that 20 percent of the people own 80 percent of the wealth of the whole world, which makes it relatable and understandable. Only that it’s wrong. By now 10% hold 90% of the wealth already. And it’s getting worse. I don’t have any sources on this right now, and I won’t go looking. Because it was only meant as an image of how this works.

Back to my clients…

There is a similar distribution of 80/20 to be found. 80% of software development teams do the same mistakes over and over again. It starts with a new green-field project. A year of development work passes. A lot of code was written. And after a year the team is frustrated with their software again and doesn’t find a way out. This is bad practice.

If you find yourself in these situations, there are ways out of it. With a lot of intrinsic motivation and the ability to learn from mistakes and external sources, you could be able to drag the ship around and sail into the sunset, happily. But there are a lot of rocks under the water that might wreck your boat. An experienced navigator for these waters could prove beneficial.

In my opinion, a good start is to look to industry standards and follow those as well as common best-practices. Find and learn the rules on how other teams work. They might seem strange; you might not understand or like them. But let me tell you something: They way you worked up until here didn’t work and brought you into this mess. Doing things as you’ve always done them won’t help you a bit.

So, don’t be smart. Find rules. Follow the rules. Stick to them and don’t deviate. Re-evaluate in 6 months. It will hurt. It might not be fun. But it will get better.

If you need a pointer, let me know by replying.

Before I get to today’s topic, I would like to say thank you, to you. My little poem yesterday seemed to resonate with you. At first, I planned to write about it and its meaning today. But your responses indicated that it spoke to you. And I wouldn’t want to ruin this with my ramblings about it. So I’ll just finish with: I enjoyed this very much.

Lately, I spent some more time on Twitter. I don’t know how to use Twitter well (enough). I always have trouble with creating threads or topics. But I (re)discovered some very interesting people, sharing their ideas in long threads.

A few days ago I came across @GeePaw Hill. I believe it was because I followed a few tweets by Kent Beck. GeePaw Hill had this thread where he encouraged people to refactor without caring for the application domain, only for the code. You can read the thread here. He even elaborated some more on his blog.

I find the idea fascinating and will continue to think about that.

Back then; I did it; I liked it much; Found it a necessary touch;

Never it challenged, I; then saw; A source without it; didn’t look too raw;

Since then it flows without it well Some purists call it living hell.