optimise code to reduce code complexity on UpdateUser method in particular.

code has been split into smaller methods, and queries on db have been optimised to make fewer, larger queries than lots of small queries. (using IN sql queries).
restructured UpdateUser to be more logical which has reduced some of the complexity of the method.
When deleting unassociated cards, the system will ensure all timelogs associated to those Ids are deleted also.
CreateGroup will now not allow duplicate groups, but will return the ID of the existing Group found in the DB.
pulled out GetUserCount method to simplify the GetUsers method.
removed empty quotes in favour of string.empty
#95

DataCenter_Windows/WindowsDataCenter/SQLiteRepository/SQLiteRepository.cs

@@ -19,6 +19,7 @@ namespace SQLiteRepository
         private readonly ILogger _logger;
         private readonly string _path;
         private const string DATABASE_NAME = "flexitimedb.db";
+        private const string UPGRADE_SCRIPT_PREFIX = "SQLiteRepository.UpgradeScripts.";
 
         public SQLiteRepository(ILogger logger)
         {
@@ -40,33 +41,14 @@ namespace SQLiteRepository
             _logger.Trace("Checking For Upgrades");
             CheckForDbUpgrade();
         }
+
         public UserList GetUsers(int pageNumber = -1, int pageSize = -1, int groupId = -1)
         {
             var ret = new UserList();
-            List<UserIdentity> users;
+            List<UserIdentity> users = GetUserList(groupId, pageSize, pageNumber);
-            int userCount;
+
-            if (pageNumber != -1 && pageSize != -1)
+            var userCount = GetUserCount();
-            {
-                users = _connection.Query<UserIdentity>(SQLiteProcedures.GET_ALL_USERS_PAGINATE,
-                    pageSize, (pageNumber - 1) * pageSize);
-                userCount = _connection.ExecuteScalar<int>(SQLiteProcedures.GET_TOTAL_USER_COUNT);
-            }
-            else if (groupId != -1)
-            {
-                users =
-                    _connection.Query<UserIdentity>(
-                        SQLiteProcedures.GET_ALL_USERS_BY_GROUP,
-                        groupId);
-                userCount = users.Count;
-            }
-            else
-            {
-                users = _connection.Query<UserIdentity>(SQLiteProcedures.GET_ALL_USERS);
-                userCount = users.Count;
-            }
 
-            if (!users.Any())
-            {
             if (pageNumber == -1 && pageSize == -1)
             {
                 ret.PageNumber = 1;
@@ -77,6 +59,9 @@ namespace SQLiteRepository
                 ret.PageNumber = pageNumber;
                 ret.PageSize = pageSize;
             }
+
+            if (!users.Any())
+            {
                 return ret;
             }
 
@@ -90,16 +75,7 @@ namespace SQLiteRepository
                 userObj.Groups = GetGroups(user.Id);
                 ret.Users.Add(userObj);
             }
-            if (pageNumber == -1 && pageSize == -1)
+
-            {
-                ret.PageSize = 10;
-                ret.PageNumber = 1;
-            }
-            else
-            {
-                ret.PageSize = pageSize;
-                ret.PageNumber = pageNumber;
-            }
             ret.TotalUserCount = userCount;
             return ret;
         }
@@ -131,19 +107,12 @@ namespace SQLiteRepository
                 foreach (var card in cards)
                 {
                     userObj.AssociatedIdentifiers.Add(IdentifierConverter.ConvertToIdentifierDto(card));
-                    //userObj.AssociatedIdentifiers.Add(new Identifier()
-                    //{
-                    //    UniqueId = card.CardUId,
-                    //    IsAssociatedToUser = true,
-                    //    Id = card.Id
-                    //});
                 }
                 userObj.State = GetUserState(GetLogDirection(user.Id));
                 ret.Users.Add(userObj);
             }
-            //TODO: figure out paging here. - should there be any?
             ret.PageSize = 20;
-            ret.PageNumber = 1;
+            ret.PageNumber = (int)Math.Ceiling((double)ret.Users.Count/(double)ret.PageSize);
 
             return ret;
         }
@@ -175,12 +144,6 @@ namespace SQLiteRepository
                 foreach (var card in cards)
                 {
                     ret.AssociatedIdentifiers.Add(IdentifierConverter.ConvertToIdentifierDto(card));
-                    //new Identifier
-                    //{
-                    //    UniqueId = card.CardUId,
-                    //    IsAssociatedToUser = true,
-                    //    Id = card.Id
-                    //});
                 }
             }
             catch (Exception ex)
