2
Vote

PerceptronModel.Generate fails in an unexpected way when given an empty collection.

description

I expected the following instructions to either fail silently, or throw an argument exception.
var model = new PerceptronModel<Student>();
var predictor = model.Generate(new List<Student>());  // notice empty set
Instead, an index out of range exception is thrown from inside the Matrix class. Consider adding guard clause or Code Contracts. For example:
using System.Diagnostics.Contracts;
using System.Linq;
...
public IPredict<T> Generate(IEnumerable<T> examples)
    {
    Contract.Requires(examples != null);
    Contract.Requires(examples.Count() > 0);

comments

SethJuarez wrote Aug 20, 2010 at 6:47 PM

Oops! Rookie mistake. I will fix that. It also looks like you are using code contracts. Would it be worth the time to look into those?

spivey wrote Aug 20, 2010 at 7:49 PM

Contracts have a number of advantages that make them worth considering. The most notable advantage is that they enable the use of Microsoft's static checker, which identifies any potential violations at compile time. Using the static checker, callers of the Generate function would receive a warning if it was possible that they could pass a null or empty set.

Advantages
  • Contracts can be applied to the IModel interface, so exceptional behavior would be standard across all implementations of the interface.
  • Contracts provide explicit documentation for users of the library.
  • Contracts can be stated once, and then verified at runtime, verified by the static checker, read by humans, and parsed by documentation generators like SandCastle.
Contracts are cumbersome to add, so the benefits come at a cost. The QuickGraph (http://quickgraph.codeplex.com) library has been using them for some time. I wonder if they've benefited from them.