5 ms·

This is one of those advice that sound reasonable when explained in the abstract, and yet when you hear it invoked in actual code reviews by other people, you w

• denton-scratch 3y ago
  > fuck off to hell Yeah, that's the button I was looking for.
• nousermane 3y ago
  > writing "return condition" instead of "if condition then return true else return false end" > using the conditional-value ("ternary") operator in any capacity Looks like author of some code I had to comb through recently, maybe had that among guidelines. Said code was replete with: if(function_that_returns_boolean()){ return true; }else{ return false; } ...and... if(foo()){ return true; }else{ if(bar()){ return true; }else{ return false; } }
  • ggambetta 3y ago
    This to me screams "I don't understand boolean variables". Why stop there? switch (byte_value) { case 0: return 0; case 1: return 1; ... case 255: return 255; }
    • drewcoo 3y ago
      It's more about guard clauses. https://en.wikipedia.org/wiki/Guard_(computer_science) https://en.wikipedia.org/wiki/Guard_(computer_science)
    • hardware2win 3y ago
      Visualizing control flow from if ladder effort: minimal Doing the same for not tiny boolean expression: way more Just because you think it is cool cuz profs forced it on ya during college is not a argument
      • ajuc 3y ago
        It's personal taste. You can subdivide boolean expressions just as easily with if-then-elses as with boolean functions. What most people object to is leaving the last level of if-then-else and returning true or false, when you could have returned simpleBooleanExpression and do away with 1 level of nesting.
      • pwdisswordfishc 3y ago
        Ironically, I have had profs tell me to use nested ifs instead of breaking loops, so…
    • HaroldBolt78 3y ago
      [flagged]
  • hayley-patton 3y ago
    My high school computing exam (by government direction - VCE, graduated 2019) was filled with many such If a And b Then Return True Else Return False End If programs. Not sure why they were so scared of Return a And b. Apparently they got a new curriculum after I graduated; my teacher was very happy about it but I didn't get to see it myself.
    • hayley-patton 3y ago
      Well, the 2019 final exam [0] lacks such questions; it must have been some other year. But the correct data type for an "AccountBalance" variable (section C, question 9) was a float, so I definitely lost marks on that question! The 2021 final exam (which is the most recent I can find) [1] somehow creates XML records for a SQL database (section C, question 7), and hints at using "data validation" to avoid an SQL injection (section C, question 11.c), which is quite exciting. And the 3.5 gigabit wireless connection (section A, question 13) is definitely not using an ISP-supplied router. [0] https://www.vcaa.vic.edu.au/Documents/exams/technology/2019/2019comp-softdev-w.pdf https://www.vcaa.vic.edu.au/Documents/exams/technology/2019/..., answers https://www.vcaa.vic.edu.au/Documents/exams/technology/2019/comp-softdev_examrep19.pdf https://www.vcaa.vic.edu.au/Documents/exams/technology/2019/... [1] https://www.vcaa.vic.edu.au/Documents/exams/appliedcomp/2021/2021AC-softdev-w.pdf https://www.vcaa.vic.edu.au/Documents/exams/appliedcomp/2021..., answers https://www.vcaa.vic.edu.au/Documents/exams/appliedcomp/2021/2021softwaredev-report.pdf https://www.vcaa.vic.edu.au/Documents/exams/appliedcomp/2021...
    • erik_seaberg 3y ago
      In an intro class using Ada I’ve seen case Big_Expression_Here is when true => Do_Something; when others => Do_Something_Else; end case; and I found myself wondering whether they realize what this is.
  • cookieperson 3y ago
    If a code reviewer pulls that on you, meet with them and explain why that's asinine. If that fails bring in someone more senior as a tie breaker. If that fails, find somewhere else to work...
  • gizmo 3y ago
    In C++ projects you see the verbose version a lot, because it's much faster to set a breakpoint on a statement than on a conditional expression. It's really annoying to repeatedly stop debugging, add the redundant return statements, and sit through a recompile cycle just because `return expr` looks nicer than `if (expr) return true else return false`.
    • regularjack 3y ago
      Alternatively, you could use a debugger that supports conditional breakpoints.
      • xxs 3y ago
        Cond breakpoints require the code breaking each time and evaluate the statement which makes it runs like snail.
• samsquire 3y ago
  Someone reading my code could probably point out profound numbers of code quality issues. But I would want to know what they've done themselves to evaluate the weight of their expertise. If they have impressive projects, their criticism means something. there was a koan about the progression of someone's Java program from beginner to intermediate, to expert to master to enlightened. The code for the enlightened looked the same as the beginner. These koans are useful too https://prirai.github.io/books/unix-koans.html https://prirai.github.io/books/unix-koans.html Edit: In other words, say a developer that criticized wordpress, do you respect what they say more than the success and value of wordpress (of the wordpress developers)?
  • okl 3y ago
    You describe an appeal to accomplishment fallacy. Whether or not someone has done x or y themselves has no bearing on whether your code has quality issues.
    • samsquire 3y ago
      The merit of the code quality issues is in question based on the authoritativeness of the person doing the criticising. I don't like the pride, superiority and smugness that comes from criticizing code and the assumption that they're the ones in the right because of their criticism, rather than the expertise of the one who wrote the code and solved the problem and did the work. I've noticed people pay more attention to the stones that people throw rather than those throwing them. Edit: In other words, criticism is cheap if you have no skin in the game. I don't think it's a fallacy, it's like hosting a wedding and inviting a guest and then being criticised for the entertainment on offer. People pay attention to the criticism, not that the criticism is coming from a guest and that their criticism is not as valuable coming from the guest than the people paying for and tastes of those hosting the wedding. Whether or not the code quality issues are true or not, their importance is dependent and weight of someone's criticism is not that high as everyone thinks.
    • yobbo 3y ago
      "Quality" is not an indisputable property of the code. Someone's judgement about code quality can only have as much weight as their competence.
• thih9 3y ago
  > Using "clever" as a criticism is implicitly an ad personam argument, passing value judgement on the person who wrote it ("Oh, they must have written it to show off") instead of an argument about the code itself. I don’t get it; why? Anecdotally, me and my colleagues are fine giving ourselves feedback that some code is too clever, without malicious undertones.
• Joker_vD 3y ago
  > any kind of metaprogramming whatsoever I am personally on the fence about this one. It's tremendously useful when writing code, and I've done quite a fair share of it myself; but on the other hand, when you are reading the code, trying to understand what the hell is actually being called and why, it really sucks when your F12 bottoms out at some meta-generic piece of machinery that invokes something when called from somewhere but what and where, exactly, is a mystery that requires a global search over the repo in the best case (in the worst case there is either nothing to search on or the search term will turn up thousands of hits).
  • tgv 3y ago
    To me, meta-programming should not creep into a function. It's ok to generate functions or initializers or whatever with it, but it should not be an inconspicuous part of an otherwise normal function body, especially not if it affects control flow. If it really, really helps readability, perhaps it could be used at the top level, but not inside a loop or conditional statement. I might make an exception for complex type declarations. But, like all programming: it should help readability and it should help to avoid bugs, not be used for its own sake, aka cleverness.
• cookieperson 3y ago
  I don't think most people would describe almost anything you wrote as clever. I hope that's not insulting... Metaprogramming tho... You want to make a magic black box that takes hours to debug and conceals real bugs, invest heavily in metaprogramming to save a hundred lines of code that can be maintained with three find and replaces/SED calls and a recompile a year... Most metaprogramming shouldn't exist in production code. Programming is social. You have to weigh your decisions heavily by who you work with now and who you might work with in the future. If you work in a niche language or toolset you might think "this is my chance to do whatever I want!" But the reality is, it should be the complete opposite. Most compilers make clever code silly. You can write a 15 line hard to read pipe or method chain, or a 25 line double for loop with the same runtime characteristics. The latter is always better to maintain, while the former is always more clever. One is good for the team the other for someone's self esteem.
  • nisegami 3y ago
    >I don't think most people would describe almost anything you wrote as clever. I hope that's not insulting... I desperately wish I could say this, but I have seen it happen
    • cookieperson 3y ago
      Sorry to hear that. I'd pitch to invest in the teams education a bit. A little time spent establishing some ground rules goes a long way. If it's your boss biting down on you for it, don't waste your time, seek a new environment if it bothers you or stunts your growth. Most code review cultures degrade into absurd rules over time because of human power structures and crappy feedback loops. I'd imagine if some of those things became rules either there's an obscure reason for it, or the environment has rotten out. I've seen both cases for some of these types of things. The rotten culture being the most common
  • josephg 3y ago
    > Most compilers make clever code silly. You can write a 15 line hard to read pipe or method chain, or a 25 line double for loop with the same runtime characteristics. The latter is always better to maintain, while the former is always more clever. Weirdly, in rust there's lots of cases where the compiler will generate better assembly if you give it map/filter/fold than it will if you give it a for loop. The reasons are weird - like, its easier for the compiler to know it doesn't need bounds checks for list.iter(), and to work around some integer overflow errors while iterating through ranges and things like that. The difference usually doesn't matter in practice, but I still find myself thinking about it in performance critical sections.
• _0ffh 3y ago
  I keep seeing "if (some_boolean == true)" and it kind of grates on me.
  • wizofaus 3y ago
    In many languages it's necessary if it's a nullable boolean...
  • denton-scratch 3y ago
    In PHP, you might need "if (some_boolean === true)". There is a fashion for inverting it: "if (true == some_boolean)", which seems arse-over-elbow to me (I do know why people do that).
    • grzm 3y ago
      The “literal/constant first” form can be used defensively to avoid mistakenly using the wrong operator for languages that have assignment operators that are similar to equality-test operators. For example, this will cause a syntax error: if (true = some_boolean) While this will silently result in an assignment of true to the some_boolean variable, not intended the equality test if (some_boolean = true) Editor hints and linting can help catch this as well, but those aren’t always available (or weren’t available in the past). I think I first saw this in Code Complete 2. Here’s some more explanation https://softwareengineering.stackexchange.com/questions/74086/if-constant-variable-vs-if-variable-constant#74087 https://softwareengineering.stackexchange.com/questions/7408... Looks like the pattern is sometimes referred to as a Yoda expression.
    • deleted 3y ago
      [deleted]
• ralphstodomingo 3y ago
  > Using "clever" as a criticism is implicitly an ad personam argument, passing value judgement on the person who wrote it ("Oh, they must have written it to show off") instead of an argument about the code itself. I don't know how you got this conclusion. I can see judgement being passed to the code with respect to not just who wrote it, but also who's going to need to understand it later, who's going to need to make changes to it. In that sense, calling code "too clever" makes sense, right? There's hyper-specific reasons why you'd need code that's going to be complex/too clever for that group, esp if it saves cost or is faster than the simpler alternatives, but in most cases there's nothing that justifies it.
• whamlastxmas 3y ago
  Routine reminder to the world that uBlock has “annoyances” filters I recommend for everyone, and reporting new sites that aren’t yet blocked is pretty easy once you’ve done it. This removes virtually any pop up or cookie banner or suedo ad on any popular website.
  • pwdisswordfishc 3y ago
    I do use that. It doesn't work here though, because the author of that website "cleverly" decided to obfuscate their CSS class names.