67

While chasing a bug, I found a few lines of code I found hard to read and improved them for readability.

This change brought the code closer to what our Coding Convention says, which also states that we should "correct code that isn't written using this style".

Now one of my peers, who has seniority let's say because of the length of his tenure there. Smart guy, devoted to the code, and has written important pieces. Pulls me aside and says:

Why are you doing this? You are changing my code. We are a team. This code has been written in such a way to be meant to be understood from anyone on the team. And has to be maintainable even after 100 years. If you don’t follow the team conventions we have to part ways, or I will talk to managers so you never have to touch that code again. This thing ... will not be understood by junior developers, and I know what you’re doing there, just because I can see the diff. I written this code and know every line in it, and now it is hard to follow it. It is not readable and I write my code so it does make clear what I think. You should do what I tell you to do – this means don’t touch my code, and if there are bugs just fix the bug and that’s it.

OK, I admit I am guilty I didn’t follow the Code Convention to a T, but generally I think my version is more readable than the version before.

I reversed the changes, and just fixed the bug which was a few lines below. But this leaves me uneasy as I am complicit in leaving messy code. It is not the only place in the code base that has long lines and multiple statements on one line, but at least I didn’t have to spend so many iterations with it.

Should I let go, or go and talk to higher ups, his managers, their managers, etc. On one hand for 3 lines it will be stupid, given the fact I recently joined so they don’t know me that well, on the other hand it is a matter of principles and this bugs me a lot.

Clarifications:

  • The person I had this discussion with is not my manager, he is a peer. We are on the same level. His perceived seniority is based on the fact he's been there longer than me.
  • The code convention clearly say that I can change code to make it in the spirit of the code convention. The convention was written by his direct manager.
  • @Fattie - I tend to agree in general that both versions of the code are garbage. I am fan of Uncle Bob, Grady Booch, Kent Back, Martin Fowler, etc... So I know what you are implying. Now I didn't go all the way to make it unrecognizable, as I was afraid just of getting into this territorial dispute. I merely followed the convention in this case.
  • I've got the impression that some of you assume that being new at that place - means unexperienced. Please, don’t assume that.
  • To me code readability is precursor for easier maintenance down the road. I know I can cut corners and leave it as it is. I am going to change ship eventually. Future devs will have to deal with it. Don't need to complicate my life for 3 lines of code. Just wanted to collect opinions if the fight is worth it. In that regard (for this case) the answer by @A.O makes the most sense.
  • Comments are not for extended discussion; this conversation has been moved to chat. – Jane S Nov 13 '17 at 22:24
  • 6
    Looking at the code in the old edit (and speaking as a programmer): I think splitting the long complicated lines into multiple lines is a good change, it makes it dramatically easier to read. The part where you introduce the % operator is more debatable. Arguably, the old code there was easier to understand, particularly for junior developers who may not use that operator often (except for FizzBuzz). Do you think he would have complained so vehemently if you had only split the lines? That said, his reaction was very poor. Be careful with this guy in the future... – thirtydot Nov 14 '17 at 10:49
  • 1
    I agree mod operator (%) adds to the trickery. But even without it I am certain the reaction would have been the same. – Stankar0x Nov 14 '17 at 22:50
  • On a general note I find coding conventions often too detailed and generally overrated. They do serve important purposes: Avoid dangerous habits and make code readable, partly by reducing style differences within a collaborative project. Focusing on details beyond these purposes is distracting and can be counter-productive. I don't want to be mean but: I often suspect people who focus on coding style details to enjoy the power granted by the rules to interfere where they otherwise couldn't. Advice: Focus on the substance. That can include substantial violations of coding rules. – Peter A. Schneider Nov 15 '17 at 10:03
  • @Stankar0x, I answered a similar question years ago based on an experience of mine over a decade ago. You can read it here, and perhaps it will be helpful: How do you stop yourself from refactoring working-but-awful code? I was in your shoes then, and I discovered that there are some good reasons to leave bad code alone (painful as it is). – user1602 Oct 11 '18 at 13:45

