4

Say I need to use s:

typedef struct tagSOMESTRUCT   // Defined by someone else; C-compatible
{
    int count;
    int elements[256];
} SOMESTRUCT;

SOMESTRUCT s;

and say I have a function like:

template<typename RevFwdIt>
std::pair<RevFwdIt, RevFwdIt> some_slice_rev(RevFwdIt rbegin, RevFwdIt rend)
{
    RevFwdIt it = basename_rev(rbegin, rend);
    RevFwdIt e = std::find(rbegin, it, 5);
    return std::make_pair(e == it ? rbegin : e, it);
}

In order to use this function, I need to say

some_slice_rev(&s.elements[s.count - 1], &s.elements[-1]);

which (IMHO) is ugly and error-prone due to the off-by-one errors.

On the one hand, I cannot simply change some_slice_rev to some_slice to use the (much better)

some_slice(&s.elements[0], &s.elements[s.count]);

because then std::find would search from the beginning instead of the end.

On the other hand, the code itself already looks broken to me, because I can't see how std::find would handle "reverse iterators" that are raw pointers.

What is the best way to fix the code in situations like this? Is there any way to work with reverse-iterators that are raw pointers? Or is there a standard refactoring mechanism for fixing this, other than changing SOMESTRUCT?

user541686
  • 205,094
  • 128
  • 528
  • 886

2 Answers2

7

I'm not quite sure I understand the question (that may be from the awkward mixing of iterator directions you seem to be trying to avoid), but I'll just direct your attention to std::reverse_iterator:

#include <iostream>
#include <iterator>

// for example
template <typename Iter>
void print_it(Iter first, Iter last)
{
    std::cout << '|';

    for (; first != last; ++first)
        std::cout << ' ' << *first << " |";

    std::cout << std::endl;
}

int main()
{
    int arr[10] = {1, 2, 3, 4};

    int *begin = arr, *end = arr + 4;

    print_it(begin, end);
    print_it(std::reverse_iterator<int*>(end),
                std::reverse_iterator<int*>(begin));
}

They work like bi-directional iterators, except ++ is internally --, and vice versa.

Note that it's a bit ugly. You might want some utility function:

#include <iostream>
#include <iterator>

// for example
template <typename Iter>
void print_it(Iter first, Iter last)
{
    std::cout << '|';

    for (; first != last; ++first)
        std::cout << ' ' << *first << " |";

    std::cout << std::endl;
}

template <typename Iter>
std::reverse_iterator<Iter> make_reverse_iterator(Iter iter)
{
    return std::reverse_iterator<Iter>(iter);
}

int main()
{
    int arr[10] = {1, 2, 3, 4};

    int *begin = arr, *end = arr + 4;

    print_it(begin, end);
    print_it(make_reverse_iterator(end),
                make_reverse_iterator(begin));
}

So I think you want this:

template<typename ForwardIterator >
std::pair<ForwardIterator, ForwardIterator>
    some_slice(ForwardIterator begin, ForwardIterator end)
{
    typedef std::reverse_iterator<ForwardIterator> rev_iter;

    rev_iter it = basename(rev_iter(end), rev_iter(begin));
    rev_iter e = std::find(rev_iter(end), it, 5);

    return std::make_pair(it.base(), e.base());
}

Relatively off-topic now, but note that s.elements[s.count] is undefined behavior if s.count is 256, because s.elements[s.count] is *(s.elements + s.count), which isn't a valid array element to dereference.

In practice, the full expression is fine because &*x cancels out to x, but you still probably want to avoid it:

some_slice(s.elements, s.elements + s.count);

s.elements[-1] may also be undefined behavior, though I think strictly speaking it might be legal by accident, because you have an int member before the array.

