Kill it with fire #2 - or fml
-
To me, this is cleaner than the alternative. I'm certain many will disagree, but this matches my thought process, and eliminating the
gotos requires more indentation levels (which I despise) and thinking time. And if it doesn't require more indentation levels, it still requires more time! I agree with not usinggotos often.void Repeater::createRepeatedObjs() {
if (!createdObjsC.empty()) throw dwl::Exception(_T("Delete created objs prior to "
"creation"));
app::ObjMap & holder = appC.objMap();
holder.setPtrPos(s_cast(0));
object::BaseObj * obj;
object::BaseObj * objToCopy;
while ((obj = holder.objAtPtr()) && obj != nullptr && obj->time() < endTimeC) {
if (obj->time() < timeC) goto breakout;
for (size_t i=1, count=numRepeatsC; i<=count; ++i) {
object::BaseObj::Type type = obj->type();
if (type == object::BaseObj::Type::TypeOne) {
object::TypeOne * typeOne = s_cast(obj);
object::ObjParent * parent = s_cast(typeOne->parent());
if (parent) objToCopy = parent;
else objToCopy = typeOne;
}
else if (type == object::BaseObj::Type::TypeTwo) goto breakout;
else if (type == object::BaseObj::Type::TypeThree) goto breakout;
else objToCopy = obj;//Don't make copies of copies! if (objToCopy->copyOf()) goto breakout; std::unique\_ptr newObj = objToCopy->copyToTick(objToCopy->time() + durationC \* i); if (newObj->type() == object::BaseObj::Type::ObjParent) { s\_cast(newObj.get())->typeOne().copyOf((object::BaseObj\*)1); s\_cast(newObj.get())->noteOff().copyOf((object::BaseObj\*)1); } createdObjsC.push\_back(newObj.get()); newObj.release(); //It is in the vector }breakout:
holder.incObjPtr();
}
}Forcing it into your way, I believe it becomes:
void Repeater::createRepeatedObjs() {
if (!createdObjsC.empty()) throw dwl::Exception(_T("Delete created objs prior to "
"creation"));
app::ObjMap & holder = appC.objMap();
holder.setPtrPos(s_cast(0));
object::BaseObj * obj;
object::BaseObj * objToCopy;
while ((obj = holder.objAtPtr()) && obj != nullptr && obj->time() < endTimeC) {
if (obj->time() < timeC) goto breakounot that I'm really arguing, but it looks like your code base between example 1 and 2 have the same level of complexity. And why use a goto to leave the loop when a break would work just fine? Maybe it's just the example. My point in the original is that I have try/catch pairs all through the code, and nothing is done in the catch. No logging, no recovery, no ASSERTS, I mean nothing. If you're not going to handle the exception, just remove the try and let it crater :)
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
-
not that I'm really arguing, but it looks like your code base between example 1 and 2 have the same level of complexity. And why use a goto to leave the loop when a break would work just fine? Maybe it's just the example. My point in the original is that I have try/catch pairs all through the code, and nothing is done in the catch. No logging, no recovery, no ASSERTS, I mean nothing. If you're not going to handle the exception, just remove the try and let it crater :)
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
The
gotos don't break out of the loop, so abreakwon't keep the same flow. I should have namedbreakoutsomething likeskipProcessingTo, but thebreakoutnomenclature comes to my mind naturally. I believe they are equivalent, except for the extra indentation level which I don't like, and theif (!(type == object::BaseObj::Type::TypeTwo || type == object::BaseObj::Type::TypeThree)) {which I despise. The
!/||construct flips my brain upside down, vs the other way which is clear. I did not notice the 'verbatim' part. At least they gave you a template to fill in! :laugh: :laugh: Their comment indicates they possibly once meant to do something. Copy/paste at its finest! Have fun making it better! -
The
gotos don't break out of the loop, so abreakwon't keep the same flow. I should have namedbreakoutsomething likeskipProcessingTo, but thebreakoutnomenclature comes to my mind naturally. I believe they are equivalent, except for the extra indentation level which I don't like, and theif (!(type == object::BaseObj::Type::TypeTwo || type == object::BaseObj::Type::TypeThree)) {which I despise. The
!/||construct flips my brain upside down, vs the other way which is clear. I did not notice the 'verbatim' part. At least they gave you a template to fill in! :laugh: :laugh: Their comment indicates they possibly once meant to do something. Copy/paste at its finest! Have fun making it better!Ha! I missed that last bracket. My bad.
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
-
Pretty little piece of code I inherited. I deleted code out of the try, the real point was the catch... that code is verbatim. :doh: // create vector of file types so we can reorder the list from newest to oldest
try
{
// do something here...} catch(...) { // New throws }Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
I used to have a developer who did his try/catch routine like this, thinking that as exceptions were caught "in the wild", he would fill in his catch clause. I had to show him that if he left his catch clauses open, none of the exceptions would be handled. At the very lease, rethrow the original exception. The most effective method I've used is to go through all your activities in the try clause and look for all the different exceptions you are capable of throwing, and then trap those exceptions one at a time and recast the exception with a more descriptive message with any relevant values and capture the original exception as an inner exception so you don't lose the stack trace. And finish it with a catch-all. It's a paranoid way of writing your catch clause, but OMG my operations guys were the first converts because they didn't have to go through the stack trace and data dumps...it's all right there in the message. It also makes for very effective unit tests too, since you can predict just every aspect of where the routine could break rather than some obtuse unhandled null reference exception.
-
I used to have a developer who did his try/catch routine like this, thinking that as exceptions were caught "in the wild", he would fill in his catch clause. I had to show him that if he left his catch clauses open, none of the exceptions would be handled. At the very lease, rethrow the original exception. The most effective method I've used is to go through all your activities in the try clause and look for all the different exceptions you are capable of throwing, and then trap those exceptions one at a time and recast the exception with a more descriptive message with any relevant values and capture the original exception as an inner exception so you don't lose the stack trace. And finish it with a catch-all. It's a paranoid way of writing your catch clause, but OMG my operations guys were the first converts because they didn't have to go through the stack trace and data dumps...it's all right there in the message. It also makes for very effective unit tests too, since you can predict just every aspect of where the routine could break rather than some obtuse unhandled null reference exception.
error checking is hard, especially in embedded systems. in 14 years of development, I have not yet heard of a cogent approach to how to fail creatively. That said, the *minimum* error handling should be a log message saying "you're f****" and why :). Hopefully with some minimal thought, you can come up with a recovery approach...
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
-
Still better than
int main() {
begin:
try {
//do stuff
}
catch(...) {
//fix stuff
goto begin;
}
return 0;
};P PS - Enjoy the cleanup on aisle one, two, three, four...!
Both codes to final! :laugh: :laugh: :laugh:
-
The
gotos don't break out of the loop, so abreakwon't keep the same flow. I should have namedbreakoutsomething likeskipProcessingTo, but thebreakoutnomenclature comes to my mind naturally. I believe they are equivalent, except for the extra indentation level which I don't like, and theif (!(type == object::BaseObj::Type::TypeTwo || type == object::BaseObj::Type::TypeThree)) {which I despise. The
!/||construct flips my brain upside down, vs the other way which is clear. I did not notice the 'verbatim' part. At least they gave you a template to fill in! :laugh: :laugh: Their comment indicates they possibly once meant to do something. Copy/paste at its finest! Have fun making it better!David O'Neil wrote:
The
!/||construct flips my brain upside down, vs the other way which is clear.I'm not familiar with C(++), but can't your if be rewritten as:
if (type != TypeTwo && type != TypeThree)
:confused: And do you even need it at all? Simply checking for TypeOne should suffice as you do absolutely nothing in case of TypeTwo or TypeThree...
if (type == TypeOne) {
// Do stuff...
}
holder.incObjPtr();Or maybe I'm wrong, but your example is mostly hard to read because indentation is way off :laugh:
Best, Sander sanderrossel.com Continuous Integration, Delivery, and Deployment arrgh.js - Bringing LINQ to JavaScript Object-Oriented Programming in C# Succinctly
-
Pretty little piece of code I inherited. I deleted code out of the try, the real point was the catch... that code is verbatim. :doh: // create vector of file types so we can reorder the list from newest to oldest
try
{
// do something here...} catch(...) { // New throws }Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
So you want to play catch? ?Maybe I should try throwing again?
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
-
error checking is hard, especially in embedded systems. in 14 years of development, I have not yet heard of a cogent approach to how to fail creatively. That said, the *minimum* error handling should be a log message saying "you're f****" and why :). Hopefully with some minimal thought, you can come up with a recovery approach...
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
I dreamed I got the source code for my life, but forgot to check for errors and my entire life is a major debugging process.
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
-
David O'Neil wrote:
The
!/||construct flips my brain upside down, vs the other way which is clear.I'm not familiar with C(++), but can't your if be rewritten as:
if (type != TypeTwo && type != TypeThree)
:confused: And do you even need it at all? Simply checking for TypeOne should suffice as you do absolutely nothing in case of TypeTwo or TypeThree...
if (type == TypeOne) {
// Do stuff...
}
holder.incObjPtr();Or maybe I'm wrong, but your example is mostly hard to read because indentation is way off :laugh:
Best, Sander sanderrossel.com Continuous Integration, Delivery, and Deployment arrgh.js - Bringing LINQ to JavaScript Object-Oriented Programming in C# Succinctly
Sander Rossel wrote:
...can't your if be rewritten as:
Possibly. It works, though, so I will leave it for now!
-
I dreamed I got the source code for my life, but forgot to check for errors and my entire life is a major debugging process.
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
Life is like a game of chess. I don't know chess.
Wrong is evil and must be defeated. - Jeff Ello
-
David O'Neil wrote:
The
!/||construct flips my brain upside down, vs the other way which is clear.I'm not familiar with C(++), but can't your if be rewritten as:
if (type != TypeTwo && type != TypeThree)
:confused: And do you even need it at all? Simply checking for TypeOne should suffice as you do absolutely nothing in case of TypeTwo or TypeThree...
if (type == TypeOne) {
// Do stuff...
}
holder.incObjPtr();Or maybe I'm wrong, but your example is mostly hard to read because indentation is way off :laugh:
Best, Sander sanderrossel.com Continuous Integration, Delivery, and Deployment arrgh.js - Bringing LINQ to JavaScript Object-Oriented Programming in C# Succinctly
Sander Rossel wrote:
I'm not familiar with C(++), but can't your if be rewritten as:
if (type != TypeTwo && type != TypeThree)
Yes, that is part of De Morgan's laws.
"They have a consciousness, they have a life, they have a soul! Damn you! Let the rabbits wear glasses! Save our brothers! Can I get an amen?"
-
David O'Neil wrote:
gotoBlech. I'm sure someone will douse me with an accelerant and light me up for this, but the only time I've felt justified in using
gotoover the last umpteen years of my career has been in embedded assembly language code viajmpinstructions of one flavor or another. If you're using any kind of higher-level language that supports structured programming, the ease of misusinggotofar outweighs it's utility. After writing millions of lines of FORTRAN, Ada, Pascal, LISP, PL/I, C, C++, and C# I've never written code using agotothat wasn't simpler, more robust, and easier to follow when written structured.Software Zen:
delete this;Somewhat recently I have found one genuine use for goto (in C# at least).
switch(someEnum)
{
case SomeEnum.FirstCase:
// do something
break;
case SomeEnum.SecondCase:
// do something
break;
default: goto case SomeEnum.FirstCase;
} -
Somewhat recently I have found one genuine use for goto (in C# at least).
switch(someEnum)
{
case SomeEnum.FirstCase:
// do something
break;
case SomeEnum.SecondCase:
// do something
break;
default: goto case SomeEnum.FirstCase;
}Really? Why not:
switch (someEnum)
{
case SomeEnum.SecondCase:
// Handle second case
break;
default:
// Handle first case
break;
}Exactly the same behaviour, with no need to use
gotoin your code. (Of course, the generated IL will probably still use the equivalent ofgotobehind the scenes.)
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
