I work at an organisation with a very small dev department, developing a project with one co-worker only. We are both junior, never used ASP MVC .NET or worked on production grade code before. The others in the department don't have MVC experience either, and have never coded in a team, so they are no help.

I appreciate that my coworker has a strong drive to do things, but find that he does not think them through well enough. As soon as he has imagined a possible solution, he jumps to implement it, without taking a moment to reflect a) whether the solution fits the problem , b) if the solution is good in itself, and 3) how will his code react if the state of the world is not what he expected it to be (simplest example: he never thinks of validating user input).

What happens is that he is ploughing through features, churning out tons of new code. But I think that the quality of his code is very low. Case in point: after returning from vacation, I found that he created about 50 new semi-hard routes, most of which could have been handled by the default MVC routing system without any need for configuration. I told him why I think this is a bad idea, but he was not convinced and I did not insist that he change it. Today, I spent hours trying to find out why my first simple attempt to use an exception filter fails, until I noticed that one of his superfluous routes is wrong and redirects my request long before the method which throws the exception is called.

I already talked with my project lead (who is also the manager responsible for our workgroup) about this situation, but she is accustomed to ugly, unmaintainable codebases, and does not see a problem, especially because she is impressed by the quantity of code he delivers, and finds it normal that he spends so much time playing whack-a-mole with bugs we accidentally find (his tests are no better than the rest of the code, if they exist). She suggested that, if he is doing something wrong, I should just teach him how to do it right.

For the concrete feature implementations we need, I cannot teach him how to do it right, because I don't know the MVC-specific solutions. If I research the solution of a feature of his worklist for a day, he spends the day implementing two other features in his own brittle way, and I get nothing from my worklist done. And as for teaching him to think of consequences and side effects before he jumps into action, this is even harder for me, it seems to be too deeply ingrained in his personality.

I don't know how I should handle the situation. 1) How do I help him to start writing better code? He is thankfully willing to listen to what I say, but I don't know what to tell him. And 2) What do I do about bad code he has written which is not obviously buggy? I have no time to fix everything for him, or even to research the proper solution for him, but writing my own part of the code becomes increasingly complex and frustrating. He hates revising something he sees no problem with, and is unlikely to come up with a better solution the second time. Any ideas?

  • 6
    Pair programming. Create the code together rather than each work independently if this is causing you such problems. – JB King Sep 12 '13 at 14:51
  • 4
    I'd also be very grateful that you're seeing what he's doing and you're not being given his code 6 months after he "finished" it – Michael Sep 12 '13 at 14:58
  • 4
    Full team code reviews! Sounds like you might be pushing to production too often as well. – Andrew Bartel Sep 12 '13 at 15:27
  • 5
    @happybuddha The "leave" option should be the last one after all other options has been tried. Otherwise you just end up being a jobhopper. – Thorbjørn Ravn Andersen Sep 13 '13 at 10:49
  • 4
    Your problem is not with your colleague. Your problem is with your project lead/manager. Low quality code is an inconvenience for you, but is ultimately her responsibility. – Roger Mar 21 '14 at 14:52

I think you really need to listen to what your supervisor is saying and not saying. Her immediate, off the top of her head and without any data to support it, answer was the fact she is impressed with the volume of work he gets done. Maybe this is her personal preference or maybe her boss and everyone else involved likes this as well.

You brought up the number of bugs and the difficulty to fix them and/or add new features. Apparently, she doesn't see these as a major problem and has been willing to accept this risk. Perception is reality. A quick turn-around on a request is percieved as getting it done quickly. Yes, the users have to point out some bugs and they take longer to fix than they should, but no one is factoring that into their definition of how long did it take to get done.

You've been given approval to try and implement some of the improvements you mentioned, but be careful, you have not been given approval to get fewer things done. You may have to start tracking your current process, so you have something to compare it to when you start making changes. The complaints about waiting too long could increase and make your newly implemented standards look bad. It will take time and some data to show fewer bugs and quicker turn-around on requests in the long-run (i.e. when things are delivered with fewer bugs.).

  • 8
    OP's in a rough spot. Often times it takes a catastrophic event like a hack or database deletion from a sql injection to get people to pay attention to standards and code quality. – Andrew Bartel Sep 12 '13 at 16:12
  • 1
    You read that correctly, she is indeed OK with bugs in production (we are an inhouse department, so we can take care of our "customers" immediately when they find something bad). But while she only cares for what is visible to the customers, I also care for the fact that I have trouble doing my own work when starting from my coworker's code. When he has quintupled a class, with tiniest tweaks in each copy, I have to make a change five times (and if I don't find all five classes, there is a bug I have produced). – Rumi P. Sep 16 '13 at 8:46

Please consider carefully if "doing it properly" is actually what the project needs.

I strongly hate and despise bad code, and I want to do everything "the right way"™, but this doesn't mean that this is always the best approach.

Maybe this is a low-budget project were fatal errors are acceptable, maybe they have a contract which binds them to a strict timeline, but they can fix it later, maybe their goal is to mantain the product, so your collegue's approach might be better for two reasons.

I don't approve nor condone these methods, mind, but you should consider that these might be the reasons for some decisions.

  • 1
    Perfect is the enemy of done :) – Juha Untinen Dec 8 '14 at 12:57

Put up a baseline of code quality and make code reviews. You need to work out the Baseline with him and your manager what level of quality you want. Things like input validation, unit tests, strict MVC separation of code etc etc etc could be part of your quality assurance rules. Then review each others code and reject code that does not meet the quality baseline. Rejected code of course should not make it to production.

As for not solving every single problem in the optimal way you might have to relax a bit. Some times good enough is good enough if it works and the code is readable and unit tested and solves the problem. That said, knowing the most common and basic scenarios of the used framework by studying or reading some examples is likely required.

  • 2
    Thanks, I wil try that. Must see how I'll sell my boss on the baseline. As for being relaxed, I try to do it. It worked well in areas I have more knowledge, such as the data model. But with MVC, I cannot differentiate between "this antipattern will bring us lots of trouble in the future" and "eh, this could have been done cleaner, but whatever". I let almost everything stay as it is, and when we do run into trouble from the first kind, I feel angry with myself for not having brought it up at the beginning. – Rumi P. Sep 16 '13 at 8:54
