views:

45

answers:

1

What is considered good practice for querying entites from a database using a simple string as a query parameters. Who should implement the method that "converts" the string to an Entity, and who should call this method? The repository, the entity, or some other object, like the controller in an MVC application?

For a more concrete example I have an ASP.NET MVC 2 application, using Sharp Architecture's IRepository and SharpModelBinder. The latter is configured so, that if a user submits a form, that has an ID in the place of an object, then it will query the appropriate repository for an object that has the same ID. I'd like to add to this example to not only be able to use an ID, but a custom string as well. Of course the method that gets the concrete ID from a generic query string is different for each Repository/Entity. How should I do this?

To be even more concrete. Suppose I have two entities:

class Doctor : Entity {
    public virtual Patient ActualPatient { get; set; };
    // some other properties, constructor, etc.
}

class Patient : Entity {
    // some properties, constructor, etc.
}

Now, if I have an editor for the Doctor entity (like a form), I could enter an ID in place of the Doctor.ActualPatient, and the model binder would take care of assigning the appropriate object. But what if the user enters a custom string (like the name of the Patient)?

Here are my current options:

  1. The Controller should take care of this custom string. So after the ModelBinder fails to bind the Patient to the Doctor (=returns null for the Patient), the controller takes the string and using some algorythm it tries to get a Patient that is represented by that string (or it fails too)
  2. The Controller takes care of this, but calls a method that is defined somewhere else not in the Controller class (look at next question)
  3. The ModelBinder should be configured to use this external method
  4. You should never do something like this, this is always considered bad practice
  5. Something else (suggestions are welcome)

And here are my suggestions on where this method should be implemented:

  1. The Repository should have a GetEntityFromAString(string) method, and the subclasses for this Repository should redefine this method.
  2. The Entity should have an Attribute that if present takes care of this conversion, and the controller/modelbinder/etc checks for this attribute and calls a method like GetEntityFromAStringUsingThisRepository(string,repository)
  3. I've already said that this is a bad practice
  4. Something else

Although the concrete example is for Sharp Architecture/ASP.NET MVC2/NHibernate feel free to answer it generally.

Thanks for the answers.

A: 

I'd manage this explicitly from the controller.

If you bind by ID you can guarantee that you'll either get the instance or null if not found. But binding by an arbitrary property is not so simple: what if there are multiple matches? Should the binder throw an exception? Should it only return the first result? As you can see, it's easy to run into unexpected results, thus violating the principle of least astonishment.

If you really know what you're doing and you want this, I'd create a custom binder with the associated attribute, which would have the name of the property to use to find the entity, e.g.

class Patient {
  int Id {get;set;}
  string PatientName {get;set;}
}
...
ActionResult SomeAction([MyCustomBinder("PatientName")] Patient patient) {...}
Mauricio Scheffer
Yes, but by adding the logic for example to the Repository class you are actually saying, that the Repository should know how to handle the situation. Or are you saying that a repository shouldn't contain such logic? Adding things like which property to search for is not good either, because more than one property might be checked. The logic to how to "convert" from string to entity can be much-much more comples, which might actually need to query the database a lot.
SztupY
@SztupY: I'm not sure I understand what you're saying... Of course the actual fetching is delegated to the Repository. If you need something more complex than that then you *definitely* don't want to do it in the binder, like I said at the beginning of my answer.
Mauricio Scheffer
Yes, but the binder is actually calling the repository, to fetch the ID. If it can use the "getID" method of the repository why can't it use another method, like the "GetEntityFromName" too? Why is delegating this call to the controller is a bad idea, but keeping the ID fetching in the modelbinder a good one?
SztupY
IMO doing it in the controller is more explicit than doing it in a binder. A binder is a more reusable piece of code, but like I said there are many possible behaviors for this lookup, so you'll eventually end up with an unpleasant surprise when it does not behave as you thought it would.
Mauricio Scheffer