12 Answers 12

59

Let it go. You are trying to compromise among 3 or 4 priorities: your preferences, the Code Conventions, the preferences of the senior dev whose code you changed, and the team's ability to make sense of the code. According to the senior dev, his coding style makes sense to the team. So priorities 3 and 4 can be combined. Now it's between you, the code conventions, and senior dev+team.

Between these options, the team's conventions, and your relationship with the senior dev, in my opinion, matter more to the team's ability to get work done, than your preference and the code conventions.

Factoring in that you are new, I would recommend to let it go and learn from it that what you think is right is not necessary what always should end up happening.

I suspect others here might push back and there is definitely room for different ordering of priorities, but this is probably the path I would have taken, at least while still a relatively new employee. Good luck!

  • 65
    Based on the coworker's quote, between the threats, bullying and exaggeration, I very much doubt that they're the type of person to be able to accurately speak for others by actually considering the opinion of anyone else. – Dukeling Nov 12 '17 at 18:33
  • 34
    Re According to the senior dev, his coding style makes sense to the team. I suspect that that is not the case. It is "his code" that this newbie had the gall to touch. I suspect nobody touches his code because (a) it's utterly unreadable but magically appears to work and (b) the guy is so touchy about "his code" that anyone who dares touch it gets a lecture. – David Hammen Nov 13 '17 at 1:08
  • 14
    @DavidHammen: Most toxic coworker ever. Reminds me of a senior colleague who was dead set against code reviews because he didn't want anyone to review his code... – Matthieu M. Nov 13 '17 at 9:43
  • 11
    While I predominantly agree with the answer, I don't agree with the justification of "According to the senior dev, his coding style makes sense to the team." (1) Everyone's code makes sense to themselves. You should check the opinions of others. (2) A senior can enforce coding standards based on being a senior, somewhat regardless of the standard he's setting. The fact that the lead dev engages OP on a personal level ("why are you doing this?" as opposed to "the coding standard is [such and such]") suggests that the lead dev is calling the shots, rather than [..] – Flater Nov 13 '17 at 13:18
  • 9
    [..] following an established guideline that is objectively defined. If there is such an established guideline, he should be referring to the guideline as opposed to being offended by OP's actions, making exaggerated "100 years of maintainability" claims, threatening to remove OP's access to the code, or arrogantly claiming that his code is (unequivocally) not to be touched. If the quote from the lead dev is in any way accurate, there is a massive issue with the lead dev's overconfidence and proactive prevention of anything that highlights an honest mistake (or imperfect solution) by him. – Flater Nov 13 '17 at 13:20
97

So your senior and you disagree on what the coding conventions means and which way is more readable. The way to resolve this is to clarify your understanding so everyone is one the same page.

Therefore, I'd have reacted as follows:

S: Why are you doing this? You are changing my code. We are a team ...

J: I tried to make it adhere to the coding convention. It says we should do XXX, and improve existing code when we see it?

And then listen to what he says and genuinely try to understand his point of view.

And once the technical debate is over (and tempers have hopefully cooled down), I'd address the social side of his feedback. Again, this best works as a question:

What should I do if I see something in your code I don't understand or think could be done simpler?

And again listen what he wants.

