I joined up at a place a few months ago as a web developer. They hired me thinking I was "green" to the industry, placing me as a junior developer, and giving me menial tasks at first.

I've since proven to them that I am competent and have been handed off more difficult tasks, but often they are tasks that involve working with someone else's code.

The developer that is considered my senior has coded multiple things I've worked with, and they have done nearly everything wrong. The code I am forced to utilize on tight deadlines is typically unacceptable, and the code itself lends the inference that the other developer is just skirting by and really has no idea what they are doing with the language. For this reason, it has become almost "nagging" of me to continually ask them why they did something. I feel obligated to fix it for the client, but it would exponentially increase the time I need to spend on projects. I have been avoiding that, but it is becoming unavoidable.

I need a way to approach the PM as well as this developer to kindly inform them that what the developer did was improper and it will require additional hours on my behalf to fix the mistakes. However, even just typing that out I feel like a jerk.

An HTML example I came across recently is by laying out an unordered list of links like so

<ul><li>item1</li></ul> <ul><li>item2</li></ul>

How does one tell someone else that they're "doing it wrong"?

  • 14
    Related (on Programmers): How do you tell if advice from a senior developer is bad? - The highest voted answer is stellar (the second one isn't that bad either ;) – yannis Oct 18 '12 at 17:44
  • 5
    Part of the problem with industries like this, however, is that by the time you're a "Senior developer", the skills you came in with are hopelessly out of date. Often you'll find senior developers "doing it wrong" because they don't have time or budget to be retrained :( – Yamikuronue Oct 18 '12 at 18:28
  • 2
    @LagWagon: do it without taking credit.... – Greg McNulty Oct 18 '12 at 18:31
  • 2
    I suggest you become a regular StackExchange contributor and do some time on the review queue. After a few weeks of reviewing, you'll stop worrying about whether fixing somebody else's work is pretentious or arrogant. – itsbruce Oct 19 '12 at 11:20
  • 3
    Personally, for each problem I'd just add a test case demonstrating why a piece of code is wrong, then fix the code to make the test case work and leave it to the revision control system/code review to sort out any disagreements about the original or revised implementation. There's no need to make a big deal about fixing errors, and it is rarely a problem unless people have made the mistake of being precious about their code. – Mark Booth Oct 19 '12 at 13:15
up vote 31 down vote accepted

A few thoughts.

Dealing with ugly code

The code I am forced to utilize on tight deadlines is typically unacceptable

Assuming you're working in The Real World, this is inevitable. It is not feasible to expect every employee to write every piece of code in an "acceptable" fashion. You will run into ugly code and most of the time you'll just have to work with it. This is the ugly truth of the computing industry; clients don't see how pretty the code is, clients see when it ships and whether or not it works. So my first piece of advice would be choose your battles wisely. It is probably not reasonable for you to expect to clean up every piece of bad code you are forced to work with. So make careful judgments about how much time it would take to fix a thing, how hard it would be to just deal with it, etc. Be decisive. Decide for sure whether or not you want to raise issue or just let it pass, and then act accordingly. Indecision is a drag and can become a huge detriment to your productivity, so eliminate it. It's OK to decide to postpone the decision, but get in the attitude of "I make decisions; I'm in control."

Being a jerk

I need a way to approach the PM as well as this developer to kindly inform them that what the developer did was improper and it will require additional hours on my behalf to fix the mistakes. However, even just typing that out I feel like a jerk.

You shouldn't feel like a jerk. There is nothing wrong with identifying and wanting to fix a problem, and certainly nothing wrong with being honest about how you feel. You cannot control what others think of you; walking on eggshells and keeping your insights to yourself will only place more of a burden on you. People make mistakes, yourself included. In my opinion, the best way to reconcile this sort of situation is to confront it, and clearly explain why you think it's wrong. However, while you do this, be open to correction. There may be some things you've overlooked. More importantly, this demonstrates that you are sincerely interested in the quality of the product, and not interested in just being Holier Than Thou. There is a delicate balance here; if you are too weak presenting your beliefs about the problem, you will just get bulldozed. If you are too strong presenting your beliefs, you will be perceived as arrogant. Make your arguments with evidence, refer to unambiguous policy and best practices, and don't act like there's solidity in your argument if you don't have such things to back it up. If you're as smart as you think you are, then people will quickly learn to appreciate your criticism, because it is correct, and leads to easier programming and a better product. Even if you're not quite as smart as you think you are, you will either still help others identify mistakes they otherwise would have missed (and people appreciate that), or the others will help correct your mistake (which you should appreciate).

Climbing the ladder

They hired me thinking I was "green" to the industry, placing me as a junior developer, and giving me menial tasks at first. I've since proven to them that I am competent and have been handed off more difficult tasks. ... The developer that is considered my senior has coded multiple things I've worked with, and they have done nearly everything wrong.

Again, the best solution is to talk with your boss about these concerns. If you think you deserve a promotion -- or even just more respect -- then ask for it! If you think a certain employee needs to be educated then discuss the situation with your boss, with his boss, or with him directly. Be honest. Be open to correction. You don't have to be completely satisfied with the outcome of the conversation, but you do need to have that conversation or else thinking about it will slowly eat away at you.

Ms Frizzle's advice

Nobody expects you to be perfect, and nobody expects you to consider them to be perfect. So go out there, take chances, make mistakes, get messy!

  • 3
    +1 The "dealing with ugly code" paragraph is what I've also noticed over my time as a programmer. Sometimes you're forced to work with a low budget or short timeframe that won't allow you to do things "right". Many managers and most clients don't care how you code it, just that it fits their specifications when its done. You should see the databases that I work with...NO keys, table/column names might as well be random characters. Columns that are populated and not used. It's a nightmare but the client that uses it doesn't have any budget for restructuring so it stays. – Isaac Fife Oct 19 '12 at 16:04

Just factor necessary technical debt fixing into your estimates. Everyone who works on a team must deal with it, some places more than others. People also improve as they gain both general experience and experience on a specific project. Work somewhere long enough, and eventually you will look in source control to see what "idiot" designed some code, and discover it was yourself.

You don't have to blame someone else, just blame the code if you must. Say it needs refactoring in order to ensure you're implementing the solution correctly. If your colleague is truly as bad as you think he is, he will eventually be naturally migrated off of design tasks like that. Include him on code reviews so he can learn from what you do.

Try to resist the temptation to completely rewrite something you don't like. Make incremental improvements using the "boy scout rule": leave a campsite a little cleaner than you found it. A good programmer can find ways to work with legacy code without needing it to be perfectly clean.

  • 44
    +1000 - 'Work somewhere long enough, and eventually you will look in source control to see what "idiot" designed some code, and discover it was yourself.' This is so true... – jmort253 Oct 19 '12 at 2:43
  • 3
    I recommend reading Working Effectively with Legacy Code – Alex L Oct 19 '12 at 8:34
  • 4
    @jmort253 by "long enough" and "eventually" I assume you mean, "within a week" and "all the time". :) This's driven me to document (typically with comments) any code smells I'm aware of when I first check them in. Knowing immediately if something looks bad because time pressure prevented cleaning up or if it was necessary because the obvious seeming simple way didn't work for case XYZ has been a real time saver. – Dan Neely Oct 19 '12 at 13:01
  • 8
    Just want to add that in general, never assume that the code is "wrong." There may be design decisions that factored in which you are not aware of. Nothing is more annoying than a junior that thinks they know it all breaking a system because they didn't know we also had to handle edge cases B, C and D in a certain way. – Andy Oct 19 '12 at 15:05
  • 3
    @Andy - This is why it's good to avoid hiring cocky people. ;) Man, there's nothing worse than someone walking through the door with an empty skull and too much energy and deciding that you know nothing and then waltzing in and crashing the live server. ;) – jmort253 Oct 19 '12 at 17:55

Be sure you are correct.

I have mixed feelings about these kinds of questions, because I've seen both sides. As a senior developer, I've learned a lot from keen juniors who have a better sense of new techniques, etc. And I know seniors who are senior in years only, so I can sympathise if you're dealing with one of those.

But, I have also run into juniors who don't have my experience and think they know better cause they read it on their favourite blog. So, just as a word of warning, be sure you are right before you start telling people how to do their jobs.

Don't worry about asking them why.

As a senior, part of the job is mentoring juniors. Most of us don't mind explaining our reasoning, as long as it doesn't come over as defending our seniority. And hey, you never know, you might learn something.

If it seems like you're breaking their flow too much, consider arranging regular times when you can sit down with this developer and have these kinds of discussions.

Don't say "You're doing it wrong."

You're right to worry that this comes over as pretentious. Rather, say "Hey, I read about this technique the other day and I wondered what your thoughts were."

Sit them down and refactor their code, show them how it is easier to read afterwards, and how you only have to make a small change to implement your new requirement. Listen to their concerns, look for answers to those concerns, continue the discussion over time.

And be prepared to hit situations where you just cannot win the argument, even if you're absolutely correct. One day you'll be that senior and sometimes you'll have to put your foot down and say "No, I really believe this way is better." And you won't want juniors being sulky about that.

But, if that happens over and over again, you might have to consider grabbing as much experience as you can stand and then going on to work somewhere else.

Don't bring the PM into this.

It's not his job to worry about implementation details. It's your's and your senior's. Keep the discussion at that level. Do, however, bring in other developers, if there are any.

In response to "This client is a massive, multi-billion dollar, high-level business."

Even Facebook have made mistakes, some by their own admission. If only we could all say we've made mistakes and still be worth billions of dollars. You can understand why people that have been around a while might think they're doing things very, very right.

In short ...

You have to concede their seniority, but you should challenge them. That's the line you have to walk.

  • I agree with what you said, but I feel you don't understand how poorly the code was written. An example would be using all position:absolute (regardless of its location), negative margins in css being abused, and still applying floats to these absolute divs. Etc. That's just the CSS. An HTML example I came across recently is by laying out an unordered list of links like so <ul><li>item1</li></ul> <ul><li>item2</li></ul> – Jbwilliams1 Oct 18 '12 at 17:51
  • 4
    @LagWagon: Like I said: You could be right. I'm just accounting for the possibility that you might not be. To talk about specifics makes this question too localised. – pdr Oct 18 '12 at 17:58
  • Right, I only listed the low-level stuff. Getting into the specifics of the languages used and the errors made would risk my job I'm sure. The point is, the code is wrong on an objective level - not a subjective level. Programmers will not get far surviving on opinions and feelings, a general understanding of a language is a requirement. Being confident that I know what I am looking at is wrong, I feel obligated to confront it. – Jbwilliams1 Oct 18 '12 at 18:00
  • 5
    @LagWagon: So that accounts for the "Be sure you are right." Now follow the rest of my answer. – pdr Oct 18 '12 at 18:01
  • 10
    The longer you work in this industry, the more you'll find that bad code is often written for sound reasons. In the bullet list example you've given, perhaps the developer found that with the list items in one list, they were visually too close together. Maybe the "correct" solution is to modify the CSS for the li element to produce the correct spacing, but then you'd have to check all the other pages to make sure that their formatting wasn't adversely affected. In that situation, most bosses would prefer the developer to use a quick fix, as long as it wasn't too ugly. – mhwombat Oct 19 '12 at 12:23

Unfortunately we don't have details and when you say "they have done nearly everything wrong" all I can take away from that is that you believe they had done things incorrectly.

You may or may not be right - it could just be you, you know... And your PM may not be in the position to be able to tell the right way from the wrong way.

Instead of going to the PM, you should be talking to your colleagues. If you don't understand why something has been done a certain way, ask. If you consistently get evasive answers or answers that clearly show that the person is out of their depth, then you have an opportunity to educate. Show them a better way - educate them and help them learn.

I understand this will not help with current projects, but going forward it should help.

  • I gave some examples above. It should be enough to understand the validity of my concerns. – Jbwilliams1 Oct 18 '12 at 17:53
  • 2
    @LagWagon - You did not give concrete examples, I'm afraid. You are talking about needing to redo things and code that is unacceptable, but these are not concrete examples, they are not detailed. – Oded Oct 18 '12 at 17:54
  • 1
    @LagWagon - Then incorporate it into your question. Comments are "second class" here. If they hold important information, you should be editing and posting such information in the question. Regardless, when I posted my comment, there were no concrete examples. – Oded Oct 18 '12 at 18:16
  • 3
    +1 - I came to say exactly this. I spent time as a consultant coming into existing code bases. They were large, poorly designed and only half worked. Most of the junior level consultants would be quick to point out how 'wrong' it was. I don't think I've ever worked on a project where some co-worker didn't say, 'We need a full re-write'. The really good consultants could wrap their head around the code, figure out was done, how to leverage it to get it doing what it needs. Rewrites rarely actually work out half as well as people intend. – user3497 Oct 18 '12 at 20:40
  • 1
    @LagWagon, I work for a massive, multi-billion dollar Fortune 100 company, that just bought another massive, multi-billion dollar Fortune 100 company. We have all sorts of problems like this. It's a fact of life when you have hundreds of people working on a site. – Alan Shutko Oct 19 '12 at 14:59

You think they did things wrong, but before you confront them, have you considered why they might have done the things they've done?

Possible reasons I can think of (and have seen) that the senior developer has coded "nearly everything wrong":

  1. They don't know how to do it right.
  2. There isn't time to do it right if deadlines are tight.
  3. They inherited crappy legacy code and don't have time to clean it up.
  4. They know it's bad and they plan to fix it later.
  5. They just don't care.

In my experience, #2 and #3 are the most common reasons for this sort of thing to happen. Sometimes they are related when #3 leads to #2.

How to talk to this person:

Say you are concerned about the maintainability of some pieces of code. Show them an example and explain why that particular example concerns you. It will help a lot if you have trouble tickets that you can directly link to poorly written code. This will open the discussion and you will be able to ask them why the code is the way it is, and how to go about changing it.


With regards to the HTML example you posted:

Ok, I agree that it is a little bizarre-looking, and I can't think of a reason to do it that way. I think it would be ok to just say to this person who wrote that:

I've never seen a list done like that before. Is this better than the standard way?

  • 5
    @FrustratedWithFormsDesigner: I didn't downvote, but I'd guess that it was because the first half doesn't answer the question and the second half says the same things others had already said. – pdr Oct 18 '12 at 18:28
  • 1
    I upvoted because the first points are definitely valid, and no one has considered these why aspects of things. Unfortunately, when we're working, the world isn't perfect, and sometimes we have to just plain take shortcuts to get from point A to point B. – jmort253 Oct 19 '12 at 2:50

You have obligations to your employer not to the client. You are performing work for your employer not for their client.

I am going to give you the benefit of the doubt that the senior developer did it wrong. I know I would love to go back and redo some of the things I did in the past and do them better. It happens as programming is a constant learning experience. So I would start by stepping back and instead of blaming anyone for what exists, figure out how to deal with it. There is no need for you to point the finger and dress down the senior.

I would get with the development team and discuss what you think needs to be done to bring the application up to par. Especially focus on if this is something that can be done incrementally so as to reduce the risk. Since if you have to overhaul the entire application you are increasing the risk of bugs. Be able to communicate the risks that exist with not fixing the problems. Realize that businesses do not want to spend money on redoing things that work fine just because there is something better. You need to be able to show that what exists is a potential problem and that it is only a matter of time before that problem becomes an emergency.

From there the team can decide how to address the issue. If a dressing down is due then management will likely get involved when they learn the hours that will have to be spent to fix it. There is no need to concern yourself with that. However if the team decides to just continue with the current code you need to be prepared for that. Your responsibility is to make your employer look good so deliver the best product you can with out making them look bad to the client. If you involve the client it will cause problems for your employer and will reflect badly on you.

  • "There is no need for you to point the finger and dress down the senior." Dress down, no. But part of addressing an issue is stopping it from getting worse. – pdr Oct 18 '12 at 18:26
  • 1
    Yeah, ok, I see your point. Seems like a passive-aggressive solution though. Personally, I like to be shown how I can do things better. I'd rather someone be upfront about it than call a meeting and point to all my code and say "ok team, how are we going to fix this mess?" – pdr Oct 18 '12 at 18:33
  • Passive-aggressive is not limited to going behind someone's back, although that is probably the most common manifestation. Passive-aggressive is essentially attacking someone while trying to appear like you're not. – pdr Oct 18 '12 at 18:46
  • That is only true if you have examples of problems from several developers. Had that been the issue described, I'd have agreed. – pdr Oct 18 '12 at 19:14
  • @PDR - you do not have examples of problems from any developers you have examples of problems existing in the work product. I am glad I do not work where ever you do. – IDrinkandIKnowThings Oct 18 '12 at 20:37

If you do it politely and properly inform all the stake holders, I don't see why it should be a problem. Look, I know many people will disagree with me BUT I think that, in this day and age, society, and the workplace along with it, place waaaaaaaaay too much importance on things like EQ and making everyone feel good about things they should and shouldn't have to feel good about. I have done exactly what you describe MANY TIMES and yes, some people were pissed and some had a more rational reaction. I think we have overengineered our emotional complex and the two clear pitfalls of that are:

  1. People have become wusses .
  2. One must constantly run checks in the back of their mind how whatever they are doing will emotionally affect everyone involved (so called "empathy"). And that's just a waste of time most of the time, IMO. (high maintenance)

So if you fix something and make it better, document it properly and demonstrate your improvement in one way or another if you are concerned that it may be considered impertinent of someone at your level of seniority to rock the more established elements. If you do that, I guarantee you that you will garner more respect than animosity in the net yield. Be polite and do your homework. If you are making an improvement overall, it shouldn't be too much to expect your surrounding to react RATIONALLY AND NOT EMOTIONALLY.

If you want to be successful, if you want to GET ANYTHING DONE, if you want to be of any effect beyond a little organizational gopher -- you will have to get used to making calculated sacrifices in terms of alliances, i.e. you will have to create some animosity. You have to be careful who you piss off though. But if you aim to please everyone and are scared of pissing people off -- you will fail miserably.

So the net effect is what counts. I love refactoring other people's code. You should do it. But beware that there are extra steps involved when you overhaul the work of someone who is still there, alive and ready to challenge your taking on his baby.

As far as "emotions in the workplace" are concerned and the infestation of emmo culture in the Western world, all I got to say is that I doubt the railroads would have been built back in 1800 if the builders were touchy-feely. That's why I like working with people who have been either in construction or military cause they are thick skinned. But you can't always choose.

I think the best way to go about it is to be positive and constructive when telling your colleague how to fix their code. Whatever the age or experience the approach is always the key. You should always make the initial approach with a positive statement. Then lean forward to constructive criticism. This way they don't feel that you're telling them they are doing it wrong, even if they really are.

Usually they will realize that by themselves and keep it quiet, but still save face at the same time. It is also very good to discuss with your colleague the best practices that are in use for that particular task. You can show them websites and cite popular documents that "Pros" use.

Then lastly you can end the session by saying that even the seniors and pros used to make similar mistakes like those when they started out. It helps gives your colleague a feeling that it's ok to get their mistakes found out because it's a learning process that even pros go through.

So the cycle kind of looks like this

  1. Approach positively
  2. Provide constructive criticism
  3. Discuss the better ways of performing the task
  4. Show that even experts were once beginners who knew nothing
  5. It's all about learning new things

TRUTH TO POWER

NEVER TALK TRUTH TO POWER, THE FIRST THING THAT A KING WOULD DO WHEN HE SEES SOMEONE SPEAKING TRUTH TO HIM, HE WOULD GET HIM TO THE GALLOWS!

YOU REMEMBER THE STORY ABOUT THE NAKED KING AND THE INVISIBLE CLOTHES, nobody was willing to speak TRUTH TO POWER i.e. the fact that he was naked, excepting a child who had no idea about POWER OF THE KING - What ever happened to that poor child! The king might have killed him! My advice is - Don't be that child!

So if the man holds power, i.e. if he has worked there for three to five years, he would have a tight network with all the people there and you will immediately make not one but one + network full of enemies. What is the nature of the person, can the person take it? Is the person receptive to feedback? That should also be considered into the equation. You should also consider if you meet the PM by surpassing him, would he consider it a betrayal? So this isn't a simple YES/NO decision. This decision requires careful analysis taking into account the personalities of people involved. Are they political? What are the ego levels of people involved.

Please don't create a pros and cons list of what will happen I if report vs what will happen if i don't

Instead try to create a mind map about the political possibilities, often there are more than 100 different ways in which this simple reporting of bad code situation can turn out into and each of these end results are going to be decided by the simple fact! What do you want?

The society in general hates people who are straight! i.e. people who speak the truth - It is the straight trees that get the axe.

So are you strong enough to take the axe? Have you witness a similar activity i.e. reporting in the organization before. Do you have a precedent to act upon?

So my advice from the little info that you have posted in this situation is

  1. You are a junior dev, he is a senior dev, your word against his to the PM, whose words do you think would be more respected!
  2. He holds sway over the project and is the one responsible for the project, you are only the sidekick helping him. Thus the senior dev has his ass on the line, not you!
  3. Going to the PM is almost always throwing rocks at the hornets nest. If you go to the boss directly the senior dev will become you permanent enemy and will never trust you with anything.
  4. He has survived for so long, may be this is the only project which he could not get right, a project with bad foundations will end up bad! Thus you should give him some more time and observe him for a few more projects, before you conclude that he does not know how to program or does not follow programming standards.
  5. It would be wise to take senior dev to a dinner or lunch and then ask him why the project is running into so may glitches and then slowly push your observations.
  6. Don't make the capital mistake of striking a conversation in the corridor, or taking the person into a conference room for "discussion about code" That will create ruckus!

Anyway handle the situation carefully, but if it was your co-dev i.e. another junior developer, you could directly go to the PM and snitch anonymously, because there is not TRUTH TO POWER involved!

I have take these frameworks from the book "DECISIVE" you can learn a lot about decision making from this book, please check it out

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.