Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm building a ASP.NET MVC app and would like some feedback for my way to query/execute statements to the ORM.

I'm using Fluent NHibernate, and have mixed the UoW provided by NHibernate with a Command pattern.

My code:

// ICommand.cs (Core layer)

using NHibernate;

namespace MyDemon.Core.Commands
{
    public interface ICommand<out TResult>
    {
        TResult Execute(ISession session);
    }
}

// ISessionExtensions.cs (Core layer)

using NHibernate;

namespace MyDemon.Core.Commands
{
    public static class ISessionExtensions
    {
        public static TResult Execute<TResult>(this ISession session, ICommand<TResult> unitOfWork)
        {
            return unitOfWork.Execute(session);
        }
    }
}

// GetUserById.cs (Core layer)

using NHibernate;

namespace MyDemon.Core.Commands.User
{
    using Entities;

    public class GetUserById : ICommand<User>
    {
        public int UserId { get; set; }

        public GetUserById(int userId)
        {
            UserId = userId;
        }

        #region Implementation of IUnitOfWork<out User>

        public User Execute(ISession session)
        {
            return session.Get<User>(UserId);
        }

        #endregion
    }
}

// AccountController.cs (Web layer)

[AjaxOnly]
[Authorize]
public ActionResult Details(int id)
{
    User userToGet = _session.Execute(new GetUserById(id));

    if (userToGet == null)
    {
        return PartialView("Partials/UserNotFound");
    }

    DetailsUserViewModel userToViewModel = Mapper.Map<User, DetailsUserViewModel>(userToGet);

    return PartialView("Partials/Details", userToViewModel);
}

What do you think? A clever design, or just another "too-much code" approach?

share|improve this question
add comment

1 Answer

This seems to be a lot of code to say the same as this:

[AjaxOnly]
[Authorize]
public ActionResult Details(int id)
{
    User userToGet = _session.Get<User>(id);

    if (userToGet == null)
    {
        return PartialView("Partials/UserNotFound");
    }

    DetailsUserViewModel userToViewModel = Mapper.Map<User, DetailsUserViewModel>(userToGet);

    return PartialView("Partials/Details", userToViewModel);
}

What problem are you trying to solve here?

(or am I missing something?)

share|improve this answer
1  
I wouldn't go as raw as having the _session in the Controller (would have a DAO or Repository layer), but definetely this is the way to go. This is the way NH is used in web apps in .NET. Unless you have a (good) reason there is no motive to come up with something so "complicated" (not because the inherent complexity but just because it not the standard) to deal with NH + DB. –  tucaz Feb 4 '11 at 11:10
 
@tucaz Agree with you there. But the session was already in the controller, so I left it there rather than complicating the answer. –  pdr Feb 4 '11 at 17:27
 
@tucaz For what reason would you have a DAO or repo layer here? Isn't NHibernate enough of a DAO in this case? (I'm new to NHibernate too so I'm curious about these things). –  Jamie Dixon May 29 '12 at 19:17
 
@JamieDixon, it is, and I'm probably should not add another layer of indirection unless needed for some other reason, but I like to think that adding it allows me to make sure that the usage of NHibernate does not get out of hand and is spreaded where it should not be and also allows me to change strategies easily later as well encapsulating the lifecicle of the ISession. –  tucaz May 30 '12 at 21:13
 
Thanks for replying @tucaz :-) –  Jamie Dixon May 31 '12 at 7:04
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.