12

Background:

I work in a small group of web developers for a website project. The project is growing significantly, and our leader had added a new developer. I largely work on the front-end aspect (concerning the web design), while the rest of the developers work on the back-end aspect (concerning the server process logic).

Let's refer to the new developer as John. To begin with, I have to admit that I am biased with my feelings for John now, but I am trying to be as neutral as possible in describing the situation. So, please take my statements with a grain of salt.

John comes from a back-end background, and has dabbled with front-end as well. Once, I was too busy with my work, and John offered to help, in which I agreed. The end product was rather messy, and I had to clean up after him. After that, I gave him pointers on where to improve, and explained how our standards worked. I also guided him on installing a plugin to ensure that his work is standard compliant, but he asked me to disable it, saying he would use it later.

Later on, he was put in charge of another module, in which he offered to do the front-end again. Once it was done, the leader asked me to go through his work to ensure the quality. John's work was functional and working, but the code was very messy. However, the leader was happy enough because it worked. It was difficult for me to clean the mess, so I sat down with John again and talked to him about the matter.

From our discussion, I learned that his way of doing things is more towards "the end justifies the means". Since this project is large, my way of doing things is more towards future-proofing it. If I were to fix his work, I would only be doing twice the work. Plus, as mentioned, our leader has no complaints. So, I handed over the front-end responsibility to him completely.

I have discussed this matter with some of my colleagues, and some of them do complain about his quality of work. We are unsure if we should bring this matter to our leader. For one, our leader only has basic technical knowledge, which means it will be difficult to explain this issue. He is also having a hard time balancing the quality and speed in this project.

John is not exactly a bad man, and I can see that he is trying his best. But, he is clearly not taking our advice, preferring to do things his way.

Question:

  1. Have I handled the matter in the best way possible? If I ever encounter similar situations, what is the best course of action?

  2. Should I raise this matter to our leader and attempt to explain this issue?

10
  • 3
    "the leader was happy enough because it worked". Is John's frontend markup/code really bad? Will it really cause issues in future because it's hard to find/fix/change things? Will you definitely need to do that? Or are you just put off because it's not the way you like to do it? That can be annoying to you personally, but often as programmers we lose sight of the goal and end up gold-plating. We also tend to see other people's code as "messy" at first, regardless of whether it is or not.
    – MGOwen
    Commented Jun 2, 2016 at 6:54
  • Yes, it is bad. Some are severe (the whole search results are saved into session without paging, then selectively picked for view), some are minor, but my main concern is maintainability. One of my colleagues had to spend a day to debug something wrong with his module, as he was away. As mentioned, I consider myself biased with how I view his work, which is why I seek opinions.
    – user51235
    Commented Jun 2, 2016 at 7:40
  • 3
    "the whole search results are saved into session without paging, then selectively picked for view". Without more information it's not clear that this is a "severe" problem. In fact, it might be a more efficient design, if it allows querying the database a single time for showing multiple pages of results...
    – user45590
    Commented Jun 2, 2016 at 9:15
  • If we have 1,000,000 results, it will load them all. Add a couple of users performing the same request, and we will be querying our database to death! :) However, while our leader stresses on scalability, we have not come to that point yet, so your point stands.
    – user51235
    Commented Jun 2, 2016 at 9:58
  • @user51235, clearly if 1,000,000 results is a plausible search outcome, the design will need rethinking.
    – user45590
    Commented Jun 2, 2016 at 10:01

4 Answers 4

18

Coding standards are the team leader's responsibility. Avoid taking responsibility for John's code unless there is a clear risk to the team.

You do highlight a potential problem, in that the team leader might not have enough technical knowledge to really lead in this area. However, based on the information in your question:

  • John's code works.
  • You haven't described any immediate problems with his code's functionality, only stylistic issues and design issues that might (possibly) have an impact in the future.

Therefore, it's possible that the team leader's emphasis on code that works is correct (or at least defensible) in this instance.

Software development always involves striking a balance between efficient development and perfectly designed code. John's method may be more "quick and dirty" than you prefer, but that doesn't necessarily make it wrong. It may be that prioritizing rapid development over future proofing is the right decision right now.

(At some point, code quality gets bad enough to simply be indefensible, but it's not clear from the information that you have provided that John's code is really that bad.)

So, I would be inclined to leave the responsibility to the team leader at this point. If his code works, leave it be.

Certainly don't rewrite John's working code to "fix" it.

Working code should not be rewritten--this is one of the fundamental rules of software development. It may have been better to future proof it, but given that the code is already done, you should just wait until the future when it actually needs to be redesigned (this future might never come).

Just to clarify this point, by "working" I mean the code will work in the expected production environment. If the code works now in testing but couldn't cope with expected real-world usage, then it is not really working code in this sense.

If you do decide to bring it up to the team leader, base the discussion on a concrete issue.

If there really is a problem with John's code beyond the theoretical "it shouldn't be done this way", bring it up on the basis of the problem and its consequences. Your team leader is non-technical, so make a business case that something needs to change.

For example, "We are worried that if John continues with his current design, it will take a lot more effort to add feature XYZ that we have planned for later", or "This search functionality may work now on our test data, but we don't think it will scale to the real website where there could be millions of search results."

If there are genuine issues that need to be addressed, try to separate style from substance and focus only on the real functionality issues.

1
  • 2
    "Therefore, it's possible that the team leader's emphasis on code that works is correct (or at least defensible) in this instance." HARD disagree. I'm guessing you're not a programmer. This is the equivalent of saying "Well, the house didn't fall down so it's fine that the contractor skipped a few building regulations"
    – Kevin
    Commented Apr 15, 2021 at 17:24
2

This is what Technical Debt is all about. You need to make sure your leader understands it to the extent that it affects you, and assesses the risks accordingly. It's sad your team has coding standards and a review process, but it's nearly a waste of time since your boss doesn't follow them either. As a result, neither does John.

Just let the leader know that if any of John's code has to be debugged or changed for new features, it is going to take much longer to correct. All code rots, but this is going to rot at a faster pace. If your boss wants this to be a "chance we'll have to take" then there's nothing you can do about it.

Don't be surprised if you have to mention this again as the reason for the delay in a client's request. Your boss will probably want to ignore it and fight back. Then you'll be looking for a gracious way to say, "I told you so."

2

Have you thought of code reviews?

Code reviews are a standard practice where you only merge your code in after there is a general acceptance of its quality, this usually comes before QA. This will mean that both you will review Johns work and John will review your work, it will usually catch surprises before they are merged and helps the team both gel and get into conflicts over minor things.

I do agree with your position, in the end the code should be read as if the same person wrote while in practice this is very hard some structural ideas should be maintained.

0

Judging of what you did, you did fine.

The problem comes from John, who doesn't want to understand what is good in term of coding. It happens a lot and I had the same problem with a colleague.

I end up telling him that he should stop doing that, because he was wasting as much time for me as if I would have done it myself. Since you already explained the problem, which I didn't, at first, my approach would be the following :

  1. Give him another chance, but force him to use the plugin and ask him to come to you when half-done, just to see what is wrong, and how he should do it.
  2. I hate this, but you will have to give the global speech : "This is a company, we have rules to ensure future ameliorations etc... If you don't do that, it will be fine at first, but people will start to complain since it is really hard to take back your code"
  3. If nothing changes, you will have to do a 3-man meeting with him and your project leader, to make them understand why it is wrong.

It is what I have done, I just didn't have to catch my project leader, but maybe you will have to do so. Don't be ashame, it is the good thing to do for everyone, especially those 2 who doesn't understand the main things about a clean code.

You must log in to answer this question.

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