size_t implicit conversion

Hi

I think the main problem is the mixture of signed and unsigned types, always a recipe for disaster !!

So what happens if you change the type of i and B to std::size_t ? This is normal practise when dealing with the STL size functions, because that's what they return.

Alternatively, consider using iterators with a normal for loop, or the for_each algorithm. There are forward, reverse and const, non const versions. That way you can avoid math to determine where they start and stop.

https://en.cppreference.com/w/cpp/language/for
https://en.cppreference.com/w/cpp/algorithm/for_each

Iterators and ranges are have libraries of their own:

https://en.cppreference.com/w/cpp/iterator
https://en.cppreference.com/w/cpp/ranges

So you could make own iterator or range if B is smaller than the size of the container, that is you wanted to iterate part of the container.

If you want to iterate the whole container, then boost has a reverse range based for loop:

https://stackoverflow.com/questions/8542591/c11-reverse-range-based-for-loop

Hope this helps, Good Luck !!

Last edited on

If you want to iterate the whole container, then boost has a reverse range based for loop

C++20 introduced the reverse range adaptor.
https://www.fluentcpp.com/2020/02/11/reverse-for-loops-in-cpp/

There is a third option, Range-v3, for reverse for loops that also don't require C++20:
https://ericniebler.github.io/range-v3/

Hi,

Something I forgot to add earlier, the reason why the loop continues:

In the expression Xdata.size()-B B is promoted from int to std::size_t If the expression is equal to zero, then the condition is true. In the comparison between i and the expression, i is also implicitly promoted to std::size_t . When i is decremented it would have a value of say 2⁶⁴ - 1 (if std::size_t is really a unsigned long long *), because std::size_t can't have negative values, so the loop continues.

* The type of std::size_t is often set to the largest unsigned type the system has, but this is not guaranteed by the standard.

So this is why it is problematic to mix signed and unsigned types when doing arithmetic.

Also there should have been a compiler warning about this, if one has the warnings turned on. Always have compiler warnings turned on to the highest level one can get away with. It's worth reading the manual to turn on specific warnings. With g++ and clang++ there quite a few warnings not turned on by -Wall -Wextra -pedantic

GeorgeP wrote:
C++20 introduced the reverse range adaptor.

Ah cool, thanks George. There is so much stuff in the ranges library, awesome.

Last edited on

Check your value of B — I don’t believe it is what you think it is.

To iterate over the final N elements in reverse, using old-style indexed-for loops, it pays to be a little redundant:

1
2

for (size_t i = xs.size(), n = 0;   (i-- > 0) and (n < N);   ++n)
    std::cout << xs[i] << "\n";

Good luck!

seeplus (6457)

Also note that since c++20 there is also .ssize() which returns as a signed type the number of elements

keskiverto (10365)

1
2
3

for (int i = Xdata.size() - 1; i >= Xdata.size()-B; --i){
         // loop body
    }

Do you get same with:

if ( B < 0 or Xdata.size() < B or Xdata.size() == 0 ) throw 42;
for (int i = Xdata.size() - 1; i + B >= Xdata.size(); --i){
         // loop body
    }

PhysicsIsFun (283)

Thank you guys, the problem seems to be what @TheIdeasMan described.

Would it help if I simply explicitly converted to int before, i.e.

 
int size = Xdata.size();
for (int i =size - 1; i >= size-B; --i){
         // loop body
    }

Last edited on

PhysicsIsFun wrote:
Would it help if I simply explicitly converted to int before, i.e.

Well you could do that, but I would recommend using the facilities of the language to take care of things, rather than old school "hand coding". It's a huge advantage of using the STL, a lot of the tricky stuff is done for you. So in other words make your life easier and better.

In your example, really you should bounds check the B variable, which is what Duthomhas code above does in a clever fashion. By using ranges and iterators, things like bounds checking can be taken care of in an easy and safe manner. I modified the code from cppreference:

https://en.cppreference.com/w/cpp/ranges/take_view