This accomplishes several things:

  1. His rather emotional response indicates that he is emotionally invested in his code and feels his expert judgment disrespected. By asking for his expertise, you show that you value his expertise, which should sooth the discussion.
  2. It communicates you did not mean to disrespect him or flaunt social norms. Rather, it makes him realize that you simply didn't know yet what was expected of you, and that you are trying to fix that.
  3. It clarifies what is expected of you to avoid further accidental conflict.
  • 1
    "It clarifies what is expected of you to avoid further accidental conflict." I think the coworker made that clear already : "Touch my code again! Touch my code one more time! I dare you ... " - So I don't think engaging in suggested conversation will be constructive. – Fildor Nov 13 '17 at 14:21
  • 11
    Listening is one of the most powerful tools for gaining trust. Sometimes we get so caught up in showing someone how our way is better we forget that it doesn't matter if our way is better if no one trusts us. And once in a while we might discover our way isn't better! – corsiKa Nov 13 '17 at 19:30
  • 2
    @Fildor: In anger, people sometimes make rash statements they recognize as less than perfect solutions upon calm reflection. In this instance, I believe revisiting necessary because the proposed solution is not sustainable: What is OP supposed to do if he is given tasks that require material modifications to this senior's code? – meriton Nov 13 '17 at 20:14
  • Generally, this is the way I approach problems of this nature. As much as I like your answer in this case it cannot be applied. I have tried before to gauge the expectations, but was brushed off with responses like: “Well the code convention is not followed.”, or “The code has been written long time ago, no body reads it anyway. Everyone in the team understands it and it should be the way it is”. – Stankar0x Nov 14 '17 at 22:43
23

You've already reversed the changes. So let it go.

That being said, the next time this happens. Be sure that there is no wiggle room with your interpretation of the coding conventions. Because, if you're going to have a battle with someone, be sure to pick a battle that you have 100% chance of winning.

This is important especially if you do not have as much social capital with management as this person has already.