@@ -262,16 +225,22 @@ namespace SQLiteRepository
 
         public void ClearUnassignedIdentifiers()
         {
+            var unassignedIdentifiers = _connection.Query<CardUniqueId>(SQLiteProcedures.GET_UNASSIGNED_CARD_LIST);
+
+            //remove the logs from the timelog db
+            _connection.Query<TimeLogDb>(
+                SQLiteProcedures.FormatInQuery(SQLiteProcedures.DELETE_TIMELOG_ENTRIES,
+                unassignedIdentifiers.Select(x => x.Id.ToString()).ToList()));
+
+            //now remove the unassigned identifiers/cards
             _connection.Execute(
                 SQLiteProcedures.CLEAR_UNASSIGNED_CARDS,
                 Constants.UNASSIGNED_CARD_USER_ID);
         }
 
-        //TODO: Check time logs table on update to ensure associated cards/unique identifiers are removed/added as appropriate.
         public OperationResponse UpdateUser(User user, out int userIdResult)
         {
             var ret = OperationResponse.NONE;
-            var cardIds = new List<int>();
 
             //Get a list of current associated identifiers, convert into a list of Identifier Objects..
             var currentCards =
@@ -279,43 +248,12 @@ namespace SQLiteRepository
                     .Select(IdentifierConverter.ConvertToIdentifierDto);
             var cardsToRemove = currentCards.Except(user.AssociatedIdentifiers).Select(x => x.Id).ToList();
             
-            #region GetUnique Identifiers
-            foreach (var card in user.AssociatedIdentifiers)
-            {
-                var existingCard = _connection.Query<CardUniqueId>(
-                    SQLiteProcedures.GET_CARDS_BY_UNIQUE_ID, card.UniqueId);
-
-                if (!existingCard.Any())
-                {
-                    var cardInsert = IdentifierConverter.ConvertFromIdentifierDto(card, Constants.UNASSIGNED_CARD_USER_ID);
-                    _connection.Insert(cardInsert);
-                    cardIds.Add(cardInsert.Id);
-                    if (ret < OperationResponse.CREATED)
-                        ret = OperationResponse.CREATED;    //only change it if my status supercedes.
-                }
-                else
-                {
-                    cardIds.Add(existingCard.First().Id);
-                    if (ret < OperationResponse.UPDATED)
-                        ret = OperationResponse.UPDATED;
-                }
-            }
-            #endregion
-
             #region Update/Create User
             int userId;
 
             if (user.UserId != -1)
             {
-                //edit..
+                UpdateUserDetails(user.FirstName, user.LastName, user.HoursPerWeek, user.IsContractor, user.UserId);
-                _connection.Query<UserIdentity>(
-                    SQLiteProcedures.UPDATE_USER_DETAILS,
-                    user.FirstName,
-                    user.LastName,
-                    user.HoursPerWeek,
-                    user.IsContractor,
-                    user.UserId
-                );
                 userId = user.UserId;
             }
             else
@@ -328,19 +266,32 @@ namespace SQLiteRepository
             }
             #endregion
 
-            #region Update Card/Unique Id entries.
+            #region GetUnique Identifiers
-            foreach (var cardId in cardIds)
+
+            var existingCards = _connection.Query<CardUniqueId>(SQLiteProcedures.FormatInQuery(
+                SQLiteProcedures.GET_CARDS_BY_UNIQUE_ID_LIST,
+                user.AssociatedIdentifiers.Select(x => x.UniqueId.ToString()).ToList()));
+
+            foreach (var card in user.AssociatedIdentifiers)
             {
-                _connection.Query<CardUniqueId>(
+                if (existingCards.All(x => x.CardUId != card.UniqueId))
-                    SQLiteProcedures.UPDATE_CARD_USER_ID,
-                    userId, cardId);
-            }
-            foreach (var card in cardsToRemove)
                 {
-                _connection.Query<CardUniqueId>(
+                    //this card is not currently in the system..
-                    SQLiteProcedures.UPDATE_CARD_USER_ID,
+                    var cardInsert = IdentifierConverter.ConvertFromIdentifierDto(card, user.UserId);
-                    Constants.UNASSIGNED_CARD_USER_ID, card);
+                    _connection.Insert(cardInsert);
+                    existingCards.Add(cardInsert);
+                    if (ret < OperationResponse.CREATED)
+                        ret = OperationResponse.CREATED;    //only change it if my status supercedes.
                 }
+            }
+            #endregion
+
+            #region Update Card/Unique Id entries/associations
+            //make sure all identifiers are associated to the right user
+            UpdateIdentifierUserId(existingCards.Select(x=>x.Id).ToList(), userId);
+
+            //make sure to remove all identifiers that have been deselected in edit are removed from user
+            UpdateIdentifierUserId(cardsToRemove, Constants.UNASSIGNED_CARD_USER_ID);
             #endregion
 
             #region Update Group Associations
@@ -375,11 +326,11 @@ namespace SQLiteRepository
                 ret.Direction = LogDirection.UNKNOWN;
                 return ret;
             }
-            else
+
-            {
-                //TODO: log when more than one comes back. should NEVER happen but....
             ident = cardIdQuery.First();
-            }
+            _logger.Warn("More than 1 Identifier returned with ID {0}, card ids returned: {2}",
+                ident.CardUId,
+                string.Join(",", cardIdQuery.Select(x => x.CardUId).ToList()));
             #endregion
 
             // Get The User Direction (are they going in or out)?
