locked
Efficiency How can I make my codes more efficient....? RRS feed

  • Question

  • User-507786106 posted

    Your help is needed, I have three types of ID's coming from different pages to the same Controller / Action.

    Each of the ID's  has an integer

    Variable names:  eId, ID, EmpID   <<--- any time the controller actions is called at least one of these IDs will have a values, but if they each have no value what should I do.

    Goal, need a condition that checks each of the values and if integer is greater than 0 done some work and if no number is found redirect .

    Can someone share expertise and help me be more .

    Tuesday, May 21, 2019 3:05 PM

Answers

  • User1120430333 posted


    Your help is needed, I have three types of ID's coming from different pages to the same Controller / Action.

    Maybe, you should consider method overloading for the ActionMethod, which is a seperation of methods with the same method name, but the selection is made by .NET based on the signature  to be used for the overloaded method.

    https://www.c-sharpcorner.com/UploadFile/0c1bb2/method-oveloading-and-overriding-C-Sharp/

     [Authorize]
            public ActionResult Edit(int id = 0)
            {
                return id == 0 ? null : View(_projectModel.Edit(id));
            }
    
            [Authorize]
            [HttpPost]
            public ActionResult Edit(ProjectViewModels.Project project, string submit)
            {
                if (submit == "Cancel") return RedirectToAction("Index");
    
                if (ModelState.IsValid && _modelHelper.IsEndDateLessThanStartDate(project, "Project"))
                    ModelState.AddModelError(String.Empty, "End Date cannot be less than Start Date.");
    
                if (!ModelState.IsValid) return View(_projectModel.PopulateSelectedList(project));
    
                var theproject = new ProjectViewModels.Project();
    
                theproject = project;
    
                theproject.ProjectType = Request.Form["ProjectType"];
    
                _projectModel.Edit(theproject, User.Identity.Name);
                return RedirectToAction("Index");
            }
    

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Tuesday, May 21, 2019 5:56 PM
  • User1520731567 posted

    Hi slimbunny,

    if ((studentId == null) && (eId == null) || (eId == string.empty))

    {

              return RedirectToAction(Index, Home)

    }

    How can I make this statement more efficient

    if ((StudentId != null) && ( update == "true"))

    {

        ID = StudentId

    }

    else if ((StudentId != null ) && (emp == "true"))

    {

          ID = StudentId

    }

    else if (eId != null)

    {

        ID = eId

    }

    else if (StudentId != null)

    {

        ID = StudentId

    }

    else

    {

       returnl......

    }

    If could,you could modify your database design to judge different scenes by one field.

    If you want to reduce the code, I suggest you could try to consider using ?: Operator.

    Best Regards.

    Yuki TAO

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, May 22, 2019 6:28 AM

All replies

  • User475983607 posted

    Your help is needed, I have three types of ID's coming from different pages to the same Controller / Action.

    IMHO, this is a poor design.  Create one action for each page which removes the need to create complex conditional logic and simplifies the code base.

    Tuesday, May 21, 2019 3:16 PM
  • User-507786106 posted

    this is what I have

    if ((studentId == null) && (eId == null) || (eId == string.empty))

    {

              return RedirectToAction(Index, Home)

    }

    How can I make this statement more efficient

    if ((StudentId != null) && ( update == "true"))

    {

        ID = StudentId

    }

    else if ((StudentId != null ) && (emp == "true"))

    {

          ID = StudentId

    }

    else if (eId != null)

    {

        ID = eId

    }

    else if (StudentId != null)

    {

        ID = StudentId

    }

    else

    {

       returnl......

    }

    Tuesday, May 21, 2019 4:40 PM
  • User753101303 posted

    Hi,

    A higher level view rather than actual code could help. For now it seems they will all end anyway into ID so why to have distinct names to start with ? Also the update flag seems to tell that maybe you always go to some general purpose action and have to use code to decide which exact operation is done (and on what ?) depending on parameters when you could just call directly an action that does right away what you want (as pointed by mgebhard) ???

    Tuesday, May 21, 2019 4:52 PM
  • User475983607 posted

    Try to avoid writing a lot of "if" conditions.  Code should not figure out what logic to run as it causes hard to maintain code and bugs.  Always tell the code what to do.  In this case, create separate actions to handle each Id.  This will simplify the code as requested.

    Tuesday, May 21, 2019 5:13 PM
  • User1120430333 posted


    Your help is needed, I have three types of ID's coming from different pages to the same Controller / Action.

    Maybe, you should consider method overloading for the ActionMethod, which is a seperation of methods with the same method name, but the selection is made by .NET based on the signature  to be used for the overloaded method.

    https://www.c-sharpcorner.com/UploadFile/0c1bb2/method-oveloading-and-overriding-C-Sharp/

     [Authorize]
            public ActionResult Edit(int id = 0)
            {
                return id == 0 ? null : View(_projectModel.Edit(id));
            }
    
            [Authorize]
            [HttpPost]
            public ActionResult Edit(ProjectViewModels.Project project, string submit)
            {
                if (submit == "Cancel") return RedirectToAction("Index");
    
                if (ModelState.IsValid && _modelHelper.IsEndDateLessThanStartDate(project, "Project"))
                    ModelState.AddModelError(String.Empty, "End Date cannot be less than Start Date.");
    
                if (!ModelState.IsValid) return View(_projectModel.PopulateSelectedList(project));
    
                var theproject = new ProjectViewModels.Project();
    
                theproject = project;
    
                theproject.ProjectType = Request.Form["ProjectType"];
    
                _projectModel.Edit(theproject, User.Identity.Name);
                return RedirectToAction("Index");
            }
    

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Tuesday, May 21, 2019 5:56 PM
  • User1520731567 posted

    Hi slimbunny,

    if ((studentId == null) && (eId == null) || (eId == string.empty))

    {

              return RedirectToAction(Index, Home)

    }

    How can I make this statement more efficient

    if ((StudentId != null) && ( update == "true"))

    {

        ID = StudentId

    }

    else if ((StudentId != null ) && (emp == "true"))

    {

          ID = StudentId

    }

    else if (eId != null)

    {

        ID = eId

    }

    else if (StudentId != null)

    {

        ID = StudentId

    }

    else

    {

       returnl......

    }

    If could,you could modify your database design to judge different scenes by one field.

    If you want to reduce the code, I suggest you could try to consider using ?: Operator.

    Best Regards.

    Yuki TAO

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, May 22, 2019 6:28 AM
  • User-821857111 posted

    How can I make this statement more efficient
    Efficient? Do you mean more concise? Easier to understand? Frankly, I can't see anything wrong with the code as it is. It's easy for anyone else to follow the logic. You could try to make it syntactically more compact, but then you might lose something in clarity.

    Wednesday, May 22, 2019 7:06 AM
  • User-507786106 posted

    yes more concise

    Wednesday, May 29, 2019 5:44 PM
  • User753101303 posted

    Seems rather a design issue.

    "I have three types of ID's coming from different pages to the same Controller / Action"

    Why ? This is a discrepancy or because you are not doing the same action ? It seems to me you need some code to figure out what needs to be done depending on parameters when you could have multiple actions and call directly the one you need.

    IMO it's a bit hard to give a specific advice without knowing about the big picture here.

    Tuesday, June 4, 2019 9:49 AM