PG uniq does not preserve order

[dlglin]

The uniq function in PGbasicmacros.pl returns an array of unique items from the input array, but it doesn't maintain a consistent order of the items in the returned array.

Consider the following MWE

DOCUMENT();
loadMacros("PGstandard.pl",
           "parserRadioButtons.pl",
          "PGML.pl");

## Do NOT show partial correct answers
$showPartialCorrectAnswers = 0;

@answers = ('a','b','c','b','a');
@options = uniq(@answers);

$mc = RadioButtons([@options],'b');

BEGIN_PGML
[_]{$mc}
END_PGML

ENDDOCUMENT();

Each time you load the problem the options can be presented in a different order, which means that even if you select the correct answer it may be marked incorrect since the array could be reordered on submit.

All of the examples that I found in the OPL (Contrib actually) take this into account and sort the resulting array, so it may be that this behaviour is understood, but there is a comment in the code that says "sort is causing trouble with Safe.??", which makes me think that the intention was to have the function sort the array.

[dlglin]

I wonder if that comment predates the addition of the PGsort methods in PGbasicmacros.pl. Would it be as simple as adding lex_sort or num_sort to the end of the uniq function? Are there downsides to that?

[drgrice1]

Note that it is documented in the POD for the uniq method that the order is unspecified. So this behavior is technically not an issue, as it is the expected behavior of the method. Of course the reason it is documented that way is probably also related to the comment you mentioned regarding sort in the safe compartment. But regardless of the reason it is implemented this way, at this point it means that sorting the result is something that the caller is expected to do if it is desired to have the result sorted.

One problem with sorting the uniq return value is choosing which sort to use. lex_sort and num_sort are different sorts and would give different results depending on the types of the elements in the list that is passed to the uniq method. In fact , you really couldn't use num_sort because then you can't even pass non-numeric values to the method (as in your example) without getting warnings. So you really have to use lex_sort. But then it may be unexpected when you get back 1, 11, 2, 3 when you call uniq(1, 2, 3, 11).

So in the end I think that it is actually best to leave this as it is, and tell the caller to sort the result if desired. The POD documentation should be made more concise regarding this though.

Another possibility is to reimplement the uniq method as

sub uniq {
	my @in = @_;
	my %seen;
	return grep { !$seen{$_}++ } @in;
}

Then the order will always be the same, but will be dependent on the order of the input. For example, if you call uniq('a', 'c', 'b', 'a', 'c') it will always return 'a', 'c', 'b'.

[drgrice1]

This was fixed in #1298.