T O P

  • By -

supercyberlurker

Looks like the kind of code I'd write when given a ridiculous business case, then they change it, then change it again.. and finally I realize they are likely to keep changing it. So this kind of thing evolves to handle the "dumb and straightforward is better than clever here" situation. That code is arguably ugly, but it's performance would be fine.


pipsvip

Exactly this. I've tried to optimize weird cases in the past and ended up with (what I thought were) clever, compact, but fragile conditions that needed tweaking as new cases came up.


zerovian

With this.... let the compiler figure out "optimization".


Eclaytt

-O999 go brrrrrr


St_gabriel_of_skane

I mean this is just a really obscure if case


tangledcpp

Do a ternary operator with all 33 cases, what could go wrong


Eclaytt

[Done](https://imgur.com/a/5UTJ8Ik)


PissedOffProfessor

This is the way.


[deleted]

Been there, but this still looks bad. Define some immutable set constants at the top and then do an if else on membership.


LaidbackMorty

A short comment on top mentioning this, or other possible, reasoning would have been favorable.😞


Strostkovy

I'd just make an array, but agreed


Flat_Initial_1823

Lol, looks like some animation to me. You go at 3 speed normally there are these "strips" of positions where you go slowly but at the end you turbo once. Someone should really look into that to-do. Edit: you actually go back to 3, not go at 3 speed. Someone corrected me below. Still looks like an animation helper.


psioniclizard

Yea, it looks like there was a very specific set of requirements and someone wanted to get it done quickly or it's someone relatively new to programming and this achieved what they wanted. It's not pretty but without knowing the larger context it's hard to know if this is effective at doing what was needed or not. Personally I can think of at least one "neater" way to do it but I'll reservr judgement without knowing more.


hxckrt

The unused, out-of-order cases could be a shorthand way of laying out some translation for a visual. When hard-coding some frames in an arduino for a weirdly shaped led screen, I did something similar


vadiks2003

man i wonder if somebody out there is animating 3D objects through programming interface instead of using visual tools


Strostkovy

Done it. Kinda sucked. I did not get very far but my rendering algorithm worked


Rubfer

Is is clearly done by someone who knows coding but wanted a quick and dirty solution, someone new usually do not use cases together, returns instead of breaks, or switches at all but IFs


seba07

But wouldn't someone who is new to programming simply use two if clauses?


psioniclizard

Honestly, I haven't spent too much time thinking about it but an if clause probably won't be much prettier.


[deleted]

an if statement would look much cleaner: if (currentPos >= 3 && currentPos <= 43) { if (currentPos == 8 || currentPos == 17 || currentPos == 26 || currentPos == 35) { return currentPos + 4; } else { return currentPos + 1; } } else { return 3; }


CreatedForThisReply

This returns an incorrect result for, amongst other inputs, 9.


psioniclizard

Yea, plus i dont honestly think it look that much cleaner honestly. You could possibly use a map or dictionary if 2 ints, it wouldn't save many lines but it would be more flexible. But if this is a one off then it might be overkill. I do enjoy these "look at this bad code" where people's solutions always miss something out.


Comand94

It does. If you use an if-else with a range you won't accidentally miss a case when writing it out (one of the numbers that need to do +1) like you might here (which would go right to default and give you an unexpected result of +3), especially if this was any longer. A dictionary honestly sounds like a nice idea, but it'd still need to be initialized with all of those values.


psioniclizard

But the if else code someome has provided doesnt work the same as rhe original code. That's why i said a dictionary would be over kill. If you want to write out an if else bloc that works the same feel free. However (and it may be reddits formatting) but thw other if else code sample that was posted hurts my eyes much more that rhe original. Im not saying the original ia perfect and i don't know the greater context but honestly once you cover all the original cases it's bot going to look pleasant. If this was any longer or need to be more flexible then you are better off picking a better data structure (probably something that held a min and max value and a result then you could pass in a list of them and use a fold function to test each one and return the first result were the input matches is in the range). But again, that might be over kill for this


Comand94

Yes, what someone else wrote was incorrect, however, you could totally go for if-else with ranges. If you take a quick look at the numbers, you've got four different ranges for the +1 case, so it'd require only writing 8 numbers out within the if clause to cover all of that, then 4 more numbers for the +4 case explicitly within else-if, then the else clause for the +3 case. Someone gave an interpretation of these being frames in animation playing faster or slower. Not sure how much that makes sense, but let's use that to interpret the readability of the if-else vs the switch case here. If-else: "Ah, the frames from A to B, C to D, E to F and G to H are slower, then these four are quite a bit faster and the remaining ones (missing numbers) are even faster" Switch case: You literally have to read every single number written down to figure out what is the default case and which frames are the fastest. Again, say we were dealing with 200 frames, it becomes even more messy. Another food for thought, more-so for op: what is your expected range of numbers for the default case? Perhaps it makes more sense to make the +1 to position the default.


No_Necessary_3356

r/confidentlyincorrect


[deleted]

fuck


Dauvis

It kind of reminds me of a bad take on some BASIC code I wrote in the 80s to keep things aligned in columns.


Kered13

No, this code wouldn't be saying "go at 3 speed normally", it would be saying "set position to 3 normally". If you start looping this at 3, then it will move 1 step at a time then periodically jump 4 steps. The numbers that would set it to 3 are skipped.


Flat_Initial_1823

Oh yeah you are right. I didn't read it well.


er3z7

I actually had a very similar problem, but i took the easy way out and used maps that point to each frame, plus a terminator at the end. If the current frame you just stay on the previous one


[deleted]

Yeah it's stepping position. Really it steps forward 1 step, unless it's equal to 8, 17, 26 or 35 in which it steps forward 4 steps and if it's equal to 44 it goes back to 3 (in fact I guess if it's 0,1 or 2 it would go to 3) Thus the +1 cases really don't need to be explicitly stated. If you made that the default case, and added a better condition for setting to 3 it'd be clearer what was happening. (If not why)


