2
\$\begingroup\$

Consider the following code:

#include <iostream>
#include <vector>
using namespace std;

struct test {
    int j;
    test& operator>>(int & i) { i = ++j%26; return *this; }
    operator bool() { return j%26; }
};

int main () {
    test t;
    int i;
    while (t >> i) {
        cout << i << endl;
    }
}

I have implemented a simple class that can be used in a similar fashion as for example std::cin. I implemented it this way, because I wanted to have an object that hides some IO-operations from me and provide a nice interface to yield the data items each one-at-a-time.

Although it does want I want, I know there is at least one issue with the implementation:

Are there probably any more?

\$\endgroup\$
2
\$\begingroup\$

For simple 10 lines 10 programs you can get away with this:

using namespace std;

Anything bigger and it starts to be a pest more than a help. Thus it best not to get into the habit of using it. Anyway it is simpler even in this case just to prefex cout with std:: (that is why standard is named std to make it short and easy to use).

You don't initialize the value of j in your class.

int j;

Thus any usage is undefined behavior.

You have two options:

  1. Add a constructor that initaizes j
  2. Always force zero-initialization of your class.

In anything but this trivial program I would go with (1) and add a constructor. But just to show zero initialization the alternative is:

 Test  t = Test();  //  The () brackets at the end of Test() force zero initialization;

Note: The equivalent for new is

 Test* t = new Test();  // force zero-initialization (when no constructor)
 Test* t = new Test;    // Use default-initialization (un-initialized)

Note 2: Normally you would think you could do this:

 Test  t(); // Unfortunately this is a forward declaration of a function.

Note 3: But you can get around the problems of (2) by using extra braces. As pointed out last time I tried to answer a question like this.

 Test  t(()); // Unfortunately this confuses people and maintainers have been
              // known to try and take out the extra braces. With mixed results.
              // This is why I still prefer method 1

 Test t = Test(); // This may look like there is an extra copy.
                  // But every compiler implements the optimization to do construction
                  // in place.
              // Thus I prefer method 1 (ie this method)

Described here: https://stackoverflow.com/questions/5914422/proper-way-to-initialize-c-structs/5914697#5914697

For such a simple class I don't think you need to worry about the safe bool idium. This just protects your class from being used in places where arithmatic would be done:

 Test t;
 int x = 5 + t;  // Here we get conversion to bool
                 // which is then converted to int (0 or 1)
                 // The safe bool idium would protect you from this:

To use it do this:

 operator bool() { return j%26; }

 // Remove the above and replace with

 operator void*() {return j%26 ? &j /* or this the value is not important as long as it is not NULL */ : NULL; } 
\$\endgroup\$
  • \$\begingroup\$ Thanks for your notes! Actually you're right, plus, in this case typing std:: two times would've been less than one time using .... Also no discussion about the value of j being indeterminate, but I can't believe this condition turns using this value undefined behavior (couldn't find a statement prohibiting this in C++03). \$\endgroup\$ – moooeeeep Sep 14 '12 at 18:41
  • \$\begingroup\$ if I overload the operator void* this would add other unexpected behavior depending on the return value, wouldn't it? (e.g., (test*)(void*)t) ? \$\endgroup\$ – moooeeeep Sep 14 '12 at 18:51
  • \$\begingroup\$ The value of j is indeterminate. That is fine up to the point where you try and use it (read it). At this point the behavior becomes undefined. stackoverflow.com/q/4279264/14065 \$\endgroup\$ – Martin York Sep 14 '12 at 22:58
  • \$\begingroup\$ @moooeeeep: Yes if you want to explicitly cast something to void* then you may do so (though since you are using C++ you should probably do so with a C++ cast not a C cast). But casting it back is undefined behavior (since it was not a pointer to start with). And there are no implicit cast back (you must do it explicitly). The only legal thing you can do with a void pointer is check if it is NULL. (Note: You may cast it back to its original pointer type if you know the pointer type it originated from (but that information is not provided here)). \$\endgroup\$ – Martin York Sep 14 '12 at 23:08
  • 1
    \$\begingroup\$ Admitadel. The link you provide is the fale-safe mechanism that is perfect (and should be used in real code). The one I did is a trick that works fine and good for code reviews like this (where you don't want to plow through 100 lines of code just to explain something trivial when it is not really needed). This article: artima.com/cppsource/safeboolP.html explains all the techniques; plus the advantages and disadvantages of each. \$\endgroup\$ – Martin York Sep 14 '12 at 23:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.