C++ and goto - can I?

[Uzytkownik]

I wrote:
....
for(int i = 0; i < somevar; i++)
{
bool b = true;
for(deque<int>iterator it = freeID.begin(); it != freeID.end(); it++)
if(i == (*it))
{
b = false;
break;
}
if(b)
ret.push_back(i)
}
....
Can I:
....
for(int i = 0; i < somevar; i++)
{
for(deque<int>iterator it = freeID.begin(); it != freeID.end(); it++)
if(i == (*it))
goto myfunction_find;
ret.push_back(i);
myfunction_find:
;
}
....

For me it's shorter, faster and more simple, but I've herd that goto in
c++ program could make problems.
Is it work everywhere and always?
Regards.

 

[Alf P. Steinbach]

* Uzytkownik:

I wrote:
...
for(int i = 0; i < somevar; i++)
{
bool b = true;
for(deque<int>iterator it = freeID.begin(); it != freeID.end(); it++)
if(i == (*it))
{
b = false;
break;
}
if(b)
ret.push_back(i)
}
...
Can I:
...
for(int i = 0; i < somevar; i++)
{
for(deque<int>iterator it = freeID.begin(); it != freeID.end(); it++)
if(i == (*it))
goto myfunction_find;
ret.push_back(i);
myfunction_find:
;
}
...

For me it's shorter, faster and more simple, but I've herd that goto in
c++ program could make problems.

It not only can make problems, it does make problems.

In your case, it's got you thinking at a too low abstraction level so
you've turned something extremely simple into something less simple.

Try (off the cuff):


typedef std::deque<int>::const_iterator Iterator;

for( Iterator it = freeId.begin(); it != freeId.end(); ++it )
{
if( 0 <= *it && *it < somevar )
{
ret.push_back( *it );
}
}


In my view this is both more clear and more efficient.

Is it work everywhere and always?

Yes, technically it's well-defined, and it will nearly always have the
effect of obscuring your code and making you do unsimple things like above.

 

[Uzytkownik]

Alf P. Steinbach napisa³(a):

* Uzytkownik:



It not only can make problems, it does make problems.

In your case, it's got you thinking at a too low abstraction level so
you've turned something extremely simple into something less simple.

Try (off the cuff):


typedef std::deque<int>::const_iterator Iterator;

for( Iterator it = freeId.begin(); it != freeId.end(); ++it )
{
if( 0 <= *it && *it < somevar )
{
ret.push_back( *it );
}
}


In my view this is both more clear and more efficient.






Yes, technically it's well-defined, and it will nearly always have the
effect of obscuring your code and making you do unsimple things like above.

More simple
freeID become set and
for(int i = 0; i < somevar; i++)
if(freeID.find(i) == freeID.end())
ret.push_back(i);
In one my code and in your code (may be you don't now whot I'd like to
do) is error.
This code should return in ret all used ID's less then somevar.
Regards.

 

[Phlip]

Alf P. Steinbach napisa³(a):

More simple
freeID become set and
for(int i = 0; i < somevar; i++)
if(freeID.find(i) == freeID.end())
ret.push_back(i);
In one my code and in your code (may be you don't now whot I'd like to
do) is error.

Alf performed on your code what is called a "Refactor". That is a
transformation of structure that leaves behavior exactly the same.

You performed what might playfully be called a "refeature". You changed
structure and behavior at the same time.

As you learn to design, and learn not to want to use 'goto', you will know
when you are refactoring, and how to strictly change only structure.

Tip 1: Don't refactor and refeature at the same time. Only do one or the
other.

Tip 2: Write lots of unit tests, and run them every 1~10 edits. If they
fail, use Undo.

 

[Alf P. Steinbach]

* Phlip:

Alf performed on your code what is called a "Refactor". That is a
transformation of structure that leaves behavior exactly the same.

Uh, I'm afraid I slipped up: I misunderstood the intent.

Uzytkownik is interested in, and Uzytkownik's original code produced, the
complement wrt. [0...somevar] of the set my code produces.

Without knowing more it's difficult to give advice about efficiency
here, but there are potentially some problems with that (as I'm sure you
noted): Uzytkownik's code has execution time on the order of
somevar*freeId.size(), and passing a large list of integers around is probably
not very efficient.

Tip 1: Don't refactor and refeature at the same time. Only do one or the
other.

Tip 2: Write lots of unit tests, and run them every 1~10 edits. If they
fail, use Undo.

I can only agree with both points, and _apologize_ for the incorrect code!

Cheers,

- Alf

 

[Phlip]

Alf performed on your code what is called a "Refactor". That is a
transformation of structure that leaves behavior exactly the same.

Uh, I'm afraid I slipped up: I misunderstood the intent.

Uzytkownik is interested in, and Uzytkownik's original code produced, the
complement wrt. [0...somevar] of the set my code produces.

I didn't actually read your code closely enough to see if it qualified for
the Refactor Lecture ... before leaping in. ;-)

 

[Maciej Pilichowski]

More simple
freeID become set and

It doesn't have to -- you can use "find" algorithm on deque too.

have a nice day
bye