How can I clean my controller actions and reference up in my ASP.NET CORE 5 application?-CodePudding

The front end of my web application has a form that the user can submit which is tied to a model called Requirement. Because there is one single form used for both the creation and editing of data I need a way to handle this process in my controller.

In an attempt to make things easier I took the incoming Id from the model as a point of reference and worked from there.

Process_Requirement

This is the action called by the form when submit is hit. It looks at the incoming Id with the condition that if it's 0 it's new and it it's above 0 it already exists.

public async Task<JsonResult> Process_Requirement(Requirement model)
{
    //It's new
    if (model.Id == 0) {
        var create = await Create_Requirement(model);
        return Json(new { model = create });        
    }
    //It exists
    else {
        var update = await Update_Requirement(model);
        return Json(new { model = update });
    }
}

This parent action references the two following actions depending on which condition is met.

public async Task<JsonResult> Create_Requirement(Requirement model) 
{
    try
    {
        var requirement = await _requirement.CreateAsync(model);
        return Json(new { success = true, data = requirement.Id, operation = "create" });                
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
    }
    return Json(new { success = false, data = model });
}

public async Task<JsonResult> Update_Requirement(Requirement model)
{
    try
    {
        var requirement = await _requirement.UpdateAsync(model);
        return Json(new { success = true, data = requirement.Id, operation = "update" });
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
    }
    return Json(new { success = false, data = model });
}

So, what's the problem here? Well, I just think that my approach here is not quite as clean as it could be, clunky and also perhaps, not quite right. YES it works! but I want to make sure I'm producing slick code that works well and thought I would post it here and see what people's thoughts were and how I could do this better?

CodePudding user response：

You are violating the SRP principle

SRP:

The single-responsibility principle (SRP) is a computer-programming principle that states that every module, class or function in a computer program should have responsibility over a single part of that program's functionality, and it should encapsulate that part ...

You should have two actions on the backend, one to create and one to edit And you have to create two states on the front End side Using the same form. If your state is in create mode, create api should be called And if it is in edit mode, update api should be called.

 public async Task<JsonResult> Create_Process_Requirement(Requirement model)
    {
        var create = await Create_Requirement(model);
        return Json(new { model = create });
    }

   public async Task<JsonResult> Update_Process_Requirement(Requirement model)
    {
        var update = await Update_Requirement(model);
        return Json(new { model = update });
    }

CodePudding user response：

Your Update_Requirement and Create_Requirement literally are doing the same thing except calling two different methods. You can merge these two methods into a single one.

    /// <summary>
    /// Create or Update Requirement object.
    /// </summary>
    /// <param name="model"></param>
    /// <returns></returns>
    private async Task<JsonResult> CreateOrUpdate_Requirement(Requirement model)
    {
        try
        {
            string operationValue = string.Empty;
            int requirementId;
            if(model.Id == 0)
            {
                operationValue = "create";
                requirementId = (await _requirement.CreateAsync(model)).Id;
            }
            else
            {
                operationValue = "update";
                requirementId = (await _requirement.UpdateAsync(model)).Id;
            }

            return Json(new { success = true, data = requirementId, operation = operationValue });
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
        return Json(new { success = false, data = model });
    }

I am not sure what your await _requirement.CreateAsync(model) and _requirement.UpdateAsync(model) actually do and what type of value they return. You can merge them too. And you can change your main method to:

public async Task<JsonResult> Process_Requirement(Requirement model)
{
    return await CreateOrUpdate_Requirement(model);
}