Skip to main content
added 84 characters in body
Source Link
Der Kommissar
  • 20.1k
  • 4
  • 68
  • 158

First off, I would use a switch on message. It's faster (though not by a lot) than ternary (?:) operators or if statements. Also, I would have each UserManager async method run at the same time. That is, save them to a var and then await each var in the IndexViewModel. (You may have to clean up syntax for that portion, I wrote it without the ASP.NET context.)

IndexViewModel

You could possibly replace fieldsproperties there that are duplicates of the ApplicationUser, and then store an ApplicationUser in it, and access the properties by IndexViewModel.ApplicationUser.Property. Though you can really only do that for the common properties.

First off, I would use a switch on message. Also, I would have each UserManager async method run at the same time. That is, save them to a var and then await each var in the IndexViewModel. (You may have to clean up syntax for that portion, I wrote it without the ASP.NET context.)

You could possibly replace fields there that are duplicates of the ApplicationUser, and then store an ApplicationUser in it, and access the properties by IndexViewModel.ApplicationUser.Property. Though you can really only do that for the common properties.

First off, I would use a switch on message. It's faster (though not by a lot) than ternary (?:) operators or if statements. Also, I would have each UserManager async method run at the same time. That is, save them to a var and then await each var in the IndexViewModel. (You may have to clean up syntax for that portion, I wrote it without the ASP.NET context.)

IndexViewModel

You could possibly replace properties there that are duplicates of the ApplicationUser, and then store an ApplicationUser in it, and access the properties by IndexViewModel.ApplicationUser.Property. Though you can really only do that for the common properties.

Source Link
Der Kommissar
  • 20.1k
  • 4
  • 68
  • 158

MergeController.cs

First off, I would use a switch on message. Also, I would have each UserManager async method run at the same time. That is, save them to a var and then await each var in the IndexViewModel. (You may have to clean up syntax for that portion, I wrote it without the ASP.NET context.)

The idea behind moving the await keywords, is that it allows all the tasks to start before requesting data from them, so they can run in parallel. As it stands now, the programme will wait for user to run, then it will wait for email to run, etc. This way, it will get them all started, and then await what's left. (Though, again, I wrote it without the ASP.NET context, you may or may not be able to use that portion in that manner.)

public async Task<ActionResult> Index(ManageMessageId? message)
{
    switch (message)
    {
        case ManageMessageId.ChangePasswordSuccess:
            ViewBag.StatusMessage = "Your password has been changed.";
            break;
        case ManageMessageId.SetPasswordSuccess:
            ViewBag.StatusMessage = "Your password has been set.";
            break;
        case ManageMessageId.SetTwoFactorSuccess:
            ViewBag.StatusMessage = "Your two-factor authentication provider has been set.";
            break;
        case ManageMessageId.Error:
            ViewBag.StatusMessage = "An error has occurred.";
            break;
        case ManageMessageId.AddPhoneSuccess:
            ViewBag.StatusMessage = "Your phone number was added.";
            break;
        case ManageMessageId.RemovePhoneSuccess:
            ViewBag.StatusMessage = "Your phone number was removed.";
            break;
        default:
            ViewBag.StatusMessage = "";
            break;
    }

    var userId = User.Identity.GetUserId();
    var userTask = UserManager.FindByIdAsync(userId);

    var email = UserManager.GetEmailAsync(userId);
    var hasPassword = UserManager.HasPasswordAsync(userId);
    var phoneNumber = UserManager.GetPhoneNumberAsync(userId);
    var twoFactor = UserManager.GetTwoFactorEnabledAsync(userId);
    var logins = UserManager.GetLoginsAsync(userId);
    var browserRemembered = AuthenticationManager.TwoFactorBrowserRememberedAsync(userId);
    var user = await userTask;

    var model = new IndexViewModel
    {
        Email = await email,
        HasPassword = await hasPassword,
        PhoneNumber = await phoneNumber,
        TwoFactor = await twoFactor,
        Logins = await logins,
        BrowserRemembered = await browserRemembered,

        FirstName = user.FirstName,
        LastName = user.LastName,
        Picture = user.Picture,
        Handle = user.Handle
    };
    return View(model);
}

ApplicationUser.cs

I can't see anything here that really needs help. There's not much going in so it's inherently clean.

ManageViewModels.cs

Again, there's not much here so not a lot to optimize.

Overall, there's not a lot going on, so there's not a lot to optimize. Most of the optimizations that could be made are purely readability/maintenance, apart from the await keyword optimizations.

Your initial concern was the IndexViewModel, which as far as I can see, doesn't require much work. (If you need all those properties for display, then so be it. Don't try cutting out properties if you know you need them, just because you think there are too many.)

You could possibly replace fields there that are duplicates of the ApplicationUser, and then store an ApplicationUser in it, and access the properties by IndexViewModel.ApplicationUser.Property. Though you can really only do that for the common properties.