Demonstrate bad code to client?

Question

A client has asked me to do a redesign of their website, an ASP.NET Webforms application that was developed by another consultant. It seemed like a relatively straightforward job, but after looking at the code, it's clear that's not the case.

This application was not written well. At all. It is extremely vulnerable to SQL injection attacks, business logic is spread throughout the entire application, there is a lot of duplication, and dead end code that does nothing. On top of that, it keeps throwing exceptions that are being smothered, so site appears to run smoothly.

My job is to simply update the HTML and CSS, but much of the HTML is being generated in business logic and would be a nightmare to sort out. My estimate on the redesign is longer than the client was aiming for. They are asking why so long.

How can I explain to my client just how bad this code is? In their mind, the application is running great and the redesign should be a quick one-off. It's my word against the previous consultant. How can I give simple, concrete examples that a non-technical client will understand?

Update

Thanks for all the responses. The SQL injection attack demonstration makes sense and I will demo this in a test environment. That is just one part of many problems in this application. I was looking for ways to explain why other parts (such as html being generated in the data layer) would need to be replaced with better practices in order for the html and css update to take place. There are many good suggestions here which I'll piece together when I talk with my client.

Answer 1

Non-techies aren't idiots (for the most part). They can understand a technical argument if you keep it high-level enough. Pick a task you thought should be simple, and walk them through why it's not.

I expected this change to be one word in one file. The most likely place to change it seemed to be here, but when I changed it there, it only worked in one place, and it broke these 7 other places. When I fixed one, it broke two more places, causing a domino effect, so a change I thought should have taken 10 minutes ended up taking 2 hours. That's just one example. There are a lot more unexpected 2 hour tasks in there.

Answer 2

Code structure, style, technical debt are one thing that - at least initially, until the client trusts you - you're going to need to live with.

Security vulnerabilities are another matter.

Personally, I would do an estimate based on the work required using the existing structure and style while making it clear that there are significant issues with the codebase. I'd raise the security implications separately: do a demonstration of a hack on the database to drive the point home during a meeting.

I had great joy doing this with a previous client with a loyalty gift card system when I put £5000 on "my" card and had him check the card on his till.

Answer 3

Some great suggestions here on how to convey and communicate this to the client. Hopefully they will pay off for you.

Major red flag here!

If the client asks you not to make any changes other than what you've agreed to (HTML and CSS) I'd pass on this project and withdraw my bid.

Even with a written and well communicated overview of all of the flaws and security issues, the potential liability is just too great for me to be comfortable with. Even if the client never took any legal action or demanded fixes after a hack or breach; your name and reputation are still attached to the work!

You may well lose far more than you stand to gain.

Answer 4

Explain and possibly demonstrate the flaw
When it's your word against his, everything you say could just be hot air as far as they're concerned. Once you show them how their app can be abused via SQL injection, then suddenly you're a person to be trusted. You're going to need credibility in order to renegotiate. And this is enough of a game-changer to give it to you.

Be charitable with respect to your predecessor
That doesn't mean pretend the mistakes aren't there, but if you come across condescending then you lose credibility. Don't say a word about the programmer except perhaps to give him the benefit of the doubt. Focus on the code, not the coder. Making them feel like you're the "good guy" will give you a lot more leeway in negotiations. And "good guys" never say mean things. When explaining existing security mistakes (such as SQL injection vulnerabilities) to the client, I prefer to say something like this:

Web application security is a rapidly-evolving field. Many of the development tools and techniques that people learn even today evolved before most of these exploits were well-understood. In order to stay ahead of security developments, you have to follow the field very closely and occasionally even change your whole development style. Most programmers don't do this.

There we go. Not a word of evil spoken about the developer; he's just "most programmers" which means he's in pretty good company. And now you've demonstrated that you're not "most programmers" which give you a bit more credibility and perhaps a reason for them to pay you more money.

Negotiate a new arrangement
Once the client understands that his app is open to abuse by the public, he's going to want it to be fixed. You are probably the person he's going to ask to fix it. You may or may not want that job, so think it over carefully before you talk to them.

At the very least, you want more time to finish the work they've already given you. You've set them enough off-guard with the vulnerability stuff that they probably won't hold you to your original estimate. But make sure the client knows what you are and are not going to be fixing as part of this arrangement.

