A possible solution to the unsigned x = -1; bug

[Paul]

Suppose I code a function to perform f() a user_specified number of times.
A natural first attempt would be

void perform_f(int numTrials)
{

for(int i = 0; i < numTrials; ++i)
f();
}

In an improvement, I decide that I don't want negative input to be
acceptable because a negative number of trials doesn't make sense.
I'm more ambitious than just writing a message about it.
I simply don't want a negative numTrials to be a possibility in the first place.

So a second attempt could simply change the signature
to void perform_f(unsigned numTrials)
If I don't want the compiler to give any warnings,
I should use unsigned in the for loop too.

But this is even worse.
If a caller of perform_f erroneously sets numTrials to -1
the number of trials performed is UINT_MAX which is unlikely to
be what the caller wants.

My solution?
Create a new integerType class
IntegerType(int underlying)
which throws an exception in the constructor when a user tries to make the instantiation of the underlying integer positive.

Is this what's done in practice?
If not, what is the standard solution?

Many thanks for your advice.

Paul

[Paul]

I meant
"exception in the constructor when a user tries to make the underlying
integer negative".

Apologies for the error.

Thanks,

Paul

[Daniel]

On Tuesday, October 2, 2018 at 3:58:35 AM UTC-4, Paul wrote:
>
> void perform_f(int numTrials)
> {
>
> for(int i = 0; i < numTrials; ++i)
> f();
> }
>
> In an improvement, I decide that I don't want negative input to be
> acceptable because a negative number of trials doesn't make sense.
> I'm more ambitious than just writing a message about it.
> I simply don't want a negative numTrials to be a possibility in the first place.
>
> So a second attempt could simply change the signature
> to void perform_f(unsigned numTrials)
> If I don't want the compiler to give any warnings,
> I should use unsigned in the for loop too.
>

> If a caller of perform_f erroneously sets numTrials to -1
> the number of trials performed is UINT_MAX which is unlikely to
> be what the caller wants.
>

I don't think that case is any different from the caller
setting numTrials to _any_ erroneous number, whether that be
2 or 2000000 or UINT_MAX or whatever. If you like, you
can validate that numTrials is within a permissible range.

Daniel

[Paul]

Daniel,

Thanks for your response.
I'm interested in the technique of validation.
With the approach in my OP,
I would write the following
Any feedback (even pointing out obvious carelessness)
would be helpful.
It has been tested as below but testing has been limited.
Since it's been copy-pasted from my compiler, apologies in advance
if the line returns look ugly.

Many thanks. It's been a very helpful newsgroup for me.
Paul

#include <exception>
#include <iostream>

class ValidInteger
{

static constexpr int min = 0;
static constexpr int max = 1000; // Suppose more than 1000 trials is unfeasible.
int NumTrials;

public:
ValidInteger(int);
bool operator< (const ValidInteger&) const;
ValidInteger& operator++();
};

ValidInteger::ValidInteger(int numTrials) : NumTrials(numTrials)
{
if(NumTrials < min)
throw(std::runtime_error("Underlying value too small"));

if(NumTrials > max)
throw(std::runtime_error("Underlying value too large"));
}

bool ValidInteger::operator<(const ValidInteger& rhs) const
{
return NumTrials < rhs.NumTrials;
}

ValidInteger& ValidInteger::operator++()
{
++NumTrials;
return *this;
}

void f()
{
std::cout << "\n f is called\n";
}

void perform_f(ValidInteger trialsNum)
{

for(ValidInteger i = 0; i <trialsNum; ++i)
f();
}

int main() // test valid and invalid cases
{
try{
perform_f(5);
perform_f(-5);
}

catch (const std::runtime_error& e)
{
std::cout << "\n" << e.what() << "\n";
}
}

[David Brown]

I would say that is over-engineering, unless your ValidInteger class
could be used in all sorts of other cases (in which case it should
probably be a template, and maybe a better name). Try this:

const int min_no_of_trials = 0;
const int max_no_of_trials = 1000;

