6

We have a big messy project on Java, a family of desktop Swing applications. It's 20 years old

If you believe you've seen the worst of it, you haven't. This is (some of) what happens when you have no code review for two decades

  • souts instead of regular logging;
  • mains in domain classes instead of tests;
  • finalize() that close resources;
  • variables named like _abcDef;
  • million of obscure flags;
  • complex multiline boolean expressions passed directly into methods;
  • new Thread(){/* overridden run() with a ton of logic */}.start();
  • raw types everywhere;
  • switch-cases;
  • if (!condition) Thread.sleep(250) in a loop
  • unexpected (and obviously undocumented) side effects all the time
  • GlobalGodObject that's partly a factory, partly a utility
  • new instantiations scattered all over the place

Also:

  1. No specification. We don't know for certain what the app should be doing and are figuring that out on the fly.

  2. No QA. There's literally nothing that can stop a bug introduced by somebody's change from reaching the end user

One little note. We develop medical applications and our bugs may kill somebody one day

But making the management hire some BAs and MQAs (basically, create two new departments of well-paid individuals at the time when business is not doing very well financially) is not something I can do

At any rate, I want to improve our standards. Be a professional. While developing one of my very first changes on the project, I realized it's not possible to do it clean without some sane dependency injection (DI) solution. I decided to introduce a DI framework. After a little research, I opted for Dagger (even though I am used to Spring, it's a worse fit for the frontend). Within a day, I have successfully introduced it in one part of the code that had to do with my task. I suggested to have a little team discussion on the topic of DI as such decisions shouldn't be taken unilaterally (even though, anybody can push anything any time)

However, the tech lead is not willing to discuss it before we're done with the backlog (100+ active issues and counting). He seems to see anything that is not directly related to those issues as a distraction. He responded that he has "only complaints" that the Java department is not keeping up, and we don't have time for that