@@ -433,10 +384,14 @@ namespace SQLiteRepository
             return ret;
         }
         
-        /*Groups*/
-        //TODO: check group name can only be entered once.
         public OperationResponse CreateGroup(Group group, out int groupId)
         {
+            var query = _connection.Query<GroupDb>(SQLiteProcedures.GET_GROUP_BY_NAME, group.Name);
+            if (query.Any())
+            {
+                groupId = query[0].GroupId;
+                return OperationResponse.NONE;
+            }
             var groupDb = new GroupDb { GroupName = group.Name };
             var resp = _connection.Insert(groupDb);
             groupId = groupDb.GroupId;
@@ -487,7 +442,7 @@ namespace SQLiteRepository
 
         public OperationResponse UpdateGroup(Group group)
         {
-            var resp = _connection.Query<GroupDb>(SQLiteProcedures.UPDATE_GROUP, group.Name, group.Id);
+            _connection.Query<GroupDb>(SQLiteProcedures.UPDATE_GROUP, @group.Name, @group.Id);
 
             return OperationResponse.UPDATED;
         }
@@ -553,6 +508,34 @@ namespace SQLiteRepository
             return OperationResponse.UPDATED;
         }
         
+        private int GetUserCount()
+        {
+            return _connection.ExecuteScalar<int>(SQLiteProcedures.GET_TOTAL_USER_COUNT);
+        }
+
+        private List<UserIdentity> GetUserList(int groupId = -1, int pageSize=-1, int pageNumber=-1)
+        {
+            List<UserIdentity> users;
+            if (pageNumber != -1 && pageSize != -1)
+            {
+                users = _connection.Query<UserIdentity>(SQLiteProcedures.GET_ALL_USERS_PAGINATE,
+                    pageSize, (pageNumber - 1) * pageSize);
+            }
+            else if (groupId != -1)
+            {
+                users =
+                    _connection.Query<UserIdentity>(
+                        SQLiteProcedures.GET_ALL_USERS_BY_GROUP,
+                        groupId);
+            }
+            else
+            {
+                users = _connection.Query<UserIdentity>(SQLiteProcedures.GET_ALL_USERS);
+            }
+
+            return users;
+        }
+
         private void CheckForDbUpgrade()
         {
             var data = _connection.Query<DbVersion>(SQLiteProcedures.GET_DB_VERSION);
@@ -576,19 +559,20 @@ namespace SQLiteRepository
 
         private void ExecuteUpgradeFromVersion(string installedVersion)
         {
-            const string PREFIX = "SQLiteRepository.UpgradeScripts.";
             var instVers = new Version(installedVersion);
             //so we have established that each script for upgrade will be <version>.sql, so now start at lowest version and work up!
             var assembly = Assembly.GetExecutingAssembly();
             var upgradeScripts = assembly.GetManifestResourceNames()
-                                    .Where(x=>x.StartsWith(PREFIX))
+                .Where(x => x.StartsWith(UPGRADE_SCRIPT_PREFIX))
-                                    .OrderBy(x=>new Version(Path.GetFileNameWithoutExtension(x.Replace(PREFIX, ""))))
+                .OrderBy(x =>
+                    new Version(Path.GetFileNameWithoutExtension(x.Replace(UPGRADE_SCRIPT_PREFIX, string.Empty))))
                 .Where(x =>
                 {
-                                                var scriptVersion = new Version(Path.GetFileNameWithoutExtension(x.Replace(PREFIX, ""))).CompareTo(instVers);
+                    var scriptVersion =
+                        new Version(Path.GetFileNameWithoutExtension(x.Replace(UPGRADE_SCRIPT_PREFIX, string.Empty)))
+                            .CompareTo(instVers);
                     return scriptVersion > 0;
-                                            }
+                })
-                                          )
                 .ToList();
             //now have an ordered list of upgrade script files, so execute in order!
             foreach (var upgradeScript in upgradeScripts)
@@ -878,5 +862,29 @@ namespace SQLiteRepository
                 }
             }
         }
+
+        private void UpdateIdentifierUserId(List<int> cardsToUpdate, int userId)
+        {
+            foreach (var card in cardsToUpdate)
+            {
+                _connection.Query<CardUniqueId>(
+                    SQLiteProcedures.UPDATE_CARD_USER_ID,
+                    userId, card);
+            }
+        }
+
+        private void UpdateUserDetails(string firstName, string lastName, float hoursPerWeek, bool isContractor,
+            int userId)
+        {
+            _connection.Query<UserIdentity>(
+                SQLiteProcedures.UPDATE_USER_DETAILS,
+                firstName,
+                lastName,
+                hoursPerWeek,
+                isContractor,
+                userId
+            );
+        }
+
     }
 }