23

I'm currently reviewing a system built by some developers that previously worked at my job. The system works pretty well from a user's point of view, but when delving into code review it's a utter mess. I'm more than convinced that the way the application is built won't hold up for future updates, let alone high increases in usage.

The problem is that I know how bad it is, but my superiors don't. How can I prove it to my manager so he actually sees the problem and can be convinced to do minimal triage on the current codebase, and in the near future start a new line of development for the next version of the application?

Community
  • 1
ChrisR
  • 779
  • 1
  • 5
  • 13
  • 6
    http://programmers.stackexchange.com/questions/59050/what-is-needed-for-a-self-taught-developer-to-be-able-to-move-into-senior-level Maybe heed how badly doing the "big rewrite" usually ends up from a business perspective. That would be what a more "senior level" programmer would do. – quentin-starin Jan 06 '12 at 08:13
  • 23
    @qes, the only thing worse than doing the "big rewrite" is a paralyzing fear of the "big rewrite." When I started my current position, I inherited a mess of Perl CGI from two or three different programmers with no source control, bug tracking, or tests. It took some convincing, but I got approval to rewrite in Ruby on Rails while maintaining the legacy code. 5 months later, the tools were more user-friendly, and we could add new features in days or weeks instead of months. The users are ecstatic, and I'm not losing my mind. The "big rewrite" requires solid justification, not total avoidance. – Jason Lewis Jan 06 '12 at 08:59
  • 1
    @JasonLewis: (I'm guessing you didn't follow the link in my previous comment?) It seems ill-advised for someone who less than a year ago self-described as lacking important skills a senior-level would posses and doesn't know where to begin when starting a new project. – quentin-starin Jan 06 '12 at 09:39
  • @qes I skimmed the linked question, but missed that it was from OP. You're absolutely right in principle, but two things: 1) OP's other question didn't pertain to lack of technical skill so much as learning project management and design aspects. 2) I was in a similar position when I found myself doing the 'big rewrite' I mentioned. The best way to learn design/architecture skills is by designing a large project. But just because you're not sure how to manage a project, doesn't mean you can't spot the WTFs in bad code. I take your point, however. – Jason Lewis Jan 06 '12 at 09:54
  • @qes You certainly have a valid point but that doesn't change the fact that i'm faced with the situation, know where it's going because i've been there and know what to do about it in the long term. It's only that i need to make my point to management and prove to them that my approach is the right approach. You don't need to be senior to see that this has gone the wrong way some time ago. Thanks for the input though :) – ChrisR Jan 06 '12 at 10:43
  • @JasonLewis, thanks for backing me up, you pretty much describe my current situation and have given some really helpful advice! thanks! – ChrisR Jan 06 '12 at 10:43
  • 5
    @JasonLewis I completely agree with you. Every time someone ask about rewriting an app in SE a lot of people comes with this "don't do a big rewrite" ideology. I think there are certainly many reasons not to do it but you shouldn't be avoiding it at all costs. I have participated in a rewrite of a 100k lines of code app and we had a lot of success, very similar to what you've described. We were even able to rewrite it module by module (first part of the UI, then the server, then the rest). 12 months later we have a very very happy customer base and everybody is proud of the decisionw we took. – Alex Jan 06 '12 at 11:56
  • 2
    This is part of a more general problem: that of your predecessor going for immediate results at long-term expense. When taking over, you have to explain why you can't keep up the same output. – David Thornley Jan 06 '12 at 15:32
  • Been in this position many times - the simple answer is, put up and shut up - or move on. Its very rare for things to change that drastically, and with every new job/codebase, its pot luck as to how “good” its going to be - thats the joy of development! – Robert Perry Jun 09 '21 at 18:05

10 Answers10

22

'But it works now' is the standard management response to the legitimate frustrations of software engineers. The first thing I would do would be to compile the documentation (if any) and use that to demonstrate contradictions between the code and the documentation.

If you can, put together a comprehensive suite of unit tests. Run these with every change so you can document regressions which can be blamed on the existing codebase.

Lastly, if you can pull in a developer from another department whose work you trust, get a second pair of eyes on the code. One developer saying 'this is crap' is easier to dismiss, than when a senior developer who has been around a while vouches for him and says, 'No, Jim, he's right. This is crap on a crap cracker.'

Of course, it all depends on your environment, company size, etc.

I always recommend taking a look at The Pragmatic Programmer If you haven't read it. Not only should it be required reading for every software professional, but it has some good suggestions for dealing with management, co-workers, users, etc. who don't view software engineering as a craft.

Jason Lewis
  • 2,113
  • 13
  • 18
  • 7
    There are no docs and the code is pretty much untestable unless we are going to refactor the hell out of it. I'm fairly confident i'm being backed up by multiple colleagues tho so getting another developer in the discussion might help. – ChrisR Jan 06 '12 at 10:29
  • @ChrisRamakers That's excellent news (the support, not the lack of docs!). It's harder (albeit not impossible) for managers to deny/reject the professional opinion of *multiple* developers. And if your supporters have been at the company longer, they may have valuable experience navigating the internal politics of the organization. Good luck! – Jason Lewis Jan 06 '12 at 15:36
  • Addtionally, if you can get ahold of some load testing software, you can show how it might break under a high load. – HLGEM Jan 13 '12 at 20:00
13

From management's perspective, there's nothing wrong with the system and you are just complaining because [you just want to rewrite it/you don't understand what previous engineers did/you want your job to be easy]. A bit of a straw man, but when someone at the top sees that things are working fine right now, they're disinclined to see a crisis where you do (I'm sure there's a climate change allegory in there somewhere...).

To some extent, they have a point. Management sees problems when releases go well beyond their original estimate, estimates seem too high for the work being done, "this is impossible with our existing code base", and high occurrences of bugs slipping past QA. If things are purring along nicely, it's easy to pat you on the head and tell you to do your best, because it's not affecting the bottom line.

What to do depends on your organization and the software itself. Fundamentally, though, I suggest documenting every complication that comes up as a result of bad legacy code. When estimating tasks, make it clear to management why you think it will take that long, with detailed explanations about what aspects of the old code base are adding this extra cost. When bugs are introduced into the code, include the reasons why this bug was able to slip in, and how issues in the old code base are responsible.

I'd suggest phrasing your concerns to the stakeholders in a way that suggests the software has outgrown its original design and is now problematic to continue enhancing.

Stefan Mohr
  • 340
  • 3
  • 8
5

There are various tools available which can do code coverage and code review. Google tools suitable for your technology, these are normally industry standard tools. I would suggest you use these tools and come up with code-quality report and present it to Manager. Make sure your criticism is contructive and non-political.

ViSu
  • 190
  • 6
  • yes, i've thought about calculating the average cyclomatic complexity and using that as argument but i'm certain that this won't prove a thing to management. But it's worth a try, thnx – ChrisR Jan 06 '12 at 10:37
  • 5
    @ChrisRamakers: Nothing can be "proven" to a manager. Forget "proof". Gather data. Facts are all you have. More facts is more convincing. But there is no "proof". – S.Lott Jan 06 '12 at 10:48
4

Pick an example that is easy to understand where managment would think it is a simple change request, but because of the existing design it is much more difficult.

You don't want them to dwell on the specific request, but make sure you let them know that this is what ANY request for change is going to be like. Nobody wants to be painted in a corner with an application. Developers believe in YAGNI, but management believes in CYA.

The suggestions for load testing could be a good approach, but they may not be convinced the increased usage concerns meets any real world growth potential (The company is not going to double in one year.).

Everything is relative. They may not want to put the resources into this project when they have plans for more important projects to spend your time on. Don't get labeled as not seeing the big picture.

JeffO
  • 36,816
  • 2
  • 57
  • 124
  • 1
    After making the change, show the diff file, which in a bad codebase will touch many different source files, have a "what does that have to do with it?" element, etc. Explain to management how much of this work, i.e., time == $$, was related to the poor codebase and that changes going forward will all have this characteristic. – Larry OBrien Jan 06 '12 at 19:20
3

When you talk about proving something, all that scientific method stuff comes into play, and part of what that means is that if you are going to accept objective standards of deciding what's true, you must accept the possibility that, upon investigation, those annoying facts turn out not to be on your side.

In your case, I think there are 2 things to be proved.

First, that the current codebase is "bad". What you probably can prove is that "the professional opinion of nearly all developers that examine this code is that it's bad".

Second, that the company would be better off rewriting the codebase. This is a problem because even if the first point is true, the second might not be. Also, you don't know enough to make this determination. This is management's job, and if you want them to respect your professional judgement about the first point, you should respect theirs about the second.

But they can't make the determination about the second point without information you provide. You need to communicate what you know about how the problems in the code will impact the business, and what you know about how a rewrite would impact the business. This is difficult, since both involve predicting a future that has a lot of uncertainty.

But you can try to state the problem in business terms. How much extra time is spent on changes and regressions? What would a rewrite cost? How quickly will the costs of the current system go up over time if not rewritten? What if there is an increase in usage, what's the chance of a disaster if the current code is kept? You can't really know any of this, but you can give a better guess than anyone else. Give a range, or something to communicate how accurately you think you can predict these things.

Most developers do not like maintaining lousy code. Which is why it can be unfortunate that code that is a no-brainer to rewrite from a developer perspective may not be worth rewriting from a business perspective.

For example, even if the rewrite ends up profitable, it might be worth less than the opportunity cost of spending the money elsewhere in the company. Or the bad codebase might take longer to change and have more regressions, but not enough to make a rewrite profitable. They may be looking to get bought out in the next few months, and spending money on a rewrite will show up on the books but buggy software won't.

Try to think about it from the business perspective, and don't cook the numbers to get what you want. A big rewrite is almost never a no-brainer from a business point of view. If you want to prove something not directly provable, try your best to disprove it. If you keep trying your best to come up with a way not to rewrite from scratch but nothing you come up with makes sense, maybe then it's really time to rewrite from scratch. And making that effort will show your management that you are serious about representing the company's interests, not your own (you are representing the company's interest, not your own, right?).

