4
\$\begingroup\$

This is an exercise to create a class that behaves exactly like an int.

Questions

  • Am I missing anything?
  • Or did I going about this about this the wrong way?
  • Is there anything I could improve on?
  • Are there any tricks that I could learn from this?
  • How about style; does the style look OK?
#include <iostream> // std::cout
#include <utility>  // std::move

class jd_int {
public:
    jd_int() = default;

    jd_int(int i)             : _i{i}                   { }
    jd_int(const jd_int& jdi) : _i{jdi._i}              { }
    jd_int(jd_int&& jdi)      : _i{std::move(jdi._i)}   { }

    jd_int operator= (int i)             { _i = i; return *this;                 }
    jd_int operator= (double d)          { _i = d; return *this;                 }
    jd_int operator= (const jd_int& jdi) { _i = jdi._i; return *this;            }
    jd_int operator= (jd_int&& jdi)      { _i = std::move(jdi._i); return *this; }

    ~jd_int() = default;

    operator bool()   { return !!_i;                    }
    operator int()    { return static_cast<int>(_i);    }
    operator double() { return static_cast<double>(_i); }

    jd_int operator+=(jd_int jdi) { return _i += jdi._i; }
    jd_int operator+ (jd_int jdi) { return _i +  jdi._i; }

    jd_int operator-=(jd_int jdi) { return _i -= jdi._i; }
    jd_int operator- (jd_int jdi) { return _i -  jdi._i; }

    jd_int operator*=(jd_int jdi) { return _i *= jdi._i; }
    jd_int operator* (jd_int jdi) { return _i *  jdi._i; }

    jd_int operator/=(jd_int jdi) { return _i /= jdi._i; }
    jd_int operator/ (jd_int jdi) { return _i /  jdi._i; }

    jd_int operator%=(jd_int jdi) { return _i %= jdi._i; }
    jd_int operator% (jd_int jdi) { return _i %  jdi._i; }

    jd_int operator++()    { return ++_i;                          }
    jd_int operator++(int) { jd_int tmp = *this; ++_i; return tmp; }

    jd_int operator--()    { return --_i;                          }
    jd_int operator--(int) { jd_int tmp = *this; --_i; return tmp; }

    friend bool operator< (jd_int lhs, jd_int rhs);
    friend bool operator> (jd_int lhs, jd_int rhs);
    friend bool operator<=(jd_int lhs, jd_int rhs);
    friend bool operator>=(jd_int lhs, jd_int rhs);
    friend bool operator==(jd_int lhs, jd_int rhs);
    friend bool operator!=(jd_int lhs, jd_int rhs);

private:
    int _i;

    friend std::ostream& operator<<(std::ostream& os, const jd_int jdi);
    friend std::istream& operator>>(std::istream& is, jd_int jdi);
};

bool operator< (jd_int lhs, jd_int rhs) { return (lhs._i <  rhs._i); }
bool operator> (jd_int lhs, jd_int rhs) { return (lhs._i >  rhs._i); }
bool operator<=(jd_int lhs, jd_int rhs) { return (lhs._i <= rhs._i); }
bool operator>=(jd_int lhs, jd_int rhs) { return (lhs._i >= rhs._i); }
bool operator==(jd_int lhs, jd_int rhs) { return (lhs._i == rhs._i); }
bool operator!=(jd_int lhs, jd_int rhs) { return (lhs._i != rhs._i); }

std::ostream& operator<<(std::ostream& os, const jd_int jdi) {
    os << jdi._i;
    return os;
}

