2

I already saw this thread, however the situation is totally different and I can't apply that solution to my problem.

I have the following constructor:

Fan::Fan(Id id, std::string name, Age age);

Where Id and Age are typedef'ed unsigned int and unsigned short. This is given to me, so I know that I must use them, as most likely the tester of the assignment will try to use numbers bigger than int (in range of unsigned int / short).

Obviously, a Fan's id and age cannot be negative numbers. In the example code (to compile against), a fan is created with numbers for id and age. I cannot use string there and check for a minus sign.

I thought it wouldn't compile when I entered negative age / id in my unit testing. However, it turned out it did compile, and it gave random values (most likely due to overflow reasons).

So to conclude - it is possible to call the constructor with negative values for the age and id, but in the constructor their values get 'thrashed' and random numbers appear, and it causes unintended behavior.

For example, this:

Fan* fan = new Fan(-1, "Name", 9);

Compiles, and at run-time the id of the fan gets some unrelated value. So I must be able to detect the negative number in the constructor. How do I "block" the negative numbers in the constructor?

Thanks for your time! I hope my question is clear.

bad_coder
  • 11,289
  • 20
  • 44
  • 72
Zephyer
  • 333
  • 6
  • 16
  • 2
    What do you want the behaviour to be? You can start by accepting signed integers and then explicitly checking for >=0. Then add an assert() which kills the program, or throw an exception if this check fails. – Trenin Jan 20 '14 at 20:05
  • Take a look at http://www.cplusplus.com/reference/climits/ and [two's complement(http://en.wikipedia.org/wiki/Two's_complement) – OldProgrammer Jan 20 '14 at 20:06
  • you could make check on input if the age/id is < 0, throw an error until the user enters a valid input. – user2699298 Jan 20 '14 at 20:06
  • possible duplicate of [What happens if I assign a negative value to an unsigned variable?](http://stackoverflow.com/questions/2711522/what-happens-if-i-assign-a-negative-value-to-an-unsigned-variable) – tumdum Jan 20 '14 at 20:07
  • 1
    clang [issues a warning](http://coliru.stacked-crooked.com/a/a04ac10e59a5cc1a) for code similar to yours, but GCC doesn't. MSVC2013 also generates warning C4245 for the same code. If you can't get your compiler to generate warnings, your only option is to have the constructor take `long` or `long long` (assuming one of those is larger than `int`) for those paramters, then cast it to `Id` and `Age` respectively after checking that the value is greater than 0. – Praetorian Jan 20 '14 at 20:17
  • The behaviour i want - to be able to detect the number as negative and throw exception. but as it is inputed in 2s complement to the parameter it goes positive and i can no longer detect it. long long as too big range - but i guess i could input long long and check sign, and then convert to Id, that's a good idea. but what if the tester puts a number bigger than Id but in long long range? @Praetorian – Zephyer Jan 20 '14 at 20:25
  • 1
    @Zephyer You'd throw an exception in that case too. – Praetorian Jan 20 '14 at 21:05
  • @Praetorian true. can you post this as an answer so i can accept it? Thank you very much – Zephyer Jan 20 '14 at 21:08
  • [`std::is_unsigned`](http://www.cplusplus.com/reference/type_traits/is_unsigned/) may be of some help. – Rubens Jan 20 '14 at 22:54

2 Answers2

3

I created a small function template that checks whether the input lies within the range of the output data type, and throws an exception otherwise. It requires that the input and output data types are integral, and that the size of the input data type is larger than the output data type.

template<typename DestType, typename SrcType>
DestType range_check_and_convert(SrcType const& src)
{
    static_assert(sizeof(SrcType) > sizeof(DestType), 
        "SrcType must be larger than DestType");
    static_assert(std::is_integral<SrcType>::value &&
                  std::is_integral<DestType>::value, "integral types only");

    if(src > static_cast<SrcType>(std::numeric_limits<DestType>::max())) {
        throw std::out_of_range("input too big");
    } else if(src < static_cast<SrcType>(std::numeric_limits<DestType>::min())) {
        throw std::out_of_range("input too small");
    }

    return static_cast<DestType>(src);
}

Live demo

Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • the static cast is another brilliant idea. if i try to static cast long long into unsigned int, what would happen? – Zephyer Jan 20 '14 at 21:55
  • 1
    @Zephyer As long as the source value lies within the range of the destination type the cast is well defined. When casting from signed to unsigned, if the source is out of range you get modulo 2^N wrap around behavior (where N is number of bits in destination type). But when casting from unsigned to signed, with a value that is outside the range of the signed type, the behavior is implementation defined. – Praetorian Jan 20 '14 at 22:09
  • Please, could you explain why do you pass the `src` argument by const reference? The only explanation that comes to my mind is to avoid copying in the case `SrcType` is not a built in type. For example if the user uses some arbitrary precision arithmetic, such as `gmp`. – Martin Drozdik Dec 08 '14 at 03:09
  • 1
    @MartinDrozdik I did that more or less out of habit. As you've pointed out there's really no reason to if you're dealing with built-in types. – Praetorian Dec 08 '14 at 04:35
2

By declaring the contructor to only take unsigned values you basically already "block" that negative numbers can be passed as parameters.

You can still write that you pass -1 to the function, but what the function then sees is the unsigned interpretation of that negative number, see this question.

For example in a 32-bit windows application passing -1 would result in an Id of 0xffffffff or 4294967295. Now you have to decide, whether this is a valid input for your application, but it probably is, because it's still a positive number. On the other hand an age of -1 is probably an invalid input, because it is unlikely to be 4294967295 years old. As you noticed, you can't check your unsigned value for -1. You have to check if age is bigger than for example 200, because -1 is effectively a large number when given as an unsigned parameter.

Community
  • 1
  • 1
typ1232
  • 5,535
  • 6
  • 35
  • 51