Duplicate users added to database

Question

I try to add a new value to my database. UserPassword and RePassword must have the same value and a user with UserName must not already exist in the database.

  public User NewUser(int HotelID, string UserName, string UserPassword, string RePassword, string FullName, string Email, bool Active, bool MasterUser)
    {
        User user = new User();
        user.HotelID = HotelID;
        user.UserName = UserName;
        user.UserPassword = UserPassword;
        user.FullName = FullName;
        user.Email = Email;
        user.IsActiveq = Active;
        user.IsMaster = MasterUser;

        var cekUser = (from c in _UserRepository.All()
                       where c.HotelID == HotelID
                       select c.UserName).ToList();
        if (UserPassword == RePassword)
        {
            foreach (string cek in cekUser)
            {
                var x = cek;
                if (UserName != x)
                {
                    _UserRepository.Add(user);
                }
            }
        }

        _UserRepository.CommitChanges();

        return user;

    }

Every time I run my code a new line is added to the database, although a user with the supplied user name already exists in the database.

Why does this happen? Which part of my code is wrong?

What is that loop intended to do? It looks like you are adding the new user to your repository as many times as you have users. — recursive, Jun 12 '13 at 02:09
i try to check, are my value exist in database or not with the loop — novian kristianto, Jun 12 '13 at 02:11
It looks like every time the `UserName` doesn't match `x`, you're adding the user to your database. Why not just check your list of strings for the `string` value? `bool ListContainsString = listOfStrings.Any(myString.Contains);'. — X3074861X, Jun 12 '13 at 02:15
Check this thread, it's a very well known use case : http://stackoverflow.com/questions/500925/to-check-if-a-string-contains-an-element-from-a-list-of-strings-is-there-a-b — X3074861X, Jun 12 '13 at 02:17
@noviankristianto You should validate all these using `ViewModels` and not in the code. You can do validations using `Data Annotations` on `ViewModels` — Bhushan Firake, Jun 12 '13 at 02:18
my username unique with HotelID. in every HotelId, like 2, username can have same value, but if HotelID is different, username can have same value — novian kristianto, Jun 12 '13 at 02:22
Say you get a, b, c in the same hotel. And your new user is b. In your loop, a != b, add b; c != b, add b. — Jason Li, Jun 12 '13 at 02:25
TIP: Your `NewUser` method should accept a user object rather than build it within. It's a good practice to separate concerns that way. `public User NewUser(User user){...}` — Chase Florell, Jun 12 '13 at 03:14

score 4 · Answer 1 · answered Jun 12 '13 at 02:23

I think your code should be something like this:

if (UserPassword == RePassword)
{
    // Also I thinks you should finish whether user existed logic in database
    // but for now, let's follow your original logic
    var existedUsers = (from c in _UserRepository.All()
                       where c.HotelID == HotelID
                       select c.UserName).ToList();

    if (!existedUsers.Any(u => u == UserName))
    {
        _UserRepository.Add(user);
        _UserRepository.CommitChanges();
    }   
}

Igor · Answer 2 · 2013-06-12T02:34:23.757

1

You have your logic wrong. If there is more than one user in a given hotel, your code will be adding more users for all users with names different from UserName.

bool found = false;
foreach(string cek in cekUser)
{
  if ( UserName == cek)
  {
    found = true;
    break;
  }
}
if (!found)
   _UserRepository.Add(user);

edited Jun 12 '13 at 02:34

answered Jun 12 '13 at 02:24

Igor

15,833
1
27
32

can you explain to me, what logic should i use – novian kristianto Jun 12 '13 at 02:30

score 0 · Answer 3 · answered Jun 12 '13 at 03:12

Just offering an alternate idea.

If you have access to the database, the best approach will be to make the Username field UNIQUE. That way, even if you get your code wrong, a duplicate insert will fail. Then you capture that fail gracefully in your Repository, and Bob's your uncle.

Duplicate users added to database

3 Answers3