views:

198

answers:

8

Say you have an object which, for the sake of example, we will call the ScoreHotChicksEngine. And say that the ScoreHotChicksEngine's constructor is expecting to be passed an IDataReader containing property values pertaining to, apparantly, Scoring Hot Chicks for Lonely Geeks.

ScoreChicksEngine(IDataReader reader);

Ok, here's what I would like to gather input on...

As a developer would you find it more useful to assume that the reader must be read before being passed into the ScoreChicksEngine

IDataReader = command.ExecuteReader();
reader.Read();
ScoreChicksEngine SCE = new ScoreChicksEngine(reader);

or would you assume that the engine itself would call that function and possibly deal with the empty values?

IDataReader  = command.ExecuteReader();
ScoreChicksEngine SCE = new ScoreChicksEngine(reader);
if (SCE.HasReaderData()) doSomething();
+6  A: 

Shouldn't you be thinking of decoupling the data collection from the algorithm and use a bridging solution in between (an iterator adapter, say)? Just my $0.02.

The implication of such a design is that the algorithm is responsible for making calls to read the data on an as-required basis via the adapter. The adapter hides the collection and any particular facets thereof not germane to the problem being solved.

dirkgently
I absolutely agree @dirkgently. The ScoreChicksEngine should not care or even be aware that it's getting its data from an IDataReader. Decoupling is the way to go here.
Bob King
+3  A: 

I'd choose the first method. The second method violates single responsibility principle. I'd also declare the input parameter of the constructor as IDataRecord and not IDataReader. Basically the SCE class constructs itself based on a single record and doesn't care about a set of records out there.

Mehrdad Afshari
+1  A: 

Unless the ScoreChicksEngine has reason to assume the reader will be in a particular state, I'd have it do all the work itself. What if you forget to call Read() before initizalizing? There's no way to really check that, so just be sure by doing it in the SCE.

Chris Doggett
+1  A: 

Well, after some years of painful .Net-based experience, I would assume the first scenario. However, every time I typed that in, I'd wish I was typing the second.

Electrons_Ahoy
+1  A: 

I would have ScoreChicksEngine do this work. My reasons:

  1. Doesn't depend on the user knowing/remembering to do this.
  2. There may be cases where it is unecessary, such as ScoreChicksEngine deciding its configuration is not complete.
  3. If it something has to be done every time, why have the caller duplicate it everywhere?
  4. Makes it easier on the user, allowing more compact clode, such as:

    ScoreChicksEngine SCE = new ScoreChicksEngine(command.ExecuteReader());

Chris Thornhill
+1  A: 

I'd use the first approach. Maybe the reader returns more than one record, then you could do:

IDataReader = command.ExecuteReader();
while (reader.Read())
    list.Add(new ScoreChicksEngine(reader));

In fact, what I have done in the past is created a wrapper class around the data reader (with methods like GetInt32(name), GetString("name"), etc.

That way the ScoreChicksEngine does not have access to the reader's methods, but only to the methods of the wrapper class.

The above sample would then look somehow like this:

IDataReader = command.ExecuteReader();
while (reader.Read())
    list.Add(new ScoreChicksEngine(new MyDataReaderWrapper(reader)));
M4N
+1  A: 

Spontaneously the second approach makes more sense, but then it depends a bit on how the engine will treat the reader:

  • Will it read the current row using the reader, or
  • Will it iterate over all rows in the reader and make a list

If the first is the case, then I would expect the engine NOT to call the Read method, if the second case is true, I would assume that the engine is in the driver's seat and takes care of calling Read.

And this probably points towards thinking about decoupling as previously suggested.

Fredrik Mörk
+1  A: 

It depends on what your ScoreChicksEngine does to the reader. If the reader is reading a small amount of data, then passing in that data might be better:

IDataReader = command.ExecuteReader();
ObservableCollection<HotChicks> Wowzer = reader.Read();
ScoreChicksEngine SCE = new ScoreChicksEngine(Wowzer);

This could also be a lazy-loaded collection, and your SCE wouldn't care. Either way, I don't thing SCE should have to call anything on the reader before it can access the results. Either your IDataReader should have internal logic to call the read function when you first try to access the collection, or should call it in the constructor. Which is better depends on your circumstance. Either way, suggest you use an ObservableCollection though!

Jon Artus