# Decoupling of validation and data access in ASP.NET

I've been asigned to a project no dev has touched in a long time. It's an ASP.NET MVC 4 application.
It appeared to be well coded, but had no testing and the database migration folder is not present.
For further development I'm given time for minimal testing to ensure that my changes won't break the code.
When writing a Unit Test for a controller I noticed, that the code is not testable, because not only are results returned, but there is a lot of DB magic going on.
Some sample:

[HandleError]
public class AccountController : Controller
{
private readonly ILog _logger = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);

public IFormsAuthenticationService FormsService { get; set; }
public IMembershipService MembershipService { get; set; }

protected override void Initialize(RequestContext requestContext)
{
// ... Init code
}

[HttpPost]
public ActionResult LogOn(LogOnModel model, string returnUrl)
{
{
if (ModelState.IsValid)
{
var dbEntities = new PortalEntity();

var userId = (from us in dbEntities.aspnet_Users
select us.UserId).FirstOrDefault<Guid>();

var userprojects = (from up in dbEntities.T_USER_PROJECT
where up.UPR_USER_ID.Equals(userId)
select up);

//NEU alle Benutzer können sich anmelden
var projectId = (from u in dbEntities.T_PROJECT
join v in userprojects on u.PRO_ID equals v.UPR_PROJECT_ID
where u.PRO_IS_ACTIVE
select u.PRO_ID).FirstOrDefault();
var loginAllowed = dbEntities.T_PROJECT.Any(x => x.PRO_ID.Equals(projectId) && x.PRO_STARTED);

var userRole = dbEntities.aspnet_Roles.Single(x => x.RoleName.Equals("Administrators")).RoleId;
var isUserAdmin = dbEntities.aspnet_Users.Where(x => x.aspnet_Roles.Any(y => y.RoleId.Equals(userRole))).Any(x => x.UserId.Equals(userId));

{
_logger.Debug("User " + model.UserName + " existiert und möchte sich nun einloggen");

return RedirectToAction("Edit", "Stage1");
}
{
errMsg = ViewResources.ErrorNoEntryAllowed;
}
}

// If we got this far, something failed, redisplay form
return View(model);
}
}
}


I abstracted a class AccountService that handels all db access:

private class AccountService {
private PortalEntity dbEntities = new PortalEntity();

public Guid GetUserId(LogOnModel model) {
Guid userId = (from us in dbEntities.aspnet_Users
select us.UserId).FirstOrDefault<Guid>();
return userId;
}

public IQueryable<T_USER_PROJECT> GetProjectsForUserId(Guid userId) {
IQueryable<T_USER_PROJECT> userprojects = (from up in dbEntities.T_USER_PROJECT
where up.UPR_USER_ID.Equals(userId)
select up);
return userprojects;
}

public int GetProjectId(IQueryable<T_USER_PROJECT> userprojects) {
int projectId = (from u in dbEntities.T_PROJECT
join v in userprojects on u.PRO_ID equals v.UPR_PROJECT_ID
where u.PRO_IS_ACTIVE
select u.PRO_ID).FirstOrDefault();
return projectId;
}

bool loginAllowed = dbEntities.T_PROJECT.Any(x => x.PRO_ID.Equals(projectId) && x.PRO_STARTED);
}

public Guid GetUserRoleGuid() {
Guid userRole = dbEntities.aspnet_Roles.Single(x => x.RoleName.Equals("Administrators")).RoleId;
return userRole;
}

public bool IsUserAdmin(Guid userRole, Guid userId) {
bool isUserAdmin = dbEntities.aspnet_Users.Where(x => x.aspnet_Roles.Any(y => y.RoleId.Equals(userRole))).Any(x => x.UserId.Equals(userId));
}
}


Resulting in the new LogOn method:

[HttpPost]
public ActionResult LogOn(LogOnModel model, string returnUrl)
{
AccountService service = new AccountService();
{
if (ModelState.IsValid)
{

var userId = service.GetUserId(model);

var userprojects = service.GetProjectsForUserId(userId);

var projectId = service.GetProjectId(userprojects);

var userRole = service.GetUserRoleGuid();

{
_logger.Debug("User " + model.UserName + " existiert und möchte sich nun einloggen");

return RedirectToAction("Edit", "Stage1");
}
{
errMsg = ViewResources.ErrorNoEntryAllowed;
}
}

// If we got this far, something failed, redisplay form
return View(model);
}
}


Issues I'm having:

• Languages english and german are mixed in both comments and logs (prefer eng > ger?)
• Magic strings (x.RoleName.Equals("Administrators")), what to do with those?
• I made AccountService a private class, should I define a base class or interface Service?
• Should I be putting all these services into a dir /services?
• Is this approach even viable / necessary?
• Do I have to DI a service in every method or one per controller?

Any feedback is appriciated.

I guess you'll find a lot of redundancy in the code since its been held by the controllers.

Your idea of making a service is a good idea, you may need to add different services, but at the end you'll have services that can be reused, avoiding code redundancy. This would help ease things more and more. So, continue in this approach. Interface or abstract, that depends on your implementation. Go with what you see best fits your needs. You can also define both if needed. And yes, defining them under a new namespace, would be better.

Do you need to do all that ? it depends. If the application will be continued and there is no new migrations. Then, I don't think it needs that, you might make changes whenever needed (like do one controller on each time you're requested to make changes on any of them). little by little, you'll have a full updated project.

However, if there is another migration process, then you need to reconsider your choices, see what needs your efforts and what's not.

For the magic strings, use Enum.

I forgot the first issue, for which language you should favor over the other, mostly English is your choice, but from a back experience, If there is some work guidelines for developers, then it should be mentioned in that guidelines, if not, then see what is the official communication language in your work (emails ..etc) and enforce it in the code as well. You can do your comments in both languages (each comment would hold English and translation to German). I usually use English even if the application comments are written in a different language, because :

1. English is an official language in most companies in my country.
2. I'm coding in English, why should I write comments in different language ? it would be an awkward code style ;).
3. It's a good way to practice your language and keep it rolling, specially if you're in a place where it's rarely used.