0

Hi everyone I have a problem with the input string, I think I know the cause of this problem but I can't resolve it.

I have a database ,I'm using a request that return null ,because the table is empty. The null I return it as a double. So i want to make a condition that verify if it is null or not.

This is my method:

 public static double CREDIT()
    {
        double Total;
        try
        {
            CLSERVICES.CON.Open();
            string req = "select SUM(F.Reste)from RELATION as R , Facture as F where R.NRelation = F.Relation";
            SqlCommand cmd = new SqlCommand(req, CLSERVICES.CON);
            SqlDataReader dr;
            dr = cmd.ExecuteReader();
            if (dr.Read())
            {
                Total= double.Parse(dr.GetValue(0).ToString());
            }
            else
            {
                Total= 0;
            }
            dr.Close();
            CLSERVICES.CON.Close();
            return Total;
        }
        catch (SqlException E)
        {
            MessageBox.Show(E.Message, "<!!!>", MessageBoxButtons.OK, MessageBoxIcon.Error);
            return 0;
        }
    }
Liam
  • 27,717
  • 28
  • 128
  • 190
Thanatos
  • 21
  • 6
  • You really really need to put some `using` statements around your objects to ensure you don't get memory leaks – Liam Oct 10 '17 at 09:24
  • `GetValue()` will return `object` that may be `null` or `DbNull`. You need to test for these before calling `ToString()` and certainly before attempting a cast to `double` – DiskJunky Oct 10 '17 at 09:25
  • 2
    Possible duplicate of [SQL Data Reader - handling Null column values](https://stackoverflow.com/questions/1772025/sql-data-reader-handling-null-column-values) – Liam Oct 10 '17 at 09:25
  • `Total= double.Parse(dr.GetValue(0).ToString());` => is this really returning numeric value? `double.Parse` will throw error if the parsed string is not numeric or decimal representation. – Tetsuya Yamamoto Oct 10 '17 at 09:25
  • the dr.getvalue(0) is null for now , because the table is empty . – Thanatos Oct 10 '17 at 09:28

4 Answers4

3

Here is a better way to write your method:

public static double CREDIT()
{
    try
    {
        string req = "select SUM(F.Reste) "+
                        "from RELATION as R "+
                        "inner join Facture as F "+
                        "ON R.NRelation = F.Relation";
        using (var con = new SqlConnection(ConnectionString))
        {
            using (var cmd = new SqlCommand(req, con))
            {
                con.Open();
                var value = cmd.ExecuteScalar();
                double total;
                if (value != null && value != DBNull.Value && double.TryParse(value.ToString(), out total))
                {
                    return total;
                }
            }
        }
    }
    catch (SqlException E)
    {
        MessageBox.Show(E.Message, "<!!!>", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
    return 0;
}

Points of interest:

  • Using a local connection object, so that it can be closed and disposed as soon as possible.
  • Wrap all IDisposable instances in a using statement
  • Use ExecuteScalar() instead of ExecuteReader() since you only want to get a single scalar value
  • Use double.TryParse instead of double.Parse
  • Break sql to lines for better readability
  • Changed the implicit inner join to an explicit one
Zohar Peled
  • 79,642
  • 10
  • 69
  • 121
0

You could:

change your double.Parse to double.TryParse (which is something you should always do when parsing a number)

and/or

test for null using https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqldatareader.isdbnull(v=vs.110).aspx

This is how to use the isDBNull:

if (!dr.IsDBNull(0))
{
    Total = double.Parse(dr.GetValue(0).ToString());
}

and probably consider making your method return a double? instead of a double if it might be null

also, another thing, think about using ExecuteScalar instead of ExecuteReader

mikelegg
  • 1,197
  • 6
  • 10
-1

your problem comes from not checking for null in this line :

Total= double.Parse(dr.GetValue(0).ToString());

dr.GetValue(0) could be null or not even a double so you could do something like this:

var val = dr.GetValue(0);

if ( val != null )
{
    Double.TryParse(val.ToString(), out Total);
}
Andrei Dragotoniu
  • 6,155
  • 3
  • 18
  • 32
  • There's not point using TryParse and checking for null. Also what happens if TryParse fails here? You never check the result. – Liam Oct 10 '17 at 09:41
  • @T.Wassim I would strongly recommend that you look at this [other answer](https://stackoverflow.com/a/46663456/542251) it handles this in a much cleaner way and addresses many of the other issues in your code like the lack of `using`s. – Liam Oct 10 '17 at 09:44
  • @liam i m trying to write the code as zohar says , and for this answer i use an else to make total =0 and it solved there is no problem att my app now thanks for ur help :) :D – Thanatos Oct 10 '17 at 09:52
  • Ok @T.Wassim but your code suffers from [memory leaks](https://stackoverflow.com/questions/5020814/memory-leaks-c-sharp). This **will** cause problems. – Liam Oct 10 '17 at 09:54
  • @Liam .. how i can fix this memory leaks ? .. thiis is just a methode in a class that has a destructor – Thanatos Oct 10 '17 at 10:04
  • @WASSIM.Tofane I'm not sure how many different ways I can tell you this... use `using` statements, as demonstrated by the [other answer](https://stackoverflow.com/a/46663456/542251)... – Liam Oct 10 '17 at 10:21
-1
Total = Convert.ToDouble(dr.GetValue(0));

use above line instead of Double.Parse() method and that will help you out.

Shahzad Khan
  • 432
  • 2
  • 14
  • it returns null...this will just throw an exception. – Liam Oct 10 '17 at 09:45
  • Hey, @Liam if you are using the double.parse() method and value is null then return the exception but for Convert.ToDouble () method if your value is null then always return the 0. – Shahzad Khan Oct 10 '17 at 09:50