std::istream& operator>>(std::istream& is, jd_int jdi) {
    is >> jdi._i;
    return is;
}
```
\$\endgroup\$

closed as off-topic by πάντα ῥεῖ, Mast, t3chb0t, IEatBagels, pacmaninbw Apr 29 at 16:57

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – πάντα ῥεῖ, Mast, IEatBagels, pacmaninbw
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ What's the primary purpose for doing so? Can you elaborate please? \$\endgroup\$ – πάντα ῥεῖ Apr 28 at 21:22
  • \$\begingroup\$ It's an exercise question by Bjarne Stroustrup. \$\endgroup\$ – John DeBord Apr 28 at 21:49
  • 1
    \$\begingroup\$ There's a number of operator definitions missing then, \$\endgroup\$ – πάντα ῥεῖ Apr 28 at 22:08
  • 1
    \$\begingroup\$ Yes, this question appears to be incomplete. We can handle large pieces of code, don't be afraid to upload it all. However, that's for next time. Your question has already been answered now, so please don't touch the code. \$\endgroup\$ – Mast Apr 29 at 6:40
  • 2
    \$\begingroup\$ Basically, you're simulating an int by comparing against an actual int. That's not simulating, that's wrapping. \$\endgroup\$ – Mast Apr 29 at 6:44
5
\$\begingroup\$

I would make the bool operator explicit (and const)

The explicit will prevent the object being auto converted to bool in situations where you don't want it too. This may break with int type but is usually a better work match.

explicit operator bool() const  { return !!_i;                    }

I know how the !! works but it is obscure and a lot of people will raise an eyebrow. At least write a comment about it.


These methods should be const

operator int()    const { return static_cast<int>(_i);    }
operator double() const { return static_cast<double>(_i); }
//                ^^^^^

They do not change the state of the class.


All the assignment operator methods are defined wrong.

jd_int operator+=(jd_int jdi) { return _i += jdi._i; }

// Should be:

jd_int& operator+=(jd_int const& jdi) { _i += jdi._i; return *this; }

The op= is modifying the current object. Thus you should return a reference (not an object). Consequentially the return should return *this. There is no need to pass the input parameter by value (this could cause an unrequited copy). Rather pass the parameter by const reference to avoid this.


All the simple operator could be done better

jd_int operator+ (jd_int jdi) { return _i +  jdi._i; }

// Should be:

jd_int operator+ (jd_int const& jdi) { return jd_int(*this) += idi; }

Pass the parameter by const reference. Then use the op-assignment to do the work.


You define all the operators as members of the class. Personally I would also do this. But there is an argument for making the free standing functions.

id_int + int  => works.             (int will be converted to id_int then addition done)
int + id_int  => fails to compile.

If you convert the above members into free standing functions then you will get either side to auto convert.


The freestanding friend functions.

friend bool operator< (jd_int lhs, jd_int rhs);
...
friend std::ostream& operator<<(std::ostream& os, const jd_int jdi);
friend std::istream& operator>>(std::istream& is, jd_int jdi);

You declare them in the function and then define them later. Why? These are trivial functions define them inside the class. Splitting the declaration and definition does not provide any benefit.

Also pass the parameters by const reference when you can.

friend bool operator< (jd_int const& lhs, jd_int const& rhs)         {return (lhs._i <  rhs._i); }
...
friend std::ostream& operator<<(std::ostream& os, jd_int const& jdi) {return os << idi._i;}
friend std::istream& operator>>(std::istream& is, jd_int& jdi)       {return is >> idi._i;}

I think you are missing a couple of operations (binary and and binary or sprint to mind).


I hate it when people use underscore as the first character of an identifier.

int _i;

There are some complex rules around its usage. Not everybody knows these rules exactly so something best avoided. Also why i why not value at least that is a bit more meaningful.

\$\endgroup\$
  • \$\begingroup\$ I am afraid I have to differ on the "pass by const reference whenever possible" viewpoint. Passing a jd_int by value is probably better than by const reference considering its size. \$\endgroup\$ – L. F. Apr 29 at 11:10
  • \$\begingroup\$ @L.F. If you make the assumption that the class will never change. Since parameters are usually passed by register you will use the same amount of register by either method. So passing by value at best will be the same cost. \$\endgroup\$ – Martin York Apr 29 at 14:14
  • \$\begingroup\$ But passing by value has its benefit, right? You don’t want to pass builtin types by value. That doesn’t really matter that much, anyway. \$\endgroup\$ – L. F. Apr 29 at 23:00
  • \$\begingroup\$ @L.F. I can see your argument. But having to take into account future maintenance I would do the const reference passing now. It may never pay off but just in case it does (and it does not cost you anything (in my opinion (apart from two minutes of time))). \$\endgroup\$ – Martin York Apr 29 at 23:37
1
\$\begingroup\$

Are there any tricks that I could learn from this?

This sometimes defeats the purpose of an exercise, but it's worth knowing about ways to reduce the amount of boilerplate. Notice how much typing you have to perform in order to define so many almost-identical functions, e.g. all the comparison operators, all the arithmetic operators. With the CRTP, you can drastically outsoure the repetitive funtionality into base class templates.

Luckily, getting started with this is easy if you allow for a dependency on Boost operators:

#include <boost/operators.hpp>

class jd_int : private boost::totally_ordered<jd_int, boost::integer_arithmetic<jd_int>> {
public:
    jd_int() = default;
    jd_int(int i)             : _i{i}                   { }

    jd_int& operator+=(jd_int jdi) { _i += jdi._i; return *this; }
    jd_int& operator-=(jd_int jdi) { _i -= jdi._i; return *this; }
    jd_int& operator*=(jd_int jdi) { _i *= jdi._i; return *this; }
    jd_int& operator/=(jd_int jdi) { _i /= jdi._i; return *this; }
    jd_int& operator%=(jd_int jdi) { _i %= jdi._i; return *this; }

    friend bool operator< (jd_int lhs, jd_int rhs) { return lhs._i < rhs._i; }
    friend bool operator==(jd_int lhs, jd_int rhs) { return lhs._i == rhs._i; }

private:
    int _i;

    friend std::ostream& operator<<(std::ostream& os, const jd_int jdi);
    friend std::istream& operator>>(std::istream& is, jd_int jdi);
};

The base class here defines the missing comparison operators based on the two provided, same for the non-mutating arithmetic operators. Implementing these base-classes yourself is probably an excellent exercise, too.

Is there anything I could improve on?

If the compiler-generated special member functions are fine, don't specify them. This is the case, no need to implement the copy or move-ctor.

And, as pointed out in the comments and by @MartinYork, not all operators that clients would expect are present. Example: shouldn't the following compile?

jd_int i = 42;

+i; // this is called "unary plus"

jd_int j = -i;  // ... and "unary minus"
\$\endgroup\$

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