How to reuse code within the same controller?

Viewed 94

I have a controller with 2 actions containing identical code. But i'm having trouble reusing it from a separate function, without resorting to returning null and checking return values. The actions are for logging in and for resending a two factor toke.

2 actions (Login and Resend)

[HttpPost]
[ValidateAntiForgeryToken]
public async Task<IActionResult> Login(LoginModel model)
{
    if (!ModelState.IsValid) return View();
    
    var user = await userManager.FindByNameAsync(model.UserName);
    if(!(user != null && !await userManager.IsLockedOutAsync(user)))
    {
        ModelState.AddModelError("", "Invalid UserName or Password");
        return View();
    }
    
    if(!await userManager.CheckPasswordAsync(user, model.Password))
    {
        ModelState.AddModelError("", "Invalid UserName or Password");
        return View();
    }
    
    // can this be improved?
    var r = await SendTwoFactor(user);
    if(r != null) return r; // error or redirect or no two factor?
    
    return RedirectToAction("Index");
}

[HttpPost]
[ValidateAntiForgeryToken]
public async Task<IActionResult> Resend(ResendModel model)
{
    var result = await HttpContext.AuthenticateAsync(IdentityConstants.TwoFactorUserIdScheme);
    if (!result.Succeeded)
    {
        return RedirectToAction(nameof(Login)); // expired
    }

    var user = await userManager.FindByIdAsync(result.Principal.FindFirstValue("sub"));
    if (user == null)
    {
        return RedirectToAction(nameof(Login)); // invalid
    }

    await HttpContext.SignOutAsync(IdentityConstants.TwoFactorUserIdScheme);        

    // can this be improved?
    var r = await SendTwoFactor(user);
    if(r != null) return r; // error or redirect or no two factor?
    
    ...

    return RedirectToAction("...");
}

then the re-used code looks something like this:

SendTwoFactor

private async Task<IActionResult> SendTwoFactor(IdentityUser user)
{
    if(!await userManager.GetTwoFactorEnabledAsync(user)) return null;

    var d = ...;
    if(!d)
    {
        ModelState.AddModelError("", "... d");
        return View();
    }

    // determine preferred two factor method
    // send two factor token

    return RedirectToAction("...");
}

Does anyone have a good pattern to improve this?

1 Answers

This would be my aproach First of all, we need to define interface with the 'common' elements inside FooModel and Foo2Model that would be used in common code like this:

    public interface iFoo 
    {
        int Id { get; set; }
        string Product_name{ get; set; }
    }

    public class Foo : iFoo
    {
        public string OtherThings { get; set; }
        public int Id { get; set; }
        public string Product_name { get; set; }
    }

    public class Foo2 : iFoo
    {
        public string OtherThings2 { get; set; }
        public int Id { get; set; }
        public string Product_name_nv { get; set; }
    }

Then we can make a private function like this:

private async Task<IActionResult> identicalFunction<T>(T model) where T : iFoo
{
    c = ...;
    if(c)
    {
        d = ...;
        if(!d)
        {
            ModelState.AddModelError("", "... d");
            return View();
        }

        ... // a lot more code similar code that can exit early
        return RedirectToAction("...");

    }
    // Note that I included the last return inside this function
    return RedirectToAction("...");
}

And then change

// identical code

return RedirectToAction("...");

into:

return await identicalFunction(model);
Related