void perform_f(int trialsNum) {
if (trialsNum < min_no_of_trials) {
throw(std::runtime_error("Too few trials");
}
if (trialsNum > max_no_of_trials) {
throw(std::runtime_error("Too many trials");
}

for (int i = 0; i < trialsNum; i++) {
f();
}
}

That puts the relevant logic where it makes sense.

[Jorgen Grahn]

No.

> If not, what is the standard solution?

Personally, I think it's the user's responsibility.

In general, I think it's very useful to create types which are locked
down in various ways. Instead of integers, I've used things like:
- Error counters which start at 0 and only support ++
- Numeric identifiers in various protocols (no arithmetics; different
protocol fields get distinct types).
- ...

I can't explain why, but I wouldn't do that here. Perhaps because it
has to be a runtime check, via an assertion.

(And perhaps because of what someone else wrote: where do you draw the
line? Is a billion tries ok?)

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[Paul]

Thanks David,

The original problem (before Daniel's response)
was avoiding applications allowing code saying things
like "Perform f() -2 times".

This opportunity is likely to be pervasive and that's why
I used my over-engineered solution.
I never thought of doing it with templates. I suppose the
reason is that we want to avoid lots of integer types becoming negative.
A template class, I think.
So instead of class ValidInteger, we could have

template<typename IntType>
class ValidRange{
// As previous code but int NumTrials; is
// replaced by IntType NumTrials;
// and things like ValidInteger::operator++ are replaced
//by ValidRange< IntType> :: operator++


}

Is that correct, and what you mean?

Thanks a lot. Glad you reminded me of the template solution.

Paul

[Paul]

Thanks, Jorgen.
So we want to lock down types in various ways.
We're interested in counting the number of times something is done.
So this starts at 0. Call this type TrialsCounter.

Now TrialsCounter can't represent a negative number of times.
So wouldn't the code for TrialsCounter be exactly like what I did?

I identified the need to lock down types, and I (and David) tried to
give an idea for doing so.

Paul

[David Brown]

My example is exactly as good at rejecting invalid values as yours, and
throws the same exceptions.

> This opportunity is likely to be pervasive and that's why
> I used my over-engineered solution.

Over-engineering is not good. Try to make the logic of the code clear.
In your case, there is a disconnect between the code that needs a
limited range, and the code that checks for the limits.

> I never thought of doing it with templates. I suppose the
> reason is that we want to avoid lots of integer types becoming negative.
> A template class, I think.
> So instead of class ValidInteger, we could have
>
> template<typename IntType>
> class ValidRange{
> // As previous code but int NumTrials; is
> // replaced by IntType NumTrials;
> // and things like ValidInteger::operator++ are replaced
> //by ValidRange< IntType> :: operator++
>
>
> }
>
> Is that correct, and what you mean?

No.

I mean make ValidRange a template taking an int parameter, to form the
maximum allowed value. Templating on the type of integer is
over-engineering again.

[Paavo Helde]

On 2.10.2018 10:58, Paul wrote:
> Suppose I code a function to perform f() a user_specified number of times.
> A natural first attempt would be
>
> void perform_f(int numTrials)
> {
>
> for(int i = 0; i < numTrials; ++i)
> f();
> }
>
> In an improvement, I decide that I don't want negative input to be
> acceptable because a negative number of trials doesn't make sense.
> I'm more ambitious than just writing a message about it.
> I simply don't want a negative numTrials to be a possibility in the first place.
>
> So a second attempt could simply change the signature
> to void perform_f(unsigned numTrials)
> If I don't want the compiler to give any warnings,
> I should use unsigned in the for loop too.
>
> But this is even worse.
> If a caller of perform_f erroneously sets numTrials to -1
> the number of trials performed is UINT_MAX which is unlikely to
> be what the caller wants.

For type conversions there is an option to use e.g. boost::numeric_cast
which can be routinely used instead of static_cast for prophylactic means:

#include <iostream>
#include <boost/cast.hpp>
#include <cstdint>

void f() {}

template<typename T>
void perform_f(T numTrials) {
const std::uint8_t n = boost::numeric_cast<std::uint8_t>(numTrials);
for (std::uint8_t i = 0; i < n; ++i) {
f();
}
}

int main() {
try {
std::cout << "Test A\n";
perform_f(25);

std::cout << "Test B\n";
perform_f(-2);

std::cout << "Test C\n";
perform_f(1000000);

} catch (const std::exception& e) {
std::cerr << e.what() << "\n";
return EXIT_FAILURE;
}
}

OUTPUT:
Test A
Test B
bad numeric conversion: negative overflow

[Paul]

Thanks, David
I don't understand why your idea improves on mine.
Possibly, the reason I don't know this is that I don't
understand precisely enough what you intend.

Paul

[Paul]

Sorry, I think I get it now.
integer templates.

You use integers between < and >
just as in my approach you use int or long
etc. between these brackets.

I'd forgotten about that.
I'll look up int templates now.
I agree that that is much better than what I did.

Great that I'm learning so much.

Paul

[David Brown]

Exactly, yes.

> I'd forgotten about that.
> I'll look up int templates now.
> I agree that that is much better than what I did.
>
> Great that I'm learning so much.
>

Marvellous. Keep asking questions, and keep learning. That's what this
newsgroup is for (despite appearances sometimes).

Once you have had a try, post your code if you are not sure about it.

[Alf P. Steinbach]

No, one just uses `int`.

Except some programmers who does use not just `unsigned`, but all kinds
of unsigned types that they pick up from typedefs in classes. They add a
heck of a lot of up front work in order to satisfy an associative ideal.
Then get a lot of extra work also later when the lunacy results in bugs.

> If not, what is the standard solution?

`int`.

Since source code is about communicating to programmers, you can
communicate a non-negative intent by e.g.

using Non_negative_int = int;

However, I can't recall actually doing that, just that the possibility
has popped up in earlier discussions about this.

> Many thanks for your advice.

You're welcome.


Cheers!,

- Alf

[peps...@gmail.com]

Good points. Thanks to everyone.

[Paul]

Thanks for your encouragement.
I am actually looking for a C++ tutor, by the way, for online sessions
via something like Google hangout.

If anyone is interested, please reply to author and we can discuss rates
and availability.

I drafted a new topic with this tutoring request, but I thought I might
get in trouble on OT grounds.

I know there are standard websites for tutoring but I think the advice
here is far more expert than what I would get from a tutoring website.

Thanks,

Paul

[Pavel]

Yet another possibility is to use saturated math. e.g.:

template <typename B/*base type*/, B L/*low limit*/, B H/*high limit*/>
class SaturatedLimited {
public:
SaturatedLimited(): val_(Limit_(B())) {}
SaturatedLimited(B val): val_(Limit_(val) {}
// operators and functions you need; use native
// B arithmetics, then apply Limit_() to get final result
private:
static B Limit_(B val)
{ return val < L ? L : val > H ? H : val; }
B val_;
};

typedef SaturatedLimited<unsigned, 0u, 1000u) NumberOfTests;

It is not as expensive as throwing exceptions but the semantics may or
may not be what you want. In case of passing the negative number of
tests to a test!running function it will silently not run a single test
-- which would seem reasonable to some people (mainly mathematicians, I
guess).

HTH
-Pavel

[Pavel]

Pavel wrote:
...

> typedef SaturatedLimited<unsigned, 0u, 1000u) NumberOfTests;

a little correction :-) :
typedef SaturatedLimited<int, 0, 1000) NumberOfTests;
what was I thinking? ...