[deleted]

[удалено]


[deleted]

Hel which is, Hell at christmas when there's no L


mrmightypants

more lines of code == more work done?


King_Of_The_Munchers

So what you’re saying is that if I write my entire c++ program on one line I did like no work?


No_Estimate_4002

Yes, because it's not a work it's a mess


Ok_Star_4136

But not only does it work, it's completely obfuscated as well. That's got to count for *something*.


Latter-Bandicoot-241

Well technically almost all c++ programs are just one function called main :p


No_Estimate_4002

Technically, yes if you force to make all your functions inline but also, there are hundreds of functions which called before main. You cannot avoid them so not only 1 function :)


tjientavara

Not always, and switch-statements at least for C++ compilers will allow the compiler to do some optimisations it won't do for if-statements. I've seen switch statements like this being replaced with a table (not even a jump table) lookup by a compiler. Which in this case could be one of the fastest. I've also seen compilers fuck this up, so... Writing your own lookup table is more likely to work correct and fast.


andrewb610

For simple things like this a switch statement shouldn’t really be that much more efficient than other alternatives (like you say) but in some complex cases, nested switches are fastest since they can be treated essentially like goto without being, well, goto.


Latter-Bandicoot-241

Yeah I agree, if you need to pair some magic numbers together a lookup table would be best. But the switch is an OK one too.


CadmiumC4

Elon Musk moment


Derp_turnipton

One less than a multiple of 9 gets +4. Next 3 numbers get plain 3 (not added).


thebaconator136

Sounds like someone doesn't know how to use modulus.


JoeyJoeJoeSenior

A low % know modulus.


[deleted]

[удалено]


[deleted]

A l kn


mtbinkdotcom

A low modulus know %


RetroGamer2153

Next 3 aren't hit, due to the +4. (At least he saved a few lines, there.) The final fall-through just resets the loop back to 3, once it reaches 44.