Also, be sure to add unit tests before you modify too much of his code. If you introduce bugs to his code, you will never hear the end of it. And when you do find something of his that is worthwhile for you to change. Then, go ahead, but be careful and follow the official conventions (and if not the official conventions don't address a particular issue, follow the Code Complete conventions at least).

If he does complain to you, then you can just tell him to fight on changing the code conventions, and that once he receives approval for those new coding conventions he has in mind, that you will be glad to take a look at them and implement them, but not before then.

Also, you should probably ask management to organize weekly code reviews. After all, if you have a chance to discuss coding decisions during code reviews. It will give you a safer and less confrontational way to understand and/or challenge some of your peers bad habits.

  • 11
    +20zillion for mentioning unit tests. ;D This will make any changes immensely safer to implement! – akaioi Nov 13 '17 at 6:24
  • 4
    @emory, I disagree. – Stephan Branczyk Nov 14 '17 at 0:54
  • 3
    @emory the entire Code Review community disagrees with that statement. – Mathieu Guindon Nov 14 '17 at 12:56
  • 4
    @emory no. unit tests cover whether the code does what it needs to do - not whether it's implemented in a readable way. And if your tests turn implementation details into specs, then your tests are casting the entire code base in cement. – Mathieu Guindon Nov 14 '17 at 14:34
  • 4
    @emory - the OP was trying to fix a bug. The code was poorly written and needed refactoring to follow and only then was the bug found to be elsewhere (a few lines below the refactored part). That's absolutely code I'd need to refactor just to follow it, and when confronted by the "senior" dev their attitude prompt an opportunity to remind them that I wouldn't have been fixing a bug if they hadn't left one there and wouldn't have had to refactor if it had met the corp standards. The only "out" I'd give them is if that section of the codebase predated those (which it may well do.) – James Snell Nov 14 '17 at 17:41
11

Saw a number of answers saying "Let it go". I can't agree with that, because the issue is going to come up over and over again.

A team needs standards, specifically to avoid arguments like this. I'd say to talk to your co-worker again, once tempers have died down a little. Ask him to review the coding standards with you, and together work out -- if possible -- how you guys think the code should be organized. If y'all can come to an agreement, coolio. If not, recognize without rancor that you're just not on the same page, and get manager to help you two resolve the issue.

You deleted your specific example (which I thought was too bad, it offers some context), but let me just add one thing... That one potentially tricky part (incrementing index modulo count to wrap back around to 0) should be understandable to J Random Programmer, but it wouldn't hurt to drop in a quickie comment on top of the line as a reminder... ;D

  • I agree that the specific example was helpful, and it is related to the modulo:because he didn't make the index calculation easier to read, he changed the index calculation. It was oldindex+ 1 >= count ? 0 : oldIndex +1; new is oldIndex +1 % count. oldIndex value 4 and a count of 3, old = 5>=3 result 0. New = 5%3 result 2. Perhaps that doesn't happen, still...code has both syntax and semantics. Changing semantics effects readability -- it changes the concept, even if it doesn't change the result. Fixing a boundary error in a for loop, doesn't change the intent, rewriting with LINQ might. – jmoreno Nov 16 '17 at 1:00
  • IMO in this case the modulo changed not just the calculated value, but the concept of what was happening. Now the oldIndex is a wrappable or circular index, where 0, 10 and 2,145,690 are all the same, equally valid value for a count of 10. – jmoreno Nov 16 '17 at 1:09
11

A lot is going on

There are quite a few things going on here. Many of these have been addressed, and I will briefly mention them and my vision of what you should or shouldn't do about them. However, there's one point I haven't truly seen addressed yet which I take to be the biggest problem and I will address that in more detail.

Bugs & style fixes

You fixed the problem while at the same time changing the code.

In my opinion, this isn't a big issue, but it is generally better to keep separate things in separate commits. This would have had the added of separating the conflict from the bug fix here as well.

Differing views

You disagree about what version follows the conventions better, is more readable and is more maintainable.

If he's your senior, it might be good to follow his lead on this. Alternatively, it might be good to have someone else from the company have a look at the code together with both of you, looking at it from those three perspectives.

Tough code

The code was hard to understand to begin with.

I've found it to be useful to ask questions about the code in such situations, especially if you know who wrote the code (and/or the code was written recently). Asking questions can both lead you to understand the code better and help the other person see why the code was hard to understand to begin with. This often brings up ways the code can be made clearer all by itself, which you can then discuss before implementing them.

The attack

The other person gets pretty aggressive by suggesting that you have to part ways and that they are going to your manager to keep you off certain code.

As written, this is pretty serious. It depends a bit on which way the difference between the actual things he said and the way you paraphrased him here sways, but purely by the way you have written it, this alone should be reason to talk to your manager. If you do, don't make it about the code, but about his attitude towards you where he seems to consider making threats to you okay.

His code

He doesn't want you to edit his code.

In my opinion, this is the biggest problem. This is a pretty toxic attitude and I think it strikes at the core of the issue. Hobbyist programmers are often very protective of their code and sometimes that can be seen with professional programmers that have worked on code by themselves as well. However, it does not have a place in any professional setting and is extremely toxic when you're responsible for the code as a team.

Ultimately, the problem is that the code isn't his. The code is the company's or perhaps the team's, but it's not his. Instead, it is code he has written. I think it's important not to refer to it as his code, but as code he has written. Without stating it isn't his code or correcting him when he says it's his code, it can be beneficial to consistently refer to it as code he wrote yourself. This goes both for the moment you are discussing the situation with him, as well as when discussing the situation with other team members or a manager.

First of all, I'd say that I would let this incident be what it is for now. However, I would also not go out of the way to avoid touching code that he has written in the future. It sounds like this is going to be a thing that is going to come up again and walking on egg shells all day isn't going to be of help to anyone.

Assuming you're in a team with more members than the two of you, the first thing you could to is talk to other team members about the issue. Ask them if they have experienced the same behavior and how they handled or would handle it. Your coworkers are a resource that shouldn't be underestimated. If there are no other members on your team, you can instead ask other colleagues that have had to work with this person in the past.

Then, if this programmer turns out to be overprotective of the code he wrote on another occasion, look to apply the suggestions from your colleagues. Beyond that, you can also ask him if he is upset because you changed code he wrote or that it's that he thinks the change affects the code negatively. These are two completely separate issues and it's important to make him aware that they are.

After that point, it seems like it would be time to escalate the issue. Talk to your team lead or manager (whichever is more appropriate) and explain to them that this person seems to take issue with you touching the code they wrote. While you are open for discussion about the quality of any code (which you should be), it seems that it's seems to be the fact that this person wants the code that he wrote not to be touched (by you - if that's what your impression is). Explain how this falsely partitions ownership, responsibility and understanding of the code and how that's detrimental to the product and increases the way the company relies on specific people. (Of course, such an explanation shouldn't need to be nearly as detailed when talking to a team lead as when talking to a manager.)

