Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

JawnV6: Jul 4, 2004; So hot ...

Break after return, while on the minor side of the horrors here, still looks awful and should be throwing a warning.

# ? Oct 20, 2017 17:38

Adbot: ADBOT LOVES YOU

# ? Jun 9, 2024 02:57

iospace: Jan 19, 2038

JawnV6 posted:

Break after return, while on the minor side of the horrors here, still looks awful and should be throwing a warning.

A lot of people ignore warnings unfortunately.

# ? Oct 20, 2017 17:56

nielsm: Jun 1, 2009

JawnV6 posted:

Break after return, while on the minor side of the horrors here, still looks awful and should be throwing a warning.

In Java, isn't unreachable code after a return an actual error? That may be one thing it does right.

(Maybe not, I do occasionally place a return early in a function to disable part of it for debugging.)

# ? Oct 20, 2017 18:17

Rubellavator: Aug 16, 2007

nielsm posted:

In Java, isn't unreachable code after a return an actual error? That may be one thing it does right.

(Maybe not, I do occasionally place a return early in a function to disable part of it for debugging.)

Yup, it causes a compile error

# ? Oct 20, 2017 18:21

AstuteCat: May 4, 2007

Dumb Lowtax posted:

Send them the equivalent short code and make them justify why it's any longer

I'd love to see their code surrounding this

Not pictured: a comment above where I did exactly this. he could have copy pasted it.

Evidently the lightbulb finally came on when I suggested he REPLs the two bits of code and compared results. His PR was finally approved

E: also without break after returns!

# ? Oct 20, 2017 19:26

Tann: Apr 1, 2009

nielsm posted:

In Java, isn't unreachable code after a return an actual error? That may be one thing it does right.

(Maybe not, I do occasionally place a return early in a function to disable part of it for debugging.)

Me too, you gotta if(true) return; though.

# ? Oct 20, 2017 20:04

QuarkJets: Sep 8, 2008

AstuteCat posted:

Yes. That is all that this function, far as I can tell, is intended to do.

Fake Edit: Update on the code review that should have really been over with by now:

Edit: I think what makes this particular egregious is that he actually has the operators in the switch statement -- like, they are RIGHT THERE.

crazy theory here: parseInt reverses the sign of the data in memory, so it's necessary to call parseInt twice on each variable in this function in order to keep the signs consistent

because writing a version of parseInt that doesn't flip the sign of whatever data is passed in would just be pointless busy work why would you ever do that

# ? Oct 21, 2017 08:33

VikingofRock: Aug 24, 2008

QuarkJets posted:

crazy theory here: parseInt reverses the sign of the data in memory, so it's necessary to call parseInt twice on each variable in this function in order to keep the signs consistent

because writing a version of parseInt that doesn't flip the sign of whatever data is passed in would just be pointless busy work why would you ever do that

If parseInt modifies the value it takes, doesn't that make this UB (if this is C/C++)? Since the evaluation order is unspecified.

# ? Oct 21, 2017 08:50

QuarkJets: Sep 8, 2008

How would a comparison on the returned values even occur if the two function evaluations didn't finish before the comparison?

# ? Oct 21, 2017 11:09

AstuteCat: May 4, 2007

This is JavaScript code so, really, there are bigger problems at play here than just the code in question.

In the end it was just a simple inversion of the logic making it less readable and a failure to engage brain when this was pointed out.

# ? Oct 21, 2017 13:48

VikingofRock: Aug 24, 2008

QuarkJets posted:

How would a comparison on the returned values even occur if the two function evaluations didn't finish before the comparison?

Good point. I didn't think that one through.

# ? Oct 21, 2017 19:03

MisterZimbu: Mar 13, 2006

Is there a version of Hanlon's Razor that says it's more likely the programmer was just a poo poo developer who develops a convoluted mess rather than any sort of higher-level thinking going on that you're missing?

Corollary: If you use ++i in any way that's not directly interchangable with i++, then you're not as clever as you think you are.

MisterZimbu fucked around with this message at 13:44 on Oct 22, 2017

# ? Oct 22, 2017 13:41

TooMuchAbstraction: Oct 14, 2012; I spent four years making
Waves of Steel
Hell yes I'm going to turn my avatar into an ad for it.; Fun Shoe

MisterZimbu posted:

