10 replies to this topic

[stitchs]

Hello community,

I am having a minor issue in my code, I have replaced an Integer variable with the std::vector.size() in a comparison statement (please see below):

[source lang="cpp"]if(selectedOption_ > (optionsList_.size() - 1)) { selectedOption_ = (optionsList_.size() - 1); } else if(selectedOption_ < firstSelectableOption_) { // Highlighted option does not trigger as going less than 0. selectedOption_ = firstSelectableOption_; }[/source]

Before, I compared SelectedOption to an Integer (numOptions) and this worked as I expected, if it went greater than said number then it would be reset to numOptions, else if it when less than 0(firstSelectableOption), it would be set to zero.

Once I replaced it with size(), if the SelectedOption goes less than zero, it triggers the 'greater-than' clause and sets the SelectedOption equal to whatever the size()-1 is. I have looked through the debugger and the numbers are coming back as expected: SelectedOption becomes -1, and size in this instance being 2. So what is going on here. The quick-fix solution I am using is this:

[source lang="cpp"]if(selectedOption_ < firstSelectableOption_) { // Highlighted option does not trigger as going less than 0. selectedOption_ = firstSelectableOption_; } else if(selectedOption_ > (optionsList_.size() -1)) { selectedOption_ = (optionsList_.size() - 1); }[/source]

This ensures that the check for going less than zero occurs first, which solves the issue I am having, but does not answer the question it puts forward.

In short, is there something I am missing that would be firing the if-else in reverse order. Also, I have checked and am not setting size equal to something different elsewhere.

Thanks in advance for your time,

Regards,

Stitchs.

[JTippetts]

std::vector::size is an unsigned type, per the docs. Subtracting 1 from an unsigned 0 value doesn't give you a negative value, it gives you the maximum value the unsigned type can hold. Edit: Furthermore if the negative integer you are trying to compare gets implicitly cast to unsigned int, then it's no longer negative, just very large positive.

Edited by JTippetts, 15 December 2012 - 04:32 PM.

[stitchs]

In this instance of the class, size is always 2, it is never 0. That did cross my mind at one point, but I thought, as long as my Integer is not unsigned, then this would not be an issue? When would my negative Integer get cast to an unsigned int? Does this happen in the comparison at all?

Regards,

Stitchs.

[JTippetts]

When would my negative Integer get cast to an unsigned int? Does this happen in the comparison at all?

Regards,

Stitchs.

Yes.

Your compiler should be warning you about the signed to unsigned comparison, as well. It's a good idea to pay attention to those kinds of warnings, as subtle bugs like this can result.

[Servant of the Lord]

A possible solution would be to cast your unsigned value (the size_t result from vector<>.size()) to a signed value.

if(myInt < (int)vector.size())

Side-effects: if your size_t ever reaches higher than 31^2 (about two billion), your value will likely loop around to -31^2 (around negative two billion).

 

if(selectedOption_ >= int(optionsList_.size()))
{
	
}
else if(selectedOption_ < firstSelectableOption_)
{
	
}

Edited by Servant of the Lord, 15 December 2012 - 05:20 PM.

[Yrjö P.]

You can fix this simply by moving the -1 on the other side of the inequality as +1, so you'll no longer get underflows.

[Paradigm Shifter]

You'll just get an overflow if selectedOption_ == INT_MAX ;) but a lot less likely

[stitchs]

I get it now, Visual Studio wasn't showing me this warning on compilation. I have just performed a clean build, and it is one of the first warnings to show itself. Seems like my initial (if brief) thought actually was the case. Thanks all for your speedy responses, I'll get on the case now.

Regards,

Stitchs.

[Paradigm Shifter]

Also, whack up your warning level to the highest possible (level 4 I think), and fix all of them. If you get warnings from 3rd party headers, you can use #pragma warning(push:####) and #pragma warning(pop) to enable/disable them around specific headers (where #### is the warning you want to disable).

[NightCreature83]

Also, whack up your warning level to the highest possible (level 4 I think), and fix all of them. If you get warnings from 3rd party headers, you can use #pragma warning(push:####) and #pragma warning(pop) to enable/disable them around specific headers (where #### is the warning you want to disable).

In that case I would suggest you also set the flag treat warnings as errors, which forces you to fix them.

[kubera]

Hi!

Visual C++ could show such warning, but the compiler must have set the correct warning level.

Why such strange comparision? Maybe you would construct a bit more complex if.
Such magic values, etc sometimes are unsafe and strange for reading.