Not all code paths return a value while loop

Question

I've made the following code to fill a list with data from a database.

 public List<Transactie> FillTransacties()
    {
        try
        {
            SqlConnection connection = new SqlConnection(connectionString);
            connection.Open();
        
            SqlCommand cmd = new SqlCommand("SELECT transactieID, opdrachtID, medewerkerID, soort, datum, bedrag FROM Financien", connection);
            SqlDataReader transactieinformatie = cmd.ExecuteReader();

            List<Transactie> transacties = new List<Transactie>();

            while (transactieinformatie.Read())
            { 
                    string transactieID = transactieinformatie["transactieID"].ToString();
                    string opdrachtID = transactieinformatie["opdrachtID"].ToString();
                    string medewerkerID = transactieinformatie["medewerkerID"].ToString();
                    string soort = transactieinformatie["soort"].ToString();
                    string datum = transactieinformatie["datum"].ToString();
                    string bedrag = transactieinformatie["bedrag"].ToString();
                    Transactie transactie = new Transactie(transactieID, opdrachtID, medewerkerID, soort, datum, bedrag);
                    transacties.Add(transactie);
                    connection.Close();
                    return transacties;
            }
        }
        catch (InvalidCastException ICE)
        {
            MessageBox.Show("De data in de database is incorrect", ICE.Message);
            return new List<Transactie>();
        }
        catch (Exception e)
        {
            MessageBox.Show("Er is een onbekende error opgetreden.", e.Message);
            return new List<Transactie>();
        }
    }

Now I know what the problem is, I return values withing the while loop. The problem is though, when I try to 'Return transacties;' outside of the while loop, the List only fills with 1 value.

My question is then, how do I solve this error in such way that the database will fill with every row in the database?

Why are you closing the database connection in the loop? Won't that make it difficult reading more data? — ProgrammingLlama, Dec 15 '18 at 13:05
You can't use simple assignment like this to store multiple database row values in single variables. Use a datatable instead — Caius Jard, Dec 15 '18 at 13:06
@John , any tips where to otherwise place it? Im new to programming so I admit I make a lot of mistakes. — Max, Dec 15 '18 at 13:07
@CaiusJard correct me if i'm wrong, but is this what you call a disconnected layer? And I'll look into that, thanks. :) — Max, Dec 15 '18 at 13:07
@John You gave me quite the tip there mate. Fixed the problem by getting the connection.Close() and return out of the while loop. I wonder now though, is this the correct way of solving it or is there any other 'best practice' to do this? — Max, Dec 15 '18 at 13:09

score 0 · Accepted Answer · answered Dec 15 '18 at 13:09

You should rewrite your code with using statements. These will automatically clean up the datareader, command, and connection once your code completes:

using(SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();
    using (SqlCommand cmd = new SqlCommand("SELECT transactieID, opdrachtID, medewerkerID, soort, datum, bedrag FROM Financien", connection))
    using (SqlDataReader transactieinformatie = cmd.ExecuteReader())
    {
        List<Transactie> transacties = new List<Transactie>();
        while (transactieinformatie.Read())
        { 
                string transactieID = transactieinformatie["transactieID"].ToString();
                string opdrachtID = transactieinformatie["opdrachtID"].ToString();
                string medewerkerID = transactieinformatie["medewerkerID"].ToString();
                string soort = transactieinformatie["soort"].ToString();
                string datum = transactieinformatie["datum"].ToString();
                string bedrag = transactieinformatie["bedrag"].ToString();
                Transactie transactie = new Transactie(transactieID, opdrachtID, medewerkerID, soort, datum, bedrag);
                transacties.Add(transactie);
        }
        return transacties;
    }
}

I've therefore removed the connection close from the loop (so you can continue reading more information), and now return the list outside the loop.

Thanks a lot this works great! Also thankyou for giving an explanation on the 'using' statements. — Max, Dec 15 '18 at 13:12
@Max Generally if something implements `IDisposable` (has `.Dispose()`) you should wrap them in a `using` statement when you use them. You can read more about `using` [in this question](https://stackoverflow.com/questions/212198/what-is-the-c-sharp-using-block-and-why-should-i-use-it). — ProgrammingLlama, Dec 15 '18 at 13:13

score 0 · Answer 2 · answered Dec 15 '18 at 13:13

Delete every line of code after your SqlCommand one (so sqldatareader and onwards) and replace it with:

DataTable dt = new DataTable();
SqlDataAdapter da = new SqlDataAdapter(cmd);
da.Fill(dt);

You will now have a DataTable object representing your database data (all of it). You can iterate over it as a set of rows and work with the data

Shortly after this you should consider reading an Entity Framework tutorial (by Microsoft) and work towards ignoring this method of getting data out of a database entirely (for now; feel free to revisit it when you become more proficient, but it's a very low level way of working and not conducive to writing good, well encapsulated, robust and performant code, especially as a newbie) ..

score 0 · Answer 3 · answered Dec 15 '18 at 13:35

Actually, when you return in while statement, the loop has been broken, you must consider it and also handling closing the connection. You need to change a little on your code, follow as:

public List<Transactie> FillTransacties()
{
    try
    {
        SqlConnection connection = new SqlConnection(connectionString);
        connection.Open();

        SqlCommand cmd = new SqlCommand("SELECT transactieID, opdrachtID, medewerkerID, soort, datum, bedrag FROM Financien", connection);
        SqlDataReader transactieinformatie = cmd.ExecuteReader();

        List<Transactie> transacties = new List<Transactie>();

        while (transactieinformatie.Read())
        { 
                string transactieID = transactieinformatie["transactieID"].ToString();
                string opdrachtID = transactieinformatie["opdrachtID"].ToString();
                string medewerkerID = transactieinformatie["medewerkerID"].ToString();
                string soort = transactieinformatie["soort"].ToString();
                string datum = transactieinformatie["datum"].ToString();
                string bedrag = transactieinformatie["bedrag"].ToString();
                Transactie transactie = new Transactie(transactieID, opdrachtID, medewerkerID, soort, datum, bedrag);
                transacties.Add(transactie);  
        }
        return transacties;
    }
    catch (InvalidCastException ICE)
    {
        MessageBox.Show("De data in de database is incorrect", ICE.Message);
        return new List<Transactie>();
    }
    catch (Exception e)
    {
        MessageBox.Show("Er is een onbekende error opgetreden.", e.Message);
        return new List<Transactie>();
    }
    finally
    {
        connection.Close();
    }
}

Not all code paths return a value while loop

3 Answers3