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, also very often I couldn’t understand what was wrong. It is really annoying having people saying that your code is wrong when they are not capable of explaining why💢 or worse they are not willing to do so.
At some point, I was obsessed with understanding the difference between Good😇 and Bad😈 code. This article is meant to share with you what I learned in my road to discover what differentiates great code from bad code. Hopefully, I will make you understand some ideas that some senior code reviewers couldn’t explain to me without the jargon of a senior dev with poor personal communication skills.
The quirks of code review feedback
There are so many things to say about code review feedback, I am going to condense the more important parts to understand the topic we are discussing. What I am trying to show you here is that code review feedback very often has involved communication problems 💬 and we as developers tend to have this kind of problem.
There are some exceptions but many professional developers are not very sociable persons. It makes sense we spend most of our time sitting in front of a computer. In fact, it is very probable that you are a developer because you are an introvert who prefers to spend more time in front of these devices than dealing with people 👀. And if you are an extrovert, hold on, there is still a chance to teach you how to program better 😅.
Our job is to communicate with computers, especially to command computers to do specific tasks. We don't interact with people like doctors, lawyers, or almost any other job, the most we code the less we interact with people(unless you are doing pair programming). Even worse on these days of the pandemic, we are more isolated than ever😷. So, compared to the professionals of other careers. It is very likely that we have personal communication skills on average or under the average.
So, with that being said. Let's explore code reviews.
The technical jargon
On one of the many code reviews to my work, a coworker put something like 'you are not using technique "X"' -rejected- and I was like wait a second how I was supposed to know that there is consensus to use technique "X". After talking to the CTO about the technique "Y", I used, it was accepted because the trade-offs of technique "X" was not worth the time to correct the technique "Y"💵.
After that, I went to the guy who did the review and asked him about technique X. He told me something like 'use google'. Then I realize that it is unfair to work in these very abstract things and having to interact with people who don't want to convey how they think🤔.
I did everything I could to learn technique "X" and what I found is that I didn't need to use it. Why do developers do this? Why do we avoid having conversations about how to program? What works or what does not work? Where does "A" work better than "B"? Where does "B" work better than "A"? Why do we cover our arguments under the appeal to authority or the 'this is the way we do this' or 'our "X" language senior says that we should do it this way' or ‘this approach is similar to the syntax of "X" framework’?🤨
Avoiding confrontation
We, humans, we are biased very often, that's why we need to make decisions based on the evidence. I have no doubt that the reason for many data-driven decision companies to be successful is that they are not taking the gut decisions of their managers but they are listening to their clients and more important to the data of their clients 📊.
We like to think that we are really good at what we do and very often we don't want to share our work with others, or we don't want to review the work of others. Be aware that the whole point to make anything meaningful is to work in a team. So, this is as important as making code or more, because it doesn't matter if you created a super-optimized snippet of code if it is unmaintainable 🙃 someday someone is going to delete it and create another version with its own new characteristics.
Share ideas, avoid technical jargon, have real conversations with your team, and be straight to the point when giving feedback is a good pattern to maintain the good health of a code base allowing it to be scalable. Do not be fooled by the urgent things that put you away from what is really important. Always take time to explain what you did in your code and even more important, always be willing to listen to others' ideas about their code and yours💡.
I don’t like how you did this
Sometimes there are code reviewers that forget that they should share the reasoning behind the review result. I have seen people rejecting code with arguments like: 'It works but I don't like how you did this', the objectivity 🎯 in code reviews is vital. Please, don’t be this person if you are going to reject the work of someone else at least have the decency to explain why. Otherwise you are making your code base unsustainable, if someday you need help with code reviews you will need to train at once a team member on your code review process, do’s/don'ts, style guide and rules.
I know this situation sounds silly, although, I have found projects with people who think that by being the only one who can approve pull requests they are protecting their jobs 🤢 . Furthermore, they reject any attempt to change the code style from whatever he/she perceives to be correct without having any objective reasons to behave in this way.
This is an anti-pattern of code maintainability, the so-called 'Single Point of Failure' and should be avoided because it is very dangerous to the code base. I saw an extreme case of this. I worked with one of these code owner 'dictators' . He sabotaged the git repository of the project and stole 😈 the clients by making it impossible to access the code again.
Most of the time, the correct way to give code review feedback when you believe it needs some corrections is starting with what is working fine and then switching to bad things like not following a style guide or not using a good practice. Anyway, there is a case when the coder has completely failed ❌. In this case my suggestion is to be as objective and direct as possible without criticizing the author and just focus on criticizing the code.
The mythical good reviewer Senior Dev
We need to address that there are some cases where code reviewers are very wise good mentors. You can learn a lot from them and not only knowledge sometimes they even teach you how to teach. They give you suggestions about how to fix your code 🛠️ and some ideas about how to improve your code and which trade-offs in an specific code snippet you should be aware of.
Value this kind of senior because they bring stability to the team. They allow the team to be more scalable, meaning you can add more people to the team more easily, also, 👉 good code reviews improve the quality of the code in general 👈, improve the communication among team members, creates trust, show respect, transparency and humility.
About Good and Bad code
Good and Bad are subjective concepts that usually underlined an specific set of values that vary from project to project, even from team to team. When someone is telling you that your 'code is bad' it is not an universal truth, you should think on this way that what he/she wanted to tell was that your 'code is not what he/she would consider to be good code' 🙃.
You should always need to ask why your code is not covering the expectations. You need to adapt your code to your team values, if those values are already defined if not you need to encourage the discussion to figure out what is the correct way to do things otherwise it will become a complete mess down the road 🛣️.
Your code and values should be in alignment with each other. If this is not the case to solve this discrepancy we need to have principles because there is no process, style guide, set of convention that we can use to address all possible cases on programming. Principles 📜 help us whenever we need to take a decision to define what are the important things for the company or project. And principles can be formulate base on values.
For example, there are cases where speed is more valuable than readability, this is very unlikely but it happens. However, most of the time I found companies that do not understand what are their values, and why they do what they do, they don't have principles most of the time they are improvising or relying on the experience of their software engineer, which sometimes can help but you should avoid the Improvising Driven Development. It is a recipe for disaster 💥.
Now, that we have talked about good, bad, principles and values. We can talk about the more broaden good and bad characteristic of code.
What is wrong
We know that 'bad code' is not universal 🌎. However, there are some more general characteristics of 'good' quality code that usually bad code doesn't have and this is what is wrong with your code most of the time. Normally you won't hear these concepts explicitly mention in online courses, or even in university courses about CS.
Professional developers learn about these concepts by reading books 📚 about programming and learning from experience the concepts and methodologies used in different companies. It is people who are serious in the business of software who are interested in these concepts. The next list outline what, I believe, we as professional programmers believe is a set of broadly accepted characteristics that moderate the quality of code:
- S.O.L.I.D. principles
- Code smells
- Refactoring
- Design Patterns (GOF)
- Separation of concerns
- Abstraction
- Function purity
- KISS
- DRY
- YAGNI
You need to understand the concepts listed above, if you want to become a real professional developer (a person who profess), one of the things that. These concepts allow you to have smart conversations with other developers thus you can collaborate with them to create good quality code 🏆.
Why is wrong
Ok, someone is telling you that your code 'is wrong' but why?. It means (mostly) that you have failed ❌ on one or more of the characteristics enlisted in the previous section.
You are not applying a well-known design pattern that could make your code more predictable in a place where is a good fit 🧩 to use it and you can't justify why you took this decision. The reviewer identified a flaw in your logic that is 100% based on his/her experience 🧙♂️ and believes it is going to be a problem later. You are not following S.O.L.I.D. 🦉 principles these principles serve as guides to create classes that are flexible and extendable. Your work has code smells 🦨 and you can't explain the reason for this which means you are trying to push to production code that needs to be refactor. You are not following the style guide or code conventions of your team 📑. Your reviewer is biased and he/she thinks he/she can do it better but he/she is just bluffing, in reality, he/she doesn't like your solution and probably is because he/she is not aware of the trade-offs related to the actual problem 🙁 in this cases you can say 'please, can you take a second look and focus on...'.
In order to be a successful software engineer you need to have the skills to explain your code. You need to defend your reasoning 🛡️ behind each decision. Otherwise, your code will be rejected more often. This topic is a complete world in its own and I would need to write another article about it but for now.
If someone is rejecting your code and you believe your code is correct, express your intentions about what you were trying and then ask why that specific approach was wrong if these other number approaches didn't seem to be good enough, show that you evaluate trade-offs between different solutions and show that you studied 🔬 the details about the problem that you are trying to solve.
These are the main reasons that I have found, to receive negative feedback on code reviews, when collaborating with teams. With the What & Why out of the way, now I can teach you one detail to have in mind when creating new code and extending existing code, that will improve or at least main the quality of your code base 💪 .
How to write good(general) code
As mentioned earlier good quality code have several aspects, however very frequently I found developers in all kinds of organizations doing this one mistake that increases 💹 the code base cost maintainability.
We can do an exercise to explore this topic, we can create an hypothetical case where we are working in a project, I am going to explain the situation and the decisions involve in creating the code base. Especially, we are going to explore how the way the code is written affects the readability of the code base, all this using a story about boxes 🔲.
Once upon a time
A junior developer named Bill 🧒 just graduated from university starts sending his resume wherever he could. On the other side of the same city, there is a guy named Terry 👨. Terry doesn't know anything about programming. The only thing that Terry knows is that he wants to build an app or webapp. Normally when he explains his ideas to other people he says he is creating the Uber/Spotify/Facebook of insert_any_words_here.
One day, Terry realizes that he needs to start looking for software developers to create this big original idea the Spotify of Italian Soups Recipes or something as weird as this. He is determined to use the fortune 💰 of his grandfather 👴 to make himself more wealthy 🤑 .
Terry starts asking his friends, he was looking for developers, so he can hire them, but he didn't have luck. Then he remembered that one of his friends had a successful business related to computers. His friend Ken had a computer store 🖥️. Sure, 'he needs to know where I can find developers', he thought. He went down to Ken's computer store asked the people in there it seems that nobody knew who Ken was. He made a phone call and Ken told him that he usually doesn't show on the store. Ken only shows himself 2 or 3 times per year in his store.
Eventually Terry asks Ken the big question 'Where can I find developers?' 🕵️ , Ken replies that 'I have no idea, the only time I needed a programmer was the time I end up paying to use a program to create web pages'. Next, Terry starts googling he find out some websites for freelancers.
He is interested in hiring the best developer of his city. Very soon he realizes that paying a software developer expert is expensive. However, he continue with his ideas, he needed to find an alternative. Eventually, he paid for an advertise in the local newspaper job and classified ads 📰.
Two days later Bill in his desperation to get a job finds Terry's ad. Next, Bill calls Terry, eventually they met and Bill along other 2 just graduated, started creating the 🌈 Spotitaliansoup 🦄 platform.
The project starts
Everyone is happy 🤗 , especially Terry. He is building a platform at what seems to be almost a tenth of the original estimated price defined by a senior software engineer freelancer.
In the meantime Bill and the Boyz 👬 start to think that after all, writing 📝 production code is not that difficult. Everyone is coding driven by what each one believes work better. There is no consistency across the entire code base they are even using multiple tools to do the same job. They didn't have a timeline, the only thing to moderate the project duration is the decision of Terry willing to pay at most 6 months of salary to each one of his developers.
☣️ 28 Days Later ☣️
One day Bill is creating the soupImporter module from scratch and as part of that module he decided 💡 to create a function called soupFoo,this function receives 1 argument then it reads a given list of soups files, next it gathers all text. After that,it goes through a transformation maybe capitalization of titles or filling empty data with default data. Finally, outputs processed data inside a JS object. It was something around 15 lines of code.
Here you have soupFoo logic in a diagram with boxes!!!:
All coming days of this project were a collection of trial and errors 🐘 .
☣️☣️☣️ 28 Weeks Later ☣️☣️☣️
The personal deadline of Terry was near and Terry found out they didn't have anything to release 😟 . Of course, they had some good prototypes and parts of the system doing what was supposed to be doing but nothing with the shape of a Minimum Viable Product.
Suddenly Terry starts asking very often to Bill and the Boyz 'why is it taking so long?' 😠 . Each of them tell Terry their current status and they can't give him a real estimation because they had no idea what they are doing. Terry starts to feel that if they continue at this pace. It is going to be more expensive than expected.
He starts looking in the internet how to simplify the problem because at this point he has spent a lot of time and money 💸 , he learned that he needed a project manager, he read about using APIs to spend less time generating content.
Eventually, he discovered an API that serves Italian soup recipes 🍲 on demand. He hired a friend's cousin named Karen 👩 . She studied arts but she was looking for a job. It was good timing for this project. After a meeting Terry presented to everyone Karen their new project manager. Her job was to maintain developers focus on the important tasks.
First task for Karen was to manage the integration of the Italian soup API. She talked 🗣️ to Bill about this task. Bill told her that the implementation could be done right away. He knew that he could implement the use of the Italian soup API inside the soupFoo function.
Next day when Bill was implementing the API. He realized 🤔 that he could put an if/else inside the soupFoo logic and send a new flag as an argument 👿 , in a way that he can know when they are going to read soup recipes from soup API or their own soup recipes. He knew that this was an ugly solution but it was a quicker implementation.
Here you have soupFoo logic v2 in a diagram with boxesss:
This was the last commit of Bill in the project 😔 . He found a better job with a friend from university . The boyz and Terry didn't see it coming. Terry rapidly hired another new just graduated junior Bob 🧑🎓 . Hopefully Bob and the boyz could manage to finish the project on time before Christmas.
18 months and 1,543 commits later...
After 18 months, no one remembers Bob, Bill, Karen and the Boyz 🌵. There are all the time new workers being hire also workers quitting their jobs, Terry's family is advising him to end this project. But Terry doesn't want people to think that he was a fool 😡. He continues pushing the project throwing more money at it 💸.
One day a project manager named Mark 🤓 , asked a new hired call Tim to replicate and fix a bug found in their end-to-end tests. It has something to do with how the application is reading soup recipes.
And this is what Tim found 😨 when he goes through the code for the soupFoo function version 17 diagram with boooooxxxesssss:
Now imagine poor Tim 😐. He needs to discover which of those boxes is related to the reported bug. He starts counting how many cases does he need to test because he doesn't have more information about how or when the bug is happening. The only thing that he knows is that something is terribly bad 👻 with this function. This situation amplifies his impostor syndrome 😢 and what happens next is Tim notifying Terry that he is quitting his job.
The Final Act 🎭
Finally, Terry has had enough 🤬 he decided he is going to hire an expert to finish the project. Now, the money is not as important as ending up looking stupid 🤪 to his friends and family due to all the wasted money. After googling 🕵🏽♂️ for a while he found Steve an expert with 20+ years of experience doing software engineer 🦄 with perfect score in a freelance platform.
Some meetings and negotiations later, Steve presents a plan 📉 with a timeline 📅 to Terry. With the following tasks in this order:
1)Documenting existing code 📄
2)Prioritizing code 🆙
3)Based on priority creating tests 🟩🟥
4)Refactoring the code that needs refactor 🧩
5)Documenting new code 📄✖️2️⃣
6)Creating new tests 🟩🟥✖️2️⃣
One day when doing the refactoring milestone, Steve 🦄 found our beloved soupFoo 🧟 function. He remembered a line from a tv show 📺 he used to see on the late 2000s. Instead of criticizing the bad shape of a computer in this case it would be the bad quality of code 🔫 .
Steve did what was needed to be done 🦉 since the beginning and along the development of the project. He identified all the needed functionality for a MVP(YAGNI) 👍, removed repeat behaviors(DRY) 👍 and simplify logic(KISS) 👍.
After 3 months of work he created 12 functions to divide all the soupFoo behavior 🌈:
Steve was amazed by how well he abstracted all functionality. He was glad with the result of his work 🎖️. He finished all the work considered in his original timeline. After this Terry 😃 could invite his family and friends to the alpha test of his 🌈 Spotitaliansoup 🦄 platform.
Some weeks later Terry got his first subscribers and then he realizes that the rate of revenue would be very slow 😓 because not many people was into italian soups. However, he asked Steve if they could adapt the platform to distribute digital books 📚 instead of italian soups and due to the flexible design this was indeed possible. The return of revenue of that project worked better for him at least to recover his money.
As the years went by many contributors of the 🌈 Spotitaliansoup 🦄 platform remembered those months of work with shame 😳 . Terry learned a lot and very often he questioned himself: 'what would have happened if I didn't find Steve 🐴 ?'
The Yoda paradox
The story is over ✋, now in this section I would like to talk about a topic related to code quality. Which is what happens when you reach the higher levels of code quality 🏆 in software engineer.
I have been lucky enough 🧐 to work with software engineers like Steve with 20+ years of experience. At higher levels of good quality code, I have found an interesting pattern. Most of this engineers when they give their wisest advise is something among these lines:
"You should know how to refactor so you never need to refactor again"
"You should know how to use design patterns so you never need to use those design patterns"
Every time they say things like these I have seen other co-workers saying that it is none-sense 😕 . But I believe these experts are not lying, these advises convey deep wisdom which is needed to create great quality code, hold your horses because there is a catch. What they really want to say is that:
You need to get wisdom 🧙♂️ , discover the liberty that exists within the limits that are defined by the principles, techniques and methods that drives software engineer
This means you should not blindly follow all the recommended principles, techniques and methods 🤖 . It is better to understand the "Whats", "Hows", "Whys", "Whens" and "Wheres" for each of this resources 🤔 .
And don't get me wrong, I am not saying that you should not follow those principles, techniques and methods. Definitely you will have a phase where you are going to follow them blindly because it is a process but that doesn't mean that it would be the same forever 🕰️ .
To get to this level of mastery, you need to take a long journey ✈️ reading dozens of books 🏝️ , spending thousands of hours programming 🏞️ , spending hundreds of hours working with other programmers 🗻.
I believe this is what Yoda wanted to teach to Luke Skywalker in one of the Star wars movies.
"You must unlearn what you have learned"
Yoda
I truly believe that this should be the goal 🏅 of any true software engineer who wants to be at top levels of competence. There are no shortcuts, anyone saying that is easy is lying, what is really needed is hard work 🏋️♀️ , take smart decisions base on all the data you can access at the given time and you need discipline.
Conclusions
About Code review
Remember the aspects of code that should be reviewed:
- Correctness and comprehension 📝
- The general style (code convention, style guide) 📔
- Language specific readability 👓
Defend your code 🛡️ on review if it is wrong demand an explanation.
Suggestion: When a code reviewer is proposing another solution that is equivalent to the original code, changing what was already done is more expensive 🙅. Original author's code should have preference unless there are clear identified downsides.
Code reviews should be considered an opportunity to share knowledge 🧠. Code reviews should be as important as writing code because the code base belongs to the whole organization it is not of the author, that's why it is needed that someone from your team review your code. Developers should find ways to overcome the anxiety and stress that come with code review. Everyone can collaborate to make this process less stressful by communicating in a correct manner.
About writing better code
My recommendation is that you should use those boxes diagrams (used to explain the story about the Spotitaliansoup) all the time. Be aware of the logic branching 🎋 generated by the used of if/else statement, any time you find yourself writing a function repeating this pattern of throwing an if/else inside an if or an else you are doing something wrong. Of course, there is a minimal amount of if/else that you need to have in order to create any program.
My recommendation is that a function in the worst case should have an if (if/else) / else or if / else((if/else)) anything deeper than that you are doing it wrong ❌ .
My claim here is that we should maintain if/else at minimum otherwise the complexity of a function grows exponentially 🤯, literally is the function 2x it is not an opinion it is a fact, which mean that the cost to test, document, debug and read that code also grows exponentially, of course there are a lot of ways to write bad code but this is the kind of code that I found more often and that represent a lot of wasted money and time, this is the reason why I wrote this article because it is very common. So, this is the take away of the soupFoo function story you need to understand that if you are not careful with the code that you create you hurt your code base, your team, your company and your family because you can lose your job if you don't know how to work correctly.
The value that we should aim to is code stability, but how can we do that? writing code for complete systems is a pretty complex task. The principle that we should follow is that we need to create code that will not fail. The secret 🙊 in order to write code that will not fail, you need to write code that can't fail or at least not in the near future. And how can you do that? avoid evaluating expressions inside if statements, as much as possible, and if you can't avoid them, make sure those expressions are evaluated with distance between them.
The closer if expressions are between each other the more probable bugs will hide in there. That's way you should keep it at minimum and away from each other. How many hours you have wasted due to a bug 🕷️ that was hidden inside an if/else, and it look something like this:
if (expression1) {
if (expression2) {
if (expression3) {
if (expression4) {
}
else {
...
}
}
else {
//Your bug was here
...
}
}
else {
...
}
}
The worst is that sometimes we are lazy 🦥 we don't care what the code does we simply add another if/else statement to branch functionality this is wrong. I did this when I was starting my career, that's why I recognize this pattern in multiple project code base. Avoid nesting if/else as much as possible.
If you don't have this problem or no one in your team have this problem in your code base 🎉 congratulations 🎉 . At least I put other topics in here. Hopefully, you learned something in here.
About striving for wisdom
Almost everything that I wanted to say is in that section. Just a recommendation is that, you can follow your passion but don't forget to be human 🧍. Your coworkers and relatives are not computers 🤖 , you still need to interact with them and you still have things to learn from anyone. So, keep teaching and more important learning.
In general
In order to create anything meaningful you need to work in team. The wizard-genius computer programmer 🧙♂️ that build companies on its own is a myth. You can build amazing things only if you are in a good team.
Every time someone needs to rewrite your code to extend it, you have failed ❌ as software engineer. Programmers need to write code assuming that it will not be modified just extended, this doesn't mean that your code should be rigid it still needs to be flexible but the idea is that anyone updating/extending your code should find something to build on 🏗️ , not something that they will want to destroy.
"Performant code is the Mc-mansion 🏘️ in Sarasota that starts falling apart after 10 years. Readable code is the old stone building 🏛️ that stands for centuries. I cannot respect someone who doesn’t see the difference."
Frank Underwood (a Twisted version) 👌
Thank you for reading 🥺 . And excuse me if my English was bad 🎩 , it is not my native language.
Top comments (13)
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.