In conclusion

So, essentially, I think that the most important thing is to separate out the different issues and tackle them individually. This should also make a it a lot easier to deal with each one of them, as you now know what's going on.

  • +1 for the Hobbyist code - many older coders come from that Hobbyist/RockStar school of "it's only other people that make bugs" and it takes a good deal of self-reflection and experience to realise why we all need the other aspects of software engineering like unit testing and readability. – James Snell Nov 14 '17 at 17:46
  • I appreciate breaking down the problem as you did. I usually use a technique by Martin Fowler and refactor the code as I go along to make better sense to me or simplify it. So the style changes and bug fixes as well as tough code are all addressed by it. This is great when you have unit tests to back your assumptions. But also works when you do manual test while chasing bugs. Also when original programmers are unavailable, or when people are busy and don’t want or have time to discuss details with you.[..] – Stankar0x Nov 14 '17 at 23:11
  • [..] Here some references for those of you interested in this: link or his book: Refactoring Improving the Design of Existing Code. – Stankar0x Nov 14 '17 at 23:12
  • @Stankar0x Interesting read (though a little dated). I'm not entirely sure that's what you're implying but under the "Bugs & style fixes" heading I never meant to say you shouldn't do that. I just meant to say that it may be better to do refactor-commit-refactor-commit-fix-commit, instead of refactor-refactor-fix-commit. I know I'm a bit on the extreme side when it comes to separating different things in different commits, but what I was saying is that if the bugfix was a couple of lines below the refactoring, it might have helped de-escalate the issue somewhat if they were separate commits. – Jasper Nov 15 '17 at 11:06
10

I don't think you can let this go: your colleague is very protective of the code he has written. So every time you change a piece of code he has written and does not have a bug, he is very likely to come to 'talk' to you about it.

And your colleague is trying to bully you, make no mistake about that.

First off, the code you are both working on is not his nor yours. It is owned by the one paying you.

Second, you should not do what he says you should do, unless he is your boss, what he is not. You should do what your employer wants you to do.

Third, I agree with you that you should leave things better then what you have found, so making the code more readable is good thing.


So, you don't want to complicate your life for 3 lines of code. Good on you! But what are you going to do the next time this happens?

I suggest to get your team lead / manager involved next time this happens. See points, one, two and three.

  • While all are valid points this situation happened to me and I can advice to avoid conflct and scalating. People are not logical, the senior guy can fell is as an insult "you code is lame, I'll change everything in a way make you looks pathetic" yeah people can be that sensitive. Management is always witth hand full of problems and can think "oh gosh now I need to waste my time solving conflits with between two big kids, and why the new guys is wasting time fixing working code what matter if it's ugly". Of course it depends a lot of you colleagues. If I'm the senior i cannot care about it. – jean Nov 14 '17 at 12:51
  • Team member conflicts are the fatest way to make your life miserable – jean Nov 14 '17 at 12:53
  • @jean, agree that team member conflicts are miserable, but the colleague here might be starting such a situation. I think that situation is best solved by making it the problem of the team leader, it's his / her team. – Flummox Nov 14 '17 at 13:04
7

While your "senior" peer may have overreacted, you should also consider the possibility that he was right in a way.

Unfortunately, the edits removed the code itself: maybe this is typically better for most non-IT users, but in this case an important piece of information was lost.

The way I see it, you've improved the readability of code by most formal metrics: the conditions are more logically organized, there are braces and stuff. However, your introduction of the % operator (integer remainder) is a Clever Trick that will confuse 90% of junior developers, and apparently some seniors as well. Now, overly verbose code is bad, redundant checks are bad, but Clever Tricks are just horrible, unless you are sure that all developers are at least as clever as you are. And you've already got the proof that they are not.