I don't believe the backlog is ever going away if nothing changes. I believe poor standards is exactly what brought us to this point and keeping doing things as usual won't help. In fact, I expect it to get even worse. Introducing code review (probably, the most important thing now, a relatively low-hanging fruit too), improving coding practices, writing tests and so forth is exactly how we can find our way out of this situation. I am certain it's not a distraction, as the tech lead seems to believe, but actually the solution. It's not going to be fast (it's likely to take years), but it's the only way, the right way

How can I make him see it? Preferably in a way that doesn't get me fired. I have under one year of experience so it won't be easy to find a new job. A tarnished record of holding down a job for only a few weeks would be an issue too

9
  • 2
    If your company has no QA, it must be either a startup or a very small company ? Is it true ? If this is a startup, then you don't have enough resources for lots of things. Maybe, the owners want to save money by cutting costs and hiring less employees. If this situation is true, then there is not much you can do. Commented May 18 at 19:10
  • 1
    You don’t; The technical lead is giving clear directions and sharing honesty way to much information, your team is behind, and their supervisor has set expectations to catch up.
    – Donald
    Commented May 18 at 21:33
  • 3
    As a medical product provider, are you aware of any specific legal or industry requirements that your company is failing to meet? Commented May 18 at 23:07
  • 3
    @Job_September_2020 You sweet summer child - startup of small company?? lol. I work at a company that has basically been been running for 40+ years, and has a significant global presence, and if it wasn't for Java, I would have said to the OP "Are you a colleague of mine?". I've also pointed out similar concerns and had them ignored - but at least we don't build medical devices.
    – Peter M
    Commented May 20 at 13:40
  • 2
    @Job_September_2020 Not only do we not have QA, we don't even have source control. And regardless of how many hidden issues I've pointed out, if the code is running at site with no obvious problems and the customer is happy, then that is all that matters.
    – Peter M
    Commented May 20 at 19:06

2 Answers 2

14

I answered a similar question to this elsewhere - but I cant find it - so I will re-iterate my answer:

Make your suggestions something that your Boss (or his boss) cares about

So - you want to do Code Clean-up because a myriad of well-articulated reasons. Your Boss doesn't care about these reasons and you can't afford to create a new department because of finances etc.

Your Bosses concern is financial

This is where you start - You don't want to do Code Cleanup - you either want to introduce new features that will be profitable or you want to reduce costs:

"Boss, I have been comparing the time it takes our Dev team to troubleshoot a Bug in new shiny module vs old crappy module - and I've noticed that, on average our Devs are spending nearly 3 times as long to fix things in the older modules - due to issues, I suggest we look at starting to align some of our older code to modern standards as this should drastically reduce the time to address bugs, decrease costs, allow us to spend more time working on new features and increase customer satisfaction"

I have written the above predicated on your Bosses primary concern being Financial - but it might not be, if they care about Security - then talk security. If they care about code size, talk about code size - the key is - present your argument as to why they should do what you want to do, because it is what they want to do.

6

Code cleanup, like any code change, involves risks of breakage and costs developer time. Like any task, it must be prioritized vs. other tasks, and often it is not a business's highest priority.

You can put it in the backlog. You can volunteer to spend time on it.

But generally the best you are going to be able to do is to suggest improvements during code review, make sure your own code is clean, try to ensure any piece of code that goes through your hands leaves a bit cleaner than it came in (modulo "least change" reliability guidelines), but you have to accept that this is software engineering, not computer science, and that legacy code is usually not what you would write if you were doing it today.

Maybe someday the company will be ready for a complete rewrite. Maybe at that time you'll get to lead that team. But until then, unless your code is remarkably stable and has all the features it needs, you are just going to have to learn to program with one hand holding your nose.

Welcome to the industry. We've all been there. You will learn what can be changed, what cannot, and what is just going to require patience. This is something that can be changed, but much more slowly than you would expect. It's very hard to make a strong business argument for changing code that is doing everything it currently needs to do.

Afterthought: A lot of what you're griping about may not be currently considered best practice, but is fairly harmless. Variable names are more documentation than function. Mains in classes not normally invoked directly are irrelevant if not invoked, and not especially harmful even if in use. Finalizers that close resources may limit reusability, but if folks know that's the expected behavior of this class that's harmless until a specific issue arises. Obscure flags are obscure to you as a newbie, but much less so to folks with a deeper understanding of the code base. And so on, and on, and on. Your argument would be stronger if you weren't raising what amount to stylistic and code cleanliness issues, and were focused on the things that really do affect maintainability and testability. You need more real-world experience and perspective. Priority is the combination of the independent axes importance and urgency, and most of what you've mentioned really is not as urgent as you seem to think.

17
  • 3
    Welcome to the industry. You would be surprised how much ugly code is not only survivable, but highly successful. if you can find a way to make a real business argument, with numbers of dollars that could be saved by making this investment, go for it. Otherwise, by all means put it on the backlog, but recognize that it is nobody's first priority. Not even yours, since your manager will expect you to be doing other things. If you really think the company is going to crash, well, you can try applying for work elsewhere, though you may not find much better code there.
    – keshlam
    Commented May 18 at 17:47
  • 6
    I'm retired after almost 40 years in the profession. I've seen a lot of good and bad cide written. I've seen people introduce bugs while trying to clean up code, I've seen code that violated principles yet, when looked at more carefully, was actually a darned good implementation. I've seen at least one newbie a year horrified that industry practice doesn't confirm 100% to their professor's ideals, and some whose professors were making statements that were flat-out inappropriate. I've learned that fixing legacy code requires time, not panic. And I've learned that sometimes you just need to exit
    – keshlam
    Commented May 18 at 19:03
  • 5
    @demavi - "We won't get away with it, I'm 100% sure of it" - I can appreciate being confident in your knowledge, but you have less than a year of professional experience under your belt, and it is clear you are not yet able to see a bigger picture. Your confidence after only a year might be one reason your tech lead is not ready to trust your judgement.
    – Donald
    Commented May 19 at 2:53
  • 4
    @demavi - "The fact that we have no code review is an emergency. The fact that our code is a mess is an emergency." - The company has been around for 20 years at least, if the application is 20 years old, the lack of code reviews and the technical debt surrounding the code is not a new issue. Please take your time to fully understand the entire landscape. Those issues existed before you joined the company, likely for years, which suggest it's not that much of a emergency. At the moment, until you are the tech lead, you are going to have to take a seat back. 25 years as a programmer.
    – Donald
    Commented May 19 at 2:56
  • 4
    He's an idealist. And he'd certainly agree with you that things could be improved; we do too. I suspect he'd also admit that "drop everything to clean up stable legacy code" is not a wise business choice, especially when many of your concerns really are cosmetic rather than functional. Stop panicking and look for opportunities prioritized evolutionary change rather than revolutionary. Unless you want to stop being a developer and spend all your time on devops and QA frameworks, in which case ask your manager if they'd be interested in assigning you some of those tasks.
    – keshlam
    Commented May 19 at 15:20

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .