0

Method that I use for handling ExecuteScalar Postgresql queries:

public T ExecuteScalar<T>(string sql, CommandType commandType, List<NpgsqlParameter> parameters)
    {
        using (NpgsqlConnection conn = Konekcija_na_server.Spajanje("spoji")) 
        {
            return Execute<T>(sql, commandType, c =>
            {
                var returnValue = c.ExecuteScalar(); //The Connection is not open.
                return (returnValue != null && returnValue != DBNull.Value && returnValue is T)
                 ? (T)returnValue 
                 : default(T); 
            }, parameters);

        }        
    }

"The Connection not open" comment is where it happends, I dont understand why I dont have connection inside, so can someone be so kind to explane me whats happening?

Execute method:

T Execute<T>(string sql, CommandType commandType, Func<NpgsqlCommand, T> function, List<NpgsqlParameter> parameters)
    {
        using (NpgsqlConnection conn = Konekcija_na_server.Spajanje("spoji")) 
            using (var cmd = new NpgsqlCommand())
            {
                cmd.CommandText = sql; 
                cmd.CommandType = commandType; 
                if (parameters.Count > 0 ) 
                {
                    foreach (var parameter in parameters) 
                    {
                        cmd.Parameters.AddWithValue(parameter.ParameterName,parameter.Value); 
                    }
                }
                Konekcija_na_server.Spajanje("prekini");
                return function(cmd); 
            }

        }

    }

My connection class:

class Konekcija_na_server
{
    public static string Connectionstring = "Server=127.0.0.1;Port=5433;User Id=postgres;" +
    "Password=*********;Database=postgres;Pooling=false;";

    public static NpgsqlConnection Spajanje(string konekcija)
    {
        bool spajanje = false;

        NpgsqlConnection conn = new NpgsqlConnection(Connectionstring);
        if (konekcija == "spoji")
        {             
            conn.Open();
            spajanje = true;
        }
        else if (konekcija == "prekini")
        {
            conn.Close();              
        }

        if (spajanje == true)
        {
            return conn;                
        }
        else return null;

    }
Naveen DA
  • 4,148
  • 4
  • 38
  • 57
Marko Petričević
  • 333
  • 2
  • 9
  • 20
  • @mjwills Nothing, coz *c* is getting declared there, so when u write *conn* ur declaring it again and we all know thats an error – Marko Petričević Oct 07 '17 at 22:32
  • 1
    Please include the source code for your `Execute` method - since the bug is in the interaction between your `Execute` and `ExecuteScalar`. – mjwills Oct 07 '17 at 22:36
  • It looks like that you are not assigning connection to command. You need to replace: using (var cmd = new NpgsqlCommand()) with using (var cmd = new NpgsqlCommand(sql, conn)) inside Execute method and also remove cmd.CommandText = sql; line below – Ivan Milosavljevic Oct 07 '17 at 23:09
  • @IvanMilosavljevic seems like it works, but can u explane why and make an answer of it? – Marko Petričević Oct 07 '17 at 23:24

1 Answers1

2

NpgsqlCommand is missing Connection object. In order to execute query command (NpgsqlCommand) needs to know what SQL it will execute (CommandText), what type of command it is (CommandType = Text, Procedure) and Connection.

In lines below you have made Command and assigned Text and Command Type but you are missing Connection.

using (var cmd = new NpgsqlCommand())
cmd.CommandText = sql; 
cmd.CommandType = commandType;  

So correct implementation would be:

T Execute<T>(string sql, CommandType commandType, Func<NpgsqlCommand, T> function, List<NpgsqlParameter> parameters)
    {
        using (NpgsqlConnection conn = Konekcija_na_server.Spajanje("spoji")) 
            using (var cmd = new NpgsqlCommand(sql, conn))
            {
                cmd.CommandType = commandType; 
                if (parameters.Count > 0 ) 
                {
                    foreach (var parameter in parameters) 
                    {
                        cmd.Parameters.AddWithValue(parameter.ParameterName,parameter.Value); 
                    }
                }
                Konekcija_na_server.Spajanje("prekini");
                return function(cmd); 
            }

        }

    }

Also, I have noticed that you are calling Konekcija_na_server.Spajanje("prekini"); to close SQL connection but instead of closing existing connection you are initializing new NpgsqlConnection and then you are closing that new connection. You already have connection inside using block using (NpgsqlConnection conn = Konekcija_na_server.Spajanje("spoji")) meaning that when you are exiting that block your connection will be automatically closed

Correct implementation would be:

public static NpgsqlConnection Spajanje()
{
    var conn = new NpgsqlConnection(Connectionstring);

    conn.Open();
    return conn;

}
Ivan Milosavljevic
  • 839
  • 1
  • 7
  • 19
  • thanks for that answer, I was hoping someone would also comment that connection class, and u did. So ur saying to me that by using `using` statement Im closing connection without even having to to call `Konekcija_na_server.Spajanje("prekini")`. Point of that class is that I have some other classes where I dont use `using` statement, so I have to close connection somehow. Having that in mind, do you have any solution how to edit that class for both cases? – Marko Petričević Oct 08 '17 at 07:46
  • Manual closing of connection will probably end badly if this is large project or project that will grow over time. You might have control of what methods are currently closing connection manually but as application grows it can end with undesirable effects ... been there done that. The short answer is that you should use "Unit of Work" pattern in your case. You can ask for more details on https://codereview.stackexchange.com/ – Ivan Milosavljevic Oct 08 '17 at 09:11
  • thanks for your help again, there is just one more thing I can't get my head around. I edited my question and added a problem I have with using ExecuteScalar and its return value. Either I don't see clearly what's happening or im using a wrong implementation for my problem, if you could look at that method and its return values, would be grateful =) – Marko Petričević Oct 08 '17 at 09:15
  • 1
    I suggest to revert question back to original one and opening new question on stackoverflow so that we can have a clean thread. – Ivan Milosavljevic Oct 08 '17 at 09:17
  • https://stackoverflow.com/questions/46629568/executescalar-not-returning-right-values-c-sharp There you go, ones more thanks for the help I appreciate it =) – Marko Petričević Oct 08 '17 at 09:31