psr
  • 12,846
  • 5
  • 39
  • 67
2

I Guess it depends what is bad about the code base. Being "Not the way I would do things" does not mean it is a bad code base.

Things that actually make a bad code-base:

  • Security holes

    Problems that leave the Server, application, and/or data vulnerable. Especially anything that leave sensitive company, client or customer data at risk. These should be easy to document.

  • Working Broken

    It is only working because you massage the data and perform maintenance on the application nearly continuously to keep it working. If you were to leave and no one took up the slack it would not longer be working. - Document how much time you are spending doing this. And note how much you could save. Quite often these processes are inefficient for users as well and you may be able to document that as well.

  • Does not actually work

    It appears to work but the results are incorrect. This generally causes problems down the line like numbers not matching, high defect rate, etc.

What is not bad codebase (just not good):

  • Written on obsolete unsupported tech. (VB6, COBOL, .net1.1 Etc.)

Note the advantages of migrating to a new technology. Try to find a migration path that moves pieces at a time so that you can minimize risk and build user and management confidence. Make sure that the logic works basically the same as the original.

  • Undocumented/poorly formatted code

This is the easiest for you to fix because generally you can add comments and correct the formatting with out actually impacting the code. But it does not require a rewrite. If you feel this is combined with other issues the first thing to do is fix this so you can better assess the viability of the code.

Community
  • 1
SoylentGray
  • 3,043
  • 1
  • 23
  • 31
1

You have answered your own question in a way.

The way to convince them to spend money on the system is to wait until it doesn't function well for the user. If you think it won't scale, either wait for that to happen or do a load test to prove it.

It's then a simple proposal of cleaning this to scale will cost X hours.

