Opened 8 months ago

Closed 5 weeks ago

#2723 closed defect (done)

Providing variable values as double[] to Dataset fails at runtime

Reported by: abeham Owned by: gkronber
Priority: medium Milestone: HeuristicLab 3.3.15
Component: Problems.DataAnalysis Version: 3.3.14
Keywords: Cc:

Description

In the Dataset class an IList is accepted as variable values, however later on this IList is cast as a List<T>.

public Dataset(IEnumerable<string> variableNames, IEnumerable<IList> variableValues)
  : base() {
}

  // line 199:
  List<T> values = list as List<T>;
  if (values == null) throw new ArgumentException("The variable " + variableName + " is not a " + typeof(T) + " variable.");

Now, a double[] is an IList, but not a List<double>. The compiler is happy, but the program crashes at runtime. In my opinion, it should be checked if it's a T[] in case List<T> fails or otherwise prevent accepting double[] in the constructor.

Change History (11)

comment:1 Changed 4 months ago by bburlacu

r14857: Add DatasetUtil static class with useful methods for dealing with Datasets. Add input validation to Dataset constructor.

comment:2 Changed 3 months ago by gkronber

  • Owner set to bburlacu
  • Status changed from new to assigned

I think r14857 is not related to the ticket description.

comment:3 Changed 3 months ago by bburlacu

  • Owner changed from bburlacu to gkronber
  • Status changed from assigned to reviewing

comment:4 Changed 3 months ago by abeham

I agree with gkronber, this change does not adress the ticket.

comment:5 Changed 3 months ago by bburlacu

r15013: Found a different solution to this issue, by using IList<T> as the return type for the GetValues<T> method.

comment:6 Changed 3 months ago by bburlacu

r15015: Fix exception in Dataset.cs when calling the ToModifiable() method.

comment:7 Changed 6 weeks ago by gkronber

r15152: merged r14857 from trunk to stable

Premature merge to stable to fix a compilation fail of the stable branch. The remaining changesets need to be reviewed an merged.

comment:8 Changed 6 weeks ago by gkronber

Reviewed r14857, r15013, r15015 and tested this with the following C# script.

    var names = new string[] {"a", "b", "c"};
    var @as = Enumerable.Repeat(1.0, 10).ToArray();
    var bs = Enumerable.Repeat("abc", 10).ToArray();
    var cs = Enumerable.Repeat(DateTime.Now, 10).ToArray();
    var values = new System.Collections.IList[] {@as, bs, cs};
      
    var ds = new Dataset(names, values);
    vars["ds"] = ds;

    Console.WriteLine(ds.GetDoubleValue("a", 0));
    Console.WriteLine(ds.GetStringValue("b", 0));
    Console.WriteLine(ds.GetDateTimeValue("c", 0));
    
    var columnA = ds.GetReadOnlyDoubleValues("a");
    Console.WriteLine(columnA[0]);
    
    var columnB = ds.GetReadOnlyStringValues("b");
    Console.WriteLine(columnB[0]);

    var columnC = ds.GetReadOnlyDateTimeValues("c");
    Console.WriteLine(columnC[0]);
    
    var aEnum = ds.GetDoubleValues("a");
    Console.WriteLine(aEnum.First());

    var bEnum = ds.GetStringValues("b");
    Console.WriteLine(bEnum.First());

    var cEnum = ds.GetDateTimeValues("c");
    Console.WriteLine(cEnum.First());
Last edited 6 weeks ago by gkronber (previous) (diff)

comment:9 Changed 6 weeks ago by gkronber

r15159: merged r15013 and r15015 from trunk to stable (all changesets merged)

comment:10 Changed 6 weeks ago by gkronber

  • Status changed from reviewing to readytorelease

comment:11 Changed 5 weeks ago by gkronber

  • Resolution set to done
  • Status changed from readytorelease to closed
Note: See TracTickets for help on using tickets.