I am unhappy with my web design assignment

[Blue Protoman]

Assignment's due, handed it in today. Hopefully next time I'll get to make something about the Pandora.

 

[Gerix]

You could always do something in your free time.

 

[Austin25]

I would have went with some thing like, "Coke: It's not worse than urine."

 

[- Theo -]

I would have went with some thing like, "Coke: It's not worse than urine."

But that would be lying

 

[urjaman]

2) if 'grep "goto" foo.c' prints anything out, you get zero by default

Example of a good exception to this rule: http://lxr.linux.no/linux+*/drivers/input/mouse/sermouse.c#L245


Just pointing out that in this type of error handling (I think most often seen in kernel driver intialization code) it is really (IMO) the cleanest solution.

 

[darfgarf]

2) if 'grep "goto" foo.c' prints anything out, you get zero by default

Example of a good exception to this rule: http://lxr.linux.no/linux+*/drivers/input/mouse/sermouse.c#L245


Just pointing out that in this type of error handling (I think most often seen in kernel driver intialization code) it is really (IMO) the cleanest solution.

yes, in that case it's a perfectly good way to do it, instead of the alternatives, but fist year c programming isn't going to get close to touching on that kind of situation, i got criticised for defining and using a DEBUG_PRINT("blah blah"); macro, as it was 'unecessary bloat'

 

[fr4n]

i got criticised for defining and using a DEBUG_PRINT("blah blah"); macro, as it was 'unecessary bloat'

Our code base at work would give that lecturer a thrombo.

 

[darfgarf]

Our code base at work would give that lecturer a thrombo.

it was one of the lab demonstrators, final year students who sposedly know a little about what they're supervising...then again they've told us you can put a transistor any way round you like and it'll work, they even insisted they were right when we intentionally made one give off the blue smoke

 

[- Theo -]

2) if 'grep "goto" foo.c' prints anything out, you get zero by default

Example of a good exception to this rule: http://lxr.linux.no/linux+*/drivers/input/mouse/sermouse.c#L245


Just pointing out that in this type of error handling (I think most often seen in kernel driver intialization code) it is really (IMO) the cleanest solution.

I disagree. I don't want to turn this thread into a design patterns discussion (god forbid) but it's really not the cleanest, far from it. A nightmare to maintain, and it's usually the sign of quick and dirty code that hides some other bugs, potentially nasty. You can always refactor to use very simple control flow constructs, and that is the cleanest solution.


My 2c.

 

[urjaman]

I disagree. I don't want to turn this thread into a design patterns discussion (god forbid) but it's really not the cleanest, far from it.

Do that function without goto and without duplicating cleanup code for me, please.

 

[- Theo -]

Do that function without goto and without duplicating cleanup code for me, please.

What is wrong with duplication? I'm not talking of duplicating the code itself, only the methods calls. A first refactoring step would be:

...<snip>...

err = serio_open(serio, drv);

if (err) 

{

    serio_set_drvdata(serio, NULL);

    input_free_device(input_dev);

    return err;

}

err = input_register_device(sermouse->dev);

if (err) 

{

    serio_close(serio);

    serio_set_drvdata(serio, NULL);

    input_free_device(input_dev);

    return err;

}

return 0;

And that's just a start. I find it cleaner and less error-prone. As the coder, you're supposed to know what you want to do in each error case and clearly specify it, not trying to optimise number of lines and hide the logic. Although to be honest I haven't done pure C for at least 10 years, and I'm now using OO languages like Java, C# or Scala, with proper support for exceptions.

...<snip>...

try {

   serio_open(serio, drv);

   input_register_device(sermouse.dev);


} catch (SerioOpenException e) {

   serio_set_drvdata(serio, NULL);

   input_free_device(input_dev);

   throw e;


} catch (RegisterDeviceException e) {

   serio_close(serio);

   serio_set_drvdata(serio, NULL);

   input_free_device(input_dev);

   throw e;

}

If you absolutely want to return an error code (a bad practice, if you ask me), then you can do that instead of throw e.





And if you really don't want duplication, you can use nested conditions:

...<snip>...

err = serio_open(serio, drv);

if (!err) 

{

    err = input_register_device(sermouse->dev);

    if (!err) 

        return 0;

    serio_close(serio);

}

