last day (16 days later) » 

22:24
40
Q: Code readability, conventions and should I let go

Stankar0xWhile 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 seni...

It's your choice whether you want to get along well with this coworker and accept not improving the readability of their code or cause conflict by raising this with your manager and/or confronting the coworker in order to be allowed to change stuff. Their response doesn't leave you with much of an alternative or middle ground.
You don't have peer-reviewed pull requests? I think that is a bigger concern than what you did.
hey @Stankar0x - I read the code in question over on the other site I am not a member of. (link just below courtesy Duke - I don't know how to do links :) ) The facts of the case are ...........
The facts of the case are .......... (1) the other guys' code is complete garbage, he should be unemployed. It's "not even a non-starter". (2) if your current company does NOT have a system in place to eliminate such crap {it sounds like the "code quality wiki" is a start - but - newsflash - why do you need such a basic "code quality wiki" amongst professional working engineers???} .... essentially, you'll want to change companies when it is convenient for you
you then have the difficult issue that your boss (or more senior person) you disagree with. this happens in a minute to minute basis in engineering. all you can do is go along with what your boss says, try to support them. (think of it like being .... in the army.) And then, get a new job at a convenient time for yourself.
@Fattie You need coding standards because coding standards are not objective nor universal.
hey @Dukeling - right, each team, whatever, has a "house style". but the fact that such an incredibly basic issue is in the code-quality-manual is a bit whacky. but it's kind of "off topic" from the core of the question. (Which is the usual question asked a zillion times by new programmers - "what to do when i think my boss is wrong" - answer "deal, or leave software")
22:24
Just an observation: fix the bug first in one submit, and then do readability improvements as a separate submission. How can you prove the readability "fixes" won't accidentally introduce another bug?
Incidentally, your code, while IMO better than the original, still doesn't conform to the 'always use curly brackets' rule. I'll leave further criticism to others . . .
Point of clarification - does Senior Dev's personal convention not meet the team's conventions?
Why are the team standards not enforced by the CI / build process ? If there are standards, they build should enforce them. Without being enforced you have coding suggestions which mean different things to different people
Drive-by code fixing without talking to the person who will maintain your changes is very impolite. All other things aside, this is the crux of the problem.
You seem to be contradicting yourself: "This change brought the code closer to what our Coding Convention says" and then "OK, I admit I am guilty I didn’t follow the Code Convention to a T". Did your changes make the code more or less compliant with your Code Convention?
22:24
@terdon It should be pretty easy to tell that he meant to say he brought it closer but not perfectly in line, which is still more compliant rather than less.
I read the history of the question, to see what the code changes were about and.. welp, you certainly didn't follow the language conventions there, and I do agree that the Modulus operand isn't making the life of the juniors any easier. His code wasn't by any means stellar, but neither was yours. And for fork's sake put those dammed brackets there >.>"
I don't know if you were re-directed here, but you might want to consider Programming.se to know if you should be modifying your code for best practice standards in a situation like this - they won't be able to help with whether or not you should speak up to your peer about this, but they will let you know if the practice is the right thing to do.
@T.Sar I agree. Neither is particularly good code.
Frankly, I'm not sure you have a leg to stand on. Next time, if you change existing code, follow the coding standard to a T or don't change it at all. Then you have a case. "Why did you change my code?" "So that it is in line with our standard. Here is where it says, in our standard, to do it this way." Otherwise, it's just your opinion vs. his, and that's just not worth anyone's time, and ultimately doesn't help anyone.
"His perceived seniority is based on the fact he's been there longer than me." That is exactly what seniority is. Seniority simply has to do with the length of employment, or status gained purely as a result of length of employment. If you disagree, please see dictionary.com/browse/seniority, merriam-webster.com/dictionary/seniority, and businessdictionary.com/definition/seniority.html.
22:24
@SeantheBean and yet, at your own 2nd link... 1 :the quality or state of being senior => merriamwebster.com/dictionary/senior 2 a :a person with higher standing or rank so I think there is a pretty obvious colloquial or nuanced usage you're glossing over.

  last day (16 days later) »