Is there a version of Hanlon's Razor that says it's more likely the programmer was just a poo poo developer who develops a convoluted mess rather than any sort of higher-level thinking going on that you're missing?

Sturgeon's Law: 90% of everything is crud. Statistically, the code you're looking at is crud.

quote:

Corollary: If you use ++i in any way that's not directly interchangable with i++, then you're not as clever as you think you are.

i++ is the horror here behavior-wise, but they're both horrors in terms of the kind of code they enable. Access and mutation should not be combined into a single operator!

# ? Oct 22, 2017 14:48

Soricidus: Oct 21, 2010; freedom-hating statist shill

TooMuchAbstraction posted:

i++ is the horror here behavior-wise, but they're both horrors in terms of the kind of code they enable. Access and mutation should not be combined into a single operator!

there are contexts where combining access and mutation is important

admittedly I can�t think of any where it isn�t also important for this to be atomic, which ++ isn�t

# ? Oct 22, 2017 16:11

Joda: Apr 24, 2010; When I'm off, I just like to really let go and have fun, y'know?; Fun Shoe

I do i++/++i array subscript sometimes, but I don't really think I'm being clever. I also realize I'm just making it harder to read for other maintainers. I'm just too lazy to write the extra line/there's some subconscious or neolithic-brain part of me that seems to think that conserving code lines is in any way a good thing.

# ? Oct 22, 2017 18:02

brap: Aug 23, 2004; Grimey Drawer

Didn�t it come about because a lot of architectures have read-then-increment, decrement-then-read, etc kind of instructions?

# ? Oct 22, 2017 18:10

Soricidus: Oct 21, 2010; freedom-hating statist shill

Yes. And that�s very useful, in an early C context where the language is a high-level assembler without any particularly sophisticated optimiser. Less so nowadays, and frankly it should never have been a part of Java or C#, but I guess the habit is hard to kick.

# ? Oct 22, 2017 19:06

sarehu: Apr 20, 2007; (call/cc call/cc)

Go got it right by making it a statement.

# ? Oct 22, 2017 19:43

KernelSlanders: May 27, 2013; Rogue operating systems on occasion spread lies and rumors about me.

sarehu posted:

Go got it right by making it a statement.

Haskell got it right by making it immutable.

# ? Oct 22, 2017 19:53

Doom Mathematic: Sep 2, 2008

Soricidus posted:

atomic, which ++ isn�t

It isn't? Doesn't it compile down to like a single processor instruction?

# ? Oct 23, 2017 17:26

MrMoo: Sep 14, 2000

Increment is three operations: fetch value, increment, store new value, the operation can be interrupted in-between the fetch and store.

On Intel x86 you get free atomic load and store operations as long as the data is aligned and equal or smaller than the machine word size, everything else needs a special function.

MrMoo fucked around with this message at 17:33 on Oct 23, 2017

# ? Oct 23, 2017 17:28

Jeb Bush 2012: Apr 4, 2007; A mathematician, like a painter or poet, is a maker of patterns. If his patterns are more permanent than theirs, it is because they are made with ideas.

Doom Mathematic posted:

It isn't? Doesn't it compile down to like a single processor instruction?

single processor instruction does not necessarily imply atomic, and the c specification doesn't guarantee it will compile to a single processor instruction anyway

# ? Oct 23, 2017 17:31

Joda: Apr 24, 2010; When I'm off, I just like to really let go and have fun, y'know?; Fun Shoe

MrMoo posted:

The load and store are separate memory accesses, the only atomic standard ops on x86 are load and store.

To be fair, if it's a small loop with not much logic, and the counted variable isn't used outside of it, the compiler could probably justify keeping it in a register if your target has enough of them.

# ? Oct 23, 2017 17:34

Soricidus: Oct 21, 2010; freedom-hating statist shill

Joda posted:

To be fair, if it's a small loop with not much logic, and the counted variable isn't used outside of it, the compiler could probably justify keeping it in a register if your target has enough of them.

yes, but if you care about whether an operation is atomic or not, you want stronger guarantees than �the compiler could probably justify ...�

so arguably ++ and -- are actively harmful, because they make something look like a single operation that is not in fact guaranteed to be one. it�s too late to fix that in C but there was really no excuse for keeping it there in Java � although at least AtomicInteger is easy to find if you need it