#include <algorithm>
#include <iostream>
#include <ranges>
 
auto print = [](char x){ std::cout << x; };
 
int main()
{
    constexpr char pi[]{'3', '.', '1', '4', '1', '5', '9', '2'};
    
    std::ranges::reverse_view reverse_pi{pi};
    const std::size_t reverse_size {3};
 
 //print last 3 digits of pi reversed
    std::ranges::for_each(reverse_pi | std::ranges::views::take(reverse_size), print);
    std::cout << '\n';
 
    // safely takes 8 chars only, i.e. min(pi.size(), 42)
    std::ranges::for_each(std::ranges::take_view{pi, 42}, print);
 
    std::cout << '\n';
}

Output:

295 3.141592

If reverse_sizewas too big for the size of the container, it does the right thing. I made the type std::size_t to avoid negative values.

Note that not all of the algorithms and constructors do bounds checking, some result in Undefined Behavior (UB). For example some of the constructors of std::ranges::subrange also have the disadvantage of std::moveing things which may not be what one wants.

https://en.cppreference.com/w/cpp/ranges/subrange/subrange

The ability to chain together range algorithms with | is a really powerful feature.

Anyway Good Luck !!!

Just to be clear, I totally agree with TheIdeasMan’s recommendations here.

Unless you have some weird restriction on what Language Standard you can compile against or prohibition against just downloading from https://github.com/ericniebler/range-v3 for use with C++14/17/20, life is so much easier if you use modern C++ methods.

And you wouldn’t think it mattered, but using ranges is often quicker than using a hand-coded index loop as well.

(I learned this when playing with bignums. https://cplusplus.com/forum/lounge/32041/)

using ranges is often quicker than using a hand-coded index loop as well.

Especially when walking through a container in reverse order.

If I needed to walk through a container that doesn't "touch" each element, but in a precise skip method such as every 3rd element, I look at using iterators. Otherwise I'd use a range-based loop.

You can create iterators on a known non-pointer type "regular" array (type arr[10]), though if it has devolved to a pointer (type*) the iterator creation will fail.

A range-based loop has the same problems when dealing with devolved to pointer arrays. That is why using std::array is preferred over a regular array if one needs a fixed size container. A std::array never devolved to a pointer.

Yes, the interface for std::array isn't as neat and clean as a regular array, but the advantages outweigh that IMO.

PhysicsIsFun (283)

I will need to look at how to use the ranges then. I am often too lazy to look at new stuff, but I guess it pays off in the long run. Cheers guys!

The quickest way to learn about ranges (somewhat) is to learn and use the range-based for loop. It was introduced in C++11.

https://en.cppreference.com/w/cpp/language/range-for

C++20 added a nice refinement of an init-statement so you can declare a separate variable within the for loop declaration. I've used this variable to "skip" elements or provide a means to output an element's index number.

It’s pretty easy, actually. Here’s a simple example:

#include <iostream>
#include <ranges>
#include <vector>

int main()
{
	std::vector <int> xs = { 2, 3, 5, 7, 11, 13, 17, 19 };

	std::cout << "forward:\n ";
	for (auto x : xs)
		std::cout << " " << x;
	std::cout << "\n";
	
	std::cout << "reversed:\n ";
	for (auto x : xs | std::views::reverse)
		std::cout << " " << x;
	std::cout << "\n";
}

forward:
  2 3 5 7 11 13 17 19
reversed:
  19 17 13 11 7 5 3 2

Requires C++20 (or to download the Ranges library from Github like I linked above).

Last edited on

I reckon it could be a guideline for people new to C++, don't write old style for loops when dealing with containers: Prefer range based for loop; or STL algorithm and containers.

Maybe it is already a guideline, if so I have just reiterated it :+)

Duthomhas wrote:
download the Ranges library from Github like I linked above

Neener, neener, neener! I mentioned the Range-v3 library first! :Þ

Sorry, I couldn't resist. It ain't often I can claim something like that.

Huh, so you did.

I swear I’m getting senile.