The Yandere Dev/Pirate Software dilemma

4 months ago by MousePotatoDoesStuff to c/godot

UPDATE: The consensus seems to be overwhelmingly in favour of the match variant. And not to worry, I have replaced the magic numbers with an enum. Will try to remember to merge the branch tomorrow

Does an if-statement block or a switch statement fit better here? For context (and advertisement), this is part of my all-purpose utility plugin ( Codeberg link)

The code:

        # Method 1 (Yandere Dev Technique)  
        if self.throw_errors and status==MpupTest.TESTSTATUS.ERROR:  
            push_error(result)  
        if self.throw_warnings and status==MpupTest.TESTSTATUS.WARNING:  
            push_warning(result)  
        
        # Method 2 (Pirate Software Technique)  
        match status:  
            MpupTest.TESTSTATUS.ERROR:  
                if self.throw_errors:  
                    push_error(result)  
            MpupTest.TESTSTATUS.WARNING:  
                if self.throw_warnings:  
                    push_warning(result)  
refalo 8 points 4 months ago

If status is modified in another thread you could have a race condition with method 1 because you're checking it twice. The two methods are not exactly the same code due to that.

path: 0 24092816, hotness: undefined, score: 8, children: 1
MousePotatoDoesStuff 4 points 4 months ago

Not to worry, the variable is local and the function is single-threaded.

The elif is a good idea, though - although I'll likely go with a switch statement in the end.

path: 0 24092816 24092886, hotness: undefined, score: 4, children: 0
lbfgs 7 points 4 months ago

Not sure about how to do this with Godot, but a common pattern for this in other languages is creating something like a 32 bit value where the lower bits are the enum and the upper bits are the status code (assuming they are <=16bit each) and then doing a switch on the resulting packed value.

path: 0 24091681, hotness: undefined, score: 7, children: 1
sp3ctr4l 5 points 4 months ago

Both these devs are far too full of themselves to think of using an enum table or anything clever with datatypes themselves.

Because that would mean the way they've been doing things was potentially grossly inefficient at runtime... and they've been doing that for years...

But a narcissist never makes a mistake, so instead, here's a bunch of reasons why actually writing optimized code is stupid, first among them being 'well it didn't occur to me, therefore doing it this way would reduce code readability for this code base that only I and no else needs to read'.

I am guessing the concept of a sentinel value or an overloadable function would make their minds implode.

But anyway, 'switch' is 'match' in gdscript:

https://docs.godotengine.org/...

match <test value>:
    <pattern(s)>:
        <block>
    <pattern(s)> when <pattern guard>:
        <block>
    <...>
path: 0 24091681 24097557, hotness: undefined, score: 5, children: 0
copygirl 5 points 4 months ago

Are 1 and 2 magic values in this case?
You could use consts or even use an enum to further improve readability:

const STATUS_ERROR   := 1
const STATUS_WARNING := 2
# or alternatively:
enum Status { FINE, ERROR, WARNING }

In the latter case, you'd change status to be the new enum type Status.

edit: Wait, nevermind. I think you're already using enums in the actual code (not screenshot).

path: 0 24092603, hotness: undefined, score: 5, children: 1
MousePotatoDoesStuff 5 points 4 months ago

Oh yeah, I forgot to update the screenshot, yes. Thanks for reassuring me that it was good decision to use an enum!

path: 0 24092603 24092914, hotness: undefined, score: 5, children: 0
protogen420 4 points 4 months ago

i am rust dev so I pretty much just use match all the time

path: 0 24091684, hotness: undefined, score: 4, children: 0
Ledivin 3 points 4 months ago

I'd personally lean towards the switch pattern

path: 0 24091360, hotness: undefined, score: 3, children: 0
darthelmet 2 points 4 months ago

Naively: Isn’t the second one slightly better because the first one will always check both variables twice but the second one only checks one of them once then the other variable once when inside?

path: 0 24102238, hotness: undefined, score: 2, children: 2
MousePotatoDoesStuff 2 points 4 months ago

You're right, it is slightly better. Not really a concern here since this code runs only once per run for each unit test (and only for debugging, it's not meant to be in the game release) so it shouldn't be a performance issue, but still.

path: 0 24102238 24117830, hotness: undefined, score: 2, children: 0
fibojoly 0 points 4 months ago
path: 0 24102238 24125541, hotness: undefined, score: 0, children: 0
tabular 1 point 4 months ago

I would be tempted to rename "result" to something more descriptive.

path: 0 24095665, hotness: undefined, score: 1, children: 3
waldfee 1 point 4 months ago

Imo result is more descriptive than even Java-esque variable names, because it tells you that this is the variable that will actually be returned, rather than just some intermediate representation

path: 0 24095665 24100905, hotness: undefined, score: 1, children: 2
tabular 1 point 4 months ago

The function name ought to describe what is being returned?

path: 0 24095665 24100905 24108557, hotness: undefined, score: 1, children: 1
waldfee 2 points 4 months ago

Yea, basically. Like, the users shouldn't have to look at your implementation to find that out what it will return. My example here would be Square.area(). Maybe they would have to look at the method's docs though to find out what unit the result is in

path: 0 24095665 24100905 24108557 24109580, hotness: undefined, score: 2, children: 0
godot
godot

@programming.dev

login for more options
7983
1194
571

Welcome to the programming.dev Godot community!

This is a place where you can discuss about anything relating to the Godot game engine. Feel free to ask questions, post tutorials, show off your godot game, etc.

Make sure to follow the Godot CoC while chatting

We have a matrix room that can be used for chatting with other members of the community here

Links

Other Communities

Rules

  • Posts need to be in english
  • Posts with explicit content must be tagged with nsfw
  • We do not condone harassment inside the community as well as trolling or equivalent behaviour
  • Do not post illegal materials or post things encouraging actions such as pirating games

We have a four strike system in this community where you get warned the first time you break a rule, then given a week ban, then given a year ban, then a permanent ban. Certain actions may bypass this and go straight to permanent ban if severe enough and done with malicious intent

Wormhole

!roguelikedev@programming.dev

Credits

  • The icon is a modified version of the official godot engine logo (changing the colors to a gradient and black background)
  • The banner is from Godot Design

go to feed...