I am a junior developer. Although I already programmed in different languages for the last 5 years I still have a hard time debugging more complex issues. On the other hand I like clean code and try to write expressive, short functions wherever possible (at present in React). However, it annoys me that around me other developers who are more senior than I am don't write clean code, produce long and snarled methods that are not completely bad but could be easier to read.

My question: Would you bring this up in a meeting - like a 1:1 with another developer? Or would you just keep your mouth shut because you are more junior and wait for the critique until you are senior enough?

  • 91
    Could they be hard to read because the problem and requirements are complex to solve? You might ask them why a given method was implemented that way. Commented Apr 24, 2019 at 9:41
  • 13
    I am kind of curious what makes you think their code is bad. Is it simply that they write long functions? is it not DRY? Poorly formatted? Everyone seems to have some pain point early in their career that contributes to what they think is good and bad, perhaps yours just isn't the same as theirs. For me as long as the code is DRY, I don't much care how complex it is, but others have had different experiences. One of my other trigger points is "Short, Expressive" code which often seems to mean "Heavily Encrypted".
    – Bill K
    Commented Apr 24, 2019 at 18:37
  • 6
    You say you've been programming for 5 years but you also say you're a junior - can you clarify on that? Commented Apr 25, 2019 at 9:36
  • Hi @BenjaminGruenbaum... Well I don't know if there is a standard for the labels Junior, Professional, Senior. I only think I could still improve and see that the people around me are faster than me. I subjectively have the impression that being a senior takes more than 5 years and differs also from person to person.
    – user104010
    Commented Apr 25, 2019 at 10:40
  • 2
    @BillK Well I think I go pretty much with what Robert C Martin and Michael Feathers teach. It is easy to write code that computers understand but hard to write code that humans understand.
    – user104010
    Commented Apr 25, 2019 at 10:43

18 Answers 18


Don't wait until you have a senior title to comment on other peoples code. My experience is that the title doesn't matter that much, as does the way you give feedback.

Ask, the senior whether you can share some feedback about the code, or if you prefer can put it as asking questions about the code.

Try to be as concrete as possible and if possible explain the impact it has on you. "I found this method hard to read, because it is very long and does two different things." or "I was confused by this method name, because it says X but the method does Y" is much easier to accept for any developer than "You write bad code"

You might also ask what was the reason for specific coding choices (often enough I found out that "bad code" was actually around for a reason.)

This kind of feedback should be inoffensive enough, and if the developer takes it well there might be a chance for follow up discussions. "Robert Martin suggests this for naming, what do you think?"

But there is also the chance that the developer doesn't agree with you or your favorite author, and doesn't care too much about your opinion. Just safe your energy on that developer, there isn't too much chance you will have a positive impact by being more verbose.

  • ...often enough I found out that "bad code" was actually around for a reason... Most of the times, this is just to cut corners. For an example same code being duplicated just to skip the time that needs to be spent planning and thinking on how the function should be written so that it is reusable. Commented Apr 28, 2019 at 19:42
  • 6
    I would focus on the reason why Robert Martin (or any other person) makes a given suggestion, as opposed to simply appealing to a particular authority.
    – mcknz
    Commented Apr 28, 2019 at 19:49
  • 3
    "You might also ask what was the reason for specific coding choices " It can be very hard "asking" a question like this without revealing your underlying disagreement . I think the better approach is saying what you think is better, and the response will contain a counter argument without you asking for one, and either you'll learn the reasoning behind the original code, or you will manage to continue with a debate until some conclusion is met.
    – NiRR
    Commented Apr 30, 2019 at 12:03
  • I see your point. I agree, that it isn't a good practice to camouflage critique as a dishonest question, but in my experience it often does help to assume that there was good reason to do something a certain way and honestly try to figure out what it was.
    – Helena
    Commented Apr 30, 2019 at 18:02
  • If you use GitHub or something similar, reviewing other PR's voluntarily can be a good initiative.
    – user47813
    Commented May 1, 2019 at 17:53

Obviously, criticizing your elders isn't a great move and should be avoided. Belittling juniors is also a bad thing.

One thing to remember is that loose coding practices doesn't necessarily lead to poor compiled code. So, if it works, it works, and the guys know how to maintain their own code. People can, and do, get defensive about their own parts of the code-base.

What you can (and should) do is maintain the best coding practice you can within your own portions of the code. Allow that to speak for itself when other team members see this in the code-base or you're talking through examples.

If you need to work on a part of the code-base that someone else has worked on and you have problems in following it, then by all means ask the developer to talk through the relevant portion so that you can quickly complete this coding task.

You don't have to point out the bad coding practices from other people - take care of your own and hope that other people notice and also take on board what you're doing.

  • 17
    In the 1st line. I suggest changing Obviously by Arguably. Some elders consider criticism as a great value. Or maybe changing criticizing by belittling; it is indeed not a great move then. Commented Apr 25, 2019 at 15:32
  • 20
    "So, if it works, it works" - most likely it doesn't work, but nobody knows it yet. "and the guys know how to maintain their own code." And eventually those guys leave, and then my company gets contracted to take over b/c nobody else can figure it out. Not that I'm complaining exactly, it means that my company will never run out of work :-)
    – industry7
    Commented Apr 25, 2019 at 19:30
  • 6
    loose coding practices doesn't necessarily lead to poor compiled code Why does this matter? In my understanding the complaint was not at all that poor source code leads to bad compiled code. if it works, it works I could write the same piece of code in 100 different ways, which would all "work" in the same way, and then show you the 20 worst versions and challenge you to say that they could be kept because they "work". In a professional context a piece of code "works" if it delivered the expected result and maintaining it is not a nightmare and doesn't require the authors to do it. Commented Apr 26, 2019 at 12:29
  • 5
    To suggest that everyone should just "do there own thing and worry about their own code" is terrible advice. Imagine giving similar advice to a sports team, for example. It's not how teams work.
    – aw04
    Commented Apr 26, 2019 at 17:17
  • 11
    I get the impression that most people up-voting this answer have little to no idea about software development and how problematic it can be to have to deal with someone else's crap code that only the original authors understood and no one else. "It works" is a necessary, but not sufficient, precondition for good code. It's ironic that this answer was written by someone who is a "developer working for one of the largest software companies in the world" (as per profile).
    – code_dredd
    Commented Apr 26, 2019 at 18:35

All code should be peer reviewed (but I've worked in a lot of places where that never happened). How clean is clean? There should be coding standards and guidelines; ask for them. As to how "picky" you should be; that depends on the code being reviewed. Some people like having blank lines pointed out to them, and spacing. Others prefer you spot potential problems.

Don't keep your mouth shut because you're junior, but do be careful about how you highlight things. The aim here is not a blame game, but to collectively, as a team, improve the codebase. Try to suggest improvements. The code may not be "wrong", but there's usually something that can be improved, personal coding styles aside.

  • 12
    Yes, the first thing to do is to push for a proper code review process. That way everyone is invited to provide feedback and it doesn't feel so much like a personal thing. Once code reviews are in place, ensure the responses are not overly critical but instead are framed as possible enhancements (don't say "this is unreadable/hard to read" when you could instead say "this might be easier to read if you did XYZ").
    – delinear
    Commented Apr 24, 2019 at 9:10
  • To avoid having to point out whitespace issues, make sure everyone has a linter plugin on their editor and if any issues get committed just add a comment saying there were lint errors added.
    – Qwertie
    Commented Apr 26, 2019 at 3:21
  • 2
    Agreed here... I would talk about best practices rather than focusing on comments on specific code that may come across as negative or personal attacks.
    – JeffC
    Commented Apr 26, 2019 at 13:49
  • 1
    This is a much better answer than Snows, as this one actually suggests that seniors and "seniors" should find and follow good coding practices, rather than just write a bunch of junk and assume everyone else can understand it and that it's good simply because "it works for me". I've seen senior devs write code they couldn't understand a week later, as well as novices write very understandable code, even if it was a bit "simple". Commented Apr 26, 2019 at 17:28
  • For code, "a bit simple" is good :-)
    – gnasher729
    Commented Apr 19, 2021 at 23:01

A few points in addition to the other answers:

Accept that, as a junior, you don't know everything :-)  There may be reasons for the style of code that you are unaware of, such as:

  • Avoiding unnecessary changes to working code (keeping diffs manageable, avoiding introducing unnecessary bugs, &c).

  • Keeping related code so it can be seen together.  (No benefit from nice short functions if that just obscures the complexity.)

  • Avoiding subtle language or performance issues.

  • Matching the style of other code.  (Don't underestimate the value of consistency in a codebase; it can make code much easier to read, understand, and maintain, reduce friction between developers, and avoid subtle bugs.)

That said, it's also entirely possible that your changes are worth making!

  • Most developers are mediocre (even though we all think we're pretty good…).

  • Previous developers may have been in a hurry, and didn't have time to consider refactoring.

  • Previous developers may not have cared enough about clean code, or seen how to improve it.

In my experience, every codebase can be improved.  Even on code that I've written and worked on many times, I often spot improvements that I'd previously missed!

As to how to handle it, I'd suggest:

  • Don't criticise your fellow developers, or act as if you know more than they do.  Even if you do, that won't win you friends — and a good working relationship is important too.  By all means, suggest improvements, but in a constructive way, without blame.  And accept that they may not agree.

  • Run significant changes past a senior colleague first.  If any of the reasons above apply, it's better to find out about it before doing lots of damage!

  • Don't rewrite code just for the sake of it.  The original author may take it personally; and you want to be seen as working with them, not against them.  (Remember that what seems an obvious improvement to you may look very different to them.)  Also, each change brings risk; a decent test suite &c can reduce that, but changes aren't free, and being conservative can save headaches.

  • Instead, make improvements as you go along: when you fix a bug or add a feature, improve the code you're already working on. That way, it's easier to account for the time spent, and it won't require any extra testing &c. And over time, the quality will increase.

  • Finally, accept that while it's important, improving code quality is just one of several priorities.  Developer time is limited, and there are always compromises.  Sad, but true.

It's great that you care about code quality!  Too many developers don't.  But try to use your concern in a productive way that's good for the team as well as the codebase.

(I've been that young developer who wants to rewrite everything…  To some extent, I still do!  But I've also watched my own code being changed pointlessly or made worse by people whose priorities I didn't agree with, or who simply misunderstood it.  And I've struggled over codebases which used inconsistent formatting, naming, organisation, and style.  Ultimately, life's much better if the codebase works well together, and the developers work well together.)

  • 27
    "Don't underestimate the value of consistency in a codebase; once you're familiar with it, it can make code much easier to read and understand." Thank you, I've had this discussion multiple times with people that wanted to refactor some small part of a huge legacy project. They usually had valid concerns, but at one point consistency is more important imo. Commented Apr 24, 2019 at 10:00
  • 6
    I'd suggest adding one more "Don't do rewrite team code without running it by a senior first." Junior dev doesn't want to find themselves a couple of months in (if senior devs don't check commit logs carefully) having rewritten massive amounts of team code one small bit at a time only to have angry senior devs showing them exactly why their changes were very bad for some reason (broke something, made the code unmaintainable, etc.), and now there's a big avoidable mess to clean up.
    – bob
    Commented Apr 24, 2019 at 14:49
  • 7
    Why does this answer have random "&c" characters in several places?
    – Aaron
    Commented Apr 24, 2019 at 16:34
  • 9
    @gidds because I was curious and others may be likewise, I looked into it then asked about it on English.SE: english.stackexchange.com/questions/496087/…
    – Aaron
    Commented Apr 24, 2019 at 21:35
  • 5
    Using &c is a really good example of a rarely used construction confusing others, instead of just using what everybody expects - "etc." or even "and so on". A peer review would probably have flagged that... Commented Apr 25, 2019 at 22:02

Don't get hung up on junior/senior. No one is a perfect developer, and everyone - regardless of title - has the opportunity to improve.

That said, it's important to consider the context. If you're picking out old work that's not really important or relevant at the moment, and then telling them why it's bad quality, that's not going to come off well. On the other hand, if you're in a code review with this person, and looking at code that's currently being worked on, you have a better opportunity. At the end of the day though, you need to understand that employers aren't inherently looking for perfection, they're looking for functionality with a certain level of sustainability. Sure, a function may be rewritable in a way that helps you understand it without a few extra seconds of thought. But that may not be worthwhile for a particular project.

Also, consider that criticize has a negative connotation. No one likes criticism. Instead of,

This is wrong


This could be better

you might want to try,

Can you explain this function to me?


Can you tell me why you did it this way?

Those open-ended questions let the other developer talk through their code and their thought process, which may lead to you realizing they had a legitimate reason to do what they did. Or, it may lead them to realizing it could be cleaner/better. Or, it may create the context for you to suggest an improvement. At the end of the day, you've made your point, and with a better chance of maintaining the relationship than if you'd just gone in cold with your criticism.

  • I like "Can you tell me why you did it this way?" in particular. It's the least offensive as it frames the question as "I don't think there is anything wrong with or code, I'm just trying to learn in order to get up to your level." But then you can always follow up with "Oh, the way I would have done it is like... and I don't understand..." And again your'e not saying that it would be better to do it your way, but simply opening up communication and getting the coder to elaborate in a way that will end up highlighting any issues, if they exist.
    – industry7
    Commented Apr 25, 2019 at 19:37
  • 1
    This is also an extremely important point: "they're looking for functionality with a certain level of sustainability" You're working for a business that needs to maintain a profit, and there's a difficult line to tread when it comes to maintenance costs. How good is good enough? How much time do we spend now to avoid potential future problems? It would be great if we could get hard objective numbers about how these choices affect the bottom line, but it's all so complicated that there will probably always be a measure of subjectivity to it.
    – industry7
    Commented Apr 25, 2019 at 20:00
  • Upvoting because this is the first answer that mentions to frame the critique as questions. Striking the right tone is the most important aspect here.
    – blues
    Commented Apr 26, 2019 at 9:46
  • Always initially assume there was a good reason. (You can later privately decide for yourself if you want!) A good initial assumption is that it was due to time constraints of one kind or another. Writing clean code in itself is not a categorical good: it is hypothetical on the basis of some organisational need. The biggest additional constraint is usually time, whether that's the time to write code, fix it, understand context, etc. The senior is probably well aware this and of their limitations. I'm afraid this won't be news to them. As a junior developer there's a great opportunity to help.
    – Dannie
    Commented Apr 26, 2019 at 10:55
  • I don't remember where, but I read that devs spend more time reading code than writing it. If the code takes minutes to figure out and can be rewritten to take seconds to understand, then it might be worth it, especially in code that gets read often. As someone else stated, it's a difficult line in maintenance costs, which comes down to speed of development and making good code that's easily readable. And sometimes there's a major performance enhancement that's gained by making it complex. It's not as often as some devs like to think, though, so YMMV. Commented Apr 26, 2019 at 17:36

30 year software development professional here. Perhaps some insight I've gleaned might be of help.

  1. Don't sh*t where you sleep. Everyone thinks everyone else's code is crap. This is a pretty natural reaction to reading any code that is difficult to understand. Railing about it, unless you have a good reason to (eg: telling your boss why a feature change is going to take way longer than she budgeted) is just going to tick off someone you have to be able to work with, and make everyone else you have to work with wonder if you're going to say equally nasty things about their code when you look at it.
  2. The proper attitude for you to take into discussions about problems with other's code is that you could be wrong. You're probably missing something, right? I try to keep to this, and am still continually annoyed at how often it turns out to be the case. Humans are imperfect and easily confused, and you're a human too.
  3. Enlist senior people. Show others (eg: cubemates) the code you're having an issue with. Typically this will result in either an explanation of what its doing and why from someone who's been through the wars, or an agreement and admission that its subpar. In extreme cases even outrage, from someone more fully invested in the codebase than yourself, and whom management will listen to. Either result is a big win for you.
  4. If you must complain, complain about behaviors, not people. It isn't that "Fred's code" is inherently crappy. What's crappy is this bit here where it nests 3 lambda declarations inside each other over about 7 screens of code. Doing stuff like that needs to be banned. Can we get the style guide changed to ban this? And names really have to be better than this. We can't have 15 interacting classes all named with some permutation of the same five words (two of which are "model" and "view"). Also, any source file more than about 1000 lines long should probably be split into smaller classes. This routine here that's 1500 lines long should just never happen.* This is costing us a lot of time making modifications and tracking bugs. Who do I talk to about getting this stuff in the style guide?

* - Sadly, all real world examples from inherited code I'm working on right now

  • 5
    I've seen code files 16,000 lines long... but I've also seen junior developers who think they know everything, trying to convert a perfectly readable and stable codebase into "microservices" or "functional programming" or whatever the newest fad is. Hard to know "who is right" here... Commented Apr 26, 2019 at 7:58
  • "What's crappy is this bit here where it nests 3 lambda declarations inside each other over about 7 screens of code." Pffft... grasshopper. :p
    – AnoE
    Commented Apr 26, 2019 at 13:20

You can criticise the code, no need to criticise the developers. My guess is they want to do better and a friendly comment from a workmate, worded with care, will be welcomed. It's normal to get in a hurry and be a little sloppy--reminders help correct that.

If you've been a developer for 5+ years then the junior/senior demarcation doesn't make a lot of sense and unless your workplace is really aggressively stratified then you are likely better off to not consider it a barrier.

One option is to ask someone to explain what one of their obtusely written functions is doing. Once done you might say something like "Oh, ok, yes, I see that now. Oh, hey! Do you mind if I rewrite this so it's a little more maintainable? That'll help me understand it better and for others later."

Presented that way, you're getting advice and asking a favour -- not performing a code critique -- but with the same outcome. A lot of people will respond with "oh, cool, thanks that would be great. I'll try to be tidier next time."

This can create context for future similar interactions.

Adjust to fit the situation and the person you're working with.

If you find your workmates aren't open then it's unfortunate but sometimes that's just how it is. The more professional they are (senior or not) the more open they will be--by definition.

It is worth making the effort.

I think my main advice is presume these people do want to improve their code quality (this is a code quality issue). But your concern about stepping on toes is always valid and should be accounted for.

Also, it is a part of your job to support the entire team's improvement.

  • 2
    Good advice. Before you consider doing this, make sure that the "senior" isn't following an established/existing pattern in that part of the code. It may not be how they would have coded it, if they had coded it all themselves from scratch. Also, be ready for the fact that you might have wished you kept your opinion to yourself (said with a warm smile, not malicious intent). Best wishes! Commented Apr 24, 2019 at 18:51

Would you dress this in a meeting - like a 1:1 with another developer? Or would you just keep your mouth shut because you are more junior and wait for the critique until you are senior enough?

I would "keep mouth shut" but not because you are not "senior enough" but because you are not their manager and it is not your job to comment on their coding style.

Here I mean "keep mouth shut" only in the context you have mentioned. You can try casually talk about your cleaner coding style in informal discussions and its benefits without implying that others are not doing what you want them to do.

Another approach, depending on what your team culture is, you could request your manager to allow you to do a "tech talk" or "lunch & learn" kind of session where you go over some new coding ideas you have. Usually teams have these sessions where everyone gets to talk irrespective of their seniority. May be your senior developers could learn something or who knows teach you something on why they do what they do!

However way you approach the idea is to just mention what you do instead of "teaching" others in 1-1 sessions on what they should be doing.

  • 2
    The more casual approach is definitely a good way to handle this sort of thing - arranging a meeting sounds far too serious. Preferably the team should provide feedback in a structured manner though, as in line with code review principles. Commented Apr 24, 2019 at 8:35
  • 3
    Managers can't comment on coding style at all. They don't know how to code. It's not their expertise. So it can't be their job.
    – Gherman
    Commented Apr 24, 2019 at 9:03
  • 3
    @Gherman This is highly dependent on the work environment. In the last couple of places I've worked, our direct managers were probably our most competent coders. Some places promote people into management instead of finding business-only managers. I'd say this answer would fit well with that type of workplace.
    – Booga Roo
    Commented Apr 24, 2019 at 9:58

To effectively raise concerns about code quality, a definition of high quality code must be established. Secondly, a code review is an ideal environment to raise these concerns in a constructive manner.

Style Guides

Does your department have a style guide for code? If not, then it's not surprising that your peers' code isn't clear or consistent. You might want to raise the lack of code standards with your supervisor. Emphasize how consistent style improves maintainability by reducing context-switching overhead. If your supervisor sees the value in this, it might be beneficial for you to work on developing these standards since it seems important to you.

Code Reviews

Ideally, stakeholders, peers, and supervisors would have the opportunity to provide feedback in a structured code review process. This would be preferable to booking one-on-one meetings because it provides a more open forum for feedback and isn't as likely to get a defensive response. You might also want to raise this with your supervisor, emphasizing that it's an opportunity for you to learn about why your peers' wrote methods in a certain way, for example.

No matter when or where you approach your coworker, you should come from a more inquisitive perspective. Rather than explain to them why there code is wrong, ask why they wrote a method in a certain way. Even if you are correct, an outright accusation is likely to be met with a defensive response. The old adage, "You catch more flies with honey than vinegar," applies. And if you are incorrect, you will be happy that you were humble in your approach.

  • 100% agree with this answer (and actually wanted to write exactly this), it's not about seniors or juniors but about the development methods in the team. Don't criticize colleagues for following the practices of their team (and if there are no processes since the team lead decided sw quality is not important, then it's not the mistake of the colleagues)
    – Sascha
    Commented Apr 25, 2019 at 7:31

Like others have said in several good answers, there are constructive ways of bringing this up with others - asking why they wrote the code like they did, rather than being critical.

I'd like to offer another potential solution: If your team does not do unit testing for the code, you should become a champion for it. Start to add unit testing to your own code, and explain how valuable it is. Since managers hate bugs, they should hopefully get on board with improving testing and testability of the code base. Unit testing (when done right) will naturally force everyone to start writing smaller, cleaner functions. You'll end up with code that's both clean and tested.

  • 1
    +1 because this suitably addresses the ambiguity in "clean code". Asking "is the code clean" should really be "is the code highly covered with linters, unit, integ and e2e tests?" "is the code tracking in Git with a CI/CD pipeline?" "are daily builds being done?" Those should be answered first and then cleanliness is mostly automatic.
    – user56657
    Commented Apr 25, 2019 at 23:00
  1. If you want to become an expertly skilled professional a field, you must first learn to question professionals in that field.
  2. Questioning someone is not the same as insulting them, unless you're doing it wrong.

It's both about when you ask and how you ask.

A good time for "when" is one-on-one time, for example while pair programming or during a code review. If neither exist, as a junior you can easily create opportunities for both. Simply ask a senior to review your changes and you got yourself a code review. Or ask a senior to pair program for half an hour, so you can learn more about that unfamiliar component you need to work on.

As for "how": if you don't understand why they would do something the way they did, simply tell them that you don't understand and ask them why the code is written that way. People have reasons for doing things a certain way. Your goal is to find these reasons. Sometimes there are perfectly valid reasons and their code is better than what you would have written. Sometimes the reasons are external (e.g. incentives to optimize "lines of code" "# of unit tests", etc). Sometimes the reasons are just about a minor difference in opinion (are 15 lines too long for a function?). And sometimes the reason is that the programmer picked up some actual bad programming habits.

If you figure out how to ask correctly, many people will freely admit that the code is rubbish and provide you with a list of reasons for why it's rubbish and what's wrong with it. I haven't seen code that wasn't rubbish on some level so far, and I have seen and written a lot of code.


Consider that writing what you call “clean code” may not be at the top of your senior colleagues priority list. And that “clean code” is not at the top either.

Often what is called “clean code” is actually misunderstood - slavish adherence to misunderstood concepts is not improving anything. Often it just adds work for no real benefit, or for very little benefit. So one risk is that you are told why your opinion is wrong- obviously that is a good thing for you.

I had one case where I had written a 300 line method and someone complained it was too long, so I told him to fix it himself. His code was all little and easy to understand functions - with half of the functionality broken. It just hadn’t occurred to him that the function was long and complicated for a reason. Even though it was very well commented (like here we do X to work around bug Y that happens when you do Z - with corresponding code thrown away). Asked why he threw away code he said “I didn’t understand it” (so why didn’t you ask me) or “I thought it wasn’t needed” (so why didn’t you read the comment and try it).

So be prepared that the senior developer actually knows what they are doing and what you call “not completely bad” might actually be very good.


Broadly speaking, "Clean Code" is nice, but not the standard. The standard is the company's style guide. No matter what you learn in some book or at some place, you will step into an organization and it will be different and they will have their own "rules of thumb".

If there's no style guide, then often the code base settles into essentially what the Code Reviewers prefer or what the tech leads are maintaining.

I won't pretend to know every single variable in your situation and I also won't advocate that these things are appropriate. But, nonetheless, they do affect code quality:

Sometimes there's a lot of pressure to push code sooner. To get it to market as soon as possible because it provides a competitive advantage, and in fact, this might be preferable. Martin Fowler talks a little about this in his refactoring book.

You're not always going to have the ideal. The ideal might be too late. However, what you can do is every time you're in a place where you can make a small change for the better, do it.

Broadly speaking if I had one piece of advice when it comes to this sort of thing: Always, broadly, advocate for the cleanest way to do something. However, never leave your ego in the code. Your job is to solve problems and this notion that you think their code is "no clean" is you putting your ego in the code. Learn to be neutral and learn that things are not always as they seem.

Do I think you should approach them? No. Do I think, broadly speaking, you should advocate for "Clean Code"? Of course I do. But change rarely comes from the bottom, work your way into a lead / senior position and then advocate for a "Clean Code" policy.

Simply telling people their code isn't clean, doesn't do much. Also it completely ignores the context of the code. Software development sometimes is more sociological than people think. Context is everything.


When you're a junior, everything looks messy. Joel Spolsky detailed this in an article.

“Is it always this messy?” I asked.

“What? What are you talking about?” the manager said. “We just finished cleaning. This is the cleanest it’s been in weeks.”

OK, so far I’ve mentioned three levels of achievement as a programmer:

  1. You don’t know clean from unclean.

  2. You have a superficial idea of cleanliness, mostly at the level of conformance to coding conventions.

  3. You start to smell subtle hints of uncleanliness beneath the surface and they bug you enough to reach out and fix the code.

There’s an even higher level, though, which is what I really want to talk about:

  1. You deliberately architect your code in such a way that your nose for uncleanliness makes your code more likely to be correct.

Seniors often are at level 4. The code is dirty to you, but not to them. It doesn't even matter if it's dirty, as long as it doesn't lead to bugs. People who are familiar enough with their code know which parts are important to keep clean and which parts can be messy.


I don't know this is as much an answer as it is my thoughts on the subject. I think this kind of depends on the code and how 'bad' it is. I've worked on a handful of applications, that have been around for 5+, even 15+ years. The code may not be the cleanest, but it works, and has been working reliably for years.

Different programmers have different views on how to code. Styles have changed over the years. Not everyone keeps up with the newest trends, and some people are just set in their ways. You have to be able to separate style from bad coding practices. In the end, what is important is that the code works reliably.

If this is old code, you might want to look into new code being written and see if that follows better practices. On older applications, you will sometimes see the coding styles change over time. The older code may not be great, but the newer code follows more modern practices.

Look into who is committing what. You may have some coders writing clean code, and some coders writing less clean code.

Talk it out. Don't know that I would bring it up for discussion at a meeting quite yet. Talk to your team mates 1:1 and sort of feel out how they feel about clean coding practices. See if there is any team wide standard on code writing. Bring it up to the lead developer. If they think this is something that should be brought up at a meeting to discuss with the team, great. Some teams care more about coding practices than others. Many teams only care if the code works and passes validation.

I agree with the others. Don't be judgmental. Ask about the code. Ask about standards. Try to learn as much as you can. Make suggestions, but don't push too hard.

Overall, I don't think it is a bad thing to bring up and discuss. You are trying to improve coding practices.


You should not criticize existing code and you definitely should not criticize the coder. You never know how people will react to this and you risk fracturing the relationship and being labeled as a "know it all". Vague criticism, or criticism against existing code is also not productive. There is little chance they will refactor code that has already been tested and potentially deployed.

What you should do is drive towards better practices going forward. Clean code is a positive thing and in a healthy development environment there should be a platform to suggest improvements to code-quality. Generally, this platform is some form of code review. Developers can learn a lot from each other and if there are no practices in place at your company for this peer collaboration then fostering this collaboration is where you should put your emphasis. You can even take the approach of saying that you would like to set up code reviews to learn from them -- and you will.

  1. Is code review taken seriously at your company? In training some new developers just yesterday, I made a point of showing a Bitbucket screen where a relatively junior developer (4 years) corrected a very senior developer (40 years). There was even some back and forth. The junior developer was completely correct. If code review is just a formality, bad code will be checked in, and it will also break. Do you think your team's product is stable, or is the code not only poorly written, but results in bad behavior in the field? (Wrong answers, crash, etc.)
  2. Does the company have a Code Quality committee? I'm not that excited by long written standards, because I understand the need for exceptions sometimes (Yes, I used goto in the last decade.) But it needs a team that shares your interest in the subject. Join it.

If the answer is that the company blows off code review and doesn't sponsor any group interested in quality issues, you should talk to your manager about this. Those are big red flags.


Seniority generally shouldn't matter that much when giving feedback. You should respect them, their way of working and their opinion either way, and try to balance that with trying to have a high-quality and consistent codebase. With someone more junior you might insist on doing something because you know better (or that's how the team does things), but you should also try to explain why you're suggesting that, to both convince and teach them, which makes it rather similar to how you should interact with everyone else.

Typically code is commented on during code reviews / on pull requests. A 1-on-1 meeting might more make sense if you're looking to discuss the code in more detail, especially to understand it, or if you're mentoring them.

But this could be vary across different workplaces.

When thinking to which degree to criticise the code of others (or doing anything else, for that matter), I would suggest thinking of a new job roughly in terms of the 3 phases listed below. There isn't necessarily a clear line between them, and you may get to changing things quite quickly if you're hired into a more senior position, but it is a good basic way to think about things (and the fitting in phase is sometimes neglected for those hired into senior or management roles).

1. Fit in

Figure out how the team works. Then do things the way they do them.

  • What coding standards do they follow?

  • How strictly do they adhere to these standards?

  • How do they do code reviews?

  • How are reviewers chosen?

  • Can anyone review anyone's code?

  • To which degree do they comment on coding standards during reviews?

  • What's the policy/stance on refactoring code?

  • How open are they to changing how they work?

    If they're open to it, and you can make a convincing case, you could potentially jump straight into changing things.

These are all questions you can ask, in some form or another, at any point from your first day, or even during the interview process if you really want. You can also find out some of the answers by just checking what people do.

It should always be okay to at least point out a bug or ask how code works or why they made certain decisions, but some workplaces may be exceptions, and anything beyond that depends on the workplace.

If you're having trouble judging this when asking about it or based on what others do, you could leave a review comment here or there and see how that's received. Start with the most significant issues. If it's received poorly, tone it down. If it's received well, you could comment on a few more things.

There's also the chance of finding the way they do things to be much better than how you are used to doing things, or they may have some constraints that make something that would otherwise be good impractical.

2. Prove yourself

You're not going to convince anyone to follow your subjective ways unless you've proven you know what you're talking about.

So prove yourself by doing your job well, writing good code, architecting good solutions, giving meaningful review feedback, mentoring others, getting promoted, etc.

3. Try to change things (if you want)

If you're not happy with how the team does things, this would be the point where you can try to change them.

You can suggest adding or changing code reviews, incorporating some coding standards, adding unit tests, adding automated syntax checking, leaving comments about more subjective things or whatever else.

How do I give good feedback?

Ask questions. There may be reasons why things are the way they are, so don't automatically assume you know better. This is especially true if you're more junior than them, but it also applies if you're more senior. Questioning something may also help lead the other person to the issue without you having to explicitly say it's a problem. "Would this work if X=3?" might be better than "This breaks when X=3", although in that case the difference between the two isn't too big.

Phrase things in terms of "I" whenever you're pointing out something subjective to emphasise that it's your opinion (which they can disagree with without you being wrong). So instead of "This is hard to follow", say "I find this hard to follow". In many cases you can also add "I think" or "I don't think" to "soften" a statement a bit.

Avoid "bad" words like "bad", "awful", "terrible", "hideous", etc. that don't give any information about what the actual problem is.

Point out the problem and suggest a solution. If you just point out a problem, they may not know the solution. If you just offer a solution, they may not know the problem. For example, "I find this hard to follow; you should split it into multiple methods".

"Soften" what you say. I find that criticism comes across better if you "soften" what you're saying a bit with words like "a bit", "maybe" or "might". To modify the above example, "I find this a bit hard to follow; it might be better to split it into multiple methods". This is a bit of a trade-off, as it could also make you come across as a bit more uncertain.

Avoid minor or excessive comments. I wouldn't put a hard threshold here, as necessary comments are necessary, no matter how many there are. But a review with too many comments may be overwhelming or disheartening for the reviewee. If you're leaving many comments, some of those might be related to minor or subjective things that are better left unsaid. A minor spelling or formatting mistake is probably okay to point out if there aren't too many other comments, but if you have a few dozen other comments, it might make sense to skip that one and possibly fix it yourself in that pull request or as part of a later one. Similarly, a comment presenting a different way to do something that's okay as is could potentially also be omitted if there are too many other comments already. Although you should expect to leave a few more comments than usual for new team members.

People are different. Different people may be more or less receptive to feedback. This isn't to say you should treat them differently, but rather understand that what works for a few people may not work for everyone. Try to find an agreeable consistent way to treat everyone, and accept that maybe you have to occasionally tell someone something that won't be received too well.

You must log in to answer this question.