serio_set_drvdata(serio, NULL);

input_free_device(input_dev);

return err;

Isn't it cleaner? (although sometimes it won't be, depends on the number of nested loop levels - more than 3 become unreadable)

 

[notaz]

Example of a good exception to this rule: http://lxr.linux.no/linux+*/drivers/input/mouse/sermouse.c#L245


Just pointing out that in this type of error handling (I think most often seen in kernel driver intialization code) it is really (IMO) the cleanest solution.

I disagree. I don't want to turn this thread into a design patterns discussion (god forbid) but it's really not the cleanest, far from it. A nightmare to maintain, and it's usually the sign of quick and dirty code that hides some other bugs, potentially nasty. You can always refactor to use very simple control flow constructs, and that is the cleanest solution.


My 2c.

gotos are fine if they are used correctly and urjaman linked you to such example. Like Linus Torvalds said once in such discussion, you've been brainwashed by uni CS people.

 

[- Theo -]

Example of a good exception to this rule: http://lxr.linux.no/linux+*/drivers/input/mouse/sermouse.c#L245


Just pointing out that in this type of error handling (I think most often seen in kernel driver intialization code) it is really (IMO) the cleanest solution.

I disagree. I don't want to turn this thread into a design patterns discussion (god forbid) but it's really not the cleanest, far from it. A nightmare to maintain, and it's usually the sign of quick and dirty code that hides some other bugs, potentially nasty. You can always refactor to use very simple control flow constructs, and that is the cleanest solution.


My 2c.

gotos are fine if they are used correctly and urjaman linked you to such example. Like Linus Torvalds said once in such discussion, you've been brainwashed by uni CS people.

I've also been brainwashed by 13 years of professional coding experience (not to mention the previous 10 years as an amateur), and throwing names will not change anything. I've been proven many times that writing clean, maintainable code is the good thing to do, and goto defeats that.

 

[notaz]

I don't consider your "improvements" maintainable code - actually it made it less maintainable as someone needing to add something to the body would have to take care not to forget to update each duplicated error handling case you added. Not to mention longer overall time wasted to read and understand that dupe code.

 

[- Theo -]

I don't consider your "improvements" maintainable code - actually it made it less maintainable as someone needing to add something to the body would have to take care not to forget to update each duplicated error handling case you added. Not to mention longer overall time wasted to read and understand that dupe code.

That's not dupe code!


If there are additional clean-up step, they should be added in the clean-up method, not in the code that calls it! And if different errors do different steps, then you should have different clean-up methods (or a method with parameters to indicate what you want it to do).


I said that was only a first step, I wouldn't code it like that in the first place. And I wouldn't use c in the first place anyway.


And I disagree with the longer overall time to read and understand it. When I see a goto, I already feel a headache starting, and I know there is going to be unexpected side-effects.


Where are the unit-tests anyway?


And what about my third piece of code above? No "duplication", no goto. Only nested ifs. Personally, I find it almost equally headache-inducing, as I much prefer controlling the flow in a simple way and refactoring to collaborator objects when nesting level becomes too high.

 

[urjaman]

refactoring to collaborator objects

Now you lost me . It is a kernel device driver. It is written in C. (Why not C++: http://www.tux.org/lkml/#s15-3)


I'm asking for a better solution in my provided example. Nested if's look real bad to me (and become cumbersome when there's more than 3 levels as you said).


Duplicating error handling code means more work when the kernel internal interfaces change (yes they do...), and more possibilities for bugs because one case wasnt changed when the another one is, etc...


EDIT:

And I disagree with the longer overall time to read and understand it. When I see a goto, I already feel a headache starting, and I know there is going to be unexpected side-effects.

That is just because I guess that you havent written a lot of linux kernel code. When I see:

if (!sermouse || !input_dev)

               goto fail1;

I already know that it is a jump to an error handling code (cues: !sermouse, fail1....).

 

[AquaAnalogue]

When I see a goto, I already feel a headache starting, and I know there is going to be unexpected side-effects.

Oh, please. I'll admit I am not yet a professional programmer and I only have about 10 (5 once I really got serious) years' experience, but when I see someone arguing something like this, it makes me incredibly angry at the widespread ignorance.


Could you please find the x86 instruction for me that corresponds to an IF statement? No, you can't, because it does not exist.


All real code is, in some way, shape, or form, eventually either compiled directly into assembly, or translated indirectly into machine code. Neither of these two things (which are really the same thing) have complex "control structures." A basic if-else statement, for example (probably unnecessarily cumbersome; I am just learning assembly and it is early morning for me):

// implementing a simple if( <int> == <int> ) / else statement


ifstatement:

              cmpl %eax, %ecx  // compare two numbers

              je   thenblock   // if equal, jump (or: "goto") label "thenblock"


elseblock:

              // code here

              jmp done         // jump (or: "goto") over "thenblock" to "done"


thenblock:

              // code here    


done:

              // continue execution

This is literally the same thing for any "control stucture" you can think of. This may shake your faith in life itself, but every time something depends on a condition to decide something (at least in the two ISA's i've worked with, MIPS and x86) there is a "goto label" there somewhere. Switch statements are an excellent example-- the cases are basically glorified labels. You even "fall through" if you don't jump out (sorry, "break;").


Arguing pointless things such as how pretty code looks is utterly stupid. Especially when you are talking about repeating code just so your code looks pretty.


Taking systems programming this quarter has introduced me to thinking on the hardware level-- and when you're thinking about hardware, you care about two things: getting something done, and doing it in as few instructions as possible. You are suggesting we replace a brilliant little piece of C with something you feel is more "sensible and proper." A useful analogy to think about would be the Revolutionary War. Are you going to be the damn British, prim and proper, lining up in human brick formation to die? Or are you going to do the sensible thing: go guerilla, hide behind trees and take fewer casualties?


You're being British. Stop it.

 

[notaz]

And I disagree with the longer overall time to read and understand it. When I see a goto, I already feel a headache starting, and I know there is going to be unexpected side-effects.

What side effects? You act like it will jump to random point of your program or a different process even while it's 100% defined and predictable. I suppose you never coded asm?


Some people just have "goto=evil" hardcoded in their heads and just deny everything else about goto without even thinking about it.

And what about my third piece of code above? No "duplication", no goto. Only nested ifs. Personally, I find it almost equally headache-inducing, as I much prefer controlling the flow in a simple way and refactoring to collaborator objects when nesting level becomes too high.

I don't know how is that better code, you device various workarounds just to avoid that 4 letter word. In this case it doesn't look too bad yet, but add a couple of nesting levels and it becomes horrible. Writing a cleanup method for every error case is still duplication, like it or not. With super-cleanup method you just shift that nested monster to another place and add unnecessary complexity.


Of course everyone has their own ways to get the job done and I'm fine with that. But if goto causes nightmares to you it doesn't mean it does the same for others, so stating goto is bad and other ways are always better is simply incorrect.

 

[- Theo -]

Oh, please. I'll admit I am not yet a professional programmer and I only have about 10 (5 once I really got serious) years' experience, but when I see someone arguing something like this, it makes me incredibly angry at the widespread ignorance.


Could you please find the x86 instruction for me that corresponds to an IF statement? No, you can't, because it does not exist.


All real code is, in some way, shape, or form, eventually either compiled directly into assembly, or translated indirectly into machine code. Neither of these two things (which are really the same thing) have complex "control structures." A basic if-else statement, for example (probably unnecessarily cumbersome; I am just learning assembly and it is early morning for me):

Of course, but it's done in a verified, controlled and presumably tested way. In other words, its behaviour is predictable.


I've done a lot of assembly when I was younger (both 386 and 68000 processors) and I liked it a lot. I actually like a lot clever programming, trying to find workarounds to succeed in what appears at first impossible, and using languages in ways they were not planned for. But I don't consider that good practice in a professional environment, and I wouldn't call that quality code.


For the same reason, I tend not to use recursion. Sure, it's a nice and fun intellectual exercise, and at some point, that's what coding is about. But the potential it has to produce nasty bugs, memory leaks, weird behaviour and more importantly, code that's impossible to test and debug, is too important.

Arguing pointless things such as how pretty code looks is utterly stupid. Especially when you are talking about repeating code just so your code looks pretty.

Thanks for the insult, but I'm afraid you misunderstood me. When I say pretty, I'm talking about producing an easy-to-maintain, simple, and high quality code. I don't care for clever finds to save one line of code (what for? Is memory that expensive these days?)

Taking systems programming this quarter has introduced me to thinking on the hardware level-- and when you're thinking about hardware, you care about two things: getting something done, and doing it in as few instructions as possible. You are suggesting we replace a brilliant little piece of C with something you feel is more "sensible and proper." A useful analogy to think about would be the Revolutionary War. Are you going to be the damn British, prim and proper, lining up in human brick formation to die? Or are you going to do the sensible thing: go guerilla, hide behind trees and take fewer casualties?

I'm going to be the Pacifist, probably. Sorry if you don't get my intent and all you see is a desire to produce "pretty" code, but I'm working on applications that have been alive for 5-10 years with a team of 12 people interacting daily with it (none of them were there at the beginning of the development, and our source control repository is at revision 48000, each one of them properly built into a continuous integration server) and if you don't have strict guidelines and your development practices, you fail because it becomes quickly unmaintainable. I've seen that over and over on "clever" code.

Oh, please. I'll admit I am not yet a professional programmer and I only have about 10 (5 once I really got serious) years' experience, but when I see someone arguing something like this, it makes me incredibly angry at the widespread ignorance.


Could you please find the x86 instruction for me that corresponds to an IF statement? No, you can't, because it does not exist.


All real code is, in some way, shape, or form, eventually either compiled directly into assembly, or translated indirectly into machine code. Neither of these two things (which are really the same thing) have complex "control structures." A basic if-else statement, for example (probably unnecessarily cumbersome; I am just learning assembly and it is early morning for me):

Of course, but it's done in a verified, controlled and presumably tested way. In other words, its behaviour is predictable.


I've done a lot of assembly when I was younger (both 386 and 68000 processors) and I liked it a lot. I actually like a lot clever programming, trying to find workarounds to succeed in what appears at first impossible, and using languages in ways they were not planned for. But I don't consider that good practice in a professional environment, and I wouldn't call that quality code.


For the same reason, I tend not to use recursion. Sure, it's a nice and fun intellectual exercise, and at some point, that's what coding is about. But the potential it has to produce nasty bugs, memory leaks, weird behaviour and more importantly, code that's impossible to test and debug, is too important.

Arguing pointless things such as how pretty code looks is utterly stupid. Especially when you are talking about repeating code just so your code looks pretty.

Thanks for the insult, but I'm afraid you misunderstood me. When I say pretty, I'm talking about producing an easy-to-maintain, simple, and high quality code. I don't care for clever finds to save one line of code (what for? Is memory that expensive these days?). When I'm proud of my code because I know it does what intended for, in a repeatable and tested way, and is maintainable and understandable, I call it pretty.

You're being British. Stop it.

I won't. I'm French, btw.

 

[- Theo -]

What side effects? You act like it will jump to random point of your program or a different process even while it's 100% defined and predictable. I suppose you never coded asm?

It will if someone else modifies it later. Have you never shared code with someone else? Or worked in a team of more than 1 person? And yes I did a lot of assembly. But even though it's very interesting to understand how everything works internally, that's as much a computer language as the wind is a human language.

Some people just have "goto=evil" hardcoded in their heads and just deny everything else about goto without even thinking about it.

Nothing is hardcoded. I explained my reasons for it.

I don't know how is that better code, you device various workarounds just to avoid that 4 letter word. In this case it doesn't look too bad yet, but add a couple of nesting levels and it becomes horrible.

Agreed, and that's what I said. That's not the word goto that I want to avoid, that's everything that breaks the normal flow of the program. I equally hate break or switch statements, they are just goto in disguise.

Writing a cleanup method for every error case is still duplication, like it or not. With super-cleanup method you just shift that nested monster to another place and add unnecessary complexity.

No it's not!


It's as much duplication as saying that using twice the word "if" or "for" is duplication. It's crazy. You can call several the same method, they're made for that.

Of course everyone has their own ways to get the job done and I'm fine with that. But if goto causes nightmares to you it doesn't mean it does the same for others, so stating goto is bad and other ways are always better is simply incorrect.

Well I never pretended to have the ultimate truth in me (If I sounded this way, I'm sorry). I'm just giving my opinion, and I'm perfectly fine with you having your own