> if isDelivered and isNotified: isDone = True else: isDone = false;
> Instead of:
> isDone = isDelivered and isNotified
Are people actually finding code like this in professional work or is this just an example? I'm self-taught and know I've got some gaps, but this example is so fundamental I find it shocking.
A difference between good programmer and a bad programmer is that the good one writes working, readable, human understandable code that does the job it's supposed to do. That's it. Someone who writes code that's shorter without being more understandable, or faster, or significantly more efficient, isn't better.
kinda like this python exagerated code:
def sum(a,b):
return operators.add(a,b)
In contrast, your example is kind of the opposite situation - it's an example of forgoing the most natural, human representation in favour of a highly Python-specific one. In other words, the first one happens when you know the abstract logic, but don't know your tools very well; your example happens when you know lots of useless details about the tool but you're struggling with the abstract logic.
Actually, the example from the article suggests to me that the author misunderstands what I regard as the most fundamental programming skill: knowing the precedence of correctness over all other concerns, including efficiency. It's better to attach the right parts together badly than to attach the wrong parts like a pro.
It was massively detrimental to readability, because it meant what should have been a 5 or 6 line function suddenly became 20 or 30 lines. A 20 line function is suddenly 100 lines. And you had to wade through each part to work out what it was doing rather than just glancing at it and it being obvious...
...and what's worse, that then gets broken up into smaller functions containing equally trivial-but-bloated code, and now you need to jump around even more to understand something that's actually very simple.
At the extreme end of the terseness scale are the APL-family languages, and the people who work with them have no problems reading or writing code like this:
https://code.jsoftware.com/wiki/Essays/Incunabulum
I think it really says something about the state of software development when you consider that a not-insignificant number of people who are perfectly fine with that level of density (and they are making $$$$), and yet there are others who will complain that even K&R's style is not verbose enough. IMHO it's a matter of education and attitude.
> return a && !(b || c) || d;
I've seen (pardon the formatting, I'm typing on my phone):
> if (d) {return true}
> else if (a) {return !(b || c);}
> else {return false}
It's usually a choice to be verbose for clarity.
// a b c d
// * * * 1 true
// * 1 * 0 false
// * * 1 0 false
// 1 0 0 * true
// 0 * * 0 false
if (d) {return true }
if (b||c) {return false}
if (a) {return true }
return falseThe major problem with code that's "overly branchy", as in containing a lot of if/else, is that you're forced to go through each case when trying to understand how it works, and it often proliferates into even more branchy code as someone makes a bugfix (with another if/else) in one of the cases, but neglects to see that a similar if not identical change must be made to some of the others.
In other words, if/else cases like your second example optimise for micro-readability when what's often important in debugging and understanding is macro-readability.
1. I'm talking about acceptable code, as opposed to "the best choice". There are good reasons to go for verbosity. Depending on the problem domain I might agree with your expansion of the parentheses, but when we get to this type of discussion, usually my overwhelming reaction is "this is bikeshedding," because...
2. If it's "overly branchy" code and you're worried about causing macro-readability issues, the answer is to refactor, not to compress. Modifying inline conditions runs just as much of a risk of becoming inscrutable. You choose between following branches and enumerating binary tables. If it's showing up too many places, you're likely extracting either expression into its own function.
When overly branch code happens, my experience is that the root cause is underthinking/overthinking the method, not taking time to design the right high level abstractions, or as you mentioned a case of too much repeating yourself. Generally, the fixes for those issues don't have that much to do with your choice of boolean expression vs conditional.
Also why would anyone store a boolean named isDone rather than just returning it as the contract of the function
And a coworker wrote his own string join method in Python (and was mad he wasn’t a senior engineer).
Personally I think programmers who are "too clever" are cancer inside a codebase. Sure you saved a bunch of keystrokes but your code golf has locked the code forever unless someone is willing to rewrite it. This is not a bad example but in general I think people who strive to do things in less characters are highly problematic.
isDelivered && isNotified
always resolves to a Boolean, either TRUE or FALSE
So you are just saying
isDone = TRUE
or
isDone = FALSE
that’s all
I was a teacher's assistant in the intro-to-programming courses in college for 3 years, and eventually realized what students were seeing. Now, with some new hires, I'm seeing the same thing:
They think the expression is "if (.. == ..) { .. }"
Not "if (..) { .. }"
The simple explanation of what boolean expressions are, and that any boolean expression can go in those parens - and that it's the same thing as storing the result to a variable and then using that variable - is like a lightbulb suddenly blinks in their head.
I felt momentarily bad about it, and myself, and then realized I have a reason:
The codebase I am maintaining is intended to be readable by people who have extremely varying levels of skill, and who have no intention of ever being professional programmers.
When possible I write stuff so you don’t have to “think like a programmer” to understand it. I’m pleased I use the conditional version more often than not, because I believe it is far more intuitive.
The shortcoming is failure to recognize your object has more than 2 states and the use of flags should be abandoned in favor of a state variable (or status bit vector, aka a state variable).
My personal rule of thumb on this is that single statement conditions aka "a = {condition}" should be using only "and" operators or only "or" operators and should generally avoid nesting unless there's a clear explanatory comment. So "a and b and c and d" is ok but "a and b or c and d" is sketchy. The reason is that "all" and "any" are idiomatic, but "some" takes more mental work to parse.
Also it's always useful to keep in the back of your mind:
!(a || b) == !a && !b
Whenever I spot either side of the equation in a condition, I stop and consider whether the alternate formulation would be more readable.
At a different company, I found stuff like:
ZERO_INDEX = 0
ONE_INDEX = 1
return arr[ZERO_INDEX]The solution to this problem is to add some sort of `NOLINT` comment to that line or, if possible, the comment that turns off that particular check. Then you link to the design document describing what "arr" is and how its interface involves accessing indices 0 and 1.
Or you could wrap "arr" up in a class or function of some sort so the code is theoretically more self documenting.
The reason we got code like this was because of outsourced contractors cargo-culting something they saw elsewhere.
Correcting this sort of stuff is the job of a senior engineer, though. If it's extremely frustrating, I'd become independently wealthy or stay away from technical leadership positions.
I was basically just told that most (if not all) numbers should be stored in constants. So I just added int twentyFive and stuff, and it annoyed me, but my teacher was happy.
Much later, I realized the missing part: Name the constant not what it is, but rather what it does. So int rightBoundary instead of twentyFive, and the code suddenly becomes much more readable.
let isBoolean = false
if (booleanComparisonOrComputation) {
isBoolean = true
}
...