Select data from SQL Server in WPF app

Question

Having problems with accessing what is inside my list in a wpf application of a mafia game I am creating.

Basically I read from SQL Server 2016, then add it to my user collection list. Later when I use my list in a display, they all are there.

However the moment I use a foreach to loop through it to set a temp user equal to a found username, it only finds hard coded users and not ones added using the data read from SQL Server. Need help.

SQL Server read code:

using (connect = new SqlConnection(connetionString))
{
    connect.Open();

    string readString = "select * from Users";
    SqlCommand readCommand = new SqlCommand(readString, connect);

    using (SqlDataReader dataRead = readCommand.ExecuteReader())
    {
        if (dataRead != null)
        {
            while (dataRead.Read())
            {
                tempEmail = dataRead["Email"].ToString();
                tempName = dataRead["Name"].ToString();

                UserCollection.addUser(tempEmail, tempName);
            }
        }
    }

    connect.Close();
}

UserCollection relevant parts

private static List<User> UserList = new List<User>();

// add a user
public static void addUser(string email, string name)
{
        UserList.Add(new User(email, name, 0, "unset", false, false, false, false, false, false,
            false, "", false, false, false, 0, 0));
}

//return list of users for use elsewhere
public static List<User> ReturnUserList()
{
        return UserList;
}

Use of a list to set tempPlayer in a wpf window

PlayersList = UserCollection.ReturnUserList();

// tempPlayer = UserCollection.ReturnAUser(sessionUser);
foreach (var element in PlayersList)
{
    if (element.UserName == sessionUser)
    {
        tempPlayer = element;
    }
}

Example of code where the list works.

// set listing of current players
ListOfPlayers = UserCollection.ReturnUserList();

var tempList = from player in ListOfPlayers
               where player.UserBlocked == false
               select new
                      {
                         Name = player.UserName,
                         Email = player.UserEmail,
                      };

this.PlayerListBox.ItemsSource = tempList;

hard coded User add that works fine and can be found by foreach statement from my app.xaml.cs

UserCollection.addUser("g", "Tom");

There's a lot of stuff going on here that's not shown, like whatever `UserCollection` is. — tadman, Jul 21 '17 at 15:43
Seconded, it's very difficult to see where the issue could be without seeing the rest of the class. It looks like your implementation is a little more complicated than it needs to be - using a method to return your user list rather than a property for example. What we would need is a minimal example, so any variables/methods you call in your DB logic. We need a verifyable example to work from, but not necesarily all 1000+ lines. — Alex, Jul 21 '17 at 15:47
@DerekBlankinship if your UserCollection all records are same for last record this is your problem? — umasankar, Jul 21 '17 at 15:48
updated and @umasankar the Username is the primary key so its not possible for overlapping records? not sure if i got what u mean — LINQtothepast, Jul 21 '17 at 15:53
is it safe to link ur Github here? new to stackoverflow when actually using for questions — LINQtothepast, Jul 21 '17 at 15:58
@DerekBlankinship Linking to Github isn't a great idea. If you can't condense the quetion into a post with everything in one place, it's likely that most users will gloss over it. Writing an answer now. — Alex, Jul 21 '17 at 16:00
@DerekBlankinship Does `UserCollection` extend a list, or is it just a container class? — Alex, Jul 21 '17 at 16:07
@DerekBlankinship check the Answer can you check and reply me. it's you exact problem. — umasankar, Jul 21 '17 at 16:11
Took awhile to come back to this. Turns out the problem was something to do with the data read from the table. I made the sql table less complicated with few columns and then read all columns. This fixed the problem with no other changes needed. Man I hate sql sometimes — LINQtothepast, Aug 04 '17 at 02:55
@LINQtothepast I would suggest either accepting an existing answer (assuming one helped), or posting an explanation of what you did as an answer, then accepting that. Stating you "made the table less complicated" doesn't highlight the resolution really. It's not far from saying "It's fixed now", and leaving at that that. — Alex, Oct 26 '17 at 13:15

Alex · Answer 1 · 2017-07-21T16:31:21.960

Firstly, is there a reason why you need a static method to add users to a collection? Even if you need access to the list via a static accessor, you are better having a static property on the same class which you're using to read the DB

The following snippet should hopefully be of some help.

public class UserManagement {

    //Static property 
    private static List<User> _users;

    public static List<User> Users {
        get { 
            if (_users == null) {
                _user = new List<User>();
            }
            return _users; 
        }
        set { }
    }

    //Static load method must be called before accessing Users 
    public static void LoadDBUsers() {


        using (SqlConnection connection = new SqlConnection(connetionString)) {
            connection.Open();
            string readString = "select * from Users";
            using (SqlCommand command = new SqlCommand(readString, connection)) {
                using (SqlDataReader reader = command.ExecuteReader()) {
                    while (reader.Read()) {
                        String tempEmail = reader["Email"].ToString();
                        String tempName = reader["Name"].ToString();
                        User user = new User(tempEmail, tempName, 0, "unset", false, false, false, false, false, false, false, "", false, false, false, 0, 0));
                        users.Add(user);
                    }
                }
            }
        }
    }
}