# ? Oct 23, 2017 18:18

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

Soricidus posted:

yes, but if you care about whether an operation is atomic or not, you want stronger guarantees than “the compiler could probably justify ...”

so arguably ++ and -- are actively harmful, because they make something look like a single operation that is not in fact guaranteed to be one. it’s too late to fix that in C but there was really no excuse for keeping it there in Java — although at least AtomicInteger is easy to find if you need it

Plenty of languages that don't have ++ still have +=, which is similarly non-atomic but potentially confusing to people with a naive understanding of atomicity. Other things that do not make any atomicity guarantees at the PL level: simple loads and stores (well, except in Java, and even Java doesn't guarantee atomicity for long and double).

I'm generally a strong believer in the ability of programming languages to promote correctness, but atomicity is really an exception, because there's a fundamental conflict of goals: programming languages intentionally try to encourage abstraction and composition, but abstraction and composition inherently break low-level atomicity unless you have a drastically invasive high-level design like transactional memory. And transactional memory is pretty much a dead idea, and for good reasons. The better design approach for PLs is to "define it away" by providing more effective tools for concurrency and memory isolation.

# ? Oct 23, 2017 21:13

QuarkJets: Sep 8, 2008

you'll only be able to take away my ++ out of my cold, dead hands; come at me bro

# ? Oct 23, 2017 22:10

TooMuchAbstraction: Oct 14, 2012; I spent four years making
Waves of Steel
Hell yes I'm going to turn my avatar into an ad for it.; Fun Shoe

QuarkJets posted:

you'll only be able to take away my ++ out of my cold, dead hands; come at me bro

i += 1 is only 3 more characters. It's time to move on.

# ? Oct 23, 2017 22:12

LOOK I AM A TURTLE: May 22, 2003; "I'm actually a tortoise."; Grimey Drawer

TooMuchAbstraction posted:

i += 1 is only 3 more characters. It's time to move on.

That doesn't need to exist either. i = i + 1 is fine.

# ? Oct 23, 2017 23:04

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

Yeah, and who needs multiplication, it's just: while((j = j - 1)) i = i + i;

# ? Oct 23, 2017 23:09

Eela6: May 25, 2007; Shredded Hen

Go literally thinks you should do that for integer exponentiation

# ? Oct 23, 2017 23:20

JawnV6: Jul 4, 2004; So hot ...

code:

*dst++ = *src++;

Get this idiomatic garbage out of here, gotta be clear with what we're doing

code:

memcpy(src, dst, sizeof(typeof(src[0])));
src = src + sizeof(typeof(src[0]));
dst = dst + sizeof(typeof(dst[0]));

# ? Oct 23, 2017 23:23

Linear Zoetrope: Nov 28, 2011; A hero must cook

JawnV6 posted:

code:
*dst++ = *src++;
Get this idiomatic garbage out of here, gotta be clear with what we're doing
code:
memcpy(src, dst, sizeof(typeof(src[0])));
src = src + sizeof(typeof(src[0]));
dst = dst + sizeof(typeof(dst[0]));

"Idiomatic" C code is, quite often, too clever by half yes. The biggest problem with the *dst++ = *src++ construction is that the operator evaluation order is kind of obtuse. Also, there's nothing wrong with

code:

*dst = *src;
dst++;
src++;

Or even

code:

*(dst + i) = *(src + i);
i++;

Where appropriate (and in many cases is arguably better because using an offset variable makes explicit an implicit assumption that i and j will be incremented in tandem). Substitute += or = ... + 1 if you wish. I can be guilty of it too but programmers place this almost religious importance on minimizing newline characters over all other characters for some reason. Newlines aren't evil, separating things into related bite-sized expressions isn't evil.

If you're working in C you absolutely should be able to read code that uses the "increment pointer and perform some operation on the location in one line" construction because it's common. The only thing *a_ptr++ really has going for it is... well... the fact that it's idiomatic. It's common and widely known so it conveys dense, easily parsable information by people who are already familiar with the construction. It's useful by inertia alone, but I'm not confident it was a construction that ever should have become popular because it's hella confusing if you're not familiar with it. Especially the K&R strcpy cousin that adds a while loop, assignment side effects, and null terminators into the mix, which is most peoples first time seeing that construction nowadays and is one of the most impenetrable things this side of languages invented for code golf if you've never seen it before.

