• Edd (unregistered)

    active = FILE_NOT_FOUND

  • Prime Mover (unregistered)
    bool SomeCommentClass::IsFrist(bool& frist)
        {
            frist = false;    // yep, sorry, false I'm afraid, you should get in quicker
            return false;
        }
    
  • WTFGuy (unregistered)

    Many of us have heard of the Schrödinger equation used in quantum mechanics to solve wave functions and such.

    We've all heard of Schrödinger's cat which is both alive and dead until you check on it. A "superposition of states" in quantum-speak.

    We've now encountered Schrödinger's method. Which is both true and false simultaneously. And goes one better than the boring old cat because even after inspection the superposition remains; it doesn't decay.

  • (nodebb) in reply to WTFGuy

    This is a very useful helper method which gives you the two options for active status, true and false.

  • Ondřej Vágner (google)
    impl Comment {
        fn is_active(&mut self, active: bool) -> bool {
            let is_it_active = if active {
                active && self.active
            } else {
                !active
            };
            let inactive = !is_it_active;
            self.active = if self.active {
                !active && is_it_active
            } else {
                active || !inactive
            };
            false
        }
    }
    
  • Sole Purpose Of Visit (unregistered)

    The slightly worrying implication is that this method is used all over the code base. God only knows how you refactor it.

  • aalien (unregistered)

    Unfortunately, no one can be told what active is. You have to see it for yourself.

  • WTFGuy (unregistered)

    Gah! The problem was staring us all in the face and we didn't see it. Hidden in plain sight as it were.

    The name of the class is SomeActivatedClass. Not SomeActivatableClass. So we know every instance of the class must always be active because, as we all know, classes are never misleadingly named. That proves the bool ref parameter is the one to use and the bool return value should be discarded.

    Maybe best to refactor by checking in a change where the IsActive method is now a void method? Preferably under another dev's credentials. What (more) could go wrong than already is? :evil_grin:

  • WTF... the film! (unregistered)

    These seems like one of those pesky situations where if you have to ask, you can't afford it, but then there's a twist and suddenly you found out the true moral of the story, which is that you had it all along.

  • (nodebb) in reply to Sole Purpose Of Visit

    First change the return to void.... Build, the errors all become "x = false". Next remove parameter.... Build, the errors all become "v=true"... remove method..

  • Robin (unregistered)

    I wonder what tests they have for this method, and whether they pass or not?

    Only kidding, obviously any company that can tolerate "code" like this will consider automated testing a waste of time.

  • Sole Purpose Of Visit (unregistered) in reply to TheCPUWizard

    Brillant! So we assume that Schroedinger's cat is definitely either alive or dead (no Miracle Max here) and that the superposition collapses correctly in all cases. (This would be your two build steps.) Compile/link job done.

    Then we assume a test environment with sufficient coverage to catch all cases where false becomes true and true becomes false. (Which of course assumes a test environment of any kind.) Test job done.

    As a prior, we assume that a collection of programmers who deal with entities that are, at base, either active or inactive, and who understand the difference between a dead cat, a live cat, and an indeterminate bouncing cat, know what they are doing with various feline splatters. It's hard to see how a half-smart cat-wrangler could fail to notice the awfulness of this method, but we are led to assume that only one of them (the OP) did.

    All joking aside, I really like your approach and I would recommend it. I just suspect that you'll end up with a huge pile of side-effect hidden indeterminate traps for the unwary, later on ... not the most concise description, but I think the acronym is appropriate.

  • (nodebb)

    I'm thinking that there's a conflict / misunderstanding - we don't have any information on what the return value means versus what the [out] parameter means. Possibilities:

    • returns false=OK, true=error for the act of discovering the state, while the [out] parameter is the answer.

    • returns the active/inactive state, and the [out] parameter is the error state for finding the answer.

    Both are bogus, naturally. Well, maybe. Depends on what "active" even means - is it the informatic object that's active / inactive, or the "problem domain" object? And in either case, are there objects of that type that are always active?

    Regardless, without more information, we're making stabs in the dark...

  • ooOOooGa (unregistered) in reply to TheCPUWizard

    The worrisome part to me is the idea that this function may possibly be overridden in some derived class so that it a) does something useful, or more likely b) has some side effects.

  • ooOOooGa (unregistered)

    I guess, fortunately for once, since this is C++ you would have to declare the function virtual before being able to override it.

  • (nodebb) in reply to Sole Purpose Of Visit

    God only knows how you refactor it.

    It's easy. First, we observe we can make a more general function.

    bool isTrueOrFalse(bool& answer)
    {
            answer = true;
            return false;
    }
    

    Then we replace the specific logic in our function

    bool SomeActivatedClass::IsActive(bool& active)
     {
             return isTrueOrFalse(active);
     }
    

    (not a C++ programmer, so apologies for any mistakes).

  • WTFGuy (unregistered)

    @CPUWizard ref:

    First change the return to void.... Build, the errors all become "x = false". Next remove parameter.... Build, the errors all become "v=true"... remove method..

    Won't quite work. Consider

    bool myActiveState = whatever; // initialization value doesn't matter
    bool myReturnValue  =  whatever; // initialization value doesn't matter
    SomeActivatedClass someActivatedObject = new someActivatedObject( /* whatever initialization parameters */ ); 
    myReturnValue = someActivatedObject.IsActive(myActiveState) ;
    ...
    
    

    The refactor needs to preserve both assignments:

    myReturnValue  = false;
    myActiveState = true;
    

    So you end up with an intermediate stage like

    myReturnValue = false;
    someActivatedObject.IsActive(myActiveState) ;
    

    You can't simply replace every method call that uses the return value with myReturnValue = false; and stop there.

  • Zygo (unregistered)

    I am disappointed that in C++

    bool active = obj.IsActive(active);

    misses a golden opportunity to have an undefined result.

  • Anoter "J" (unregistered) in reply to ooOOooGa

    Well... I commonly override non-virtual functions.

  • (nodebb) in reply to ooOOooGa

    I guess, fortunately for once, since this is C++ you would have to declare the function virtual before being able to override it.

    You have to declare it virtual in the base class. We don't know if this is the base class. (Derived classes are allowed but not obliged to redeclare the virtualness.)

  • NoLand (unregistered)

    I like this a lot! :-)

    Regarding Schrödinger's cat, we now finally know, it's actually about our modus operandi: If we ask the cat, wether it is alive or not, it will tell us that it is dead (so how is it able tell us anything at this point), while, ehen we're going to inspect it, we will find it alive. In other words, Schrödinger's cat is a slacker.

  • Sole Purpose Of Visit (unregistered) in reply to Zygo

    That's a nice easy refactoring.

    bool SomeActivatedClass::IsActive(bool& active)
        {
            active = true;
            const bool * const undefinable = 0;
            return *undefinable;
        }
    

    The const keyword is added as a "master of the universe" way of insisting that I am a C++ guru.

    Even better, you could (I think: I have no intention of trying this) define an std::terminate_handler, and implement the perfectly reasonable "solution" of using a random number generator to allow 99 out of 100 cases to avoid an abort.

    It is my fervent belief that the most cromulent way of avoiding an abort, in this case, would be to embed a longjmp into the handler. But again, I have yet to try this.

  • (nodebb) in reply to ooOOooGa

    "The worrisome part to me is the idea that this function may possibly be overridden in some derived class so that it a) does something useful, or more likely b) has some side effects."...

    NOPE: C# requires that the method be declared virtual in order for it to be overridden... It can be hidden, but that is a whole different set of issues.

  • (nodebb) in reply to WTFGuy

    @WTFGuy - you only did the first sentence of my post...not the second....

  • gnasher729 (unregistered)

    How do you refactor it: I change the declaration and definition to xxxisActive and press "Build". Then I locate the errors and change each of them appropriately. And then I have someone review my code.

  • WTFGuy (unregistered)

    @TheCPUWizard

    Not quite. I saw your second part intending to replace the method call with parameter to become an assignment v = true;.

    But I'd over-interpretted your first part as replacing the entire method call with x = false;. Which would leave nothing to error out at that location in your second phase.

    Bottom line that (I think) we agree on is that the correct refactor is NOT either x = false OR v = true. It's either v = true (if the return value is discarded) or v = true AND also x = false (if the return value is kept).

  • MaxiTB (unregistered)

    Haha, this is funny ... I think a lot of people here haven't been coding before Clean Code was published and managed languages weren't around.

    This is clearly a simple setter. The return is, as was usual for C an error code.

    Now I know what you are thinking, like why name it IsActive?

    Well, perhaps someone did a fluent interface and this method was a terminator?

    Maybe it is just a were purely named method from a time or developer long before naming coventions beyond hungarian were really a thing?

    Or perhaps the mother tounge of the dev is not English and that resulted in a poor grammar construct?

    In see not big WTF there without more context, if it's not part of a fluent method group, I would just rename it accordingly so that English native speakers don't get to much confused.

  • t (unregistered) in reply to MaxiTB

    So .. it sets whichever bool you give it to true and tells you there was no error while doing so?

  • Best Of 2021 (unregistered)

    Oh this is a work of art

    @MaxiTB it's not a setter, it doesn't set anything in instance state, it only assigns its parameter

  • 516052 (unregistered) in reply to t

    Most likely, yes. Odds are good that there was some logic on the side of that which could have generated an issue and got removed in the meantime leading to this stub.

  • nomi (unregistered)
    Comment held for moderation.
  • idviking (unregistered)

    ID Viking: Your #1 Trusted Source https://idviking.ph/

Leave a comment on “Are You Active Enough?”

Log In or post as a guest

Replying to comment #528307:

« Return to Article