To use from another class :

UserManagement.LoadDBUsers();
var dbUsers = UserManagement.Users;

If you have another list of users (say from a file), you could add a loader method to your class which will handle that for you.

If at some point you need to clear down the list of users, you can simply set it to a new instance...

UserManagement.Users = new List<User>();

A slightly better way to do this would be to remove the static property Users first, change the method definition to return a List<User> from the LoadDBUsers method - eg.

public static List<User> LoadDBUsers() {
    List<User> Users = new List<User>();

    //Handle the database logic as in the previous example..
    //Then at the end of the method..
    return Users;
}

and call as follows

List<User> dbUsers = UserManagement.LoadDBUsers();

Using the latter approach, you don't need to worry about multiple locations in your code maintaining a static property. Just call and assign it to a variable. It also has the advantage that you will always get the most current list of users from the DB without having to clear down and reload the list before you access it from the static property.

An added advantage of not using a global static property is that it can avoid potential memory issues. Static properties can be difficult to dispose by the garbage collector if a reference to them is held open. With instance variables, it's quite obvious when one goes out of scope and is not referenced anymore, but with static variables, the reference is sometimes not disposed until the program ends.

In many cases, this isn't an issue, but in larger systems it can be a pain to debug.

I will try this method then most likely check mark it. tempEmail and tempName were both declared earlier of type string. I shoulda added that as well. I will try rewriting how the DB is called in. So i thought static typing was fine most of the time, guess i'll use it a little less. Its my default i always tack on — LINQtothepast, Jul 21 '17 at 16:24
@DerekBlankinship If you need to have a single object which stores some data but is accessed from multiple places in your application, I would suggest that you look up the Singleton design pattern. It is called using a static method, but when first accessed, it creates an instance of itself and returns that. If there is an instance created already, the static call returns that instance. It still can have issues with references not being destroyed, but it's much easier to manage and debug. Static objects are useful, but can very easily become unwieldy. — Alex, Jul 21 '17 at 16:27

score 0 · Answer 2 · answered Jul 21 '17 at 16:08

0

 tempEmail = dataRead["Email"].ToString();
 tempName = dataRead["Name"].ToString();

tempEmail,tempName you are declare above on

using (connect = new SqlConnection(connetionString))

the tempEmail and tempName are reference type so, if loop it's first record add after second time loop tempName and tempEmail values update also previously added so,because it's also pointing same memory. so the data's are duplicate record list so, the users cannot add in user properly.

so, you can change your code

 var tempEmail = dataRead["Email"].ToString();
 var tempName = dataRead["Name"].ToString();

and before tempEmail,tempName remove the declaration before using statement.

I Hope this is Helpful for you.

answered Jul 21 '17 at 16:08

umasankar

599
1
9
28

Assuming that those types are Strings and defined somewhere in the class as static, assigning a value to them will just replace the existing value. – Alex Jul 21 '17 at 16:24
ya they were already defined as private strings. changing them to var still had the same error unfortunately. I'm not fully doing coding at the moment but i will try alex's answer i think. Thanks for the try at helping tho – LINQtothepast Jul 21 '17 at 16:53

score 0 · Answer 3 · answered Jul 21 '17 at 17:25

I believe your while loop was causing the issue. Since "using" was in effect, the global, assumed, variables "tempEmail" & "tempName" remained null. I tested the code using MySql on my end and this solution was effective.

private List<User> PlayersList;

public Tester()
        {
            using (SqlConnection connect = new SqlConnection(connectionString))
            {
                connect.Open();

                string readString = "select * from user";

                SqlCommand readCommand = new SqlCommand(readString, connect);

                using (SqlDataReader dataRead = readCommand.ExecuteReader())
                {
                    if (dataRead != null)
                    {
                        while (dataRead.Read())
                        {
                            string tempEmail = dataRead["Email"].ToString();
                            string tempName = dataRead["Name"].ToString();

                            UserCollection.addUser(tempEmail, tempName);
                        }
                    }
                }

                connect.Close();
            }

            PlayersList = UserCollection.ReturnUserList();
        }

        public class User
        {    
            public string email;
            public string name;

            // A constructor that takes parameter to set the properties
            public User(string e, string n)
            {
                email = e;
                name = n;
            }
        }


        public static class UserCollection
        {    
            private static List<User> UserList = new List<User>();

            // add a user
            public static void addUser(string email, string name)
            {
                UserList.Add(new User(email, name));
            }

            //return list of users for use elsewhere
            public static List<User> ReturnUserList()
            {
                return UserList;
            }
        }

This is the area of concern.

while (dataRead.Read())
{
    //Set local variable during while loop
    string tempEmail = dataRead["Email"].ToString();
    string tempName = dataRead["Name"].ToString();

    UserCollection.addUser(tempEmail, tempName);
}

Select data from SQL Server in WPF app

3 Answers3