3

Currently when working in a scrum team in a software service company, who has a clean business domain. I am working on adding new features to an existing module. As the existing module is legacy, and new features need to be integrated to the system all the times, I sometimes feel the need to refactor on the legacy code and also on the new code. For instance, sometimes I would like to just rename a Java class so that it is more close to the business domain.

However, my manager seems not like this at all and get fed up with even my micro-refactor such as renaming one class name or a method name. She tries to persuade me not to do that politely, but I indeed have difficulty to tolerate this as what she suggested are totally against what I have learned from Clean Code and the Domain Driven Development. On the other hand, I understand her worry in that the company's code review process recommends coders to do separate pull requests on actual code change and refactor, but to get separate pull requests merged will take extra time according to the company's code review process

Question:

  1. Should I really follow my manager's command?
  2. Is the company's pull request recommendation proper?

IMHO, if I follow her suggestion, the code base will gradually become the rolling big ball of mud

11
  • How much coding experience do you have? – Robert Harvey Oct 8 '20 at 22:21
  • @RobertHarvey since 2007 till now. But just got to learn clean code practice and domain-driven development in recent years, after which time I started to refactor more and more on existing code – Rui Oct 8 '20 at 22:22
  • How good was your code before you learned "clean code" and DDD? – Robert Harvey Oct 8 '20 at 22:25
  • before learning the clean code and DDD, my code had lots of problems, mainly not maintainable – Rui Oct 8 '20 at 22:29
  • 1
    @Rui: That's a very broad question. On the one hand, I'd say that seniority is not something that you can actually "lose", and it's normal that a senior developer does not get excited too quickly about the latest fad, because he or she has seen many of them come and go. On the other hand, this quality can become a liability if one absolutely refuses to accept all technological advancements. You should also know that "clean code" is not a new thing (regardless of what the homonymous book claims) but a decade-old field of debate. Chances are you aren't proposing anything new to your colleagues. – Christian Hackl Oct 9 '20 at 14:47
7

There is a danger, when picking up a new skill from a book, blog or video, to go "oh, a shiny new thing!" and use it everywhere.

Resist that temptation.

The problem with bringing new tools like DDD and clean code to an existing shop is that the shop already has a culture and history. You need to be sensitive to that. Name changes are especially problematic; the team you're on is already familiar with the names, and they've gotten used to finding things quickly by remembering those names.

Every change I make to a code base is carefully thought out. Is what I'm about to change going to be a net gain, or a net loss? For some things, the cost might outweigh the gain.

There are many ways to write good code. "Clean code" and "TDD" are just two of them. If you're finding yourself trying to bend the will of an existing code base to the philosophies of Uncle Bob and Eric Evans, first ask yourself: "Is the code already adequate, from an organizational standpoint" and "are the benefits worth the costs?"

Further Reading
Are bad programming practices typical within the software industry?

5
  • Thanks so much! Your answer indeed helps me a lot (Y) – Rui Oct 8 '20 at 23:02
  • One question in my mind again today is that is the culture and history you mentioned really proper in my company? The culture limits developers' innovation. So I am thinking should I work in such environment at all – Rui Oct 9 '20 at 8:42
  • Define "proper." – Robert Harvey Oct 9 '20 at 12:19
  • "proper" should be on base of the company's cost from the aspect of business. If the current culture really resuls in low cost, it should be fine. But seems that the company did not cut cost instead the testing team is becoming bigger and bigger. To me I still like to practice the DDD and clean code whenever I could and I am welcomed to get any negative feedback. Negative feedback helps improve, we learn from mistakes. If I never would put DDD or clean code into practice, I am never able to use appropriately – Rui Oct 10 '20 at 20:11
  • Yes. So that is the tradeoff the company decided to make. It chose more testers over "cleaner code." Unfortunately they may be too invested in that decision to change their mind now. – Robert Harvey Oct 10 '20 at 20:22
4

Symmathesy Its worth a read, and even finding a presentation on it by her.

In short the code base is not 100% checked into source control. It also exists in the heads of your fellow developers/business analysts/testors/architects. It is a lot harder to update these parts of the code base.

It is great that you have updated your own internal programming library. Before you apply this to the code though, you will need to update the libraries of your collaborators.

Imagine this scenario:

Frank is a new team and comes from a Functional background (if your functional choose a foreign sounding background). They take a look at your code base and immediately start to apply the rules of DDD and clean code. When you look at the code they have applied monads, functional decomposition, currying and a whole host of strange sounding patterns.