Jonno
  • 1,738
  • 10
  • 17
  • 8
    The only problem is that if he has principal responsibility for the system, he'll get blamed when it no longer functions. "But it worked before *you* started!", they'll say. Which is why I advised a proactive approach. Warn them in advance, document the issues, and then his ass is covered when it breaks, and not only can he say 'I told you so to his boss', but he can say 'I told *him* so' to his boss's boss. – Jason Lewis Jan 06 '12 at 09:03
  • 3
    @Jason - I see your point, but in my experience denial operates downwards and 'told him so' going up the chain will be met very very quickly with 'non team player' on the way down. – Jonno Jan 06 '12 at 09:18
  • 2
    It very much depends on the individual workplace, but I agree with you... I could have phrased it better... My main point was documenting the reasons it's *going* to fail in advance, arguing the point, and keeping the documentation available for when it does... best case scenario, you're allowed to fix it. Average, you're not screwed when it fails. Worst, you deeply grok the system and its weaknesses and how to fix them enough to impressively explain (in vivid detail) how and why you left your last position to your prospective employer when you end up job hunting after it fails ;-) – Jason Lewis Jan 06 '12 at 09:26
  • 1
    @JasonLewis If players in the company are using phrases like `I told you so` then that is a toxic culture of blame and not a place that the OP would want to work at for long. A good manager doesn't blame, a good manager acknowledges problems and comes up with a plan. – maple_shaft Jan 06 '12 at 15:00
  • @maple_shaft I agree. I didn't mean those words literally, I was more referring to covering all the bases. Ideally, we would all have good, supportive managers, all the way up the chain of command. Often, however, we put up with fair-to-middling so we can use the job we have as stepping stone to the job we want. – Jason Lewis Jan 06 '12 at 15:30
1

First determine the accumulated technical debt in your codebase. Then take a look at this question and answers, where it is explained how to convince management to pay off the technical debt before proceeding further.

Community
  • 1
BЈовић
  • 13,981
  • 8
  • 61
  • 81
1

This is a tough situation to be in because from the user's perspective everything is at an acceptable and stable point now. Primarily with non-technical managers, there is nothing to be concerned about at the moment, but asking to rewrite the codebase is an enormous decision and one that should not be taken lightly, especially on the opinion and efforts of a single man (yourself).

So you are in a tough spot of trying to make a case for a rewrite because of overwhelming technical debt (in your opinion, we don't have any examples and have to take your word for it) as well as being in the tough spot of having the difficulty of maintaining and adding features into this system.

Your estimates for new features will be high, justify these high numbers with facts, proving that these things will indeed take a great deal of time. If you don't convey this properly then your manager may just assume you incompetent and you certainly don't want that. When he questions the high estimates then explain why you feel the accumulated technical debt is hindering the ability to quickly and cheaply add features to the current software. If your manager has two brain cells to rub together he will start to understand very quickly.

The point is that you should not try to convince your manager, but steer your manager with appropriate (and carefully selected) information that he/she can convince themselves that a rewrite is an acceptable course. You have to make the manager think it was his/her idea all along.

maple_shaft
  • 26,401
  • 11
  • 57
  • 131
0

There is reason why all mature software looks like mess:

  1. All the mess is encoding the critical logic that the business relies on. Without it nothing works. Every bit of it was necessary to solve user problem.
  2. It's using slightly outdated tech. Current programmers seem to think that if it doesn't use some template magic, it's just a mess. This is broken view. Anyone coming late to any bigger project will have this stage while learning all the details.
  3. The requirements which were critical some time ago are no longer so critical. This is because the software already fixed those problems. Coming late to the project, it might seem like the software is complex for no good reason -- the problem no longer exists, but the complexity in the code is still there.
tp1
  • 1,902
  • 11
  • 10
  • This type of attitude leads programmers to excuse the massive technical debt the incur out of ignorance or laziness. A programmer's job is to encode business logic through the use of abstractions. Mutable specifics should live in metadata, not hard-coded magic numbers. Secondly, 'slightly outdated' can be subjective. IMHO, 'slightly outdated' means the framework/platform is still actively developed, and I have an upgrade path. The alternative is 'obsolete.' Number 3 is **inexcusable.** You have just described cruft. No one has refactored or cleaned up the code base, and this is acceptable? – Jason Lewis Jan 07 '12 at 03:56
  • sure, but what's the result after you have encoded the business logic through the use of abstractions... The code is going to look complicated. That's the definition of "mess". the (3) is not cruft, since the complexity is still required; the need to have it did not disappear even after the problem is solved. – tp1 Jan 07 '12 at 06:14