Have you ever received a code review report saying that your code is "wrong"?. It happened to me multiple times while working on a unicorn startup,...
For further actions, you may consider blocking this person and/or reporting abuse
Thanks for the post. It is rather long so maybe better as a multi part post but I look forward to going through it more in detail tomorrow after skimming it now. LOL at your Yoda reference. And I like the SOLID etc. you put on there. Going to use those references in my notes.
Regarding dev opinions - using a linter is a great way to get everyone to use the same code style before they submit for code review. You can set it up on your CI that the PR can't even be merged until someone applies the lint fixes and pushes the code.
Regarding the multiple nested if statements - one of the measures around this is "code complexity". Some linters will warn you that you have nested too many levels deep.
Also a bit more of my 2 cents. In my team, if a PR has to go through a
round of changes because it failed code review twice, we make an effort to talk face to face about the PR. Perhaps there is an underlying bad understanding of the problem or inappropriate solution which is better to discuss verbally as at a high level rather than in a few spread out comments that don't make a lot of sense read separately or in the wrong order.
Regarding linters I can recommend prettier for JS and Black for Python. They will lint your code and even format it for you. Best if you setup VS Code to format on save. :)
They both have sensible defaults and pride themselves on limited config options compared with others like Flake8 for Python.
This is to deliberately prevent devs having to decide (ie argue) should we configure the linter use double or single quotes and semicolons or not and tabs or spaces and where do we wrap and when do we use commas...
The linter tells you what you need to use and you stick to that and move on!
And they give a bit flexibility so you configure them if you want.
See also Bikeshedding which in programming means when you argue about style issues like tabs vs spaces rather than on decisions which affect performance, architecture, business logic, etc.
en.wiktionary.org/wiki/bikeshedding
I use Black in python project here locally and in VS Code and in GH Pages
No config. Just defaults.
Makefile
I would say that it's not "your code" that is "wrong" but the attitude of those people! 'Seniors' with that sort of attitude aren't worth being called senior IMO. If the only thing they can say "your code is wrong" and they aren't able to give a decent explanation then they're not worth their salt.
Besides, the verdict "wrong" is of course very ... wrong :-) ... criticism should always be constructive. If your company has that kind of employees then they've hired the wrong people, team players is what you need, not prima donnas.
Indeed. Mentorship and code review and coaching are part of the required skills in my company to become a senior I guess because it makes the team stronger. Having a rockstar developer who can't teach anyone is going to cause a gap when they leave and no one knows how to maintain their code
Right, plus they can be toxic for their team if they have the wrong attitude ...
Great, thorough write up. If you’re interested, @ben and @jess just did an episode of DevDiscuss on this very topic with Rina Artstain that you can find here.
Interesting... I am on it. Thank your for sharing.
Oh BTW on the SOLID part yes knowing that theory (and also applying it) gives one an approach to architecture using patterns and best practices for main table code. So I agree with studying up on those.
I read a book on TDD which said the authors knew a team which have super SOLID code and code coverage with tests etc. So looking at stats the codebase looks healthy. But the codebase was a nightmare to work with because for example the abstraction was so high that you have to look at 7 classes or functions which are strung together in order to make a change or even just read what is going on.
I currently work on a codebase at work which is that spagetthi code in a sense where one class inherits from 5 classes and they each inherit... so it becomes magical and hard to reason about.
I like a philosophy or "Zen of Python". About don't try and be clever and show off how few lines you can use or what obscure code you can use. Rather make the code readable and editable for yourself and fellow devs.
And sorry to hear about your poor experiences in receiving vague code reviews. I've generally had good experience in that regard and I am to give non emotional feedback with reasons. I also separate what I would prefer from my style and what is best practice if we had the time and what is right given the problem (let's not over-engineer and knitpick but rather focus on something else more important)
That problem that you mention about having multiple is a smell identified by Martin Fowler. It is called 'shotgun surgery', I believe that happens when developers forget about abstraction. At some point someone didn't do his job and throw code without giving a refactor.
This article really made my day! It's damn good😀
Very good Insights on how code reviews should be, what authors should do and what reviewers shall do.