Typically the developer (you) would prefer to redo the whole thing from scratch. And in cases like this, that might even be an option. But even then, the client is going to want something that can keep his business running until the new app is built. This means that even though you're starting over, you're probably still going to have to update the old app a bit.

Answer 5

I started this as a comment, because at first I thought it was an aside, but it probably really isn't.

I would fully document everything that you feel is should be redesigned, and why (what happens if they don't make the change), and an estimate on fixing the issue. I would be particularly meticulous with anything you perceive as a security risk.

I would do this before touching any code, and make sure that your client has a copy of this report, preferably with some kind of timestamp. It may take some time, but it will also cover you if one of these security risks ever comes to fruition. Even better if you can get something signed that says they received the document.

Sure, you can point to source control of the original code you inherited if it ever does happen, but it will be much easier to point to this document and say, in a more professional manner, "See? I told you so."

This document can be the launching point of further discussions, and it may even be used by your client to get the "right people" to give the permission to make some or all of the changes.

That being said, once the client undestands the risks, grin and bear it if they say to do the work anyway, or walk away.

Answer 6

Remember that the client is going to you for help with maintaining their application. It is your job as a professional to point out any issues you find with their application. The client likely has no idea these issues exist and they should be made aware of them. Explain these issues in a manner that they can understand and let them decide how they want to proceed.

Use real world examples to illustrate these issues, such as a car breaking down or a washing machine needing repair. To point is to use examples they are already familiar with. For explaining SQL injection, I would simply demonstrate what that is and why it's an issue.

In the end you want to convey that you care about the success of the application you are being asked to work on.

Answer 7

I like to use analogies the client can relate to. The amount of work I put in upfront in winning the job would depend on the amount of money the client was intending to spend ($100 is far different from $20,000). Notice I said "intending". Your personal estimate of the value involved doesn't mean much if you don't get what you're asking.

In your situation - again depending on the money - I might draw a box with one line coming out from each side and say to the client "This is how you visualize the software now. Data goes in one end and comes out the other, all looks nice and clean and simple". "This is what you think the software looks like on the inside" and then draw a third line connecting the two lines inside the box.

Then I'd draw another box just like the first with the input and output lines on the outside, except this time I'd say "Here's what the software really looks like inside the box right now." and then to connect the two lines this time I'd draw a random pile of spaghetti mess, possibly with breaks and joins and scribbles.

Finally I'd say, "Now what you're asking me to do is this..." and draw a simple shape inside the first box perhaps a small half circle touching the line and then say "but to do that, I'd have to do this..." and draw a tornado looking kind of spiral shape around the line and continue ... "in order to get around all this....." and point to the spaghetti in the other box.

I would think that would drive the point home in about 2 minutes time. If they insist you do it anyway, then document it as others mention above.

Answer 8

How can I explain to my client just how bad this code is?

Perhaps you can use an analogy like plumbing in a house that over time, after fixes and remodels, becomes so fickle and coupled that when fixing one thing, affects and possibly breaks something else that then needs fixing and there's just no way for you to know all the places that this will occur.

It's my word against the previous consultant, so how can I actually give simple, concrete examples that a non-technical client would understand?

You're right, it's word against whatever visual the previous consultant has created in their heads. My suggestion is to do just what you're asking, give simple, concrete examples. Since this is a redesign, show how an HTML fragment defined in the compiled code is displayed with the rest of an HTML page and how changing that affects or doesn't, the rest of the page. Perhaps that same compiled code renders markup after applying some "business" rule. Show the difference.

This is a hard and VERY common problem. Good luck with it.

Answer 9

Be honest and be direct.

But most importantly do not take on a job that will not meet your expectations. Most people do not realize that a contractor can fire a client, they can and should if the job is more trouble than it is worth.

Answer 10

Here's an analogy I've used (though I do not vouch for it's effectiveness): Imagine their website is a physical machine, like a mechanical printing press that somehow accepts input.

They probably think of the machine as having on component that does X and another that does Y. In reality, it's 20 or so mostly-similar machines. Some of them no longer do anything, all of them attempt to preform functions the others do already and no one besides the previous consultant has ever seen anything exactly like them before.

"See this gizmo here that parses the post variables and then sends this component down a rabbit-hole of if-elses? There isn't just one of these, there is one of these in every page (or whatever), some of them sanitize the input and some don't (or all don't) and without reading the entire thing I can't know which."

Answer 11

One point not really mentioned yet is that you might simply be overstepping what your client really wants from you in this case. Overachieving is great and can give you lots of job satisfaction. But if the client simply doesn't care, thinks current performance is "good enough" and just wants some minor updates, it may be impossible to persuade them to make a large investment in you to overhaul the codebase.

At that point you'll probably need to decide whether to stand on principles and refuse to take a job that would force you to attach your good name to a embarrassing code mess or whether you can hold your nose, get in, get the job done with some duct tape, and get out with your payment.

If you do decide to go ahead with the duct tape job though, make sure to document, document, document and be as transparent as possible. The last thing you want is to get blamed for something going wrong in the future that is a result of an application flaw you warned the client about but that the client decided wasn't important enough to deal with at the time.

As far as the SQL injection risks go, as others have said you should be able to demonstrate to dangers of that to them in a way that shows the risks without actually doing anything destructive in production. But again, if they see it and don't care enough to pay you to fix it, you've done your good faith diligence in this case.

Answer 12

It's noob sauce to come in to a project and suggest a rewrite first thing, perform some small subset of the modifications and use those to illustrate how much simpler and cheaper it could have been. Then you have a demonstrable case on why the increased cost of cleaner development will lead to lower maintenance costs and faster development over the long term given a little font side cost.

Never forget that fundamentally you are asking for them to pay you to make your own life easier, in their mind the just need to find 'the guy' who can X features at Y cost and magnifying the complexity of your project may just eliminate the opportunity for you. It's a hard road when you're a month into the rewrite and you have meeting with the original developer only to realize the entire app was written in an extremely contracted window by a developer who fully understood all the compromises that were made. If the app internally looks horrible but externally functions well, as you say, it is very likely this will be the case. Often the technical debt within a codebase is a product of the resource constraints the code was developed in and if they aren't building a team and are instead contracting things out... they are probably still not serious about maintenance.

I'm just sayin'

Answer 13

I'm going to play devil's advocate here (somewhat along the lines of what @khrome is saying: "you are not paying clients to make your life easier"). I would even go as far as stating that the case you presented is too one-sided because you described the case in a general manner. Most incoming consultants to a new project would shine a bad light to the previous one...I'm not saying that's what you are doing here, but until we see examples, I cannot simply take your word for it.

That said, I'm going to try to address the issues to you point by point:

• SQL Injections. OK, so I guess the programmer was using string concatenations instead of parameterized queries and/or stored procedure. This is very easy to fix especially in ADO.NET...I personally would mention it to the client but not make too much of a big deal out of it.
• HTML is being generated in business logic and would be a nightmare to sort out. OK, dude, this is one of those where you give me more details. Unless you're using MVC, this is a tendency to happen...but it's not necessarily a bad thing...it's one of those things where most programmers would say "goto is bad; never use it" but you know what? I've used goto where it made sense! So, are you sure they're not using helper classes that happen to be sharing the same namespace as the business code DLL? Again, it's not that difficult to isolate.
• business logic is spread throughout the entire application, there is a lot of duplication, and dead end code that does nothing.. And? Client is just asking you to change HTML/CSS. Why would you care about these issues at all?
• it keeps throwing exceptions that are being smothered, so site appears to run smoothly. Again, very vague. Exceptions are normal in any applications, that's why we have try/catch clauses in our code. Unless they bubble up in UI and ruin the user experience (like displaying HTTP 500's unnecessarily), I don't think this is something that you should care about, either..

So in short, I would advise you to take the high road. If you think it's not worth your time and you want to rewrite it at the expense of your client, then walk away from the job. Seriously, at the end, the client pays for your time to make the whole thing work with the least amount of $$$.

In my many years of experience in the field, I always say that the best programmers I've come across are the ones who can make a system stable by writing the least amount of code, not by rewriting the whole thing.

Edit: I already see that my answer is not the most popular one (I already expected this) but I stand by my response. I edited this to make it less snarky. ;-)

Answer 14

Certainly the SQL injection attacks and other functional flaws in the application took precedence, but you can also "demonstrate" bad code quality and practices. With code metrics tools you can clearly demonstrate how bad is the code and show him how much this will it add up in cost for any future changes and bug fixing. I'm not familiar with .net environment but I'm sure there are several from which to pick up.