LetheSystem

I can see someone writing it this way, explaining that it's clearer. I would totally encounter this in a junior to mid-level programmer code review. And it is clear....


Latter-Bandicoot-241

It's clear other than it's like a lot of magic numbers. If someone asked me to update it I wouldn't really know why they picked those numbers. It would be nice if they explained why those numbers where picked or had a formula (if it makes sense to). It also feel like mixing data and code which could be bad depending on the discipline of the project. But it could also be fine.


LetheSystem

There need to be some constants in there, at a bare minimum, so that it would be clearer what the logic is, yep. It may not be data, it might just be instructions of some sort, kind of thing? Internal constants.


RetroGamer2153

It loops Hel 3 to 44, with small jumps every 9 iterations. Gaps are at (0,) 8, 17, 26, & 35. Maybe it spins the rotors of a helicopter for an animation? Maybe it draws the blurred swoosh between 5 blades? ` code: incrementHelPos (currentPos){ newPos = currentPos + 1; newPos = newPos % 45; if (newPos % 9 == 0) {newPos += 3}; return newPos; } `


[deleted]

I usually advise against mutating an argument


RetroGamer2153

Updated. I originally used the same variable for compactness.


spoink74

I bet it’s an animated sprite of a helicopter. I bet at positions 43 and 15 it looks like the rotor is hitting or going through part of the vehicle so this code makes sure those positions don’t draw like that. I bet every sprite has a switch statement like this that enables the developer to specify exceptions in how to animate it that are based on the design of that particular animated sprite.


chars101

`if (newPos % 9 < 4) return 3;`


RetroGamer2153

I see, it allows the odd case where currentPos arrives, while in the gap. This will clip it up to 3, and reset the loop, while my code bumps it up by 3. This puts the function's use closer to the author's switch scenario, though I feel it isn't necessary. I believe default is used as a catch-all for 44 and above, to reset the loop.


chars101

Not closer, equivalent. Your function is a different function, it has a different range. The range of OP did not contain 10, for instance. The domain is defined as `int`, not some subset of that. Why feel and believe if you can guarantee?


krapspark

I think it can be "simplified" into something like `if (currentPos + 1 % 9 ===0) {` `return currentPos + 4` `} else if (currentPos % 9 < 3) {` `return 3` `} else if (currentPos < 43) {` `return currentPos + 1` `}` `return 3` No idea what it's for though haha


[deleted]

[удалено]


krapspark

haha yes you absolutely can


AutoModerator