Also, while your company documents say that you should "correct code that isn't written using this style", it would probably be better to mention such changes to the original author, if he or she is available. A quick message along the lines of "Look, I had problems debugging this code, don't you think it's more readable this way?" could have saved you both some embarassment.

  • +1 for asking for input before making the change. That can already defuse these kinds of situations before they're happening. Most people with such a strong sense of code ownership will be ok with changes if they can leave their mark or do it together. – CodeMonkey Nov 14 '17 at 8:14
  • A trick is only clever if it works. In this case using % instead of the conditional changes the behavior. The resulting code may be incorrect. Tricky correct code is always better than "clear" incorrect code. Of course, with care, you can achieve clear and correct code. – Brandin Nov 14 '17 at 11:08
  • @Brandin IMHO there was no behaviour change. The original code said basically: (if x+1 >= length then 0 else x+1). The Clever Trick was to rewrite it as ((x+1)%length) . That's exactly the same. The main difference is that the Cleaver Trick is not obvious. – IMil Nov 14 '17 at 19:39
6

When you are new it is usually a good idea to take some time to observe the dynamics and get a feel for the team, etc before taking it upon yourself to make any big changes. A good rule of thumb is "If it ain't broke, dont fix it", or a corollary to that, "If it was broken, someone would have fixed it by now." If the code in question was a big issue, it probably would (or at least should have been fixed by now. It is possible that some code you come across at this job will need fixing, but wait until you are more familiar with the group's system before you decide if any such code qualifies.

Despite how much effort goes into developing universal coding/documentation standards, EVERY WORKPLACE WILL HAVE THEIR OWN STANDARDS and it takes time to infer what exactly what those are.

  • 2
    Re the CAPS... Standards are manifestly not standards if they have to be "inferred"... They're standards if they are clearly defined in policy documents. Something that only exists as a set of social dynamics and conventions, passed along by observation alone, is not a standard. No workplace can enforce that, especially not with the aggressiveness of the colleague in question here. I don't think the advice to sit down and shut up is good; if the OP's employer has formal standards, s/he is right to be baffled by such a contrary and emphatic reward for adherence (assuming they read it right) – underscore_d Nov 13 '17 at 18:57
  • 2
    Is that just double yelling or triple yelling when it's all UPPERCASE AND BOLDED? :) – cale_b Nov 14 '17 at 3:35
  • If it was broken, someone would have fixed it by now. - you clearly never worked on proper legacy code. :) – Daniel Nov 14 '17 at 9:41
  • @underscore_d Oh trust me I agree with every word you said. But in practice, I think we both know how fuzzy some "standards: can be. – Bryan Goggin Nov 14 '17 at 18:01
  • @cale_b No it was more for emphasis. See other answers where headings are emboldened or upper cased. I was just doing that in the middle of a sentence to emphasis a main point. – Bryan Goggin Nov 14 '17 at 18:02
5

You might let this incident go, but you shouldn't feel in the wrong. You just have to pick your battles.

Your mentality is absolutely correct. Whenever we revisit old code, if it is not easy to maintain we must first comprehend. Once that comprehension has been achieved, if we don't document or ease the route to that comprehension, we're wasting the time of further maintainers down the road much like the original programmer has kicked that cost down to you. Therefore its the duty of any programmer worth their salt to either document or improve code as you go (the latter is less risky when you have a robust and exhaustive test suite that ensures refactors and improvements don't break functionality). Leaving comments is not ideal, but it does leave a smell that the code needs some love in the future.

Your team is also taking the correct approach to coding conventions. Conventions are not rules, so there must be some leeway to breaking them when it makes sense to do so. But they do represent an ideal that may not have always been achieved and it's more practical to enforce code conventions on new and updated code rather than exhaustively refactoring. In an ideal project, the code should never look like any specific individual wrote it. This enables all developers to check their ego at the door and treat the code as the team's code, worry about code quality rather than who wrote what.

This brings us to your colleague. Their behaviour is not unusual as you might think as programmers do have a tendency to feel pride and ownership over their code (or look to blame others, sometimes, only to see themselves in the history) and can develop ego despite efforts to resist doing so. Others do not resist at all. "My code" is a bad attitude to have, all code should belong to the team. There is an inconsistency between his claim that he's coded things to be understandable by the team if it is in contravention of the team's coding guidelines. He makes it clear that it's coded to be understandable by him first, not the team, and he's upset that you've ruined his comprehension by having the audacity to change things (all good code should be written to be easy to maintain and delete, not be immortal).

It gets worse when he claims that you can never understand his code and threatens to use his influence to put you out of your job. That alone is massively unprofessional, but in response to a little formatting is hugely blown out of proportion.

You do need to pick your battles and I'd let the formatting side of things go. If you're new on the team and someone says that you're not meeting the code convention, all you can really do is defer to their experience and suggest you update the documentation if it has become misleading. If this causes friction, then you need to appeal to your boss as keeping one standard on paper and another in practice (especially at the behest of individual developers) is discordant and undermining.

However, I'd be far more likely to focus on the threat levelled at you. If it was done verbally, you don't have good evidence at this point and you have no good option to act at this point, but I'd be very careful around him in future and try to get everything he says in writing, e.g. interact by email as much as possible. If he threatens you again in writing, you can start to talk to your boss about it. From the sounds of things, his ego is fragile enough that you're going to run into problems with him again, sooner or later.

Where you might run into problems is that you don't know the lay of the land. If this guy really is as bad as he seems, it's possible that large swathes of code have been written by him and have become unmaintainable by anyone but him. As such, he's effectively made himself unfirable as the company can't afford to lose him, which has enabled his poor behaviour. This can be good in the short-medium term for job security, but long-term it's degenerate, counter-productive, and can ruin your reputation. Don't aspire to this.

With that in mind, be careful you're not putting the company in a position to choose as they will put the interests of the business first. Treat your current job as a learning experience in dealing with difficult people. Spend time with your colleagues and work out how they deal with this guy and don't pass up the opportunity to learn other stuff from them too. In the meantime, keep an eye on other opportunities and be ready to land on your feet in case he does have the clout and does kick you out for some other perceived infraction.

4

The answer to this strongly depends on what is the general approach to "process adherence" within the company, and you should have already a feeling about this.

I have worked in a company which was, on paper, certified ISO9001, but the CEO always bugged us with quotation requests made by quick and incomplete phone calls, though we had a request form where all the info could be collected. You probably guess that in that case enforcing standard was a lost battle.

You have two alternatives:

  1. The company has a culture of process adherence: you are right to follow the company Coding Convention, and if the thing escalate, just make clear you followed the standard.
  2. Process adherence is just on paper: forget about the manual, and follow the "hint" of this senior.
  • 1
    Process adherence is just on paper. Key to survival, don't make any unilateral decisions and make sure you document your adherence to the company processes, in triplicate if you can. If someone is checking for actual productivity you might have to take some risks and do something creative but be careful not to stand out. – Jodrell Nov 13 '17 at 15:01
0

"The code convention clearly say that I can change code to make it in the spirit of the code convention. The convention was written by his direct manager."

I can't help thinking this is a terrible directive. By all means generate a ticket for "improving" a specific section of code, or bringing it up to standard, but "change it if you don't like how it looks" is a recipe for disaster. How do you control these changes when they're made as part of another fix? How do you test the changes?

0

As far as I see your problem with your peers code is the formal code convention of which you have a different understanding then the senior.

There are lots of tools out there which will automatically format the code for you by a set of configurable rules. They don't leave much space for interpreting the conventions.

So in a regular team meeting (a scrum-retrospective preferably) you might refer to your discussion with the senior an that you feel unhappy with the situation and then suggest that your team (or at least you) starts using such an auto formatter and nominate the senior to configure the rule set.

Next time you change the seniors code (to fix a bug) the reformatting is done by a tool that he configured. I'm curious how he can put the blame on you then...

protected by Community Nov 13 '17 at 13:32

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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