A question of indentation!
-
Indentation is nice. In fact code that is not indented is an absolute pain to even look at. But then sometimes you get into absurd levels of indentation. Right now I am working with the PGP SDK. For certain operations I need to call about 7-10 functions sequentially. The problem is that each of these functions can be called ONLY if all the previous functions are successful. Thus I have something like this.
if(call1())
{
if(call2())
{
if(call3())
{
if(call4())
{
if((call5())
{
if(call6())
{That's just a sample, just the tip of the large iceberg. Often it get's a LOT more deeply nested than I have shown above! In such situations can we actually do away with indentation at least partially? For example would it be considered okay to do this.
if(call1())
{
if(call2())
{
if(call3())
{
if(call4())
{
if((call5())
{
if(call6())
{I have maintained a little indentation, but it's not perfectly done! Your comments are welcome
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
The indentation isn't the problem, you're just doing too much in one function ;) Personally I'd try and wrap the api in some classes and make sure that each method does one clearly defined thing. I'd be inclided to make these classes report errors via exceptions, so at the user code level you just have straight line code. Len Holgate www.jetbyte.com The right code, right now.
-
I typically just use a singe character space vs none and then 4 or 5 in these cases. It is still readable, just does not stand out as much. To be conscious that you are ignorant of the facts is a great step towards Knowledge. Benjamin Disraeli
Michael A. Barnhart wrote: typically just use a singe character space vs none and then 4 or 5 in these cases. It is still readable, just does not stand out as much. That'll lessen the issue but won't eradicate it. Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
Sometimes the following code structure:
if(call1())
{
if(call2())
{
if(call3())
{
...
}
}
}Can be safely replaced by:
if(!call1())
return;
if(!call2())
return;
if(!call3())
return;If the semantics of the API I'm using is too C-ish, I'll wrap it up in C++ classes and this usually takes care of the excesive indentation and allows the use of the second code structure.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class ImplementationEddie Velasquez wrote: if(!call3()) return; I can't do that. Since I need to do some cleaning up as well :( Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
Eddie Velasquez wrote: if(!call3()) return; I can't do that. Since I need to do some cleaning up as well :( Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
Nish - Native CPian wrote: I can't do that. Since I need to do some cleaning up as well That's why you should wrap up the API in C++ classes and let destructors take care of the cleanup.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class Implementation -
Daniel Turini wrote: That's why exception handling was invented. Try throwing exceptions instead of nesting if's. Not necessarilly. Exceptions should only be used for reporting "exceptional" conditions and not all function returns mean and exceptional condition. e.g. In some cases an end-of-file is a normal situation, but if reading a header of a structured file it means that the file is corrupt and should raise an exception.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class ImplementationI thought that in PGP SDK a return value of false indicates an error condition. Crivo Automated Credit Assessment
-
That's why exception handling was invented. Try throwing exceptions instead of nesting if's. Two ways: encapsulating each call in a function or method, or:
try
{
if (!call1)
throw PGPException();if (!call2) throw PGPException(); if (!call3) throw PGPException();}
catch (PGPException)
{
}Crivo Automated Credit Assessment
Daniel Turini wrote: That's why exception handling was invented. Try throwing exceptions instead of nesting if's. Cool! I never thought of that! I just don't know much about exception handling though. What's this PGPException thing? Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
if (call1() && call2() && call3() ....)
{
}Of course, you can insert line breaks between call[N]() and call[N+1](). Tomasz Sowinski -- http://www.shooltz.com
- It's for protection
- Protection from what? Zee Germans?Tomasz Sowinski wrote: if (call1() && call2() && call3() ....) That won;'t work. I am not supposed to call call-n unless call-n-1 returns true!
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
2 spaces for indentation, and if I get that deep I wonder if I'm wrong with my design, should break this down to some helper functions, etc. I wouldn't recommend your 2nd indentation style, as it suggests to be "indented correctly", but isn't. trick: instead of
ok = hamlet();
if (ok) {
ok = ophelia();
if (ok) {
ok = polonius();
if (ok) {
ok = laertes();
}
}
}I use
do { // while(0)
ok = hamlet();
if (!ok) break;
ok = ophelia();
if (!ok) break;
....
} while(0);not from the source book of clean and accepted code, but it helps a lot (esp. with a #define HBRK if (FAILED(hr)) break ) Peter
The earth is not dying. It is being killed.
peterchen wrote: do { // while(0) ok = hamlet(); if (!ok) break; ok = ophelia(); if (!ok) break; ....} while(0); Cool! I like your do while(0) idea. That's what I am gonna use I think :-) Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
I use the first one with a comment at the closing bracket (so that I know from which call that closing bracket belongs.
if(call1())
{
if(call2())
{
if(call3())
{
if(call4())
{
if((call5())
{
...
} /* Call5 */
} /* Call4 */
} /* Call3 */
} /* Call2 */
} /* Call1 */Mauricio Ritter - Brazil Sonorking now: 100.13560 Trank :beer: The alcohol is one of the greatest enemys of man, but a man who flee from his enemys is a coward. :beer:
Mauricio Ritter wrote: I use the first one with a comment at the closing bracket (so that I know from which call that closing bracket belongs. Good idea Mauricio! But my problem was with the indentation level going too deep! Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
Tomasz Sowinski wrote: if (call1() && call2() && call3() ....) That won;'t work. I am not supposed to call call-n unless call-n-1 returns true!
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
Nish - Native CPian wrote: I am not supposed to call call-n unless call-n-1 returns true! Geez, Nish, you're disappointing me again :-D How can you think about earning almost $90K without basic C++ knowledge? Don't you know how logical operators work? call-n isn't going to be called when call-n-1 returns false. Tomasz Sowinski -- http://www.shooltz.com
- It's for protection
- Protection from what? Zee Germans? -
The indentation isn't the problem, you're just doing too much in one function ;) Personally I'd try and wrap the api in some classes and make sure that each method does one clearly defined thing. I'd be inclided to make these classes report errors via exceptions, so at the user code level you just have straight line code. Len Holgate www.jetbyte.com The right code, right now.
Len Holgate wrote: The indentation isn't the problem, you're just doing too much in one function The problem is that each of these functions require something from the previous function. For example if fn_1 allocs something, then fn_2 uses that and allocs something else used by fn_3 and so on. On failure at any point I also have to call the respective deallocing PGP functions in reverse order Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
Daniel Turini wrote: That's why exception handling was invented. Try throwing exceptions instead of nesting if's. Not necessarilly. Exceptions should only be used for reporting "exceptional" conditions and not all function returns mean and exceptional condition. e.g. In some cases an end-of-file is a normal situation, but if reading a header of a structured file it means that the file is corrupt and should raise an exception.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class ImplementationAn exception does not necessarily mean an error. Christian I am completely intolerant of stupidity. Stupidity is, of course, anything that doesn't conform to my way of thinking. - Jamie Hale - 29/05/2002
-
Daniel Turini wrote: That's why exception handling was invented. Try throwing exceptions instead of nesting if's. Cool! I never thought of that! I just don't know much about exception handling though. What's this PGPException thing? Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
Try/Thow/catch ( and finally in C#, dunno about C++ ) are the obvious way to deal with your situation. Christian I am completely intolerant of stupidity. Stupidity is, of course, anything that doesn't conform to my way of thinking. - Jamie Hale - 29/05/2002
-
Nish - Native CPian wrote: I can't do that. Since I need to do some cleaning up as well That's why you should wrap up the API in C++ classes and let destructors take care of the cleanup.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class ImplementationEddie Velasquez wrote: That's why you should wrap up the API in C++ classes and let destructors take care of the cleanup. Yeah, I gotta do that. I really must! I am not a naturally OOPish guy, you know! Nish :-)
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
Daniel Turini wrote: That's why exception handling was invented. Try throwing exceptions instead of nesting if's. Not necessarilly. Exceptions should only be used for reporting "exceptional" conditions and not all function returns mean and exceptional condition. e.g. In some cases an end-of-file is a normal situation, but if reading a header of a structured file it means that the file is corrupt and should raise an exception.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class ImplementationWell, the indentation of this thread has gone totally haywire. All my replies are appearing under the wrong persons :( Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
I thought that in PGP SDK a return value of false indicates an error condition. Crivo Automated Credit Assessment
Daniel Turini wrote: thought that in PGP SDK a return value of false indicates an error condition. Well, I was talking in a general sense and didn't realize you were referring to the PGP SDK in particular. My bad.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class Implementation -
Try/Thow/catch ( and finally in C#, dunno about C++ ) are the obvious way to deal with your situation. Christian I am completely intolerant of stupidity. Stupidity is, of course, anything that doesn't conform to my way of thinking. - Jamie Hale - 29/05/2002
Christian Graus wrote: Try/Thow/catch ( and finally in C#, dunno about C++ ) are the obvious way to deal with your situation. I don't even know how to implement exceptions :-( Might have to do some reading Actually after reading all the suggestions, I liked peterchens do{} while(0) idea :-) Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
Nish - Native CPian wrote: I am not supposed to call call-n unless call-n-1 returns true! Geez, Nish, you're disappointing me again :-D How can you think about earning almost $90K without basic C++ knowledge? Don't you know how logical operators work? call-n isn't going to be called when call-n-1 returns false. Tomasz Sowinski -- http://www.shooltz.com
- It's for protection
- Protection from what? Zee Germans?Tomasz Sowinski wrote: Geez, Nish, you're disappointing me again How can you think about earning almost $90K without basic C++ knowledge? Don't you know how logical operators work? call-n isn't going to be called when call-n-1 returns false. Nope! Not with PGP. When I wrote the original post I simplified things. PGP calls return a PGPError struct and I have to call IsPGPError() on it to find out whether it's an error :-( Not a true/false thing at all!!! Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
-
An exception does not necessarily mean an error. Christian I am completely intolerant of stupidity. Stupidity is, of course, anything that doesn't conform to my way of thinking. - Jamie Hale - 29/05/2002
Christian Graus wrote: An exception does not necessarily mean an error. An exception should always indicate an error... that's why they're called "exceptions"... because they're exceptional. :) Or did I miss something in your post?
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class Implementation -
Tomasz Sowinski wrote: Geez, Nish, you're disappointing me again How can you think about earning almost $90K without basic C++ knowledge? Don't you know how logical operators work? call-n isn't going to be called when call-n-1 returns false. Nope! Not with PGP. When I wrote the original post I simplified things. PGP calls return a PGPError struct and I have to call IsPGPError() on it to find out whether it's an error :-( Not a true/false thing at all!!! Nish
Regards, Nish Native CPian. Born and brought up on CP. With the CP blood in him.
Well, it seems that you need some kind of object wrapper over PGPError. The object could convert that to true/false or even throw some exception. Tomasz Sowinski -- http://www.shooltz.com
- It's for protection
- Protection from what? Zee Germans?