Is this method a good aproach to get SQL values from C#?

Question

I have this little method that i use to get stuff from SQL. I either call it with varSearch = "" or varSearch = "something". I would like to know if having method written this way is best or would it be better to split it into two methods (by overloading), or maybe i could somehow parametrize whole WHERE clausule?

private void sqlPobierzKontrahentDaneKlienta(ListView varListView, string varSearch) {
        varListView.BeginUpdate();
        varListView.Items.Clear();
        string preparedCommand;
        if (varSearch == "") {
            preparedCommand = @"
                SELECT t1.[KlienciID],
                CASE WHEN t2.[PodmiotRodzaj] = 'Firma' THEN
                          t2.[PodmiotFirmaNazwa] ELSE 
                          t2.[PodmiotOsobaNazwisko] + ' ' + t2.[PodmiotOsobaImie] END AS 'Nazwa'
                FROM [BazaZarzadzanie].[dbo].[Klienci] t1
                INNER JOIN [BazaZarzadzanie].[dbo].[Podmioty] t2
                ON t1.[PodmiotID] = t2.[PodmiotID]
                ORDER BY t1.[KlienciID]";
        } else {
            preparedCommand = @"
                SELECT t1.[KlienciID],
                CASE WHEN t2.[PodmiotRodzaj] = 'Firma' THEN
                          t2.[PodmiotFirmaNazwa] ELSE 
                          t2.[PodmiotOsobaNazwisko] + ' ' + t2.[PodmiotOsobaImie] END AS 'Nazwa'
                FROM [BazaZarzadzanie].[dbo].[Klienci] t1
                INNER JOIN [BazaZarzadzanie].[dbo].[Podmioty] t2
                ON t1.[PodmiotID] = t2.[PodmiotID]
                WHERE t2.[PodmiotOsobaNazwisko] LIKE @searchValue OR t2.[PodmiotFirmaNazwa] LIKE @searchValue OR t2.[PodmiotOsobaImie] LIKE @searchValue
                ORDER BY t1.[KlienciID]";
        }
        using (var varConnection = Locale.sqlConnectOneTime(Locale.sqlDataConnectionDetails))
        using (SqlCommand sqlQuery = new SqlCommand(preparedCommand, varConnection)) {
            sqlQuery.Parameters.AddWithValue("@searchValue", "%" + varSearch + "%");
            using (SqlDataReader sqlQueryResult = sqlQuery.ExecuteReader())
                if (sqlQueryResult != null) {
                    while (sqlQueryResult.Read()) {
                        string varKontrahenciID = sqlQueryResult["KlienciID"].ToString();
                        string varKontrahent = sqlQueryResult["Nazwa"].ToString();
                        ListViewItem item = new ListViewItem(varKontrahenciID, 0);
                        item.SubItems.Add(varKontrahent);
                        varListView.Items.AddRange(new[] {item});
                    }
                }
        }
        varListView.EndUpdate();
    }

Answer 1

The better approach would actually be to use a stored procedure instead of hardcoding SQL into your application. You can pass the where clause parameter to your stored procedure and handle the logic on the database side.

This approach also offers the advantage that if you need this logic in another application (like a JAVA app, for example) the logic is centralized in the database, so you don't have to rewrite it again.

Answer 2

I am sure there are better approaches to this, like using a Stored procedure, But if you really need to do this You need:

     string preparedCommandTemplate = @"
                        SELECT t1.[KlienciID],
                        CASE WHEN t2.[PodmiotRodzaj] = 'Firma' THEN
                                  t2.[PodmiotFirmaNazwa] ELSE 
                                  t2.[PodmiotOsobaNazwisko] + ' ' + t2.[PodmiotOsobaImie] END AS 'Nazwa'
                        FROM [BazaZarzadzanie].[dbo].[Klienci] t1
                        INNER JOIN [BazaZarzadzanie].[dbo].[Podmioty] t2
                        ON t1.[PodmiotID] = t2.[PodmiotID] {0}
                        ORDER BY t1.[KlienciID]";
string whereClause="WHERE t2.[PodmiotOsobaNazwisko] LIKE @searchValue OR t2.[PodmiotFirmaNazwa] LIKE @searchValue OR t2.[PodmiotOsobaImie] LIKE @searchValue"

 if (string.Emtpy.Equals(varSearch )) {
            preparedCommand = string.Format(preparedCommandTemplate,string.Empty)
        } else {
            preparedCommand = string.Format(preparedCommandTemplate,whereCaluse)

        }

Answer 3

If you don't want to use a stored procedure, at least parameterize the where clause. This kind of code can quickly get out of hand. I would also consider using a free text index or something like Lucene.NET to implement this kind of "like" search across multiple fields.

Answer 4

Sucks on MULTIPLE levels:

• NO DAL at all - this means your SQL code is plastered over all the forms. Terrible maintenance - put at least all the SQL handling into one class.

• It is a lot of manually writen code, as such it is bad performance wise (as in: programmer performance). Look at BLToolkit on how you can have all the code GENERATED at RUNTIME (from an attribuite with the SQL and an abstract method - the subclass with the real method is bytecode generated).

That said, unless I can convince you to use a real data access layer / ORM like NHibernate.

FOr 1 I would fire you as programmer (welcome back as trainee). Having SQL in forms just is not my idea of spending time when reworking a database - as such, it is not testable and a pain to maintain. THis is btw. ,not ".net specific" - isolating SQL is something I did 20 years ago (nearly) in smalltalk and C++ already ;)

For 2 I would - well - it would not happen due to guidelines ;)

Answer 5

Given that you want to continue using hard-coded SQL statements in your code (and not switch to LINQ2SQL or Entity Framework or another ORM tool), the one thing you definitely do not want to do is add the where clause as a parameter to your method (if that's what you mean by 'parameterize the where clause'). This makes the clients that use this method dependent on your data access technology (in this case, a SQL database).

Compare the following two calls:

sqlPobierzKontrahentDaneKlienta(lv, "something");

and

sqlPobierzKontrahentDaneKlienta(lv,
    "WHERE t2.[PodmiotOsobaNazwisko] LIKE '%something%' OR " +
    "      t2.[PodmiotFirmaNazwa] LIKE '%something%' OR " +
    "      t2.[PodmiotOsobaImie] LIKE '%something%'")

Which one looks better?