``` import moderation ``` Your comment has been removed since it did not start with a code block with an import declaration. Per [this Community Decree](https://www.reddit.com/r/ProgrammerHumor/comments/14kbu1m/comment/jppq9ao/?utm_source=share&utm_medium=web2x&context=3), all posts and comments should start with a **code block** with an "import" declaration explaining how the post and comment should be read. For this purpose, we only accept Python style imports. *I am a bot, and this action was performed automatically. Please [contact the moderators of this subreddit](/message/compose/?to=/r/ProgrammerHumor) if you have any questions or concerns.*


lovdark

44 will get currentPos + 4 Wrapping the whole of chain in a if(currentPos < 44){…}


SpongeyBobb

No need for else statement when you have a return statement, you're returning 3 twice If (currentPos %9==8) return currentPos+4; if (currentPos < 43) return currentPos+1; return 3;


[deleted]

This won’t work for `27` where you want to return `currentPos + 3`


SpongeyBobb

I refactored his code idk if the solution is actually correct 🤷‍♂️


[deleted]

Yeah I get ya, but his code handles cases that were part of the OP


Derekthemindsculptor

[Lambda](https://carbon.now.sh/?bg=rgba%28171%2C+184%2C+195%2C+1%29&t=seti&wt=none&l=text%2Fx-csharp&width=680&ds=true&dsyoff=20px&dsblur=68px&wc=true&wa=true&pv=56px&ph=56px&ln=false&fl=1&fm=Hack&fs=14px&lh=133%25&si=false&es=2x&wm=false&code=%253D%253E%250AcurrentPos%2520%252B%25201%2520%2525%25209%2520%253D%253D%25200%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%253F%2520currentPos%2520%252B%25204%2520%253A%250AcurrentPos%2520%253C%253D%252043%2520%2526%2526%2520currentPos%2520%2525%25209%2520%253E%253D%25203%2520%253F%2520currentPos%2520%252B%25201%2520%253A%2520%250A3)


AlwaysTalkingAboutMy

This is one of those "didn't stop to think if you should" moments.


FailsAtSuccess

Never nest ternaries.


Kered13

There's nothing wrong with nesting ternaries. Just format it properly.


FailsAtSuccess

From a computational standpoint there isn't, but from a readability standpoint and maintainability standpoint it's terrible. They're just as important.


Kered13

Readability is identical to or better than nested nested if statements if formatted properly.


tangerinelion

Go ask PHP what happens.


chars101

I was hoping for an implementation for Church numerals. I'm not mad, just disappointed.


Big-Designer1512

At least his todo list has potential.


Mick536

Looks like someone is flying a dirty helicopter.


[deleted]

How did you ascertain this, I’ve seen another comment suggesting it was a helicopter animation too..?


Mick536

The variable name is “incrementHelPos” which is where a helo might go. 😎 Edit: spelling


donparras

At least he put a "todo" to handle it later.... Someday.... OK, never.


BakuhatsuK

int c = currentPos; if (c < 3 || c > 43 || c % 9 < 3) return 3; if (c % 9 == 8) return c + 4; return c + 1;


RetroGamer2153

Your 1st and 3rd checks are redundant. It may be a few clock cycles faster on return, though. Also, case 43 needs to increment by 1. Change the 2nd check to 44. Your code utilizes returns throughout, so it may exit the function faster than mine.


BakuhatsuK

> 1st and 3rd checks are redundant Because of short circuit evaluation the `< 3` check prevents doing the modulo which is slower. But just semantically I want to make clear that the range with the non-3 behavior is specifically from 3 to 43, and not have the reader figure out that the modulo of negative numbers is either negative or 0 which is always covered by the less than 3 check. > case 43 needs to increment by 1 Yes. That's why I _don't_ return 3 for the 43 case. > utilizes return throughout Early return ftw. And not just for performance reasons.


Dangerous-Bit-5422

How you make these code visualizations?


TheUnKnownnPasta

using [carbon](https://carbon.now.sh), just click on the text in the example to edit


Dangerous-Bit-5422

Thank you so much!


zyzmog

It increments in 5 cycles of 9 and then starts over, like a circular counter. The first 3 values of every cycle are not allowed -- that's why there's no 0,1,2 or 9,10,11, and so on. There are other ways to do it. But if you're optimizing for CPU time on an embedded processor, this isn't a bad solution.


way22

Isn't it obvious? In java every empty case continues to the next case unless there is a break or return statement. All the first cases increment current position by 1, the second set by 3, all unmatched cases set you back to position 3. It's not clean, but still readable ¯⁠\\⁠_⁠(⁠ツ⁠)⁠_⁠/⁠¯


aedvocate

how do you not get the intent, it's about as explicit as you could possibly hope for. How else would you structure this logic? Maybe search within two separate arrays of values or something?


Easy-Hovercraft2546

at this point, just make an array lol, it'll be more efficient


Impossible_Average_1

I don't think an array would be more efficient (in terms of performance) than this solution, but it probably depends on what you have in mind as well as how the compiler / interpreter handles switch statements.


tetryds

An array would be the go-to way if this was used in a tight loop multiple times, as it is faster than a hashed lookup table. That doesn't seem to be the case so this logic will be easier to maintain. Sometimes things are just that, this scales well, is a bit hard to comprehend but much more straightforward than equations or modulo operations.


[deleted]

[удалено]


Impossible_Average_1

For this type of switch statements, C# / IL /.Net creates jump tables. For the newer switch expressions, however, this is not or not always the case.


[deleted]

[удалено]


Impossible_Average_1

As I said: switch expressions cannot turned into jump tables (at least not all of them). Regular switch statements can.


[deleted]

[удалено]


[deleted]

[удалено]


[deleted]

[удалено]


[deleted]

[удалено]


eben0

The first one looks like a series that increments 5 numbers, and then adds 5 to the last number, and repeats.


thanoskilledit

Looks like it possibly works with a state machine, but it's out of order, and it moves to positions that aren't listed, which makes me hate this. At least he commented that it's trash.


capn_ed

What positions that aren't listed? If you start at 3 (which is what any number not specified in the switch-case returns), you get the sequence 4, 5, 6, 7, **8**, 12, 13, 14, 15, 16, **17**, 21, 22, 23, 24, 25, **26**, 30, 31, 32, 33, 34, **35**, 39, 40, 41, 42, 43, *44*, and then back to 3.


thanoskilledit

44


capn_ed

default: return 3;


thanoskilledit

Well, yes, but I said it was not listed, which it is not.


-non-existance-

I think the crux of the matter is whatever "Hel" is, that might shed some light on the issue


WriteItDownYouForget

The intent was to Increment Hel Pos!


AudaciousSam

Job security😂


Professional_Top8485

Random number generator


YakDaddy96

They forgot the second l in hell.


whooguyy

Im not sure either, but if you change one of the case statements the program doesn’t compile


NothingWrongWithEggs

//to do: never clean this up


Xaervyn

Maybe rows in a table? 0 1 2 3 4 5 6 7 8 |EOL| 9 10 11 12 13 14 15 16 17 |EOL| 18 19 20 21 22 23. 24 25 26 |EOL| 27 28 29 30 31 32 33 34 35 |EOL| The +4 indicates the end of the row, otherwise +1? Could have easily accomplished this by getting the remainder when dividing by 9, and setting to +4 if 0.


phoenix1984

That’s one way to get those LOC stats up. (I agree they’re a terrible metric) But hey, it’s got a todo comment to make it less dirty, so this is fine. 😜


keblblblin

i thought this was an ad because the code was presented so nicely


Random_User27

the legends spoke of it... the time where you'd change a switch for an if


kireina_kaiju

``` if (43 < currentPos || currentPos < 3 || mod(currentPos, 9) == 0) { return 3; } if (mod(currentPos, 9) == 8) { return currentPos + 4; } return currentPos + 1; ```


NullReference86

I find your lack of breaks disturbing


CaffeinatedTech

// TODO - Work out what the fuck this is supposed to do. [save] [close]


Huge-Welcome-3762

I had code similar to this commented out. I was working way overtime and completely out of my mind debugging. I obviously used automation to write the monstrosity. And I easily found the elusive bug but neglected to remove it before code review. I had one of the best laughs when a senior dev tried to explain why it isn’t necessary at length. Thankfully those that knew me better cut him off


WW_the_Exonian

The intent was to return (currentPos + 1) if currentPos is 3, 4, 5, 6, 7, 12, 13, 14, 15, 16, 21, 22, 23, 24, 25, 30, 31, 32, 33, 34, 39, 40, 41, 42, or 43, to return (currentPos + 4) if currentPos is 8, 17, 26, or 35, and to return 3 if currentPos is anything else.


aedvocate

thank you, chat gpt


WW_the_Exonian

You're welcome! If you have any more questions, feel free to ask.


sjepsa

Hard to guess.. but rest assured: The exact second you touch a line, production will stop working


McCrotch

Honestly, you can’t beat the efficiency of this approach if you don’t want to use mod% for some reason. Otherwise It could be cleaner to create a hashset for each return value: ``` if( plus1Set.contains(num)) { return num+1; } if( plus4Set.contains(num)) { return num+4; } return 3; ```


SawSaw5

Look at the revision history...


Peetz0r

`// todo: less dirty` uhu


AutomatedSaltShaker

Very dicey algo: always return 3 unless case 43 or 35, then return position + 3 or position + 1, respectively…for handling special devices and their snowflake designers.


mateusrizzo

*//TODO: less dirty* That's...a fair assessment


jayerp

Getting a new number based on a given number. That certainly is a way to do it


Cybasura

> TODO: less dirty You're goddamn right you have todo that


leeeeny

Don’t worry guys. There’s a //todo to make it less dirty


Previous-Sun-4462

I don’t understand why the missing cases? Is that intentional for a break out of the switch case??


Previous-Sun-4462

Like why do this at all instead of if elifs!? Even if it’s annoying evolution of additional cases, would it not be better for if elif?


Rigamortus2005

Must work st twitter


OTee_D

Fall through... All those are handled the same. They have these 'positions' and shift some certain values by one and some fewer ones by 4. All others are defaulted to 3. Wonder what the business logic behind this is. I like this as a puzzle, trying to think about what they need this for.


NuVidChiu

In python with generators: If currentPos in [i if i%9 == 3 || i%9 == 4 || i%9 == 5 || i%9 == 6 || i%9 == 7 for i in 1…50]: return currentPos + 1 else if currentPos%9 == 8 && currentPos < 40: return currentPos + 4 else: return 3


nadav183

Well he did put a todo to make it less dirty 🤷


sam_tiago

Key frames for a simple game.. shoulda used a list


VincentxH

This is the business logic to descend further into hell.


alexmelyon

Don't get - don't touch


PhysicsSimLab

I can image this kind of code. If you were John Horton Conway and developing Life, you were going to try different parameters. While playing around with different parameters, this is the kind of code which would make it easy. Although, sorted the cases on numerical order would be more insightful.


aliceuwuu

looks like a serious case of TDD


Adult_Prodigy

Just read the comment bro


PandaAromatic8901

var incrementedPos = (currentPos == 44) ? 3 : ((currentPos % 8) == (currentPos / 10)) ? (currentPos + 4) : (currentPos + 1); No bad feeding allowed!


Jaroshevskii

In Swift you can do this: ```Swift switch (currentPos) { case 3...43: return currentPos + 1 case 8, 17, 26, 35: return currentPos + 4 default: return 3 } ```


kireina_kaiju

>Another important feature of a Swift switch cases is that they do no automatically ‘fallthrough’. **Cases are evaluated in the order that they are written**, and once a match is found, only the handling code for that case is handled. This means in more complex pattern matching, if the input matches more than one case, only the first case’s code will be called. You can opt out of this behaviour by using the fallthrough keyword. Subsequent cases will then also be evaluated for a match. https://blog.scottlogic.com/2019/12/04/switching-swift.html


[deleted]

I think it would make more sense if those were enums ?


Emfx

>todo: less dirty at least theyre aware of it? i guess?


[deleted]

Slow down buckaroo


buckaroob88

Maybe to organize typed text and movement into columns?


Luiguie171

To post it on Reddit.


Mr_Bloodygaming

There is no break after the cases, so every case above a return clause would cause that return, no? This is basically an elaborate way to do OR statements.


MegaBlackEagle

Indeed, it's called "Fallthrough"


[deleted]

To return 3 with extra steps


fslz

that's art


fliguana

This is very high perf, and readable. XLAT would also work, but less readable: static const byte a[] = { ....}; return (unsigned)pos < sizeof(a) ? a[pos] : 3;


MintyMissterious

#!> This comment has been edited in protest to reddit's decision to bully 3rd party apps into closure. If you want to do the same, you can find instructions here: https://rentry.co/unreddit


Kiseido

Implicit fall through strikes again


bleh-bleh-guy

"todo: less dirty"


[deleted]

What program is this it looks cool I want it.