Parenthesis-itis
-
We've got a dev who has a certain, well known, fondness for parenthesis. He will use them even if he doesn't need to.
For example, any if statement always gets an extra set of parenthesis around any logic check, like so:
if ((value1 == foo) && (val2 == bar)) { // something something dark side }
I occasional stumble upon something so utterly filled with parenthesis, that I have to take a few moments to mentally parse the thing before I set about simplifying it. This is one such occasion...
if (model.Height != null && (((model.Height.HeightFeetModel != null) && (TryValidateModel(model.Height.HeightFeetModel) == false)) || ((model.Height.HeightMetresModel != null) && (TryValidateModel(model.Height.HeightMetresModel) == false)))) { model.HeightMajorUnit = null; model.HeightMinorUnit = null; } // same with current weight if ((model.CurrentWeight != null) && (((model.CurrentWeight.WeightStonesModel != null) && (TryValidateModel(model.CurrentWeight.WeightStonesModel) == false)) || ((model.CurrentWeight.WeightKilosModel != null) && (TryValidateModel(model.CurrentWeight.WeightKilosModel) == false)) || ((model.CurrentWeight.WeightPoundsModel != null) && (TryValidateModel(model.CurrentWeight.WeightPoundsModel) == false)))) { model.CurrentWeightMajorUnit = null; model.CurrentWeightMinorUnit = null; } // same with start weight if ((model.StartWeight != null) && (((model.StartWeight.WeightStonesModel != null) && (TryValidateModel(model.StartWeight.WeightStonesModel) == false)) || ((model.StartWeight.WeightKilosModel != null) && (TryValidateModel(model.StartWeight.WeightKilosModel) == false)) || ((model.StartWeight.WeightPoundsModel != null) && (TryValidateModel(model.StartWeight.WeightPoundsModel) == false)))) { model.StartWeightMajorUnit = null; model.StartWeightMinorUnit = null; }
Let's make some improvements, shall we?
bool isFeetInvalid = model.Height?.HeightFeetModel != null && !TryValidateModel(model.Height.HeightFeetModel); bool isMetresInvalid = model.Height?.HeightMetresModel != null && !TryValidateModel(model.Height.HeightMetresModel); if (isFeetInvalid || isMetresInvalid) { model.HeightMajorUnit = null; model.HeightMinorUnit = null; } bool isCurrentStonesInvalid = model.CurrentWeight?.WeightStonesModel != null && !TryValidateModel(model.CurrentWeight.WeightStonesModel); bool isCurrentKilosInvalid = model.CurrentWeight?.WeightKilosModel != null && !TryValidateModel(model.CurrentWeight.WeightKilosModel); bool isCurrentPoundsInvalid = model.CurrentWeight?.WeightPoundsModel != null && !TryValidateModel(model.CurrentWeight.WeightPoundsModel); // same with current weight if (isCurrentStonesInvalid || isCurrentKilosInvalid || isCurrentPoundsInvalid) { model.CurrentWeightMajorUnit = null; model.CurrentWeightMinorUnit = null; } bool isStartStonesInvalid = model.StartWeight?.WeightStonesModel != null && !TryValidateModel(model.StartWeight.WeightStonesModel); bool isStartKilosInvalid = model.StartWeight?.WeightKilosModel != null && !TryValidateModel(model.StartWeight.WeightKilosModel); bool isStartPoundsInvalid = model.StartWeight?.WeightPoundsModel != null && !TryValidateModel(model.StartWeight.WeightPoundsModel); // same with start weight if (isStartStonesInvalid || isStartKilosInvalid || isStartPoundsInvalid) { model.StartWeightMajorUnit = null; model.StartWeightMinorUnit = null; }
Ahhhh, much better
-
@DoctorJones If I engrave a hammer with
Operator Precedence
, would you hit him over the head with it?
-
@RaceProUK I've given up. I now spend most of my time hunting down and killing superfluous parens.
-
@DoctorJones
I felt a grave disturbance in the force. As if a million parenthesis cried out in anguish, and were suddenly silenced.
-
...I strongly suspect that if I were to learn to code, I might actually do something similar, just to compartmentalize expressions and whatnot to save my brain from having to try and parse them out later.
-
int x = y = z
looks weird.
if mystring = 'hello' and error = 144 then
looks like a candidate for a compiler error
bitwise operator 'and' has no overloaded version for string ('hello') and integer (error)
-
@DoctorJones is he using some sort of tool to generate the expressions?
That looks like the rubbish that Access generates when you're building SQL queries in the wizard.
-
So, in the legacy system we work with here, the ancient custom language we use does not have operator precedence, so that sort of code is incredibly common here. Except all of those variables and methods would be globals.
I'm now worried you've got one of our rare escapees.
-
I was expecting Lisp.
I am disappoint!
-
@DoctorJones said in Parenthesis-itis:
Ahhhh, much better
Except it does something slightly different.
-
I tend to use more parentheses than are strictly necessary, because I think it's often more readable.
OTOH, I'm also a fan of splitting out complex combinations of
&&
and||
into their own variables like @DoctorJones did in the refactor there, because that's also a lot more readable
-
@Grunnen said in Parenthesis-itis:
if mystring = 'hello' and error = 144 then
looks like a candidate for a compiler error
bitwise operator 'and' has no overloaded version for string ('hello') and integer (error)
It makes perfect sense if "=" is the equality comparison (rather than assignment) and "and" is the logical conjunction.
-
@dkf said in Parenthesis-itis:
Except it does something slightly different.
Are you calling @DoctorJones' functions impure?
-
@anotherusername said in Parenthesis-itis:
@DoctorJones is he using some sort of tool to generate the expressions?
Nope, it's just his damage
-
@dkf said in Parenthesis-itis:
Except it does something slightly different.
It's acceptably close. Was surprised it took so long for a to reply.
-
@Khudzlin Yes, but in some languages it can depend on the context whether '=' is an assignment or a comparison, and whether 'and' is a logical or a bitwise operator.
Enough potential for bugs in comparison to the effort of writing a few parentheses.
-
@Grunnen Overloading "and" so it can be logical or bitwise isn't a problem if boolean is its own type (frowning at C here), because the. However, using the same operator for equality and assignment is a stupid idea (less so if the context can easily be determined - say if assignments aren't possible inside expressions).
-
@anotherusername said in Parenthesis-itis:
@DoctorJones is he using some sort of tool to generate the expressions?
That looks like the rubbish that Access generates when you're building SQL queries in the wizard.
It looks like my pre-refactoring code. I tend to type the conditions in where I see fit, run the tests, see I'm missing an edge case, slap it at the end of the chain, rinse, repeat and only when my monstrosity works I'd go and simplify the mess.
Seems like he just skipped the last step.
-
@Maciejasjmj said in Parenthesis-itis:
Seems like he just skipped the last step.
He's probably just waiting until the entire codebase is finished
-
@DoctorJones said in Parenthesis-itis:
@Maciejasjmj said in Parenthesis-itis:
Seems like he just skipped the last step.
He's probably just waiting until the entire codebase is finished
Functionality first, refactoring never!
-
@DoctorJones said in Parenthesis-itis:
any if statement always gets an extra set of parenthesis around any logic check
That's common if you come from an environment where different languages have different precedence rules. Always forcing the order of evaluation by parenthesizing keeps you from having to worry whether some later maintainer will misinterpret your intent.
Also, PL/I uses parentheses for everything, including both expression grouping and as syntax for certain conditional statements. do while (n=0); requires the brackets not because n=0 is a condition, but because the do statement insists upon it.
-
@DoctorJones said in Parenthesis-itis:
We've got a dev who has a certain, well known, fondness for parenthesis. He will use them even if he doesn't need to.
That's easy to fix. Make him code in Common Lisp for a while.
@asdf said in Parenthesis-itis:
I was expecting Lisp.
I am disappoint!Sorry, just now getting to this thread.
-
@antiquarian I honestly refuse to learn lisp just because those parenthesis annoy me.
-
@fbmac A good editor will match the parens for you. A better one will even indent the code for you based on the parens and what you're doing with them.
-
@antiquarian an even "better" editor will add an extra brace when you don't want it, and proceed to completely fuck the indentation up for you, in the whole file.
-
@antiquarian A better editor will detect you're using Lisp and refuse to let you continue.
-
-
@RaceProUK said in Parenthesis-itis:
@asdf said in Parenthesis-itis:
I was expecting Lisp.
I am disappoint!
Ith thith better?
Now I'm trying to imagine what @RaceProUK has in her mouth...
-
@DoctorJones said in Parenthesis-itis:
We've got a dev who has a certain, well known, fondness for parenthesis.
I would bet a small sum that it's a MISRA compliance habit. One of its rules is 'Thou shalt not rely on operator precedence'.
-
@clatter said in Parenthesis-itis:
MISRA
Found their website, and saw this:
Yep, that's about right for motor industry software QC
-
Rainbow-delimiters-mode to the rescue! Set up some
nicecolours and increase the size for every layer, and presto! you've got this:Now isn't that pretty...
-
@Mikael_Svahnberg I'm now wondering if this is how the Discodevs have their IDEs set up. Would certainly explain the 'Discourse is rainbows' rhetoric.
-
-
@Mikael_Svahnberg The blue is a little too dark for the black background, but otherwise, it works pretty well.
-
@Mikael_Svahnberg said in Parenthesis-itis:
Rainbow-delimiters-mode to the rescue! Set up some
nicecolours and increase the size for every layer, and presto! you've got this:Now isn't that pretty...
EMACS USER! GET HIM!
-
-
@Mikael_Svahnberg said in Parenthesis-itis:
Rainbow-delimiters-mode to the rescue! Set up some
nicecolours and increase the size for every layer, and presto! you've got this:Now isn't that pretty...
I just showed this to the dev in question, he loved it
-
@Mikael_Svahnberg The heresy thread is
-
@Mikael_Svahnberg said in Parenthesis-itis:
Rainbow-delimiters-mode to the rescue! Set up some
nicecolours and increase the size for every layer, and presto! you've got this:Now isn't that pretty...
Seems like something I might want to have in Linqpad or Visual Studio ...
-
@aliceif Definitely seems like the sort of thing you could do pretty easily with an addon... I wonder if anyone has?
-
@aliceif I found it!
-
-
@izzion Rainbow braces doesn't actually seem to do anything :/
-
@Magus
Racing to post testing google results to see if they actually work.
-
@izzion Viasfora works!
-
@Mikael_Svahnberg said in Parenthesis-itis:
Rainbow-delimiters-mode to the rescue! Set up some
nicecolours and increase the size for every layer, and presto! you've got this:Now isn't that pretty...
Kinda want!
-
@Magus said in Parenthesis-itis:
@izzion Viasfora works!
Luckily it doesn't alter the fonts like the example screenshot...