enum and operator++

[Saeed Amrollahi]

Dear all
I am struggling with a problem that's simple per se.
I have an enum and a prefix ++ operator:
enum Dir { North = 1, East = 2, South = 3, West = 4, Max_ = 5 }; //
clockwise

Dir operator++(Dir d)
{
Dir dd;
switch (d) {
case North:
dd = East;
break;
case East:
dd = South;
break;
case South:
dd = West;
break;
case West: // wrap
dd = North;
break;
};

return dd;
}

void f()
{
for (Dir d = North; d < Max_; ++d)
// do something
}

The problem is: in the above loop the ++ doesn't work,
indeed the loop is infinite. If you replace // do something with
output statement, it prints 1 forever.

It is really annoying.

please throw a ligth.

Regards,
-- Saeed Amrollahi

 

[Saeed Amrollahi]

----->for (Dir d = North; d<  Max_; d=++d)






--- news://freenews.netfront.net/ - complaints: (e-mail address removed) ---- Hide quoted text -

- Show quoted text -

Yu
That's Excellent.
Why ++d isn't enough and we have to use d = ++d?

-- Saeed

 

[Richard Herring]

In message

Dear all
I am struggling with a problem that's simple per se.
I have an enum and a prefix ++ operator:
enum Dir { North = 1, East = 2, South = 3, West = 4, Max_ = 5 }; //
clockwise

Dir operator++(Dir d)
{
Dir dd;
switch (d) {
case North:
dd = East;
break;
case East:
dd = South;
break;
case South:
dd = West;
break;
case West: // wrap
dd = North;
break;
};

return dd;
}

void f()
{
for (Dir d = North; d < Max_; ++d)
// do something
}

The problem is: in the above loop the ++ doesn't work,
indeed the loop is infinite.

The ++ works perfectly. You've defined it to cycle through North, East,
South, West, and that's exactly what it does. No amount of
"increment"-ing will ever produce Max_.

If you replace // do something with
output statement, it prints 1 forever.

It is really annoying.

please throw a ligth.

Decide what you really want the increment operator to do. Should it
cycle, or just step once through the range? It can't do both, so if you
need both behaviours perhaps you should consider using named functions
instead of operator++.

 

[REH]

Dear all
I am struggling with a problem that's simple per se.
I have an enum and a prefix ++ operator:
enum Dir { North = 1, East = 2, South = 3, West = 4, Max_ = 5 };  //
clockwise

Dir operator++(Dir d)
{
  Dir dd;
  switch (d) {
  case North:
    dd = East;
    break;
  case East:
    dd = South;
    break;
  case South:
    dd = West;
    break;
  case West:  // wrap
    dd = North;
    break;
  };

  return dd;

}

void f()
{
   for (Dir d = North; d < Max_; ++d)
    // do something

}

The problem is: in the above loop the ++ doesn't work,
indeed the loop is infinite. If you replace // do something with
output statement, it prints 1 forever.

Of course it's forever. Your comment says it all: West wrapped back to
North. You never hit Max.

Incidentally, you can write your operator much, much simpler than
using a case statement.

REH

 

[REH]

Yu
That's Excellent.
Why ++d isn't enough and we have to use d = ++d?

No, that is undefined behavior. Don't do it.

REH

 

[Richard Herring]

In message

I missed the missing & here :-(

The ++ works perfectly.

Except (of course) that it takes its argument by value, not reference!

You've defined it to cycle through North, East, South, West, and
that's exactly what it does.

No amount of "increment"-ing will ever produce Max_.

That much is true.

 

[Saeed Amrollahi]

Of course it's forever. Your comment says it all: West wrapped back to
North. You never hit Max.

Incidentally, you can write your operator much, much simpler than
using a case statement.

REH- Hide quoted text -

- Show quoted text -

Hi

You are right. My code had two problems. Yu found one of them
and you found the wraping one. I removed the wrap logic in switch
statement.

BTW, why I have to write
d = ++d;
rather than
++d; ?

Thanks,
-- Saeed

 

[Saeed Amrollahi]

In message










The ++ works perfectly. You've defined it to cycle through North,  East,
South, West, and that's exactly what it does. No amount of
"increment"-ing will ever produce Max_.




Decide what you really want the increment operator to do. Should it
cycle, or just step once through the range? It can't do both, so if you
need both behaviours perhaps you should consider using named functions
instead of operator++.

Hi Richard

I removed the wrap item from my code.

Thanks,
-- Saeed

 

[REH]

Hi

You are right. My code had two problems. Yu found one of them
and you found the wraping one. I removed the wrap logic in switch
statement.

BTW, why I have to write
  d = ++d;
rather than
  ++d; ?

NO. As I said, that is undefined behavior. ++d is all you need.

REH

 

[Richard Herring]

In message

Hi

You are right. My code had two problems. Yu found one of them
and you found the wraping one. I removed the wrap logic in switch
statement.

BTW, why I have to write
d = ++d;
rather than
++d; ?

Because your operator++, despite its name, doesn't modify its argument,
and doesn't return a reference to it. That's very confusing.

You should probably have started by writing
Dir & operator++(Dir & d)
{
// code to "increment" d
return d;
}

 

[Richard Herring]

In message

NO. As I said, that is undefined behavior.

Not in this case. His operator++ is a user-defined function. Besides,
it takes its argument by value, not reference, and therefore doesn't
modify d at all ;-)

 

[REH]

Not in this case.  His operator++ is a user-defined function. Besides,
it takes its argument by value, not reference, and therefore doesn't
modify d at all ;-)

Ah, you're correct. Good catch. But I think that's not what he
intended, going by his original code.

REH

 

[REH]

Hi

You are right. My code had two problems. Yu found one of them
and you found the wraping one. I removed the wrap logic in switch
statement.

BTW, why I have to write
  d = ++d;
rather than
  ++d; ?

Per your wrapping issue, and the value/reference issue noted by Mr.
Herring, I would change your operator definition to:

inline Dir& operator++(Dir& d)
{
return d = Dir(d + 1);
}



REH

 

[James Kanze]

On Jan 20, 10:34 am, Saeed Amrollahi <[email protected]>
wrote:

No it's not.

No, that is undefined behavior. Don't do it.

There's no undefined behavior given his definition of
operator++; in fact, it's rather difficult to get undefined
behavior with a user defined operator, because of the additional
sequence points it introduced.

There are, in fact, two problems with his code. The first is
that his operator++ doesn't actually increment anything; it just
returns the next value. Normally, an operator++ takes a
non-const reference as argument (or is a non-const member, but
that's not an option for enum's), and modifies its argument.
Anything else falls under the label of obfuscation, or lying to
the reader. The second reason is that because his operator++
wraps before reaching Max_, it can never cause the value to
become >= Max_; even a correctly written operator++ will result
in an endless loop with this definition of incrementation.

 

[Saeed Amrollahi]

No it's not.


There's no undefined behavior given his definition of
operator++; in fact, it's rather difficult to get undefined
behavior with a user defined operator, because of the additional
sequence points it introduced.

There are, in fact, two problems with his code.  The first is
that his operator++ doesn't actually increment anything; it just
returns the next value.  Normally, an operator++ takes a
non-const reference as argument (or is a non-const member, but
that's not an option for enum's), and modifies its argument.
Anything else falls under the label of obfuscation, or lying to
the reader.  The second reason is that because his operator++
wraps before reaching Max_, it can never cause the value to
become >= Max_; even a correctly written operator++ will result
in an endless loop with this definition of incrementation.

Hi James
As always, you explained the problems in my code, in a nutshell.

Everybody thanks.
-- Saeed