GManNickG
  • 494,350
  • 52
  • 494
  • 543
  • Ohh... wow. I didn't know it's legal to instantiate `reverse_iterator` like that... thought it must be instantiated internally. +1 that's helpful. But, a follow-up: do you know of any generic way to create a `some_slice` wrapper around `some_slice_rev` instead? (I can't simply say `some_slice(make_reversed(end - 1), make_reversed(begin - 1))` because that would break on non-raw-pointer types.) – user541686 Mar 24 '12 at 08:35
  • @Mehrdad: I'm afraid I'm back to not understanding it. :) Perhaps knowing that `std::reverse_iterator` has a `base()` function, to getting the underlying iterator, helps answer your question. – GManNickG Mar 24 '12 at 08:42
  • Sorry, I'll try to make it more clear: The problem is that I want to be able to use `find` on a C-style array, but starting from the end of the array. Of course, this works on any container with reversed iterators (`rbegin()` and `rend()`), except that it doesn't work for C-style arrays, which use pointers. I would **like** to simply create a function `XYZ` that uses forward iterators in order to do this, instead of making an awkward `XYZ_rev` function, but I don't know of a good way. Any suggestions? – user541686 Mar 24 '12 at 08:44
  • @Mehrdad: If you replace `print_it` with `std::find` in my examples, that works just fine. So you want a function that takes forward iterators originally, but internally reverses them for a step? – GManNickG Mar 24 '12 at 08:46
  • Yes, exactly. :) How would I go about doing that? Do I just have to avoid using `find` altogether, and make my own `find_rev` function? :\ – user541686 Mar 24 '12 at 08:48
  • @Mehrdad: I've edited the answer, I'm pretty sure that's what you want. I'll leave some debugging up to you though, as I'm still conceptually unsure what's going on. :) – GManNickG Mar 24 '12 at 09:01
  • Thanks a lot. :) The trouble is the expression `basename(rev_iter(end), rev_iter(begin))`: even though it fixes the pointer problem, it would break on non-pointer iterators (because it would require decrementing from the beginning iterator, which is not always defined). – user541686 Mar 24 '12 at 09:05
  • @Mehrdad: Hm, you don't need to modify that code for any specific iterators, it works for them all. `reverse_iterator`s internally subtract at the right time to Do the Right Thing. So given `rev_iter r(end)`, `*r` is not `return *end;` but actually `temp = end; --temp; return temp;` – GManNickG Mar 24 '12 at 09:10
  • What happens if `begin == end`? – user541686 Mar 24 '12 at 09:12
  • @Mehrdad: With respect to which function? All algorithms should ensure the iterator they're about to dereference is not `== end` before they dereferencing it, for example, and `reverse_iterator` comparison can just compare unmodified internal iterators, so it's always safe to do. – GManNickG Mar 24 '12 at 09:15
  • Ooooooh, right... I didn't realize that it could compare the unmodified iterators. Thanks a lot! – user541686 Mar 24 '12 at 09:17
  • `&s.elements[256]` (address one past the last element of an array) is an idiomatic expression in both C and C++. In the code `&x[i]` stands out more than `x + i`. – jfs Mar 24 '12 at 11:09
  • @J.F.Sebastian: I didn't say lots of people don't do it, that doesn't make it right. :) I'm just being technical. – GManNickG Mar 24 '12 at 20:50
1

One straightforward solution is to write a wrapper iterator class to simulate reverse iterator, and then use it instead of raw array iterators, with std::find as usual. So when std::find calls ++it, it invokes operator++ which internally decrements the actual iterator --rawIt. That is what other standard reverse iterators do.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • I was thinking about that... is that the 'canonical' solution, so to speak? Or is there no well-known solution for this? – user541686 Mar 24 '12 at 08:21
  • @Mehrdad: I think, that is what other std reverse iterators do. – Nawaz Mar 24 '12 at 08:21
  • Hmmm... do you know of any generic way to create a `some_slice` wrapper around `some_slice_rev` instead? (I can't simply say `some_slice(make_reversed(end - 1), make_reversed(begin - 1))` because that would break on non-raw-pointer types.) – user541686 Mar 24 '12 at 08:34
  • @Mehrdad: I think, specialization would help doing that. Just handle each case differently, using little metaprogramming. – Nawaz Mar 24 '12 at 08:46
  • Yes, specializing for pointers is of course not a problem. But how would I keep my function compatible with other iterators which are not pointers? (`begin - 1` isn't necessarily valid on them.) – user541686 Mar 24 '12 at 08:46
  • @Mehrdad: The primary template will handle non-pointer iterators, and the specialization will handle pointer iterators. – Nawaz Mar 24 '12 at 08:47
  • How do you handle non-pointer iterators though? I can't think of a neat way to do this, because I don't have access to the original container from within `some_slice`, so I can't call `some_slice_rev(container.rbegin(), container.rend())`. – user541686 Mar 24 '12 at 08:49
  • @Mehrdad: You can mark when `it` reaches `begin`, then the next increment will go beyond end. No need to write `begin-1` anyway, if that is causing problem. – Nawaz Mar 24 '12 at 08:49
  • Would you mind posting a example of what you mean, for both non-pointer iterators and pointers (including the usage of `some_slice` and/or `some_slice_rev`)? I think the detail I'm trying to point out is a little hard to show without an example... unless I'm misunderstanding what you're saying. Thanks! – user541686 Mar 24 '12 at 08:54