2
\$\begingroup\$

I want to get the row of data that matches the user ID I have. Then I want to return that view to my view to be displayed in my textboxes for editing.

Here's the action that gets called when a get request to sent to the controller:

public ActionResult EditUserAccount()
{
            UserAccountViewModel editUserAccountViewModel = new UserAccountViewModel();
            db_user user = new db_user();
            editUserAccountViewModel.UserName = UserSession.GetValue(StateNameEnum.UserName, StateNameEnum.UserName.ToString()) as string;
            var Title = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.Title).ToString();
            var FirstName = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.FirstName).ToString();
            var LastName = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.LastName).ToString();
            var PhoneNumber = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.PhoneNumber).ToString();
            var AltPhoneNumber = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.AltPhoneNumber).ToString();
            var EmailAddress = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.EmailAddress).ToString();

            editUserAccountViewModel.Title = Title;
            editUserAccountViewModel.FirstName = FirstName;
            editUserAccountViewModel.LastName = LastName;
            editUserAccountViewModel.PhoneNumber = PhoneNumber;
            editUserAccountViewModel.AltPhoneNumber = AltPhoneNumber;
            editUserAccountViewModel.EmailAddress = EmailAddress;
            return (PartialView("~/Views/Account/UserAccount.cshtml", editUserAccountViewModel));

}

The problem is the repetitious calls to get specific fields from the database so that I can set my viewmodel properties to them. Is there a better way to do this?

\$\endgroup\$
2
\$\begingroup\$

Use an anonymous type to store a single object retrieved from the database and then populate your view from this temporary (and anonymous) object.

Answering from my mobile, so I apologize if this doesn't compile.

public ActionResult EditUserAccount()
{
            UserAccountViewModel editUserAccountViewModel = new UserAccountViewModel();
            db_user user = new db_user();
            editUserAccountViewModel.UserName = UserSession.GetValue(StateNameEnum.UserName, StateNameEnum.UserName.ToString()) as string;

            var user = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId)
                .Select(x => new {x.Title, x.FirstName, x.LastName, x.PhoneNumber, x.AltPhoneNumber, x.EmailAddress } );

            editUserAccountViewModel.Title = user.Title;
            editUserAccountViewModel.FirstName = user.FirstName;
            editUserAccountViewModel.LastName = user.LastName;
            editUserAccountViewModel.PhoneNumber = user.PhoneNumber;
            editUserAccountViewModel.AltPhoneNumber = user.AltPhoneNumber;
            editUserAccountViewModel.EmailAddress = user.EmailAddress;
            return (PartialView("~/Views/Account/UserAccount.cshtml", editUserAccountViewModel));

}

This lets you hit the database once instead of once per property.

\$\endgroup\$
  • \$\begingroup\$ One question, the x's that appear after x.Title, it says "The name 'x' does not exist in current context". \$\endgroup\$ – Kala J Jul 20 '15 at 1:44
  • \$\begingroup\$ I wasn't (still are't) in front of my ide. Try the query syntax from the page I linked to. \$\endgroup\$ – RubberDuck Jul 20 '15 at 2:09
  • \$\begingroup\$ Wait... I was missing a new { ... } in there. Try the updated code. \$\endgroup\$ – RubberDuck Jul 20 '15 at 2:11
  • \$\begingroup\$ Okay, thank you!! Just curious why would I need a new {} there? \$\endgroup\$ – Kala J Jul 20 '15 at 2:23
  • 1
    \$\begingroup\$ @KalaJ because you are creating a new amonymous object. \$\endgroup\$ – Heslacher Jul 20 '15 at 5:18
1
\$\begingroup\$

Only talking about these lines of code

      UserAccountViewModel editUserAccountViewModel = new UserAccountViewModel();
       db_user user = new db_user();
       editUserAccountViewModel.UserName = UserSession.GetValue(StateNameEnum.UserName, StateNameEnum.UserName.ToString()) as string;
       var Title = context.db_user.Where(x => x.UserId == editUserAccountViewModel.UserId).Select(x => x.Title).ToString();  
  • I don't see the db_user user being used anywhere, so you should remove it.
  • Class names shouldn't use underscores in their name and also should use PascalCase casing based on the naming guidelines.
    Based on the same guidelines variables should be named using camelCase casing.

  • you are filling the UserName property of the UserAccountViewModel by calling GetValue() of the UserSession and later in the queries you are refering to the UserId property which seems isn't filled at all. If this is magically done by the setter of the UserName property then I would call this a side effect which shouldn't happen. If this isn't filled by the setter, then I would say that the queries are senseless because the UserId property will be set to a default value which won't be found in the database.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.