Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

>> I am not suggesting any nesting of anything. Repeatedly checking at the same indentation level.

This could be considered wasteful. You end up checking if something is NULL, if it is you jump to the cleanup code and immediately check again. Nesting may be more elegant.

>> What matters more to you, getting stuff right or repeating adages that other people have said out of context? `goto` is probably the cleanest way to do it in plain C.

Writing code in a way consistent with the team I work with and the established codebase.

>> What if a year later some future maintainer totally unfamiliar with the code needs to add another one?

Then they need to read what the function is doing and understand it before they mess with the code, just like in any other situation.

>> Doing that is a lot harder if the free statements are all over the place and repeated several times instead of in a single block.

More verbose certainly, but if the code is written in small, discrete functional blocks then it shouldn't really impact much.

He repeats frees, you repeat tests. An indenter would repeat neither but has indent readability to consider.

Frankly, don't trust anyone that tells you that they have the one true way to do things.



> This could be considered wasteful.

It's true that there is an extra compare. I think it's a small cost for maintainable code.

> Then they need to read what the function is doing and understand it before they mess with the code, just like in any other situation.

Sounds great, however, the time they spent figuring out your haphazard, repetitive and confusing free() statements could be better spent somewhere else. When I said that my way is more "composeable" I meant that adding, removing, or re-ordering operations is a cheaper operation for the programmer. Follow this style and you'll spend less time trying to read and figure out code because it will fit the existing convention and will be bleedingly obvious where the buffers are released.

I may have been a bit hyperbolic with calling this "correct" or "supposed" however I didn't make up these conventions, I advocate them because I have seen them work really well and I have seen yours create mounds of inflexible spaghetti.


>> It's true that there is an extra compare. I think it's a small cost for maintainable code.

And I don't think it's the only way to achieve maintainable code. That's all I'm saying.

>> Sounds great, however, the time they spent figuring out your haphazard, repetitive and confusing free() statements

If it's the coding standard of the product that you have a small block of this at the top before you start actually writing the function then it doesn't take any more time for a coder to understand than any other way around, the important part is consistency.

>> I advocate them because I have seen them work really well and I have seen yours create mounds of inflexible spaghetti.

What's mine? I'm not advocating any of them, just sticking to one. I still don't think yours is any better than (for instance) -

    type function()
    {
        type2 *thing = (cast) malloc (size);
        type2 *thing2;

        if (thing)
        {
            thing2 = (cast2) malloc (size2);
            if(thing2)
            {
                // do some stuff here
                // and some more stuff
                ...
                free(thing2);
            }
            free(thing);
        }
        return code;
    }
A pattern which auto-unrolls as it exits without the need for more tests, and to me is every bit as maintainable as the goto cleanup; pattern.

Spaghetti code (to me) is more about encapsulation and modularisation failures than it is about the content of any individual function.


So you avoid a few null checks but you introduce the indentation problem you mentioned a few comments ago.

I think we can agree this is better than the alexkus example but there are tradeoffs involved.

(PS: I am bothered at how you cast the return value of malloc, this is C we're talking about right, not some other language with more plus signs? :-))




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: