Avoid redundancy in multiple controllers in C# code

Question

I'm writing an application that inserts data into 6 different databases. Each database has its model and controller and views.

The problem is all of them have the same methods with a slight difference here and there but in general, they are all the same. This is redundancy and a lot of similar code in each controller.

Is there a way to reduce that? Would you suggest a design pattern to study to implement it here because I feel what I'm doing here is a bad programming practice?

Am I correct or just paranoid? Thanks

controller 1:

namespace Manage_account.Controllers
{
    public class HRLoanController : Controller
    {

        private Entities db = new Entities();

        string EmployeeLogin = System.Security.Principal.WindowsIdentity.GetCurrent().Name.Split('\\')[1];

        [HttpGet]
        public ActionResult Create()
        {
            ViewBag.DepID = new SelectList(db.Departments , "DepID", "Department1");
            return View();
        }

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult Create(AccountVM vM)
        {
            if (ModelState.IsValid)
            {
                #region Insert in account and lob of HRLoan database
                var chk_accountName = db.Accounts.Any(x => x.AccountName == vM.Accountex.Name);
                try
                {
                    if (!chk_accountName)
                    {
                        Account account = new Account();
                        account.AccountName = vM.Accountex.Name;
                        account.IsActive = true;
                        db.Accounts.Add(account);
                        db.SaveChanges();
                        TempData["success"] = "account saved successfully !";

                    }
                    else
                    {
                        TempData["warning"] = "Data already exists";
                    }

                    var Find_AccountId = db.Accounts.Where(x => x.AccountName == vM.Accountex.Name).FirstOrDefault();
                    var theID = Find_AccountId.AccountID;
                    var chk_lobName = db.LOBs.Where(x => x.AccountID == theID).Any(x => x.LOB1 == vM.LOBex.Name);
                    if (!chk_lobName)
                    {
                        LOB oB = new LOB();
                        oB.LOB1 = vM.LOBex.Name;
                        oB.LOBValue = vM.LOBex.Name;
                        oB.IsActive = true;
                        oB.AccountID = theID;
                        oB.DEPID = vM.Accountex.Dep_ID;
                        db.LOBs.Add(oB);
                        db.SaveChanges();
                        TempData["success"] = "lob saved successfully !";
                    }
                    else
                    {
                        TempData["warning"] = "Data already exists";
                    }

                }
                catch
                {
                    TempData["error"] = "Something went wrong :( .Please, try again later.";
                }

                #endregion
            }

            return RedirectToAction("Create");

        }

        public ActionResult CreateLob()
        {
            ViewBag.DepID = new SelectList(db.Departments, "DepID", "Department1");
            ViewBag.AccountID = new SelectList(db.Accounts.OrderBy(x=>x.AccountName), "AccountID" , "AccountName");
            return View();
        }

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult CreateLob(LOBex lOBex)
        {
            #region insert in Lob table in HRLoan database 
            if (ModelState.IsValid)
            {
                try
                {
                    var chk_LobName = db.LOBs.Where(x => x.AccountID == lOBex.Account_ID).Any(x => x.LOB1 == lOBex.Name);
                    if (!chk_LobName)
                    {
                        LOB oB = new LOB();
                        oB.LOB1 = lOBex.Name;
                        oB.LOBValue = lOBex.Name;
                        oB.IsActive = true;
                        oB.DEPID = lOBex.DEPID;
                        oB.AccountID = lOBex.Account_ID;
                        db.LOBs.Add(oB);
                        db.SaveChanges();
                        TempData["success"] = "Data saved to LOB Table !";
                    }
                    else
                    {
                        TempData["warning"] = "Data already exists !";
                    }

                }
                catch
                {
                    TempData["danger"] = "Something went wrong :(. Please, try again later.";
                }

            }
            return RedirectToAction("CreateLob");
            #endregion

        }

        [HttpGet]
        public ActionResult CreateUser()
        {
            ViewBag.RoleID = new SelectList(db.Roles.OrderBy(x => x.Role1), "RoleID", "Role1");
            return View();
        }

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult CreateUser(AddUser addUser)
        {
            #region Insert new user in Users table in HRLoan database 
            if (ModelState.IsValid)
            {

                var chk_user_existence = db.Users.Any(x => x.UserID == addUser.UserID); //check if user exists in db or not
                if (!chk_user_existence)
                {
                    User user = new User();
                    user.UserID = addUser.UserID;
                    user.UserName = addUser.UserName;
                    user.UserEmail = addUser.UserEmail;
                    user.IsActive = true;
                    user.InsertedOn = DateTime.Now;
                    user.UpdatedBy = EmployeeLogin;
                    user.GradeID = addUser.GradeID;
                    user.RoleID = addUser.RoleID;
                    db.Users.Add(user);
                    db.SaveChanges();
                }
                else
                {
                    TempData["warning"] = "This user already exists in Users table !";
                }
            }
            else
            {
                TempData["error"] = "Something went wrong :/ . Please, try again later!";
            }

            return RedirectToAction("CreateUser");
            #endregion
        }

        [HttpGet]
        public ActionResult CreateBoss_Department()
        {
            var get_Admin = db.Users.Where(x => x.RoleID == 1).Where(x=>x.IsActive==true).ToList();
            ViewBag.UserID = new SelectList(get_Admin.OrderBy(x=>x.UserName) , "UserID", "UserName");
            ViewBag.Indexs = new SelectList(db.LOBs , "Indexs", "LOB1");
            ViewBag.AccountID = new SelectList(db.Accounts.OrderBy(x=>x.AccountName) , "AccountID", "AccountName");
            return View();
        }

        #region populate user's ID with their specified names 

        //populate user's names
        public JsonResult GetUserName_toUserLogin(string usr)
        {
            List<User> getUser = new List<User>();
            getUser = (from r in db.Users where (r.UserID == usr) select r).ToList();
            List<SelectListItem> User_ID = new List<SelectListItem>();

            foreach (User name in getUser)
            {
                User_ID.Add(new SelectListItem { Text = name.UserID, Value = name.UserName.ToString() });
            }
            return Json(new SelectList(User_ID, "Value", "Text"));

        }
        #endregion

        #region Populates Lobs according to the selected account in the Create boss-department view 
        //populate Lobs
        public JsonResult GetLOB_toAccount(int acc)
        {
            List<LOB> getLob = new List<LOB>();
            getLob = (from r in db.LOBs where (r.AccountID == acc) select r).ToList();
            List<SelectListItem> Lobs = new List<SelectListItem>();

            foreach (LOB lob in getLob)
            {
                Lobs.Add(new SelectListItem {Text = lob.LOB1, Value = lob.Indexs.ToString() });
            }
            return Json(new SelectList(Lobs , "Value" , "Text"));
        }
        #endregion



        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult CreateBoss_Department(Boss_Dep boss_Dep)
        {
            #region Insert new record for admins only in Boss_Deapartment table in HRLoan database 




            if (ModelState.IsValid)
            {
                var Chk_UsrAccess = db.Boss_Department.Where(x => x.BossLogin == boss_Dep.BossLogin).Any(x => x.LOBIndexs == boss_Dep.LOBIndexs);
                if (!Chk_UsrAccess)
                {
                    Boss_Department boss_ = new Boss_Department();
                    boss_.BossLogin = boss_Dep.BossLogin;
                    boss_.LOBIndexs = boss_Dep.LOBIndexs;
                    boss_.IsActive = true;
                    boss_.ToMail = boss_Dep.To_mail;
                    boss_.InsertedOn = DateTime.Now;
                    boss_.UpdatedBy = EmployeeLogin;
                    db.Boss_Department.Add(boss_);
                    db.SaveChanges();
                    TempData["success"] = "Data saved successfully !";
                }
                else
                {
                    TempData["warning"] = "Data already exists in Boss_Department table !";
                }

            }
            else
            {
                TempData["error"] = "something went wrong :( !";
            }

            return RedirectToAction("CreateBoss_Department");
            #endregion

        }





        protected override void Dispose(bool disposing)
        {
            if (disposing)
            {
                db.Dispose();
            }
            base.Dispose(disposing);
        }
    }
}

controller 2:

namespace Manage_account.Controllers
{
    public class HrperonalController : Controller
    {
        private personal personal = new personal();

        string EmployeeLogin = System.Security.Principal.WindowsIdentity.GetCurrent().Name.Split('\\')[1];


        //GET: create
        [HttpGet]
        public ActionResult Create()
        {
            ViewBag.DepID = new SelectList(personal.Departments, "DepID", "Department");
            return View();
        }


        //POST: create
        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult Create(AccountVM vM)
        {
            if (ModelState.IsValid)
            {
                #region insert account in account table in HRPersonnal db
                var chk_accountName = personal.Accounts.Any(x => x.AccountName == vM.Accountex.Name);
                if (!chk_accountName)
                {
                    Accounts accounts = new Accounts();
                    accounts.AccountName = vM.Accountex.Name;
                    accounts.IsActive = true;
                    personal.Accounts.Add(accounts);
                    personal.SaveChanges();
                    TempData["success"] = "Account has been saved successfully :) !";
                }
                else
                {
                    TempData["warning"] = "account already exists";
                }
                #endregion

                #region insert Lob under that account in LOB table in HRPersonnal db
                var Added_account = personal.Accounts.Where(x=>x.AccountName == vM.Accountex.Name).FirstOrDefault().AccountID;
                var chk_lobName = personal.LOB.Where(x => x.DEPID == vM.LOBex.DEPID)
                    .Where(x => x.AccountID == Added_account).Any(x => x.LOB1 == vM.LOBex.Name);
                if (!chk_lobName)
                {
                    LOB lOB = new LOB();
                    lOB.LOB1 = vM.LOBex.Name;
                    lOB.LOBValue = vM.LOBex.Name;
                    lOB.AccountID = Added_account;
                    lOB.DEPID = vM.LOBex.DEPID;
                    lOB.IsActive = true;
                    personal.LOB.Add(lOB);
                    personal.SaveChanges();
                    TempData["success"] = "Lob has been saved successfully :) !";
                }
                else
                {
                    TempData["warning"] = "Lob already exists on that account";
                }
                #endregion
            }
            else
            {
                TempData["error"] = "Something went wrong.Please, try again later :/.";
            }
            return RedirectToAction("Create");
        }


        //GET: createLob
        [HttpGet]
        public ActionResult CreateLob()
        {
            ViewBag.AccountID = new SelectList(personal.Accounts, "AccountID", "AccountName");
            ViewBag.DepID = new SelectList(personal.Departments , "DepID", "Department");
            return View();
        }


        //POST: createLob
        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult CreateLob(LOBex oBex)
        {
            #region insert Lob in LOB table in HRPersonnal db
            if (ModelState.IsValid)
            {
                var chk_Lob = personal.LOB.Where(x => x.DEPID == oBex.DEPID).Where(x => x.AccountID == oBex.Account_ID).Any(x=>x.LOB1 == oBex.Name);
                if (!chk_Lob)
                {
                    LOB lOB = new LOB();
                    lOB.LOB1 = oBex.Name;
                    lOB.LOBValue = oBex.Name;
                    lOB.DEPID = oBex.DEPID;
                    lOB.AccountID = oBex.Account_ID;
                    lOB.IsActive = true;
                    personal.LOB.Add(lOB);
                    personal.SaveChanges();
                    TempData["success"] = "Lob has been saved successfully :) ! ";
                }
                else
                {
                    TempData["warning"] = "Lob already exists !";
                }

            }
            #endregion
            else
            {
                TempData["error"] = "Something went wrong.Please, try again later :/ !";
            }
            return RedirectToAction("CreateLob");
        }


        //GET: createuser
        [HttpGet]
        public ActionResult CreateUser()
        {
            ViewBag.RoleID = new SelectList(personal.Roles , "RoleID", "Role");
            ViewBag.DepID = new SelectList(personal.Departments, "DepID", "Department");
            return View();
        }


        //POST: createuser
        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult CreateUser(PersonnalUser user)
        {
            if (ModelState.IsValid)
            {
                var chk_user = personal.Users.Any(x => x.UserID == user.usrID);
                if (!chk_user)
                {
                    Users usr = new Users();
                    usr.UserID = user.usrID;
                    usr.UserName = user.Name;
                    usr.RoleID = user.Roleid;
                    usr.IsActive = true;
                    usr.GradeID = user.Gradeid;
                    usr.InsertedBy = EmployeeLogin;
                    usr.InsertedOn = DateTime.Now;
                    usr.Email = user.Email;
                    usr.PositionName = user.PositionName;
                    usr.DepartmentName = user.DepartmentName;
                    personal.Users.Add(usr);
                    personal.SaveChanges();
                    TempData["success"] = "User has been saved successfully :) !";
                }
                else
                {
                    TempData["warning"] = "User already exists !";
                }
            }

            return RedirectToAction("CreateUser");
        }


        //GET: create boss-department
        [HttpGet]
        public ActionResult CreateBoss_Dep()
        {
            var get_this_Admin = personal.Users.Where(x => x.RoleID == 1).Where(x => x.IsActive == true).ToList();
            ViewBag.UserID = new SelectList(get_this_Admin.OrderBy(x => x.UserName), "UserID", "UserName");
            ViewBag.Indexs = new SelectList(personal.LOB, "Indexs", "LOB1");
            ViewBag.AccountID = new SelectList(personal.Accounts, "AccountID", "AccountName");
            return View();
        }


        #region populate user's ID with their specified names 

        //populate user's names
        public JsonResult GetUserName_toUserLogin(string usr)
        {
            List<Users> getUser = new List<Users>();
            getUser = (from r in personal.Users where (r.UserID == usr) select r).ToList();
            List<SelectListItem> User_ID = new List<SelectListItem>();

            foreach (Users name in getUser)
            {
                User_ID.Add(new SelectListItem { Text = name.UserID, Value = name.UserName.ToString() });
            }
            return Json(new SelectList(User_ID, "Value", "Text"));

        }
        #endregion

        #region Populates Lobs according to the selected account in the Create boss-department view 
        //populate Lobs
        public JsonResult GetLOB_toAccount(int acc)
        {
            List<LOB> getLob = new List<LOB>();
            getLob = (from r in personal.LOB where (r.AccountID == acc) select r).ToList();
            List<SelectListItem> Lobs = new List<SelectListItem>();

            foreach (LOB lob in getLob)
            {
                Lobs.Add(new SelectListItem { Text = lob.LOB1, Value = lob.Indexs.ToString() });
            }
            return Json(new SelectList(Lobs, "Value", "Text"));
        }
        #endregion


        //POST: create boss-department
        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult CreateBoss_Dep(Boss_Dep boss_Dep)
        {
            #region Insert new record for admins only in Boss_Deapartment table in HRLoan database 

            if (ModelState.IsValid)
            {
                var Chk_UsrAccess = personal.Boss_Department.Where(x => x.BossLogin == boss_Dep.BossLogin).Any(x => x.LOBIndexs == boss_Dep.LOBIndexs);
                if (!Chk_UsrAccess)
                {
                    Boss_Department boss_Department = new Boss_Department();
                    boss_Department.BossLogin = boss_Dep.BossLogin;
                    boss_Department.LOBIndexs = boss_Dep.LOBIndexs;
                    boss_Department.IsActive = true;
                    boss_Department.ToMail = boss_Dep.To_mail;
                    boss_Department.InsertedOn = DateTime.Now;
                    boss_Department.UpdatedBy = EmployeeLogin;
                    personal.Boss_Department.Add(boss_Department);
                    personal.SaveChanges();
                    TempData["success"] = "Data saved successfully !";
                }
                else
                {
                    TempData["warning"] = "Data already exists in Boss_Department table !";
                }

            }
            else
            {
                TempData["error"] = "something went wrong :( !";
            }

            return RedirectToAction("CreateBoss_Dep");

            #endregion

        }

    }
}

Answer 1

Congratulations for spotting the repetitive nature of your code. That's a good start.

On the flip side, you have a long way to go, in that, there's a lot you'll need to learn.

Upon a study of your code structure, it looks like this:

(1) Controller Class
-- (2) HTTP GET METHOD / ACTION 
-- (3) HTTP POST METHOD / ACTION
-- -- (4) If the model state is valid
-- -- -- (5) If the object does not already exist in the database
-- -- -- -- (6) Do the thing. In your case, create a record in the database and save it.
-- -- -- -- (7) While doing it, if there's an error, write it to TempData.
-- -- -- (8) Else if the object already exists in the database
-- -- -- -- (9) Write an error to the temp data
-- -- -- (10) End of check if the object exists in the database
-- -- (11) Else if the model state is not valid
-- -- -- (12) Write an error to TempData
-- -- (13) End of check if model state is valid or not
-- -- (14) If while doing the whole method thing, there's an error, write it to TempData
-- (15) End of method.

Fortunately, this isn't a new problem at all, or even a difficult one to crack. It's basically the meat and potatoes of all simple, CRUD applications.

The problem is shouting loud and clear, and you have an inkling that something's wrong. Here's the problem.

All is mixed up. In sophisticated language, you're mixing concerns.

So, you need to separate out your concerns.

This is a basic design principle rather than a specific design pattern.

Separation of Concerns

You have code that sits close to the UI, i.e. your controller actions, your business or model objects, i.e. classes such as User, Account, etc. your utility classes such as methods that convert stuff to JSON and back, all in one place.

Besides, you've made even your utility methods that do the JSON and back conversion from your business objects into public methods, making them callable as actions on your controller.

All these mistakes are in violation of good design principles.

You need to separate out these, so that your code reads more like two or three lines like so:

public void PostAction(ModelObject model)
{
  if (!ModelState.IsValid)
  {
    // Some error handling strategy common to the whole
    // application that sits well with the kind of interface
    // this application has. Are you making a website or is this
    // a web API?
  }

  // You're here because the model state is valid

  // Get the repository object somehow:
  // (a) Either through dependency injection in the controller's constructor, and
  //     when you get the dependency, get it in the lowest / basest possible interface
  //     so that it is polymorphic.
  // (b) Or as a service location strategy, which means new it up here

  repositoryObject.doTheThing();
}

public class MyCentralErrorHandler : HandleErrorAttribute
{
  ...
}

In separating them, I noticed I applied a few patterns:

1. Dependency Injection
2. Service Location
3. Repository

I also applied the SOLID principle of interface seggregation. That is, I got my dependency using a polymorphic compile-time type like so:

public class MyController : Controller
{
  public MyController(IRepository repositoryObject) : base()
  {
  }
}

So, to answer the other aspect of your question about what design pattern to apply, I'd say you're still falling in the same trap that all rookies fall. It's not applying a design pattern you need to learn at this stage. But you need to code more.

Code a lot for practice until your own intuition develops to a stage where, just like you spotted that there was something messy about your code, your own intuition clearly and unequivocally tells you boundaries of what needs to go where, and you begin to work on your intuition and at the end of following its instruction, you realize, "Hey, what I did just now, it's called such and such a design pattern. Viola! I never did it consciously."

So, in a nutshell, study them all. Learn all the design patterns for now. Learn them thoroughly and give it a few years to roll in your head. And then throw them all out of your head when you're writing code. You're not supposed to be super-imposing design patterns on your code. They're just supposed to come out of you like poop does. And believe me, it'll happen. All you need is just more years of practice.

Meanwhile, write plenty of code. Plenty of code.

Another matter-of-fact answer to your question, as I already stated above is, you need to apply these things:

1. The design principle of separation of concerns;
2. The repository pattern, which is itself a manifestation of the principle of separation of concerns;
3. Service Location
4. Dependency Injection
5. The SOLID principle of programming to an interface rather than to a concrete implementation
6. A proper error / exception handling strategy. This is a lot of thinking that's due here. You need to ask yourself a lot of questions about what kind of an interface your application has and what do you intend to do at each stage. It's not something I can write here and be done with. You need a mentor or plenty of years of practice.

One more thing I must bring to your attention is that design patterns overlap. So, there are a few more obvious design patterns that actually also go by more than one name that are already applied in the solution I highlighted above. They're basically different names for the same thing. So, these are nebulous ideas that are demarkated on the basis of their function and even the context.

For e.g just the principle of programming to an interface can also be referred to as interface seggregation though the term interface seggregation has another, wider connotation in the context of system design. The same thing can also be noticed to be polymorphism at play. And I am sure there's another pattern name that can be ascribed to the same common-sensical, obvious solution.