Reputation: 4753
I am working on an asp.net mvc application.
This is the approach that i am following right through the application.
Here is my model:
public class EmployeeModel
{
//Properties
// Constructors
// Methods
}
My View: (strongly typed with model Properties) for example : Some Edit View
My Controller:
[httppost]
public void save(Employeemodel m) // ajax call that gets updated model details
{
m.update() // making database call from controller action
}
Are there any serious design issues with my approach of MVC. May be i mis understood MVC?
Is it appropriate to call model methods(m.update() ) in controller action?
Model contains methods that manipulate data ? IS it correct?
please help/suggest the correct approach to follow in MVC
Upvotes: 0
Views: 19303
Reputation: 5176
I would recommend you to have separate layer to save data to the database, because its not just MVC that one should follow.
So its not about MVC rather the best practice one should follow to architect an application.
Further we have several options that we must consider before planning architecture of an application.
So, it depends on the level of abstraction you want, but as far as MVC concern I would recommend you to have extra layer of separation of ViewModel MVVM.
Its better to ask these type of questions in stachexchange chat.
Example
ViewModel
public class User
{
public Guid Id {get;set;}
public string Name {get;set;}
}
Model
public class UserModel
{
public void AddUser(User user)
{
//add to the database
}
public void UpdateUser(User user)
{
//update in the database
}
}
Controller
[HttpPost]
public ActionResult UpdateUser(User user)
{
UserModel user = new UserModel();
user.UpdateUser(user);
}
Upvotes: 2
Reputation: 1938
Your logic is fine. But as K D said, you should have viewmodel that represents view. I would suggest you write your methods in view model since you will pass only model entity to database or anywhere else.
public class Employee
{
//Properties
}
public class EmployeeViewmodel
{
// Employee model object
//Constructors
//Methods
}
return view("View name", EmployeeViewModel);
So you can update model, pass Employee alone to Database via OR/M ..This is basic flexible approach. You can have a utility model class which contains common data retrival like City, state, gender .. so you wont mix entity with other models.
Upvotes: 0
Reputation: 5989
In general practice you should now follow this methodology. Although this is the default MVC behavior of accepting entire model as an argument you should have a middle layer called as DTO(Data Transfer Object) or ViewModel which represents UI. And after accepting and validating View model you can transform it to your main business entity. Offcouse it depends how you have written code in your update method but the main hack is this case is that.... any body can pass any known property value to this method and can hack your system. for example suppose you have following values in your Employeemodel { Id, Name, SecurityCode, ... }
and your edit screen just have Name input to update it. Any body can add extra html for SecurityCode and can add bad value to it :) I hope i didn't confused you. For start try to implement Repository pattern MVC... Google it and you'll find the basic usage of it. :)
Cheers
Upvotes: 1