mclow.lists added inline comments.
================
Comment at:
test/std/algorithms/alg.modifying.operations/alg.random.shuffle/random_shuffle.pass.cpp:35
@@ -30,1 +34,3 @@
}
+
+template <class Iter>
----------------
gribozavr wrote:
> gribozavr wrote:
> > mclow.lists wrote:
> > > This is not how I would rewrite this test.
> > > I would write a routine that took two "iterators", and called
> > > `random_shuffle`, and then checked for the desired results.
> > >
> > > Then call it with pointers. and RA iters, etc.
> > > for example:
> > >
> > > template <Class Iter>
> > > void test(Iter first, Iter last, Iter resFirst, Iter resLast);
> > >
> > > test(nullptr, nullptr, nullptr, nullptr);
> > > int source[] = {1, 2, 3, 4};
> > > int res [] = {1, 2, 3, 4};
> > > const unsigned size = sizeof(source)/sizeof(source[0]);
> > >
> > > test(source, source + size, res, res+size);
> > > test(random_access_iterator<int*>(source) .... );
> > >
> > >
> > I actually thought about this, and it is hard to rewrite it like that for
> > two reasons. First, `random_shuffle` mutates the elements, so one needs to
> > restore the original sequence between calls to `test()` (otherwise it is
> > not obvious that it was mutated). Second, this overload of
> > `random_shuffle` takes randomness from global state, so one can't just
> > specify one expected result in the test. That's why I first check for the
> > initial shuffle exactly, and then only check that the output is a
> > permutation of input.
> >
> Ping?
Which is why I think that your use of `is_permutation` is good below.
What do we want to know after calling `random_shuffle`?
1) The same items are in the range, only in a different order. (right? Can it
ever "do nothing")
2) Nothing else is changed.
If you have two collections with the same contents, you should be able to
shuffle one over and over, and call `is_permutation` to compare the two after
each call.
Tests that have duplicates are good, too.
[ Yes, I know that your bug report (and fix!) are very localized, but these
tests are really lacking ]
If you don't want to do this, I'll rewrite these tests sometime in the next
couple weeks, and then we can revisit your patch.
Repository:
rL LLVM
http://reviews.llvm.org/D14686
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
