Check if the user exists

Question

Scenario:

• Check if the user exists
  • If yes: update the details
  • Else: insert the user details

---

public bool AddUsers(IEnumerable<UserDetail> users)
    {
        using (var dbTransaction = System.Data.Entity.Database.DefaultConnectionFactory.CreateConnection("MyCon").BeginTransaction())
        {
            using (var entities = new MyDbEntities())
            {
                try
                {
                    foreach (var user in users)
                    {
                        // LINQ under observation
                        var userId = entities.tUsers.Where(u => u.UserName == user.UserName).Select(x => x.UserUID).FirstOrDefault();

                        if (userId > 0)
                        {
                            //update

                        }
                        else
                        {
                             //insert
                        }

                    }
                    dbTransaction.Commit();
                    return true;
                }
                catch (Exception ex)
                {
                    dbTransaction.Rollback();
                    clsErrorHandling.WriteLog("ImportUsersHandler", "AddUsers", ex.Message.ToString());
                    return false;
                }
                finally
                {
                    dbTransaction.Dispose();
                }
            }
        }
    }

Please find the comment //LINQ under observation.

Is there a better way to find the user?

Because if I do this way:-

 tblUser newUser = new tblUser() { Username = user.Username } ;

If the user exists then no issue then in case not then how do I check whether I need to do insert or update.

Answer 1

using (var dbTransaction = System.Data.Entity.Database.DefaultConnectionFactory.CreateConnection("MyCon").BeginTransaction())
// ...
finally
{
    dbTransaction.Dispose();
}

You're already using the transaction in a using statement which is shorthand for try/finally, where the finally block disposes the IDisposable. So you can remove your manual finally block completely, otherwise you're calling Dispose on the transaction twice.

clsErrorHandling.WriteLog("ImportUsersHandler", "AddUsers", ex.Message.ToString());

ex.Message is already a string, no need to call ToString on it.

Answer 2

LINQ expression to find user

c# < 6

var user = entities.tUsers.FirstOrDefault(u => u.UserName == user.UserName);
var userId = user == null ? default(int) : user.UserUID;

c# 6

var userId = entities.tUsers
    .FirstOrDefault(u => u.UserName == user.UserName)?.UserUID;

---

tblUser newUser = new tblUser() { Username = user.Username };

Don't create new instance of entity. Use Entity which was found by LINQ expression.

---

entities.tUsers.Where(u => u.UserName == user.UserName)

It isn't good idea to find user by UserName when UserName is not unique.

Is UserName in your database primary key or unique?

Answer 3

You tagged your post with "entity framework". I'm using an older version of EF, but hopefully my comments won't be too far off the mark.

                    var userId = entities.tUsers.Where(u => u.UserName == user.UserName).Select(x => x.UserUID).FirstOrDefault();

                    if (userId > 0)
                    {
                        //update

                    }
                    else
                    {
                         //insert
                    }

The //update comment hides some rather important stuff from us. Is it the entity you are updating? If so, you presumably have to perform another query to fetch the entity itself.

How about simply fetching the entity to begin with?

var userent = entities.tUsers.FirstOrDefault(u => u.UserName == user.UserName);
if (userent == null) // then we need to insert a new one
{
    userent = entities.tUsers.CreateObject(); // or new myentity(); whatever you prefer
    entities.tUsers.AddObject(userent);
}
AutoMap.Mapper.Map(user, userent); // or assign these properties yourself

Finally, a transaction? Where is your call to entities.SaveChanges()? (I'm assuming "entities" is your EF context)

Generally speaking, EF will handle the transaction part for you and will complain loudly if one of your entities can't be saved for some reason.

So just put a entities.SaveChanges(); after the for-loop and ditch the transaction object.

Finally, someone suggested:

var userId = entities.tUsers.FirstOrDefault(u => u.UserName == user.UserName)?.UserUID;

Avoid this. This fetches the entire entity from the database (use SQL Profiler!) and then leaves you using only that single property. I.e. a select * followed by only accessing a single field. An entity can be quite complex (involving joins and interesting bits) and one should take care to .Select only relevant fields (like you did initially).

Mitigating factor: EF might cache the entity within your current context, so in this instance it probably isn't that bad. Regardless of this, my recommendation is to use Select() whenever possible. Unlike plain LINQ, a lack of .Select() will force EF to do more work than what is sometimes necessary.

Answer 4

I don't think one big transaction is good. If one fails everything gets rolled back. Consider putting each update / insert in a transaction.

If you do TSQL you use a merge statement so you don't need a transaction at all. Search on upsert.