Learn More
Resend Email
12
piratefox
5y

How do you debate the "it's more complex in my opinion" statement?

So, some months ago I was looking at some code which has stuff as 300 lines of code function(s) and I could feel the bad smell irl...

I analyze it a bit and there is a lot of stuff which is misplaced, repeated or unsafe.
I first re-arrange it and remove redundancy, then break it down in about five functions (plus a caller), all is now readable and assignIcon k(made-up name) only assigns an icon, it doesn't also send a rocket in space.

But then I put the code in review and the previous author of the code says that it's now unreadable, because s/he has to look as multiple functions. I counter by showing how s/he does not need to read 300 lines of code to find a bug, but approximately 60, and I point at how misleading having an `assignIcon` function which also sends rockets in space is.
The counter? "But it looks confusing to have smaller functions, revert it."

How would you debate that? I am shy and hate myself a lot, so I have issues debating good points, but I am really really sure a lot of bugs I encountered were due to stuff like this so I would like to be able to explain my point in a more efficient way, for future teams.

question
Favorite
Ranter
Comments
  • 9
    5y
    Ideally? You don't argue at all. You need a consistent, defined coding standards document which everyone adheres to. Then you need to use tools which, as far as possible, pick up on violations of that standard (it won't do everything, but there are tools / IDEs that will generate warnings for repeated code, functions that are too long, etc.) and make sure those are distributed and available for the team to use. You then need to make sure that those warnings fail code review automatically, unless it has *specific* sign off from seniors as a deviation.

    For this case though? Depends on your position. If you're junior, you likely swallow it, do your year or two, then move on quickly if you're seeing this behaviour as a repeated pattern. If you're senior, you fight your corner, bringing in your manager if necessary, and you use the opportunity to push for longer term change (as mentioned above.)
  • 3
    5y
    Explain how the "confusing to have smaller functions" is an oxymoron to him. And ask him to learn Unix Philosophy
  • 1
    5y
    @asgs i have tried, but the counter argument was: "yeah but for ME it remains harder to understand", which I tried to counter by: "you THINK it is harder now, but what about in a month?"

    (Pointless to say the answer was the same)
  • 2
    5y
    @piratefox Single responsibility principle is a thing - it applies to code you write as well as your work. You made it better, then there is no countering your point. Think of selling reasons: why your approach is objectively reducing maintenance cost in future (instead of why assignIcon sending rocket is bad)? Crush them with analytics ffs, if it's SO getting hard in their head.
    For more decency, I'd recommend to make it in form of deposition: place some middlemen to witness the argument.
  • 1
    5y
    @vintprox I am glad you brought up this argument, I tried to use it as well... Srp and clean code should be the core for every dev IMHO...

    Altough it didn't work this time it is very nice that I find a match in your opinion! (I am changing job because of this issue and other red flags, so right now it's more a question to understand what I highlighted correctly and what I didn't rather than a "I really wish to go back to this person" )
  • 1
    5y
    @AlmondSauce I agree... I tried to enforce a standard with pre-commit hooks with the pre-existing linting configuration, but it was refused by the team... Alongside with the lint-on-save automation I was using in my personal IDE (which tells a lot about it).
    As for warnings they were there, but they were ignored due to the fact that the original team (which was very organized) moved away and was slowly replaced by this team.
  • 2
    5y
    It doesn't matter if you're junior or senior. Build your case. Quote programming authorities and explain best practices, show in examples why smaller functions are better. Show them programming antipatterns, use liners and code analysis tools. Find a programming guru in your company and ask him if he can help you convince others.
  • 3
    5y
    Be like water. Fill his lungs while no one is looking.
  • 0
    5y
    Most of the time code reviews feel pointless. It's mostly one self-appointed guru claiming "this is unreadable..." and other abstract concepts. Usually these gurus have one or two minions nodding their heads rhythmically to please their master.

    "I feel this is better", "this is more readable", "this is clearer", "this is better"....WTF do these claims mean? One guy thinks something is better and that is supposed to be the golden guideline? And the same guys own code look like random unreadable file of turd. "Yeah well, I think 3 spacebars instead of 2 makes the code readable". So you cannot understand it otherwise? Remove one space and the code goes wild? Feeling dizzy?
  • 1
    5y
    These things shouldn't be matters of opinion. Style guides exist for this reason. It's either objectively better or objectively worse. What he feels personally is irrelevant.
  • 2
    5y
    how do you debate something like that?

    precisely the way you just debated it in your post here.

    that is, if you're willing to continue to work at a company where something this basic needs to be argued to the person doing code review...
  • 1
    5y
    @piratefox "yeah, but for ME it's harder to understand"

    yeah, but code review should not be about YOU, but about everyone else who will ever have to work with the code.
devRant © 2021 Hexical Labs LLC
Privacy Policy  |  Terms of Service
Add Comment