You as an experienced dev have been asked to implement shiny new feature on top of this code, and urgently too. Can you? Probably not, not because you lack experience, but because you have now been asked to read something indistinguishable from ancient greek.

This is essentially what you are forcing onto your colleagues. Your boss is right to ask you not to do this, they have to maintain a team capable of updating this code. They also need to be able to bring on new hands quickly enough to be useful when people move on.

Now if you can improve the code quality, then yes you should improve it. First though you need to be clear about what would improve code quality. Second you need to ensure that the team can still work with the improved code.

At my work we share books on these principles and discuss them. We then pilot a small project with the principles and compare them to what we were doing a year ago. Sometimes we move forward, sometimes we go back. We actively discuss what and how to remodel sections of the code base, and follow up with sessions about what was changed.

1
  • 1
    Enforcing peer reviews for every change can help with that - at least one other team member has to understand the change and agree with it. Might still lead to problems if others are opposed to it, but better than nothing. – Hulk Oct 9 '20 at 9:57
4

One of the most fascinating things I learned when I read Martin Fowler's Refactoring book was how many types of refactors come in pairs, one that is the exact reverse of the other. There aren't any refactors that are universally beneficial. It always depends on the circumstances, and often other developers are aware of circumstances you aren't.

Sometimes the merits of a refactor are not immediately obvious, and you need to make your case. That case shouldn't be something like "DDD says" unless the reviewers are very familiar with DDD. It should be something like, "The old name makes it difficult to tell this concept apart from this other one. This new name is closer to how our customers would refer to this concept in their requirements. This will make it easier for me to make x change I'm planning in a subsequent pull request."

In my experience, most developers with concerns like yours are not making bad changes, they just aren't good at selling them.

Putting it in a separate pull request also has a good reason. It's not just for the sake of bureaucracy. It's easier to review 10 pull requests than one big pull request that should have been 10. If I see a pull request that renames a class, and the author has made a good case for it, I can quickly glance through the changes and see "yep, this is just that rename," and hit approve.

Alternately, if you've stated your reasons well and I'm aware of circumstances you aren't, it's easier for me to correct you. "While your term is more popular broadly, in our particular industry the existing term is more common, for x historical reason." And I can reject the pull request without rejecting other changes I don't disagree with, or feeling like I'm going to make you feel bad.

If that rename is mixed with other changes, I have to painstakingly, line by line, determine if this line changed due to the rename or to something else, or both. If I have objections about the refactor, I may not want to look petty picking on a rename when there are meatier issues to consider, so I have to balance my review. That extra mental effort makes me annoyed about the rename, even if I would have supported it in a separate pull request. It's irrational, but that's how human brains work, and that's one reason why we make the rule about refactors in separate pull requests. It also feels more important if someone took the time to single it out.

In other words, there are not just technical factors to consider, there are human factors to consider, and you will be more successful if you find ways to help other developers be happy about your changes.

2

What the books you mentioned don't tell you is how to do get the team on-board with such refactorings.

If you are going to suggest a class or method renaming in a team, the only way of doing it properly is to ask at least one of the other senior devs what they think about it, and get a second opinion.

If they are fine with it, tell your manager that you all agree upon this change and make sure the renaming is properly communicated to all people who might have already worked with that part of the code base.

4
  • Asking senior devs probably would not help. Seniors in the company seems not recommend refactor. They are not interested in domain-driven design, where refactor plays an important role. One of the seniors in our team told he/she never have heard about DDD – Rui Oct 9 '20 at 7:22
  • 1
    @Rui: Refactoring has nothing to do with DDD (and as I said, those books don't tell you how to communicate in a team). Cleaning up existing code to keep it evolvable is not a new idea, it is way older than the term "refactoring" itself, I would expect every professional senior at least consider it from time to time. If your "seniors" don't understand the value of such clean-ups and reject every clean-up by principle, I would recommend to polish your CV and start looking for other opportunities. – Doc Brown Oct 9 '20 at 11:41
  • Thanks a lot for your point of "those books don't tell you how to communicate in a team" :) This is too true, and exactly in my case. I confess that I might not be a good communicator and focus too much on technical stuff :D – Rui Oct 9 '20 at 11:47
  • Today found this post by chance: softwareengineering.stackexchange.com/questions/393895/… I am almost on the same boat with this poster. I actually decided to change the company. I indeed found lots of senior developers in the company don't know what is DDD, they don't practice Clean Code with refactor, either. Most of their code is indeed not readable and difficult to maintain – Rui Oct 10 '20 at 19:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.