patterncsharpMinor
Keeping data processing out of controllers
Viewed 0 times
keepingcontrollersprocessingdataout
Problem
I would like to start by mentioning that I am LAMP stack guy who happens to be making my first ever .NET C# web app and I'm seeking general advises for best practices and to see how more experienced people would have done it.
I am using code first development pattern with latest Entity Framework . I have a
I believe that smaller methods are the better. So, I am trying to keep controllers as small as possible. I made classes which handles all interactions with database and keeps business logic out of Models and Controllers. First class is
( "AppDB" is inheriting from DbContext and holds all DbSets)
```
public abstract class MasterRep
{
protected AppDB _db { get; set; }
protected AppUser _currentUser { get; set; }
protected int _companyId { get; set; }
protected string _applicationUserId { get; set; }
public MasterRep(string applicationUserId, AppDB db = null)
{
_db = db ?? new AppDB ();
_currentUser = GetUserByApplicationId(applicationUserId);
_companyId = _currentUser.Company.Id;
_appl
I am using code first development pattern with latest Entity Framework . I have a
AppUser model. The user could be on of few types, one of them is employee.public class AppUser{
public int Id { get; set; }
public UserType Type { get; set; }
public Title Title { get; set; }
public string FirstName { get; set; }
public string MiddleName { get; set; }
public string LastName { get; set; }
public virtual AddressDetails AddressDetails { get; set; }
public virtual Company Company { get; set; }
..................................................
}
public class AddressDetails
{
public int Id { get; set; }
public string AddressLine { get; set; }
public string City { get; set; }
public string County { get; set; }
public string PostCode { get; set; }
public float Latitude { get; set; }
public float Longtitude { get; set; }
[JsonIgnore]
public virtual AppUser User { get; set; }
}I believe that smaller methods are the better. So, I am trying to keep controllers as small as possible. I made classes which handles all interactions with database and keeps business logic out of Models and Controllers. First class is
abstract MasterRepository ,which has logic for Database Update/Add/SaveChanges. All other "repositories" inherit it.( "AppDB" is inheriting from DbContext and holds all DbSets)
```
public abstract class MasterRep
{
protected AppDB _db { get; set; }
protected AppUser _currentUser { get; set; }
protected int _companyId { get; set; }
protected string _applicationUserId { get; set; }
public MasterRep(string applicationUserId, AppDB db = null)
{
_db = db ?? new AppDB ();
_currentUser = GetUserByApplicationId(applicationUserId);
_companyId = _currentUser.Company.Id;
_appl
Solution
Your brace style is inconsistent. Allman style is usually applied in C#.
Underscores are usually only used for fields; properties should be PascalCase:
Don't abbreviate unnecessarily: I don't know what
Is this a field or a property:
The
Avoid "negative checks" like you do here:
Invert that logic and things become a lot clearer:
Also, since you call
public class AppUser{
// snipped
}
public class AddressDetails
{
// snipped
}Longtitude is a spelling error, it should be Longitude.Underscores are usually only used for fields; properties should be PascalCase:
protected AppDB _db { get; set; }
protected AppUser _currentUser { get; set; }Don't abbreviate unnecessarily: I don't know what
Rep is, I do know what Repository is.public class EmployeeRep : UserRepIs this a field or a property:
private EmployeeRep _employeeRep { get; set; }The
{ get; set; } says "property", but the private and the underscore at the start of the name says "field".Avoid "negative checks" like you do here:
_employeeRep = (User.Identity.GetUserId() != null)
? new EmployeeRep(User.Identity.GetUserId())
: null;Invert that logic and things become a lot clearer:
_employeeRep = (User.Identity.GetUserId() == null)
? null
: new EmployeeRep(User.Identity.GetUserId());Also, since you call
User.Identity.GetUserId() twice, consider calling it once, storing the result in a variable and then using that variable.Code Snippets
public class AppUser{
// snipped
}
public class AddressDetails
{
// snipped
}protected AppDB _db { get; set; }
protected AppUser _currentUser { get; set; }public class EmployeeRep : UserRepprivate EmployeeRep _employeeRep { get; set; }_employeeRep = (User.Identity.GetUserId() != null)
? new EmployeeRep(User.Identity.GetUserId())
: null;Context
StackExchange Code Review Q#113388, answer score: 5
Revisions (0)
No revisions yet.