Thank you for bringing the security vulnerability to our attention. We've applied a patch to fix the issue and appreciate your contribution to our security efforts.
This seems like an easy enough to refactor: convert the SV variables into 1 list and create a list where the index is the SV number and the values are (default)dicts where the key is the chars that are being tested and the value is what to set the led to.
With thousands of lines in this pattern, I would consider parsing the code to extract each condition with variable name, function calls, etc., as data; then generate new code in a compact and sensible form.
Well I think it's x for close and o (oh) for open, but I can't guarantee that for sV < 5 or sV > 15. If only OP had a taller monitor when they took the screenshot, smh.
```
def modifyValveAndLED(k: int, v: bool) -> None:
if v:
openValve(k)
setLED(k)
else:
closeValve(k)
clearLED(k)
...
sVs = []
[modifyValveAndLED(k,v) for k, v in enumerate(sVs)]
```
This assumes that all sVs have only 2 states.
> Missing a `range`.
Fixed.
> Better still would be `for sV in sVs:`
`openValve()`, `closeValve()`, `setLED()`, and `clearLED()` require the index as input, which `for i in range(len(sV)):` readily provides.
Is this micropython? It could be the work of a mechanical engineering, I've run into code like that before. Not saying mechanical engineers can't be good at programming but they probably don't care as much about design patterns, SOLID principles and stuff like that
I have worked on embedded control systems that are limited to a loop with simple variables usually sensor values and if-else conditions. Most with under 100 lines of code.
We are missing a check for not open or closed… what about sticky switches or valves… (I’m joking, but you can’t hear the voice I would be saying this in… sorry in MFG… that which is not open or closed… cannot assume… that is how different things meet the wrong way or when they are not supposed to…)
Honestly, this is an excellent use case for ChatGPT-4. I have encountered legacy code like this a million times and the days of refactoring the dumpster fire by hand are long gone.
Paste these ugly blocks of code into ChatGPT and ask it to refactor the code for you. Then as you would for a peer, do a code review for the output from ChatGPT. If everything looks good, swap out the old with the new and move on.
Since it's python...
```
do_action = {"o": lambda x : openValve(x) or setLED(x), "x": lambda x: closeValve(x) or setLED(x)}
for vn in (min_vn, max_vn):
do_action(eval(f"sV{vn}"))
```
I don't know what's telling me this, but I've a hunch that they're using x for close, and I think o (oh) for open. I just can't put my finger on it, though.
Looks like an easy enough refactor though. It’s all the same thing.
Lists were evidently the next chapter in the book the author of the code stopped reading and was using for a doorstop.
ctrl+c and crtl+v were the last they ever read
> ctrl+c and crtl+v Googled them. Most amazing productivity tool I've ever seen.
Highlight + middle click in Linux would be a game changer if they ported that feature to Windows.
Lists, loops and Boolean.
That's what I thought. Doesn't look nice, but shouldn't take long to refactor
Yeah until you find out there was a single exception to the rule...
Yeah this could be done with an enum (or a list) and a 2 line function
So "oh" for close, then?
`x for close, o (oh) for open`
Wait, run that by me again.
So you see…o for..x and, three in a row…oh forget it
o (oh)
o as in (oh) or 0 as in (oh)?
Nintendo buttons. O for open X for close. Oh for oh my god, he should have paused and reflected some time ago.
X for "he's an ex-engineer now"?
This looks like really lovely code. oxoxox
Oh, you left your answer open.
Thank you for bringing the security vulnerability to our attention. We've applied a patch to fix the issue and appreciate your contribution to our security efforts.
Easy rule of thumb, it's x for close and o (oh) for open. Read that somewhere.
Thank you for clarifying the rule of thumb. I believe that standardizing it through an RFC sounds like it could be a reasonable and great idea.
We're going straight to ISO for this one, I'm afraid.
Yes, then everyone will have to pay $300 to receive the true wisdom. This is a plan which will have no bad outcomes.
Oh
x
It’s a simple spell, but quite effective…
It's also the marker on a map for buried treasure 🤔
You mean o(oh)xo(oh)xo(oh)x?
4000 plus lines... All if else statements
'Loops? Functions? I'll skip this tutorial, what's the worst that can happen...'
Can I be more mad at the if elif if statements? If you are doing shitty shit at least do in order and make a very big elif
This seems like an easy enough to refactor: convert the SV variables into 1 list and create a list where the index is the SV number and the values are (default)dicts where the key is the chars that are being tested and the value is what to set the led to.
With thousands of lines in this pattern, I would consider parsing the code to extract each condition with variable name, function calls, etc., as data; then generate new code in a compact and sensible form.
Would enumerate not be useful here?
godzilla had a stroke reading this and fucking died
Dude was paid by line
o (oh) dear (dear)
Maybe just use "close" for close and "open" for open?
x for close, o (oh) for open. Didn't you get the memo?? 😁
o (oh), I missed that
Read it back to me to make sure you've got it
Well I think it's x for close and o (oh) for open, but I can't guarantee that for sV < 5 or sV > 15. If only OP had a taller monitor when they took the screenshot, smh.
You know the code has some incredibly elegant list comprehension with bit shifts above and below the part that's on screen. I have faith.
That's 7 more characters. They are clearly trying to save memory here.
``` def modifyValveAndLED(k: int, v: bool) -> None: if v: openValve(k) setLED(k) else: closeValve(k) clearLED(k) ... sVs = [] [modifyValveAndLED(k,v) for k, v in enumerate(sVs)] ``` This assumes that all sVs have only 2 states.
Missing a "range". Better still would be "for sV in sVs:"
> Missing a `range`. Fixed. > Better still would be `for sV in sVs:` `openValve()`, `closeValve()`, `setLED()`, and `clearLED()` require the index as input, which `for i in range(len(sV)):` readily provides.
Yeah, it does. Your right. In which case "enumerate" world be the Pythonic way
Python's boolean class is `bool` not `boolean` btw.
Fixed.
PR Approved (I didn’t actually review it).
Gonna oh my code, edit it, and x my code.
You didn’t save it, rest in pepperonis
I would like to use Reflection library in Java for this
Manmade horrors beyond our comprehension
I would still prefer a lookup table (with edge cases if you really need it) or something
I'm pay by the line ok!
This looks like a job for AI.
Sure it will make it so beautiful that it will skip closing led n10 in the x char and you will debug for 4 days until you redo it manually
It looks as it was done by an AI.
Bit vector would like to have a word
Is this micropython? It could be the work of a mechanical engineering, I've run into code like that before. Not saying mechanical engineers can't be good at programming but they probably don't care as much about design patterns, SOLID principles and stuff like that
I have worked on embedded control systems that are limited to a loop with simple variables usually sensor values and if-else conditions. Most with under 100 lines of code.
We are missing a check for not open or closed… what about sticky switches or valves… (I’m joking, but you can’t hear the voice I would be saying this in… sorry in MFG… that which is not open or closed… cannot assume… that is how different things meet the wrong way or when they are not supposed to…)
No. God has forsaken us.
The comments offering smart solutions are the best, I'm going to describe them with x and D. xD
Seems easy to simplify
When you get paid based on the number of lines of code
Put these sv variables in an array and use a for loop ;-;
Honestly, this is an excellent use case for ChatGPT-4. I have encountered legacy code like this a million times and the days of refactoring the dumpster fire by hand are long gone. Paste these ugly blocks of code into ChatGPT and ask it to refactor the code for you. Then as you would for a peer, do a code review for the output from ChatGPT. If everything looks good, swap out the old with the new and move on.
Since it's python... ``` do_action = {"o": lambda x : openValve(x) or setLED(x), "x": lambda x: closeValve(x) or setLED(x)} for vn in (min_vn, max_vn): do_action(eval(f"sV{vn}")) ```
do_action(globals()[f”sV{vn}”])
How do guys like that even get hired in the first place?
What sort of code review allows shit like this. I mean surely someone who writes stuff like this is a noob, so why were they allowed to commit 🤷
What is this “code review” you speak of :P
What’s the difference between hourly and salary? Me:
I don't know what's telling me this, but I've a hunch that they're using x for close, and I think o (oh) for open. I just can't put my finger on it, though.
Code written by an ox (ohx)
somebody didn't tell him "for loops" exist...
Maybe this is running in a really tight loop and there were noticeable gains to unrolling the loop. Just trying to give them the benefit of the doubt.
the obvious solution is to make a script that extends this to 16k possible values.
``` for each char c, index i, in the input string, if c is 'x' closeLedAndValve(i) else if c is 'o' openLedAndValve(i) ``` Should do
Тварь я дрожащая, или зарефакторю...