Current project:
- ASP.NET 4.5.2
- MVC 5
I have what might be a bit of an odd question, but I am trying to improve the efficiency of my code.
Right now I am:
- pulling data out of the database into the model for that table, then
- stuffing it into a very limited “formatting model” (which contains a fraction of the fields of the database table, but where the data needs to be manipulated prior to display based on the user), which is then
- embedded in the view model.
I do not put DB intensive procedures into the view because the rendering speed depends on it; hence the “formatting model” which makes data rendering decisions based on what class of user (there are only 2) is accessing the messaging system.
I am also trying to generally follow the “fat model, thin controller” paradigm, but I am not going through extreme lengths to do so. More importantly, I am trying to push as much business logic to the model as possible, which I feel is more important than explicitly having a thin controller.
Step 1 is simple:
var UserId = User.Identity.GetUserId();
var UserRole = User.IsInRole("HEO");
IEnumerable<Messaging> data = await db.Messaging
.Where(x => x.MessageTo == UserId && (UserRole ? x.MsgSender.RecruiterProfile.ProfileActive : x.MsgSender.HEOProfile.ProfileActive))
.OrderByDescending(x => x.MessageSent)
.ToListAsync();
“Pull all messages where the user is the recipient, as well as limiting the list to where the Sender is still active on the system; order by sent date.”
Then I stuff the “formatting model”:
InboxViewModel model = new InboxViewModel(data, UserRole);
Which looks like this:
public class MessageListModel {
public Guid MessageId { get; set; }
public bool MessageRead { get; set; }
public string MessageName { get; set; }
public string MessageSubject { get; set; }
public string MessageSent { get; set; }
}
public class InboxViewModel {
public IEnumerable<MessageListModel> Inbox { get; set; }
public InboxViewModel() { }
public InboxViewModel(IEnumerable<Messaging> data, bool UserRole) {
Inbox = data.Select(x => new MessageListModel {
MessageId = x.MessageId
, MessageRead = (x.MessageRead != null ? true : false)
, MessageName = (UserRole ? (x.MsgSender.RecruiterProfile.CompanyName.Length > 16 ? x.MsgSender.RecruiterProfile.CompanyName.Substring(0, 16) + "…" : x.MsgSender.RecruiterProfile.CompanyName) : (x.MsgSender.HEOProfile.UsualName.Length > 32 ? x.MsgSender.HEOProfile.UsualName.Substring(0, 32) + "…" : x.MsgSender.HEOProfile.UsualName))
, MessageSubject = (x.MessageSubject.Length > 64 ? x.MessageSubject.Substring(0, 64) + "…" : x.MessageSubject)
, MessageSent = x.MessageSent.ToString("yyyy-MM-dd")
}).ToList();
}
}
As you can see, the MessageListModel is my formatting model, whereas the InboxViewModel is the view model that contains my formatting model.
And then I pass the entire enchilada onto the View:
return View(model);
My main question is, while it is correct to limit the initial list to only those messages where the user is the recipient (this dramatically reduces the dataset being returned), should I put subsequent filters, such as ensuring that the sender is still active on the system, further down toward the eventual view model? For example, should I be moving the second half of the .Where() clause from the controller:
var UserId = User.Identity.GetUserId();
var UserRole = User.IsInRole("HEO");
IEnumerable<Messaging> data = await db.Messaging
.Where(x => x.MessageTo == UserId) // Much shorter now!
.OrderByDescending(x => x.MessageSent)
.ToListAsync();
InboxViewModel model = new InboxViewModel(data, UserRole);
into the model?:
public class InboxViewModel {
public IEnumerable<MessageListModel> Inbox { get; set; }
public InboxViewModel() { }
public InboxViewModel(IEnumerable<Messaging> data, bool UserRole) {
Inbox = data
// Second half of .Where() is here:
.Where(x => (UserRole ? x.MsgSender.RecruiterProfile.ProfileActive : x.MsgSender.HEOProfile.ProfileActive))
.Select(x => new MessageListModel {
MessageId = x.MessageId
, MessageRead = (x.MessageRead != null ? true : false)
, MessageName = (UserRole ? (x.MsgSender.RecruiterProfile.CompanyName.Length > 16 ? x.MsgSender.RecruiterProfile.CompanyName.Substring(0, 16) + "…" : x.MsgSender.RecruiterProfile.CompanyName) : (x.MsgSender.HEOProfile.UsualName.Length > 32 ? x.MsgSender.HEOProfile.UsualName.Substring(0, 32) + "…" : x.MsgSender.HEOProfile.UsualName))
, MessageSubject = (x.MessageSubject.Length > 64 ? x.MessageSubject.Substring(0, 64) + "…" : x.MessageSubject)
, MessageSent = x.MessageSent.ToString("yyyy-MM-dd")
}).ToList();
}
}
I would think that a .Where() that filters the sender for if their account is active or not (something that would filter very few items out) is more efficient after a dataset has been drawn from the database (having it in the actual db query would slow the query down more than if done post-extraction). Whereas the first half of the .Where(), which limits the data set to just those messages meant for the user, is far more important to be in the controller because it dramatically reduces the data set being returned in the first place, and so makes the whole DB call that much more efficient.
In particular, the limiting of messages that get displayed to those where the sender is still active on the system is technically business logic, whereas the initial DB call for messages sent specifically to the current user is not. So IMHO the first half of the .Where() should be in the controller, whereas the second half of the .Where() should be in the model.
Suggestions?
