Opened 2 years ago

Closed 22 months ago

#2721 closed defect (done)

Shuffle extension method throws on empty enumerable

Reported by: abeham Owned by: abeham
Priority: medium Milestone: HeuristicLab 3.3.15
Component: Random Version: 3.3.14
Keywords: Cc:

Description

It throws an IndexOutOfRangeException because the 0th element is returned even if the length of array is 0.

Change History (5)

comment:1 Changed 2 years ago by abeham

  • Owner set to abeham
  • Status changed from new to accepted

comment:2 Changed 2 years ago by abeham

Following options:

  1. Query for an empty enumerable and throw an ArgumentException instead of an IndexOutOfRangeException
  2. Return an empty enumerable

I went with option 2) because it leads to cleaner code at the caller site. Option one requires the caller to check on that condition which leads to evaluating a condition twice or requires two calls to ToList().

1)

var aList = Enumerable.Range(0, count).Where(x => SomeCondition(x)).ToList();
if (aList.Count == 0) {
  // handle empty case
} else {
  aList = aList.Shuffle().ToList();
}

// or

IList<int> aList = null;
if (Enumerable.Range(0, count).Any(x => SomeCondition(x))) {
  aList = Enumerable.Range(0, count).Where(x => SomeCondition(x)).Shuffle().ToList();
} else {
  // handle empty case
}

2)

  var aList = Enumerable.Range(0, count).Where(x => SomeCondition(x)).Shuffle().ToList();
  if (aList.Count == 0) {
    // handle empty case
  }

comment:3 Changed 2 years ago by abeham

  • Owner changed from abeham to jkarder
  • Status changed from accepted to reviewing

r14522: check if enumerable contains at least one element before returning the first element

comment:4 Changed 2 years ago by gkronber

  • Owner changed from jkarder to abeham
  • Status changed from reviewing to readytorelease

Reviewed r14522

comment:5 Changed 22 months ago by abeham

  • Resolution set to done
  • Status changed from readytorelease to closed

r14669: merged r14522 to stable

Note: See TracTickets for help on using tickets.