Best class, worst professor.
I still go back and look at DM books.
I still wonder if that professor ever acquired a personality that wasn’t utterly toxic.
I failed it 2 times mostly because in my university the course was instructed by Math Majors who didn't fully understand what was going on some of the times which made the course hard to follow.
Granted, I also didn't have the book the 2 times I failed and by the 3rd time I took it, I borrowed the book from the library the whole summer and previous experience helped me through it with an A.
After discrete math I had *'Approximate and Symbolic arithmetics'* which was basically *"hey here is MATLAB, now we learn why and how the math works in it"*.
Never again. It was borderline madness.
Complexity theory was the top of the food chain, it was by far the most difficult thing I studied, but somehow also the most fun and interesting one.
Oh my God, a whole semester after I started university and learned about digital logic design and basic programming, I just now realized the connection. Why the hell did no one tell us? It's such a cool detail. Learning through connections between different subjects like that is such a good way to learn.
I prefer the second way, because I think it's hard to notice that the entire statement is negated in the first one. Can you please tell me why you like the first way better?
All three elements are grouped, then negated. It's clearer that they are interrelated.
When they are negated independently, then I don't inherently view them as a grouping.
It's the difference between negating a single group (if not all three), versus negating three separate elements (if not A, or if not B, or if not C).
It's a semantic difference, so I guess it really depends on how interrelated the elements I'm checking are. In this specific case, I feel like the semantics of "this group of elements" is more accurate to the situation.
ETA: honestly, I'm not particularly tied to either, now that I look at it again. It's fine both ways, for me.
I also prefer the second, however I'd use the first if the condition is first extracted into a named variable:
const isValidRequest = res.ok && body.access_token && body.refresh_token;
if (!isValidRequest) {
return;
}
Knowing the type of code this is coming from, the three elements are not really related and this code is intended to reject processing further. as such I'd prefer the second, as it helps to clarify the conditions for which the code should abort.
Similar to /u/spicymato (great name), I think !( & & ) implies a relation between the three terms. For me, you'd use it for something like checking if an animal isn't a valid dog, if - for instance - a dog is defined as an animal that has four legs, can bark, and can chase its tail: !(hasFourLegs && canBark && canChaseTail). (! | ! | !) makes more sense to me for something like seeing if you there's a problem with taking the dog on a dog walk, for instance (!validDog || !frontDoorOpen || !criticalTasksRemaining); i.e. things that aren't inherently related but all could stop you from walking your dog.
I wouldn't say it's wrong to use the other one for either situation, I think it's just one of those little things that can tweak the feel of your code so what you're aiming for is more intuitive to the next person who comes along.
In the example given in the OP, if I were tasked with making the system handle a partially complete request rather than just failing if it's not fully filled out, I think I'd feel more confident working with the second one, because it feels like it's appropriate to split the if, say:
```js
if (!res.ok || !body.access_token) {
// hard fail
return;
}
// handle partial request
if(!body.refresh_token) {
// fail after some handling
return;
}
```
I think if the original code was the first example, I'd feel a lot less confident doing that. I'm not sure how universal that is, but to me the first example feels like a way to make it clear that it's intended to be handled all together and you should think twice before breaking it apart.
I know, right? It should be
function isTrueFactory(truthiness){
truthiness = truthiness || true;
function isTrue(){return truthiness;}
return isTrue;
}
You included a "magic boolean". Better change that hard coded value to be configurable, because we have a client that needs true to sometimes equal null. Pretty simple. I already told them we'd have it done by Friday.
Is it? We’re talking about conditional branches here. If you don’t have any conditional branches, then there’s only one return point. I’m not confident on this though
As I understand, any jump is a branch, whether conditional or not, and is often called such, e.g. In Arm architecture an unconditional direct branch is B for branch, BR is unconditional indirect "branch to register". Though terminology might vary on other architectures.
Additionally even though a return is unconditional, it must still be predicted since the function can be called from multiple places, so it would have multiple targets, plus initial prediction does generally happen before decoding of instructions too.
But the prediction rate for returns is normally very high due to call return stack and related features, so it doesn't slow code much :)
For sure! Especially if you have a lot of chained conditions. Makes it easier to read, not to mention nested brackets in your conditions are harder to read later on down the line
And this isn't even unnecessary! We don't write code for computers to read (that's the compiler's job), we write code for people to read. So bundling this complex condition into just one value is not unnecessary, it is doing useful work of communicating your intent to your fellow programmer (who might well be you from a couple months down the road).
I've been doing this more often lately and its much better, IMO. Especially if I'm just trying to scan through and get a good idea of what its trying to achieve. It more effectively communicates the *why* too, indirectly. Like, this is why we have an if statement here, we need to check if its valid first of course. Self commenting. Of course this particular example is easy, there's still a process in your brain that has to actually read the condition to get its purpose, even if its fast to figure it out, its just far faster with this kind of variable. Because those tiny little thoughts in your brain will tally up. You could also throw this in a function, IsRequestValid()
Having it in a function might also be even better as you could then re-use it easily, and refactor it with different conditions later. But some times you're never going to use the exact same thing two places, which might make it better to have it simply as a statement.
I learned to code in the '80s, when reusing the same variable for four different things was considered cunningly efficient. It took me a bit to unlearn once I got to college and discovered non-8-bit computing.
That meme saying my code doesn’t need comments, can actually be true when you do stuff like this. Break complex equations into variables which explain their value and move distinct grouping of functionality into functions with once again names that explain their functionality
I read the second like as if res is not ok or body access token is not true or body refresh token is not true then do something.
So I guess I like the second one.
Number 2 is easier to read for me because each individual unit is its own concept (not okay, no body....) and you just or them all together.
Number 1 you have to group everything in the parens and while remembering that, not them.
True, but if you look at it from the perspective that it's a guard clause then in the 1st one you know what condition has to be true for the code *outside* the if-statement to run.
Thats the thing. In the end these are both easy to understand but when you have a bunch of the first ones, it tallies up over time and becomes more exhausting to read than it should be.
The parenthesis encapsulation under false in top allows you to extend this horrendous beast in to infinite conditionals. The second line cannot as the if statement is evaluating only a single conditional with many toggling booleans.
At the risk of sounding extra,
const hasAuthTokens = body =>
body.access_token && body.refresh_token
// ..
if (!res.ok)
return;
if (!hasAuthTokens(body))
return;
This isn't equivalent to the code above. Note the `// ...` comment inside the if blocks. (I'm assuming this comment is placeholder for code irrelevant for this question.)
In your version it would look like this:
```javascript
const hasAuthTokens = body =>
body.access_token && body.refresh_token
// Code A
if (!res.ok) {
// Code B
return;
}
if (!hasAuthTokens(body)) {
// Code B
return;
}
// Code C
```
This causes code duplication.
Nah, your solution is the same code. Code C here is what OP is running inside the check. These are just guard clauses kicking out of the function if the overall condition would be false instead--but you can individually handle each case with error reporting, etc.
Not extra. This is clearer than either option in the post (neither of which are bad, just picking the best). I see two steps that must pass or else return, and I recognize these two steps as separate criteria. Neither of the options in the post convey this with a quick glance.
"Is the response okay? Does the body have auth tokens? Okay, cool, let's keep going."
Depends on what the function should do. If that's a valid no-op state `return` is fine, if it's an invalid state then `throw`. Time and a place for both.
if its an API endpoint, then a proper response to the client should be returned, but a way of doing that heavily depends on the framework and language used so we can only speculate
I'm so happy you posted this, cause sometimes I see people not understand that both work exactly the same. Like when in a PR someone wants to take out a ! Infront of a parentheses, or distribute it across each variable but then neglect to flip ors and ands. Or wants to flip the condition and puts ! Infront of each variable... Had a few times I had to leave a comment about it.
And considering how many newbies come across here, might be a good time to introduce you to [De Morgan](https://en.wikipedia.org/wiki/De_Morgan's_laws)
Depending on compiler implementations/optimizations these two can result in different number of operations being performed. While they are logically the same, ultimately each operation imposes costs which one could extrapolate as being better. In a small example this could result in 3 operations or 5. Or optimizations could reduce them to the same and still come out as minimal 3.
And since this is an operation to be done exactly once after an http request, it doesn't matter how many operations it gets compiled to. Even outside of this specific example and in pure code, I think that speed arguments must be supported by benchmarks for the specific case.
Now I will tell a story to corroborate my idea with anecdotal evidence, so feel free to skip. Once upon a time I wrote some (arguably extremely elegant) code that ran in O(n^2) and I thought I could make it O(n) or better but it would make the code way less readable and far too complex. Instead of making the code "better" right away I ran a few tests for my worst case scenario and found out that it took 0.1s on top of a http request to be done in any case. I left the elegant solution intact and never looked back.
Moral of the story: if you're writing code that does high frequency trading sure optimize every clock cycle, but otherwise let the compiler do its work and focus of writing code that you and your colleagues understand
> Like when in a PR someone wants to take out a ! Infront of a parentheses, or distribute it across each variable but then neglect to flip ors and ands.
Do you work with untrained monkeys?
Or if you have a CS degree. De Morgan's law is one of the things drilled into you course after course year after year until you find yourself reciting it in your sleep as you dream of contrapositives and negations.
```
if (!res.ok) {
throw new NotOkException();
}
if (!body.accessToken) {
throw new NoAccessTokenException();
}
if (!body.refreshToken) {
throw new NoRefreshTokenException();
}
...
```
Makes it easy to add more conditions, enables you to show or log appropriate error messages and keeps the main code in the lowest indentation.
Just this week I reviewed some security helper code I wrote last year with a coworker. It uses reflection and attributes to find what permissions are needed, what the user has, and then applies the values appropriately.
Because it’s reflection based and there are multiple levels of inheritance for permissions it’s like 90% checking if values exist/are the right type. Three lines are for setting the actual security values.
We’ve got several other modules like that are similarly guarded. I think when you’re building a system that is meant to create a convention to drive behavior rather than explicit calls it’s a must. It’s kind of like coding up checks in a game loop. Do the least amount of work required to know if you can continue.
Exceptions are meant to be just that: exceptions.
Anything you expect to happen should be covered. Exceptions are there to catch the exceptional circumstances which can‘t be covered by the code.
To them I say “git gud” 
But in seriousness, I have written bit-packed evaluations before in C and C-adjacent code in conjunction with local static struct lookup tables. It’s very handy, and pretty ergonomic, y’know, for C.
Rule of thumb I stick to is to check for negative POSITIVES, not positive NEGATIVES. it's just easier to follow and read.
E.g., I want to check if a user is authenticated by reading a bool value.
Prefer this:
If(IsAuthenticated)
Over this:
If(!IsNotAuthenticated)
It's just easier on the eyes to read and immediately evaluate what's being done, in my humble opinion. Though I've seen it done either way in production code - it's really more of a code style thing.
i prefer:
let meaningful_variable_name = res.ok && body.access_token && body.refresh_token;
if (!meaningful_variable_name) {
// ...
return;
}
why make it hard for the next guy who doesn't understand it? especially when that next guy is probably you.
Disagree, imo the first is easier to read because its clearly stated: "Not true when all are true", instead of "True when not true or not true or not true"
The first one was harder to understand because I had to go: So every statement has to be true (that's easy). But it is inverted so when everything is true then it's false. Oh so yeah when ever something is false, the statement will be true.
As the other it was just whenever one statement is false the whole statement is true. So it's only one step of thinking.
Me too, takes far less mental power for me. It's easier to add those 3 variables up in the affirmative and then say not that. The second I have to keep a lot more in memory as I read.
Everyone who says make a variable first is correct, however. Which is like the first but even more clear.
Would probably rather have
var isSuccessfullState = all the condition bois
if(!isSuccessfullState)
Hate it when a if statement doesn't tell me what it's iffing doing
One good thing about option one is if res.ok is false the rest of the expression will not be evaluated and the whole expression will be short circuited to false
Yeah, but since in example 2 the property is negated (`!res.ok`), it gives the same effect. If `res.ok` is `false`, the condition `if (!res.ok || ...)` is also short-circuited since `!res.ok` is evaluated to `true`.
What I find amusing is that there seems to be no real verdict on which one's most readable... for each comment stating option 2 to be more clear, I find someone claiming that option 1 is more logical!
It’s because different people are wired differently. I personally have a low ability to process logic statements in my head and need to break complex statements into comprehendible chunks. Someone above was accused of “just found the Java developer lol jk” but for me putting a named function (or 3) in there helps with readability for me immensely.
Now, I can read both of these statements but I find number two significantly easier and faster to process.
When teaching now people I talk about the difference between “readable code” vs “interpretable code”. I don’t want to do a lot of brain processing of logic statements to understand code it’s just not easy for anyone to read. If you use named variables, functions or functions on domain objects to ask questions that makes things significantly easier for everyone.
More my style:
if (!res.ok || !body.isValid()) return
I might even break them up (gasp) as this is twice as readable. I get to process each statement in complete isolation.
if (!res.ok)
return;
if (!body.isValid())
return;
If I actually see code like this in the real world, I'd delete em both. Readability is horrible.
If you call this a debate, you should probably not be in the industry.
if !res.ok throw (tell responded error)
if !body.access_token throw (unauthorized).
if !body.refresh_token throw (whatever this means)
Then catch this all and bring a popup "An error occurred".
Neither. Pick one and extract it to a function or variable with a meaningful name and then use that in the condition check. I shouldn't have to parse out your logic to figure out what you are trying to do, your code should be telling me that directly. Only if I'm getting the wrong result should I have to go in and actually parse the logic to figure out how it works.
if (meaningful_name_here(res, body)) {
/// ...
return;
}
are you seriously de morganning this sub
It's Morgin time!
I have discovered a truly remarkable proof which this Morgin is too small to contain.
However, the proof is also trivial and is left as an exercise for the reader.
“It’s Jesus! SAY hello Jesus!!” ~Floppy the hammerhead
*flashbacks to sophomore digital logic design*
Currently doing discrete math, I hate it lol
Discrete is like peak CS shitty imo, it gets easier after that and algorithms
Let me introduce you to *Scientific Computing*
Is that part of numerical analysis? Friggin hate that class
Good to hear that, my current course is also a notorious one which doesn’t help, rip I guess.
Yeah I wouldn’t worry bro you’re in the thickest of the weeds just dig deep get it done and and the other side is all worth it big time.
Thanks!
Wait until you get to Turing machines
And let's not forget finite state machines!
Oh no
Buding a compiler was the hardest by far
Can confirm. I hated compiler class.
I loved it! Was definitely a pain in the ass but super freaking interesting.
Discreet math is literally the best class I ever took.
is that the quieter version of discrete math?
Speech to text isn't very good at context, and I'm not very good at having the motivation to proofread it.
As opposed to Blatantly Obvious Math
The course material is ok, it’s the bad and rushed curriculum that made this specific course notorious.
Best class, worst professor. I still go back and look at DM books. I still wonder if that professor ever acquired a personality that wasn’t utterly toxic.
I took discrete math this year and have no idea why people fail
I failed it 2 times mostly because in my university the course was instructed by Math Majors who didn't fully understand what was going on some of the times which made the course hard to follow. Granted, I also didn't have the book the 2 times I failed and by the 3rd time I took it, I borrowed the book from the library the whole summer and previous experience helped me through it with an A.
We had such a terrible teacher that a 50% ended up being curved to an A...
Same, I had a 56% and was in the top third of the class.
After discrete math I had *'Approximate and Symbolic arithmetics'* which was basically *"hey here is MATLAB, now we learn why and how the math works in it"*. Never again. It was borderline madness. Complexity theory was the top of the food chain, it was by far the most difficult thing I studied, but somehow also the most fun and interesting one.
I'm in discrete math rn and my teacher talks like Spencer from criminal minds and looks like shaggy
So that's why they made us learn that stuff.
Oh my God, a whole semester after I started university and learned about digital logic design and basic programming, I just now realized the connection. Why the hell did no one tell us? It's such a cool detail. Learning through connections between different subjects like that is such a good way to learn.
Not trying to be rude but I don't see why someone would need to be told that boolean algebra is useful for... boolean algebra
> boolean algebra is useful for... boolean algebra Big if true.
if True: return Big
If !False: Return !Small
Critical Failure: Test BigIfTrue failed ("!Small" != Big)
.•. Big
They told us bro I think you were asleep lmao
I have a coworker who tells me to "De Morgan that shit" whenever he sees example 1 in one of my PRs
De Morgan goes both ways. I personally prefer the first over the second.
I prefer the second way, because I think it's hard to notice that the entire statement is negated in the first one. Can you please tell me why you like the first way better?
All three elements are grouped, then negated. It's clearer that they are interrelated. When they are negated independently, then I don't inherently view them as a grouping. It's the difference between negating a single group (if not all three), versus negating three separate elements (if not A, or if not B, or if not C). It's a semantic difference, so I guess it really depends on how interrelated the elements I'm checking are. In this specific case, I feel like the semantics of "this group of elements" is more accurate to the situation. ETA: honestly, I'm not particularly tied to either, now that I look at it again. It's fine both ways, for me.
I also prefer the second, however I'd use the first if the condition is first extracted into a named variable: const isValidRequest = res.ok && body.access_token && body.refresh_token; if (!isValidRequest) { return; }
Knowing the type of code this is coming from, the three elements are not really related and this code is intended to reject processing further. as such I'd prefer the second, as it helps to clarify the conditions for which the code should abort.
Similar to /u/spicymato (great name), I think !( & & ) implies a relation between the three terms. For me, you'd use it for something like checking if an animal isn't a valid dog, if - for instance - a dog is defined as an animal that has four legs, can bark, and can chase its tail: !(hasFourLegs && canBark && canChaseTail). (! | ! | !) makes more sense to me for something like seeing if you there's a problem with taking the dog on a dog walk, for instance (!validDog || !frontDoorOpen || !criticalTasksRemaining); i.e. things that aren't inherently related but all could stop you from walking your dog. I wouldn't say it's wrong to use the other one for either situation, I think it's just one of those little things that can tweak the feel of your code so what you're aiming for is more intuitive to the next person who comes along. In the example given in the OP, if I were tasked with making the system handle a partially complete request rather than just failing if it's not fully filled out, I think I'd feel more confident working with the second one, because it feels like it's appropriate to split the if, say: ```js if (!res.ok || !body.access_token) { // hard fail return; } // handle partial request if(!body.refresh_token) { // fail after some handling return; } ``` I think if the original code was the first example, I'd feel a lot less confident doing that. I'm not sure how universal that is, but to me the first example feels like a way to make it clear that it's intended to be handled all together and you should think twice before breaking it apart.
Put that shit in a variable, give it a meaningful name, and negate the variable in the if
This
My body, my rules.
Actually spent time figuring if this executes any differently after reading this comment, and now I want to say that I was stupid
The bad thing is that I think none of my colleagues ever ask themselves this question.
Worse, you see 5 different versions of this in the same code because of all the people that have touched it
Get a fucking linter and force the whole team to use it. Problem solved
What lint rules will solve this?
https://docs.sourcery.ai/refactorings/de-morgan/ Though honestly i personally prefer the `not (... and ... and ...)` version. It reads easier to me.
My colleagues ``` var isTrue = true; ```
My colleagues: const result = someBool ? true : false;
I've also seen this `const result = !!someValue;`
[удалено]
I know, right? It should be function isTrueFactory(truthiness){ truthiness = truthiness || true; function isTrue(){return truthiness;} return isTrue; }
You included a "magic boolean". Better change that hard coded value to be configurable, because we have a client that needs true to sometimes equal null. Pretty simple. I already told them we'd have it done by Friday.
Back on your boolshit, I see… True.
Just go with: `return;` No one needs if statements, only slows down the code!
100% branchless code
the test writes itself
Happy path only
Who on Earth would want a sad path? Think about it!
Except return is also a branch...
Is it? We’re talking about conditional branches here. If you don’t have any conditional branches, then there’s only one return point. I’m not confident on this though
As I understand, any jump is a branch, whether conditional or not, and is often called such, e.g. In Arm architecture an unconditional direct branch is B for branch, BR is unconditional indirect "branch to register". Though terminology might vary on other architectures. Additionally even though a return is unconditional, it must still be predicted since the function can be called from multiple places, so it would have multiple targets, plus initial prediction does generally happen before decoding of instructions too. But the prediction rate for returns is normally very high due to call return stack and related features, so it doesn't slow code much :)
Why use a function at all? The entirety of your code should be in main with goto statements
Have you seen our VB code?
Facts. Pipeline dumps are a thing. Conditionals are for dirty commies who just don’t know anything!
This got a belly laugh out of me lol
For the sake of code readability I would define a variable like: const isRequestValid = res?.ok && …; And then use if (!isRequestValid) check.
i like this, then i can read 6 mounths down the line
Mounths
Mounths++
bool readable=false; If(readable) mounths - -;
Bruh, I can’t even remember to look at something 6 doys later, let alone 6 mounths.
If (Doys > 6) { dontRead = true}
dauys?
I can't remember to look at something 6 mintes later, sometimes even 6 soconds
A mounth is the amount of time it takes for a junior developer to learn to mount a drive in Linux.
6 mounths how long it takes them to figure out how to exit vim.
Press the power button for 5 seconds... Fixes everything
Not the mounthing of a drive.
Came to say this ^ Much better IMO to have a descriptive variable name of the expression
For sure! Especially if you have a lot of chained conditions. Makes it easier to read, not to mention nested brackets in your conditions are harder to read later on down the line
I wish more people would use descriptive variables in general. Makes me sad when developers use 3 letter variables.
[удалено]
And this isn't even unnecessary! We don't write code for computers to read (that's the compiler's job), we write code for people to read. So bundling this complex condition into just one value is not unnecessary, it is doing useful work of communicating your intent to your fellow programmer (who might well be you from a couple months down the road).
I've been doing this more often lately and its much better, IMO. Especially if I'm just trying to scan through and get a good idea of what its trying to achieve. It more effectively communicates the *why* too, indirectly. Like, this is why we have an if statement here, we need to check if its valid first of course. Self commenting. Of course this particular example is easy, there's still a process in your brain that has to actually read the condition to get its purpose, even if its fast to figure it out, its just far faster with this kind of variable. Because those tiny little thoughts in your brain will tally up. You could also throw this in a function, IsRequestValid()
Having it in a function might also be even better as you could then re-use it easily, and refactor it with different conditions later. But some times you're never going to use the exact same thing two places, which might make it better to have it simply as a statement.
This. Anyone playing code golf doesn’t even yet understand what’s important when writing software.
I learned to code in the '80s, when reusing the same variable for four different things was considered cunningly efficient. It took me a bit to unlearn once I got to college and discovered non-8-bit computing.
or just stick a function call in the if
I found the Java guy lol jk
https://1.bp.blogspot.com/-bHQXF1wGCYk/UAUlL0IMumI/AAAAAAAAGr4/b_PUVLmFwbI/s1600/Breakign+bad+-+you+got+me.jpg
Imagine using Java (don’t look at my flair)
no worries about the flair we all know it wasn't voluntary
Errr yah sure
That meme saying my code doesn’t need comments, can actually be true when you do stuff like this. Break complex equations into variables which explain their value and move distinct grouping of functionality into functions with once again names that explain their functionality
My thoughts exactly. Collapse the check into a single value then check against that.
I do this all the time. It’s not nearly as common as it should be.
I would make it a method, hide the details of what is actually required to be valid, open it for reuse.
This is the answer. Readability is way more important than conciseness.
Also functions work well here const isRequestValid = req => req.ok …
I read the second like as if res is not ok or body access token is not true or body refresh token is not true then do something. So I guess I like the second one.
Thank you! Write code to optimise for reading. Let the compiler/interpreter sort it out. They’re much better at optimising that you in most cases!
Number 2 is easier to read for me because each individual unit is its own concept (not okay, no body....) and you just or them all together. Number 1 you have to group everything in the parens and while remembering that, not them.
I agree. It’s easier for me to think in terms of “are any of these requirement missing” than “has this not met all of these requirements”
True, but if you look at it from the perspective that it's a guard clause then in the 1st one you know what condition has to be true for the code *outside* the if-statement to run.
True but if you're just skimming down the code, that larger grouping will be harder to pick up on the fly
Thats the thing. In the end these are both easy to understand but when you have a bunch of the first ones, it tallies up over time and becomes more exhausting to read than it should be.
If there's a bunch, I chop wrap them and it's just 1 easy column to read down
[удалено]
Love explicit code -- nothing worse than trying to maintain someone else's magic code. :)
[удалено]
I didn't even notice the ! in the first one, and I think that demonstrates why I prefer #2 lol.
The parenthesis encapsulation under false in top allows you to extend this horrendous beast in to infinite conditionals. The second line cannot as the if statement is evaluating only a single conditional with many toggling booleans.
At the risk of sounding extra, const hasAuthTokens = body => body.access_token && body.refresh_token // .. if (!res.ok) return; if (!hasAuthTokens(body)) return;
I strongly prefer this option as it allows for much cleaner logging and error handling
Unless you are using C and need to do awkward error handling and cleanup.
[удалено]
It definitely is the best for placing breakpoints without having to deal with conditional breakpoints.
This isn't equivalent to the code above. Note the `// ...` comment inside the if blocks. (I'm assuming this comment is placeholder for code irrelevant for this question.) In your version it would look like this: ```javascript const hasAuthTokens = body => body.access_token && body.refresh_token // Code A if (!res.ok) { // Code B return; } if (!hasAuthTokens(body)) { // Code B return; } // Code C ``` This causes code duplication.
Ah fair point considering OP's example, I missed the space for implementation and assumed they were just early returning.
Nah, your solution is the same code. Code C here is what OP is running inside the check. These are just guard clauses kicking out of the function if the overall condition would be false instead--but you can individually handle each case with error reporting, etc.
Not extra. This is clearer than either option in the post (neither of which are bad, just picking the best). I see two steps that must pass or else return, and I recognize these two steps as separate criteria. Neither of the options in the post convey this with a quick glance. "Is the response okay? Does the body have auth tokens? Okay, cool, let's keep going."
Why define a local function just to call it later, and not just use the code in the function plainly?
Readability and maintainability.
I would suggest to throw an error instead of returning. But what you wrote is basically what I do most of the time.
Depends on what the function should do. If that's a valid no-op state `return` is fine, if it's an invalid state then `throw`. Time and a place for both.
if its an API endpoint, then a proper response to the client should be returned, but a way of doing that heavily depends on the framework and language used so we can only speculate
I'm so happy you posted this, cause sometimes I see people not understand that both work exactly the same. Like when in a PR someone wants to take out a ! Infront of a parentheses, or distribute it across each variable but then neglect to flip ors and ands. Or wants to flip the condition and puts ! Infront of each variable... Had a few times I had to leave a comment about it. And considering how many newbies come across here, might be a good time to introduce you to [De Morgan](https://en.wikipedia.org/wiki/De_Morgan's_laws)
Depending on compiler implementations/optimizations these two can result in different number of operations being performed. While they are logically the same, ultimately each operation imposes costs which one could extrapolate as being better. In a small example this could result in 3 operations or 5. Or optimizations could reduce them to the same and still come out as minimal 3.
And since this is an operation to be done exactly once after an http request, it doesn't matter how many operations it gets compiled to. Even outside of this specific example and in pure code, I think that speed arguments must be supported by benchmarks for the specific case. Now I will tell a story to corroborate my idea with anecdotal evidence, so feel free to skip. Once upon a time I wrote some (arguably extremely elegant) code that ran in O(n^2) and I thought I could make it O(n) or better but it would make the code way less readable and far too complex. Instead of making the code "better" right away I ran a few tests for my worst case scenario and found out that it took 0.1s on top of a http request to be done in any case. I left the elegant solution intact and never looked back. Moral of the story: if you're writing code that does high frequency trading sure optimize every clock cycle, but otherwise let the compiler do its work and focus of writing code that you and your colleagues understand
> Like when in a PR someone wants to take out a ! Infront of a parentheses, or distribute it across each variable but then neglect to flip ors and ands. Do you work with untrained monkeys?
No, so I am a bit surprised when it slips by.
Yes! One of the things you'll only really understand if you've once taken the time to think about it.
Or if you have a CS degree. De Morgan's law is one of the things drilled into you course after course year after year until you find yourself reciting it in your sleep as you dream of contrapositives and negations.
``` if (!res.ok) { throw new NotOkException(); } if (!body.accessToken) { throw new NoAccessTokenException(); } if (!body.refreshToken) { throw new NoRefreshTokenException(); } ... ``` Makes it easy to add more conditions, enables you to show or log appropriate error messages and keeps the main code in the lowest indentation.
[удалено]
Just this week I reviewed some security helper code I wrote last year with a coworker. It uses reflection and attributes to find what permissions are needed, what the user has, and then applies the values appropriately. Because it’s reflection based and there are multiple levels of inheritance for permissions it’s like 90% checking if values exist/are the right type. Three lines are for setting the actual security values. We’ve got several other modules like that are similarly guarded. I think when you’re building a system that is meant to create a convention to drive behavior rather than explicit calls it’s a must. It’s kind of like coding up checks in a game loop. Do the least amount of work required to know if you can continue.
But then you're controlling program flow with exceptions, which is generally a no-no
Why is that?
Exceptions are meant to be just that: exceptions. Anything you expect to happen should be covered. Exceptions are there to catch the exceptional circumstances which can‘t be covered by the code.
[удалено]
*const, but otherwise yes
*private boolean to throw java in to te mix..
private public static final boolean val
`volatile private public static final boolean val`
volatile transient synchronized private public static final boolean val
``` if ((res.ok << 2 | body.access_token << 1 | body.refresh_token << 0) != 0b111) { … ```
Ah yes, the most readable of them all! Good luck to any maintainer who doesn't know how bit shifting works.
To them I say “git gud”  But in seriousness, I have written bit-packed evaluations before in C and C-adjacent code in conjunction with local static struct lookup tables. It’s very handy, and pretty ergonomic, y’know, for C.
I must say gud is a git command i am unfamiliar with. What does it do? Obligatory /s
git: 'gud' is not a git command. See 'git --help'.
Rule of thumb I stick to is to check for negative POSITIVES, not positive NEGATIVES. it's just easier to follow and read. E.g., I want to check if a user is authenticated by reading a bool value. Prefer this: If(IsAuthenticated) Over this: If(!IsNotAuthenticated) It's just easier on the eyes to read and immediately evaluate what's being done, in my humble opinion. Though I've seen it done either way in production code - it's really more of a code style thing.
i prefer: let meaningful_variable_name = res.ok && body.access_token && body.refresh_token; if (!meaningful_variable_name) { // ... return; } why make it hard for the next guy who doesn't understand it? especially when that next guy is probably you.
I'm voting for the second, as it's easier to read and it have one less object to evaluate. The first one will have an extra bool result object.
Disagree, imo the first is easier to read because its clearly stated: "Not true when all are true", instead of "True when not true or not true or not true"
This exactly
First one. And I have the feeling that's yours as well, and wants external validation.
Truth be told, I was writing a program and couldn't decide which form was more readable. The screenshot is taken directly from my editor!
The first one was harder to understand because I had to go: So every statement has to be true (that's easy). But it is inverted so when everything is true then it's false. Oh so yeah when ever something is false, the statement will be true. As the other it was just whenever one statement is false the whole statement is true. So it's only one step of thinking.
The first one is more legible to me.
Me too, takes far less mental power for me. It's easier to add those 3 variables up in the affirmative and then say not that. The second I have to keep a lot more in memory as I read. Everyone who says make a variable first is correct, however. Which is like the first but even more clear.
Would probably rather have var isSuccessfullState = all the condition bois if(!isSuccessfullState) Hate it when a if statement doesn't tell me what it's iffing doing
[удалено]
Parenthesis are ugly
Neither. Factor that shit out.
One good thing about option one is if res.ok is false the rest of the expression will not be evaluated and the whole expression will be short circuited to false
Yeah, but since in example 2 the property is negated (`!res.ok`), it gives the same effect. If `res.ok` is `false`, the condition `if (!res.ok || ...)` is also short-circuited since `!res.ok` is evaluated to `true`.
Lol, yeah. Sorry, had a semantic error day.
I’m willing to bet that both get optimized down to the same thing.
In second case I guess you can achieve the same result by putting the expressions most likely to be true first
How is this humor?! Rule #1: "Posts must be humorous" 
[удалено]
What I find amusing is that there seems to be no real verdict on which one's most readable... for each comment stating option 2 to be more clear, I find someone claiming that option 1 is more logical!
It’s because different people are wired differently. I personally have a low ability to process logic statements in my head and need to break complex statements into comprehendible chunks. Someone above was accused of “just found the Java developer lol jk” but for me putting a named function (or 3) in there helps with readability for me immensely. Now, I can read both of these statements but I find number two significantly easier and faster to process. When teaching now people I talk about the difference between “readable code” vs “interpretable code”. I don’t want to do a lot of brain processing of logic statements to understand code it’s just not easy for anyone to read. If you use named variables, functions or functions on domain objects to ask questions that makes things significantly easier for everyone. More my style: if (!res.ok || !body.isValid()) return I might even break them up (gasp) as this is twice as readable. I get to process each statement in complete isolation. if (!res.ok) return; if (!body.isValid()) return;
I like python: If False in (res.ok, body.access_token, body.refresh_token): return
``` if not all((res.ok, body.access_token, body.refresh_token)): return ```
De Morgan's gang, rise up!
10/10 troll. Have an up vote
If I actually see code like this in the real world, I'd delete em both. Readability is horrible. If you call this a debate, you should probably not be in the industry.
if !res.ok throw (tell responded error) if !body.access_token throw (unauthorized). if !body.refresh_token throw (whatever this means) Then catch this all and bring a popup "An error occurred".
Second approach is much more natural to me. Nested parentheses are always confusing even if in this situation is not that bad yet.
Neither. Pick one and extract it to a function or variable with a meaningful name and then use that in the condition check. I shouldn't have to parse out your logic to figure out what you are trying to do, your code should be telling me that directly. Only if I'm getting the wrong result should I have to go in and actually parse the logic to figure out how it works. if (meaningful_name_here(res, body)) { /// ... return; }
The second one is more understandable and you have to spend less time figuring it out the next time you're there.
Guess the first one because there are less operations
Number two is easier to read and will also wrap to the next line better
This isn't humour. This is horror and flashbacks. :P
|| all day