views:

65

answers:

3

Hello there, I got a generic repository - should the repository be able to throw exceptions or should I keep it dumb? If I chose to throw exceptions from it, should the service layer then catch it and throw an exception with a more friendly message before it goes to the Controller?

  • Pos1: Repository (Throw) => Service(Catch, Throw new) => Controller(Catch)
  • Pos2: Repository => Service(Throw) => Controller(Catch)
+1  A: 

Generally speaking, you should simply return null from your repository if your query returns no data. You can then test for the null in the repository, if you wish, before sending the data to your view.

public ActionResult Details(int id) {

    Dinner dinner = dinnerRepository.FindDinner(id);

    if (dinner == null)
        return View("NotFound");
    else
        return View("Details", dinner);
}

http://nerddinnerbook.s3.amazonaws.com/Part4.htm

For edits I would let your Data Access Layer or your repository throw an exception, and catch it in the controller, like this:

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Edit(int id, FormCollection formValues) {

    Dinner dinner = dinnerRepository.GetDinner(id);

    try {

        UpdateModel(dinner);

        dinnerRepository.Save();

        // Success!
        return RedirectToAction("Details", new { id=dinner.DinnerID });
    }
    catch {

        // Old-school data validation from ASP.NET MVC 1.0
        foreach (var issue in dinner.GetRuleViolations()) {
            ModelState.AddModelError(issue.PropertyName, issue.ErrorMessage);
        }

        // Return original view, so user can correct errors.
        return View(dinner);
    }
}

http://nerddinnerbook.s3.amazonaws.com/Part5.htm

Robert Harvey
What about if I need to update an record.. then I would have to throw an exception in either my Repository or Service?
ebb
+2  A: 

Should the repository be able to throw exceptions or should I keep it dumb?

Yes - the repository should be able to throw exceptions. Keeping something 'dumb' doesn't mean it's not comletely self-aware :)

The caveat that 'exceptions should be exceptional' still applies - you might find this "Creating More Exceptional Exceptions" article of interest and it's also of relevance to your question.

If I chose to throw exceptions from it, should the service layer then catch it and throw an exception with a more friendly message before it goes to the Controller?

Generally, I've not re-thrown exceptions although there's merit in doing that. A pragmatic half-way-house would be to do that for exception types which you want to handle gracefully for some reason.

If you're using something like the Ms Ent Libs Application Logging Block you can set a policy (via config) that allows you to control what happens when exceptions occur - re-throwing them or otherwise; so that would be a useful approach to stop hard-coding yourself into a specific result.

Also this might be of interest: http://stackoverflow.com/questions/180937/are-exceptions-really-for-exceptional-errors

Adrian K
A: 

Definitely Option 1.

I would also replace the term "dumb" with "separation of concerns" in your thinking. There is no reason for a Repository to be dumb. It has a job to do and that will involve exceptions.

It will also involve throwing them for two reasons:

  1. To package a real error that has happened for the consuming code.

  2. To throw an exception under given conditions that violate what you want this class to do. These conditions might not involve an exception thrown by the framework and can be solely related to the "intelligence" you want your Repository to have.

The Repository has to involve encapsulating ALL of this intelligence, leaving the calling code to simply need to know how to deal with a limited set of exceptions. Otherwise, your calling code needs to deal with, for example, the full panoply of LINQ exceptions, coupling it to a technology that should be the exclusive domain of the Repository.

So part of the Repositiory's intelligence has to be throwing a well known, but limited set of exceptions related to its specific purpose.

The same reasoning applies to the Service Layer, if you have one. It needs to deal with exceptions in exactly the same way: encapsulating the "intelligence" that is specific to its task. And again, the same happens with the controller. It should interpret the exceptions it receives from the Service Layer (if there is one) according to its own purposes and concerns.

So separation of concerns, but never dumb. Not even Mute: each layer needs to squeal when it has to.

awrigley