Thread: Is this an approved use of goto?

  1. #16
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665
    Well, I think the OP's code is probably fine if it's true C-style. Like he said, he's only using a C++ compiler because his IDE doesn't know how to C. phantom's example with the myriad functions is a good approach as well. goto's can always be circumvented and it's fundamentally up to the coder to decide what's more readable. I know, good and common practices and all that jazz but the OP's code isn't a total plate of spaghetti. But judging from the amount of flack it's incurred here, I can't honestly recommend a goto to the OP. And I think I've even made similar topics like this in the past...

    Otherwise, I think it should almost be intuitive that destructors should handle resource deallocation as it's just so convenient but this is C++. But we have to be careful though because we'll start putting everything into a class and then we wind up with Java and that's not cool. So we have to keep it classy but not too much.

    And dang, phantom be dropping the hammer.
    Last edited by MutantJohn; 03-18-2014 at 11:29 PM.

  2. #17
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    But we have to be careful though because we'll start putting everything into a class and then we wind up with Java and that's not cool. So we have to keep it classy but not too much.
    O_o

    There is an entire universe of difference between using existing tools and creating new classes which serve no purpose other than emulating those tools.

    Code:
    //using std::shared_ptr; // or
    //using boost::shared_ptr; // or
    //using std::vector;
    vector<shared_ptr<HANDLE> > sProcesses(queryAllProcessIds(/**/));
    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  3. #18
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    bahamut123:
    Refer to topic #73 of Herb Sutter's "C++ Coding Standards"
    Throw by value catch by reference
    http://books.google.com.ua/books/abo...d=mmjVIC6WolgC

    Note he also states that where possible (which is most places in fact), catching by const-reference preferred.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  4. #19
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Personally, I don't like the use of goto in the OP. I also disagree with suggestions to use C++ exceptions - it is not the purpose of exceptions.


    The only concrete suggestion made so far that I'd accept is Soma's of breaking the code into component functions.

    As an alternative (which I don't claim is better than Soma's approach, but is arguably simpler if component operations don't need to be reused) the OP's function can be restructured like this;
    Code:
    systemSnapshot * createSystemSnapshot( )
    {
        systemSnapshot * snapshot = ( systemSnapshot * )createArray( NULL, 1, sizeof( systemSnapshot ), zeroArray );
     
        if (snapshot)
        {
            int succeed = ((snapshot->processIds = ( DWORD * )queryAllProcessIds( &snapshot->arraySize ) ) != NULL));
            if (succeed && (snapshot->processNames = ( char ** )createArray( NULL, snapshot->arraySize, sizeof( char * ), zeroArray ))
            {
                HANDLE * processHandles;
                if (succeed = (processHandles = ( HANDLE * )queryProcessHandlesFromIds( snapshot->processIds, snapshot->arraySize ) != NULL))
                {
                    for ( int i = 0; i < snapshot->arraySize; ++i )
                    {
                        snapshot->processNames[i] = ( char * )getProcessNameFromHandle( processHandles[i], false );
                        CloseHandle( processHandles[i] );
                    }
                }
                free( processHandles );    /* has to be freed regardless */
            }
            else
                succeed = 0
            if (!succeed)
            {
                free(snapshot->ProcessIds);
                free(snapshot);
                snapshot = NULL;
            }
        }
        return snapshot;
    }
    It produces the same result, with only minor reordering, and explicitly makes use of free(NULL) being a no-op.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  5. #20
    Registered User
    Join Date
    Nov 2013
    Location
    Norway
    Posts
    40
    Quote Originally Posted by phantomotap View Post
    You are a tremendously awful C++ programmer.

    No wonder you need your "trick"...
    Is that what pinkie pie would say, my friend? :C
    "Derp, derp, derp" - Me

  6. #21
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    Is that what pinkie pie would say, my friend?
    O_o

    There is a quote from the incredible Fred Rogers, one of the best men to ever live, in my signature, yet you ask about the cartoon pony?

    *shrug*

    Nope. That isn't what Mr. Rogers would say, but Mr. Rogers neighbors weren't crap programmers so he didn't have to do maintenance on their crap code.

    Of course, that would not matter in the slightest because Mr. was a spectacular individual.

    Really though, that only goes to show that I am not as good a person as Mr. Rogers.

    *shocker*

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  7. #22
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by grumpy View Post
    It produces the same result, with only minor reordering, and explicitly makes use of free(NULL) being a no-op.
    I don't like it because it produces spaghetti ifs instead. If statements all over the place where you must remember which else connects to which if and tons and tons of nesting. This is typical Microsoft code.
    RAII makes it clean without spaghetti gotos or spaghetti ifs.
    But this is C++ - is it supposed to be C++ or just C with a C++ compiler? If it's the later, then just restructuring the code might be a way to go. If it's C++, then I'd argue RAII.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  8. #23
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by Elysia View Post
    I don't like it because it produces spaghetti ifs instead. If statements all over the place where you must remember which else connects to which if and tons and tons of nesting.
    I suggest you're exaggerating somewhat. Basically all the nested if's are doing is a series of (at least) four operations that can succeed or fail, and that failures mean it is necessary to cleanup up preceding operations that succeeded.

    Quote Originally Posted by Elysia View Post
    RAII makes it clean without spaghetti gotos or spaghetti ifs.
    But this is C++ - is it supposed to be C++ or just C with a C++ compiler? If it's the later, then just restructuring the code might be a way to go. If it's C++, then I'd argue RAII.
    My suggestion was in response to a message I received, loud and clear, from preceding posts that it is "just C with a C++ compiler". The fact it is using free() to release memory also supports that belief.

    Which negates use of techniques like RAII. If it was pure C, I'd also eliminate the type conversions. But that would make it uncompilable with a C++ compiler.

    But I stand by the position that the OP has not demonstrated a worthy use of goto and that (contrary to suggestions of others) using exceptions for flow control is not a particularly good idea.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  9. #24
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by grumpy View Post
    I suggest you're exaggerating somewhat. Basically all the nested if's are doing is a series of (at least) four operations that can succeed or fail, and that failures mean it is necessary to cleanup up preceding operations that succeeded.
    True, I am exaggerating a little for an example this small, but I just wanted to make a point that I dislike approaches like these.

    But I stand by the position that the OP has not demonstrated a worthy use of goto and that (contrary to suggestions of others) using exceptions for flow control is not a particularly good idea.
    In this we are in agreement.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  10. #25
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,795
    Quote Originally Posted by phantomotap View Post
    There is a quote from the incredible Fred Rogers, one of the best men to ever live, in my signature, yet you ask about the cartoon pony?
    I really like his duets with Dolly Parton

  11. #26
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    Basically all the nested if's are doing is a series of (at least) four operations that can succeed or fail, and that failures mean it is necessary to cleanup up preceding operations that succeeded.
    O_o

    You aren't wrong, but in such a case for a resource returning function, you'll likely need to separate the "cleanup" for the resource for an interface available to clients. Combine the fact with the partial lack of relative dependency and I can further refine the code you've provided by placing the "extra" checks you've done into the implementation for that "cleanup" interface.

    Code:
    systemSnapshot * createSystemSnapshot( )
    {
        systemSnapshot * snapshot = ( systemSnapshot * )createArray( NULL, 1, sizeof( systemSnapshot ), zeroArray );
      
        if (snapshot)
        {
            /* I'm still breaking these lines because the massive `if' condition would be repellant. */
            /* These two components are dependent on `snapshot', but they don't depend on each other. */
            int haveIds = ((snapshot->processIds = ( DWORD * )queryAllProcessIds( &snapshot->arraySize ) ) != NULL);
            int haveNames = ((snapshot->processNames = ( char ** )createArray( NULL, snapshot->arraySize, sizeof( char * ), zeroArray ) ) != NULL);
            HANDLE * processHandles = NULL;
            if (haveIds && haveNames && (processHandles = ( HANDLE * )queryProcessHandlesFromIds( snapshot->processIds, snapshot->arraySize ) != NULL))
            {
                for ( int i = 0; i < snapshot->arraySize; ++i )
                {
                    snapshot->processNames[i] = ( char * )getProcessNameFromHandle( processHandles[i], false );
                    CloseHandle( processHandles[i] );
                }
            }
            else
            {
                destroySystemSnapshot(snapshot);
            }
            free( processHandles );    /* has to be freed regardless */
        }
        return snapshot;
    }
    The change is by no means huge, but to my mind, the change is invaluable. To me, this says exactly what needs to happen, why, and how to do it.

    using exceptions for flow control is not a particularly good idea
    O_o

    Congratulations on your entry into the "Understatement Awards"...

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  12. #27
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    I really like his duets with Dolly Parton
    O_o

    Fred Rogers > Kenny Roger

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  13. #28
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665
    Actually, are we allowed to ask about the pony? It's just so... the exact opposite of who you are on these forums. Like, the pony is all innocent and non-threatening and that's just not you, phantom.

    Granted, I'm not actually Pleiades but I mean, I really like the colors blue and black. And space is cool.

  14. #29
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,795
    Quote Originally Posted by phantomotap View Post
    Fred Rogers > Kenny Roger
    Oh. You're right, my mistake I got confused. I will goto the naughty corner. Do you approve of this use?

  15. #30
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    Actually, are we allowed to ask about the pony?
    O_o

    Sure. You may ask, but don't complain to me if you don't like the answer.

    Defined gender roles can kiss my shiny metal ass. I do the "daily workout" thing, and I've eaten enough bacon to be legally classified as part pig, and I'm even more domineering and aggressive in real life. (I apologize if your head came off.) Despite all of that sort of thing, I'm the least "macho" person I know. I had great role models that don't fit those roles, yet I'm not those people. I'll never be as good as those people; I'll never be as good a man as Mr. Rogers. That doesn't mean that I don't appreciate those people, those values; I cherish my role models even if they are far beyond me.

    Watching Mr. Rogers be himself, genuine and kind, is moving, heartening because he was such an amazing individual. The pony is not really different along the same path; I like Pinkie Pie because she is goofy and aggressive--in her way--without being like me.

    So for lack of better wording, I like Pinkie Pie precisely because she is "the exact opposite" while still having some of the few traits I find value in myself.

    [Edit]
    Mister Rogers defending PBS to the US Senate - YouTube
    Fred Rogers Acceptance Speech - 1997 - YouTube

    Obviously, I am very definitely not Mr. Rogers, but how could one not cherish the man?
    [/Edit]

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Approved Nintendo Programmer for hire
    By DScoder in forum Projects and Job Recruitment
    Replies: 0
    Last Post: 07-24-2007, 09:24 PM
  2. Why is goto so bad?
    By Munkey01 in forum C++ Programming
    Replies: 14
    Last Post: 01-02-2003, 07:11 PM
  3. Approved Slang..
    By Cheeze-It in forum A Brief History of Cprogramming.com
    Replies: 16
    Last Post: 10-06-2002, 09:58 PM