When the reviewer doesn't understand my Javascript it's his fault
-
@gleemonk said in When the reviewer doesn't understand my Javascript it's his fault:
Finally getting there: Applied
prettier
to all files.The biggest source of diffs was that wrote
if(condition) {
and wroteif( condition ) {
(though with forgetting the space before the closing paren half the time). I don't know how either got the idea, I just tried not to squint every other line while reading. Glad this is over.Those should both be
if (condition) {
-
@boomzilla said in When the reviewer doesn't understand my Javascript it's his fault:
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
@gleemonk said in When the reviewer doesn't understand my Javascript it's his fault:
Finally getting there: Applied
prettier
to all files.The biggest source of diffs was that wrote
if(condition) {
and wroteif( condition ) {
(though with forgetting the space before the closing paren half the time). I don't know how either got the idea, I just tried not to squint every other line while reading. Glad this is over.So you fixed them both to
if (condition) {
?is up with that brain fart of a format?
Are you talking about
if (condition) {
?The idea is that function calls are never written with a space whereas keywords are.
This gives just a bit more warning when you are diagonally reading a function so that you can spot a one-line
if (somethingsomething) return false;
.(Whether the one-line
if
is a good idea at all is of course another debate).
-
@JBert said in When the reviewer doesn't understand my Javascript it's his fault:
@boomzilla said in When the reviewer doesn't understand my Javascript it's his fault:
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
@gleemonk said in When the reviewer doesn't understand my Javascript it's his fault:
Finally getting there: Applied
prettier
to all files.The biggest source of diffs was that wrote
if(condition) {
and wroteif( condition ) {
(though with forgetting the space before the closing paren half the time). I don't know how either got the idea, I just tried not to squint every other line while reading. Glad this is over.So you fixed them both to
if (condition) {
?is up with that brain fart of a format?
Are you talking about
if (condition) {
?The idea is that function calls are never written with a space whereas keywords are.
This gives just a bit more warning when you are diagonally reading a function so that you can spot a one-line
if (somethingsomething) return false;
.I don't want any of that garbage.
if( condition ){
is sooo much easier to read.
-
@boomzilla wrong.
-
@Gąska oh, sure, who am I going to believe, you or my lying eyes?
-
@dkf said in When the reviewer doesn't understand my Javascript it's his fault:
No more ?
Who knows? isn't around.
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
So you fixed them both to
if (condition) {
?Oh I didn't. I didn't change no files. Not me. No. That's not my style.
prettier
might have done so though. And might have crafted two massive commits from that. And might have gotten to accept the pull requests because wouldn't touch them. True to form.@HardwareGeek said in When the reviewer doesn't understand my Javascript it's his fault:
No.
if (condition) {
Well there were some leftovers in that style.
prettier
was indiscriminate though.@dcon said in When the reviewer doesn't understand my Javascript it's his fault:
I wish that was our code standard (it is mine!). Ours is:
if (condition) { // stuff } else { }
Which is how God intended it
Oh, except when it's:
if (condition) { } else if (condition) { } ...
. There is no God.
-
@boomzilla said in When the reviewer doesn't understand my Javascript it's his fault:
I don't want any of that garbage.
if( condition ){
is sooo much easier to read.Specially when it's
if( !condition )
, I find.
-
@Zecc
!
always gets a space on both sides. So does;
insidefor( ; ; )
-
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
for( ; ; )
ITYM
while(true)
-
@Zecc said in When the reviewer doesn't understand my Javascript it's his fault:
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
for( ; ; )
ITYM
while(true)
BUTBUTBUT
#define ever (;;) ... for ever { }
-
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
!
always gets a space on both sidesYou know, jokes aside I actually find this less readable. I prefer
if( !something || !somethingelse )
to
if( ! something || ! somethingelse )
as I can more easily scan what's negated, believe it or not. Specially if
something
is a parenthesized expression.
-
AWOOOOOGA AWOOOOOOGA Holy War detected, everyone out!
-
@Steve_The_Cynic It took you over a month to detect that?
-
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
@Steve_The_Cynic It took you over a month to detect that?
No, it's just that I found the thread to be more than unusually boringly boring until now.
-
@Steve_The_Cynic said in When the reviewer doesn't understand my Javascript it's his fault:
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
@Steve_The_Cynic It took you over a month to detect that?
No, it's just that I found the thread to be more than unusually boringly boring until now.
That seems like an unusually cynical attitude.
-
@HardwareGeek said in When the reviewer doesn't understand my Javascript it's his fault:
@Steve_The_Cynic said in When the reviewer doesn't understand my Javascript it's his fault:
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
@Steve_The_Cynic It took you over a month to detect that?
No, it's just that I found the thread to be more than unusually boringly boring until now.
That seems like an unusually cynical attitude.
But not wrong. We did take a lot longer to ramp things up then usual...
-
@dcon Did you miss the implied ?
-
@dcon said in When the reviewer doesn't understand my Javascript it's his fault:
I wish that was our code standard (it is mine!). Ours is:
if (condition) { // stuff } else { }
..."all
if
must haveelse
clause (even if empty)"?I'm sorry.
Oh, except when it's:
if (condition) { } else if (condition) { } ...
Oh god, no. What is that.
-
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
What is that.
Punishable under the Geneva Convention against Warcrimes, that's what it is.
-
@HardwareGeek said in When the reviewer doesn't understand my Javascript it's his fault:
@dcon Did you miss the implied ?
Probably. I'm trying to wrap my head around some Unicode/UTF8 hell right now.
-
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
..."all if must have else clause (even if empty)"?
I'm sorry.No. There's an implied
// stuff
in that empty block (I'll edit)
-
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
So you fixed them both to
if (condition) {
?
-
@JBert said in When the reviewer doesn't understand my Javascript it's his fault:
(Whether the one-line if is a good idea at all is of course another debate).
IIRC this is why Ruby decided to do it in the opposite order,
do_something if foo
-
@HardwareGeek said in When the reviewer doesn't understand my Javascript it's his fault:
@dcon Did you miss the implied ?
Just to reassure you: I didn't miss it. Well played.
-
@Zecc said in When the reviewer doesn't understand my Javascript it's his fault:
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
!
always gets a space on both sidesYou know, jokes aside I actually find this less readable. I prefer
if( !something || !somethingelse )
to
if( ! something || ! somethingelse )
as I can more easily scan what's negated, believe it or not. Specially if
something
is a parenthesized expression.Some languages complement
if (condition) stuff
with anunless (condition) stuff
.I remember Perl 5's love of
die "Invalid parameter" unless $input =~ /f(oo)+/;
sanity checks.Which also leads to
unless ... else ...
-
@jinpa said in When the reviewer doesn't understand my Javascript it's his fault:
@Shoreline said in When the reviewer doesn't understand my Javascript it's his fault:
@gleemonk Interesting. A few points...
- People who use phrases like "you know that don't you" usually deserve atomisation.
Eh. Sometimes you need to find out whether the person you're talking to is the one who doesn't understand something, or you are. Seems a reasonable question, depending on how it's said.
Yeah but people who affix "you know that, don't you" or equivalents are:
a) often wasting words
b) often being patronising
c) often telling me something irrelevant or incorrect.In the rare cases where c) isn't true, you're quite right, it's reasonable to check. b) depends on personality and how well equipped I am to tolerate them. a) doesn't bother me unless they should have been able to work out that adding emotion or colour to their statement is only going to hurt their case, but now it's my work to do because they fancied being rude.
Example:
Me: "The bank will charge me daily analogous to how far into my overdraft I am."
Jackass: "No they won't. Don't you know how a bank works?""Don't you know X" is a slightly ruder variation on "you know that, don't you".
Counter-example:
Me: "How do I do X?"
Teacher: "Start from Y and do Z. You know this."In this context, a similar phrase "you know this" is used to remind you that you've trained in this area before. It's informal but it's not intended to belittle or condescend. It's intended to imbue confidence.
In conclusion, it's always good to check your facts, as I might not have understood properly the first time, or the facts might have been updated by enriched data, but people don't have to be an asshole about it.
-
@Zecc said in When the reviewer doesn't understand my Javascript it's his fault:
@PleegWat said in When the reviewer doesn't understand my Javascript it's his fault:
!
always gets a space on both sidesYou know, jokes aside I actually find this less readable. I prefer
if( !something || !somethingelse )
to
if( ! something || ! somethingelse )
as I can more easily scan what's negated, believe it or not. Specially if
something
is a parenthesized expression.This is a problem with javascript. One of those two statements should be causing a parse error in a sane language. Otherwise we end up having to use a linter. Languages should not require a linter.
And I'm talking as someone who likes javascript.
-
@Shoreline said in When the reviewer doesn't understand my Javascript it's his fault:
One of those two statements should be causing a parse error in a sane language.
Not following you.
-
-
@Shoreline said in When the reviewer doesn't understand my Javascript it's his fault:
And I'm talking as someone who likes javascript.
:well_theres_the_problem.js:
-
First of all, not sure how I missed the notifications. I'd apologise for necroing a six-month-old thread, butt suck it.
@Zecc said in When the reviewer doesn't understand my Javascript it's his fault:
Not following you.
Is it because of the "sane" part? :P
If you're serious, my assertion is that having such a lackadaisical approach to language syntax, such that you can write one thing in two ways, is going to create problems. Partly because now you have to look for two ways to write a piece of code when you're asking the question "what the hell is causing this".
@HardwareGeek said in When the reviewer doesn't understand my Javascript it's his fault:
@Shoreline said in When the reviewer doesn't understand my Javascript it's his fault:
And I'm talking as someone who likes javascript.
:well_theres_the_problem.js:
I was told that you like something because..., but you love something although...
Maybe I love javascript a little... although...
-
@Shoreline Are you serious?
For the benefit of others, here's a quick summary.
I said in When the reviewer doesn't understand my Javascript it's his fault:
You know, jokes aside I actually find this less readable. I prefer
if( !something || !somethingelse )
to
if( ! something || ! somethingelse )
as I can more easily scan what's negated, believe it or not. Specially if
something
is a parenthesized expression.Then you said in When the reviewer doesn't understand my Javascript it's his fault:
This is a problem with javascript. One of those two statements should be causing a parse error in a sane language. Otherwise we end up having to use a linter. Languages should not require a linter.
Languages providing their own linter seems like a good idea to me. Stuff like signalling unused variables or useless assigments is a good idea.
I find causing a parse error in this situation to be overkill. This would be a notice at best. It's a purely stylistic choice.
Significant space around an unary operator when there's no ambiguity seems to me to be the insane option.
Now if there was a line break after the
!
, then I agree. Whoever would write that is either an idiot or malicious.
-
@Shoreline said in When the reviewer doesn't understand my Javascript it's his fault:
Is it because of the "sane" part? :P
Good joke btw.
-
@Shoreline said in When the reviewer doesn't understand my Javascript it's his fault:
having such a lackadaisical approach to language syntax, such that you can write one thing in two ways, is going to create problems.
*cough* Perl
Filed under: TMTOWTDI
-
@Zecc And this is why all languages should have a
not
operator.!
is horrible because it's so easy to miss next to the opening parenthesis.
-
@dfdub For this very reason Kotlin recently introduced an
isNotEmpty
method on collections, which inlines to!isEmpty
, and there's a compiler warning every time you actually write!x.isEmpty()
.
-
@gleemonk said in When the reviewer doesn't understand my Javascript it's his fault:
@Zenith said in When the reviewer doesn't understand my Javascript it's his fault:
I started my career in languages where this was common practice. There's nothing inherently wrong with it.
Show me one project where local variables are ALL_CAPS because they are
const
orfinal
and I might reconsider.The Java code where I work does this.
-
I use the existence of conflicting coding styles as an indication of "Copy & Paste" from some unknown place and requires attention for review and refactoring. At the very least after review, I understand what the part is trying to do and reformat(refactor if necessary) it to fit the codebase.
-
One open source project that I sometimes contribute to, simply has a unit test that lets your code fail if it doesn't adhere to the style guide. I'd expect that commercial companies would at least work as professionally as that?
-
@Grunnen said in When the reviewer doesn't understand my Javascript it's his fault:
One open source project that I sometimes contribute to, simply has a unit test that lets your code fail if it doesn't adhere to the style guide.
We have such checks, though not technically as unit tests (they're a different phase of the CI acceptance flow…)
-
@Grunnen Do they make this clear upfront or let you discover it after you've wasted time trying to contribute?
In a work situation, this sort of horseshit should be front and center right with the job's location, salary, and other requirements but it's always guarded like a state secret.
-
@Zenith Why do you have a problem with reformatting your code to match the style guide? As far as I'm concerned, there's no reason not to use a style checker. Au contraire, nothing's more infuriating and hurts readability more than such annoying inconsistencies.
-
@dfdub said in When the reviewer doesn't understand my Javascript it's his fault:
As far as I'm concerned, there's no reason not to use a style checker.
Unless there's a bug in it. Or screws up formatting of your files for some other reason.
-
@Gąska
Of course, but that's just another way of saying "flaky and broken tests are worse than no tests", which I'm pretty sure is common sense.
-
@dfdub said in When the reviewer doesn't understand my Javascript it's his fault:
flaky and broken tests are worse than no tests
I disagree. There can still be value in them. It all depends on how often they break and in what way exactly they break.
-
@dfdub said in When the reviewer doesn't understand my Javascript it's his fault:
@Gąska
Of course, but that's just another way of saying "flaky and broken tests are worse than no tests", which I'm pretty sure is common sense.I feel like you're fishing for someone to come and say "you should unit test your tests" and "it's tests all the way down".
-
This post is deleted!
-
@Gąska said in When the reviewer doesn't understand my Javascript it's his fault:
accuse a programmer of drinking too little coffee
I don't (usually) drink coffee. Mountain Dew or Dr. Pepper is my preferred caffeine source.
-
@boomzilla said in When the reviewer doesn't understand my Javascript it's his fault:
@brie said in When the reviewer doesn't understand my Javascript it's his fault:
@gleemonk said in When the reviewer doesn't understand my Javascript it's his fault:
Finally getting there: Applied
prettier
to all files.The biggest source of diffs was that wrote
if(condition) {
and wroteif( condition ) {
(though with forgetting the space before the closing paren half the time). I don't know how either got the idea, I just tried not to squint every other line while reading. Glad this is over.So you fixed them both to
if (condition) {
?is up with that brain fart of a format?
It makes complex conditions line up with a tab (if using a 4-space tab width):
if (condition_part_1 && condition_part_2) {
-
@djls45 said in When the reviewer doesn't understand my Javascript it's his fault:
It makes complex conditions line up with a tab (if using a 4-space tab width):
if( condition_part_1 && condition_part_2 ) {
Ah yes, so it does.