Linear Zoetrope fucked around with this message at 00:00 on Oct 24, 2017

# ? Oct 23, 2017 23:51

qsvui: Aug 23, 2003; some crazy thing

What the hell is typeof, that can't be standard C.

# ? Oct 24, 2017 01:59

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

qsvui posted:

What the hell is typeof, that can't be standard C.

It's a common extension. It's also completely unnecessary here because sizeof can take an expression operand.

# ? Oct 24, 2017 02:13

JawnV6: Jul 4, 2004; So hot ...

rjmccall posted:

And transactional memory is pretty much a dead idea, and for good reasons. The better design approach for PLs is to "define it away" by providing more effective tools for concurrency and memory isolation.

Wait I wanted to touch on this. More than the first x86 rollout being patched away, what's happening with it? It was *the* concurrency savior back when I was learning.

Is "something went wrong, I guess" at the end of a giant batch of memory ops rolling back the entire thing not a useful behavior?

# ? Oct 24, 2017 02:48

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

It's just lost a lot of interest. TM works alright if you basically don't have contention. If you do have contention, though, guaranteeing progress is hard even in practical cases � you basically need all other threads to gradually stop trying to begin transactions, even unrelated ones, because you can't easily know ahead of time whether a transaction is indeed unrelated � so either the overhead is much higher than advertised or you have really hard-to-predict and hard-to-analyze failure modes. That statement about progress is also true of things like compare-and-swap loops but much less likely in practice, in part because of the nature of things generally being attempted but also because progress is supposed to be the sort of thing you're aware of as a programmer using atomics, and nobody ever said atomics were a feature for novices, whereas people do make grandiose claims about how easy TM makes lock-free concurrency.

Also, people have been pinning their performance hopes on hardware solutions, but hardware TM is inevitably going to have fixed transaction size limits, which means that a lot of things which would be merely bad ideas suddenly become impossible. For example, you can use TM to splice something out of a linked data structure (like a doubly-linked list) because you only need to touch some bounded number of nodes, but you generally can't use hardware TM to safely walk the entire data structure because the transaction would grow linearly with the size.

# ? Oct 24, 2017 04:14

Zopotantor: Feb 24, 2013; ...und ist er drin dann lassen wir ihn niemals wieder raus...

Linear Zoetrope posted:

Or even

code:

*(dst + i) = *(src + i);
i++;

code:

dst[i] = src[i];
i++;

is even shorter and 100% equivalent.

# ? Oct 24, 2017 05:51

NihilCredo: Jun 6, 2011; iram omni possibili modo preme:
plus una illa te diffamabit, quam multæ virtutes commendabunt

JawnV6 posted:

code:

*dst++ = *src++;

Ah, the whomst'd've of programming.

# ? Oct 24, 2017 07:19

Sagacity: May 2, 2003; Hopefully my epitaph will be funnier than my custom title.

rjmccall posted:

I'm generally a strong believer in the ability of programming languages to promote correctness, but atomicity is really an exception, because there's a fundamental conflict of goals: programming languages intentionally try to encourage abstraction and composition, but abstraction and composition inherently break low-level atomicity unless you have a drastically invasive high-level design like transactional memory.

I wholeheartedly agree. If you're a developer that has a tenuous grasp on what atomicity is and you're not sure what ++ does, then you probably shouldn't be trying to write a lock-free data structure (or whatever) in the first place.

# ? Oct 24, 2017 08:09

Adbot: ADBOT LOVES YOU

# ? Jun 9, 2024 02:57

Athas: Aug 6, 2007; fuck that joker

JawnV6 posted:

Wait I wanted to touch on this. More than the first x86 rollout being patched away, what's happening with it? It was *the* concurrency savior back when I was learning.

Is "something went wrong, I guess" at the end of a giant batch of memory ops rolling back the entire thing not a useful behavior?

Software transactional memory (as seen in functional languages) also hasn't seen as wide usage as was hoped. I think what happened is that a bunch of people invented techniques to make parallel programming safe, but then realised that the real challenge is to also make it fast. You see this happening over and over again (although some research twists the story by indeed focusing on performance, albeit asymptotic cost on implausible machine models).

# ? Oct 24, 2017 09:02

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »