Missing commas from a string

Question

I'm trying to write a custom query generator for a small database that I'm making, but the comma that should appear in between all the entries to the string aren't appearing only the one at the end is.

private void BTN_advancedSearch_Click(object sender, EventArgs e)
    {
        // Creates the variable part of the custom query
        string customwhereclause = "";

        if (CHK_enableGameName.Checked == true)
        {
            Connectqry(customwhereclause);
            customwhereclause += "Game.GameName LIKE '%" + TXT_addGame.Text + "%'";
        }

        if (CHK_enableGenreName.Checked == true)
        {
            Connectqry(customwhereclause);
            customwhereclause += "Genre.GenreID =" + genreID + "";
        }

        if (CHK_enableConsoleName.Checked == true)
        {
            Connectqry(customwhereclause);
            customwhereclause += "Console.ConsoleID =" + consoleID + "";
        }

        if (CHK_enablePlayers.Checked == true)
        {
            Connectqry(customwhereclause);
            customwhereclause += "Game.Players >=" + NUD_players.Value + "";
        }
        if (CHK_enableDisc.Checked == true)
        {
            if (CHK_discOwned.Checked == true)
            {
                Connectqry(customwhereclause);
                customwhereclause += "Game.Disc ='" + "yes" + "'";
            }
            else
            {
                Connectqry(customwhereclause);
                customwhereclause += "Game.Disc ='" + "no" + "'";
            }
         }
         if (CHK_enableCompleted.Checked == true)
         {
            if (CHK_completed.Checked == true)
            {
                Connectqry(customwhereclause);
                customwhereclause += "Game.Completed ='" + "yes" + "'";
            }
            else
            {
                Connectqry(customwhereclause);
                customwhereclause += "Game.Completed ='" + "no" + "'";
            }
        }

        //varible query code being passed back to search form.
         frm_search.Cstmqry = customwhereclause;

        //close the form and reopen the other one.
         this.Close();
         frm_search.Show();
    }

    private void Connectqry(string s)
    {
        if (s == "")
        {
            Console.WriteLine("the query is blank");
        }
        else
        {
            s = s + " , ";
            Console.WriteLine(s);
        }
    }

the output is currently this:

the query is blank

Game.GameName LIKE '%name%' ,

Game.GameName LIKE '%name%'Genre.GenreID =0 ,

Game.GameName LIKE '%name%'Genre.GenreID =0Console.ConsoleID =0 , 

Game.GameName LIKE '%name%'Genre.GenreID =0Console.ConsoleID =0Game.Players >=1 ,

Game.GameName LIKE '%name%'Genre.GenreID =0Console.ConsoleID =0Game.Players >=1Game.Disc ='no' ,

I'm not sure why it's removing the commas that be in between the string.

You should return the new string (s) from Connectqry and add this to customwhereclause before the rest of the statement. — Tjasun, Sep 23 '15 at 12:08
It's not removing the commas. They're there at the end, exactly as you wrote: `s = s + " , "` If you want commas between the predicates, well then, you'd better code that in, because `BTN_advancedSearch_Click` does not add commas at all. — Rob, Sep 23 '15 at 12:08
You might also want to consider using `string.Join` instead. Or at least consider using a `StringBuilder` versus string concatenation. — juharr, Sep 23 '15 at 12:15
This post may help; http://stackoverflow.com/questions/10792603/how-are-strings-passed-in-net — Serif Emek, Sep 23 '15 at 12:23
Code for me is poor readable. 1 ` if (CHK_enableCompleted.Checked == true)` is reedundant, give ` if (CHK_enableCompleted.Checked )` 2. method `Connectqry` name is misleading — Jacek Cz, Sep 23 '15 at 13:24
**Warning: your code is open to *SQL Injection Attacks***. Rather than embedding parameter values in query strings, you should **always** construct parameterized queries. See: [How do parameterized queries help against SQL injection?](https://stackoverflow.com/q/5468425/3744182) and [Why do we always prefer using parameters in SQL statements?](https://stackoverflow.com/q/7505808/3744182). — dbc, Jun 09 '22 at 15:11

score 0 · Accepted Answer · edited May 23 '17 at 11:51

0

You should add the code:

if (!string.IsNullOrEmpty(customwhereclause))
{
    customwhereclause += " AND ";
}
customwhereclause += // Your condition

in all your conditions. It'll add an AND operator everywhere it's necessary.

Even better:

private static string computeCondition(string current, string newCondition)
{
    if (!string.IsNullOrEmpty(current))
    {
        current += " AND ";
    }
    return current + newCondition;
}

private void BTN_advancedSearch_Click(object sender, EventArgs e)
{
    // Creates the variable part of the custom query
    string customwhereclause = "";

    if (CHK_enableGameName.Checked == true)
    {
        Connectqry(customwhereclause);

        customwhereclause = computeCondition(customwhereclause, "Game.GameName LIKE '%" + TXT_addGame.Text + "%'");
    }
    ...

To avoid too big code dup

Or even better:

private void BTN_advancedSearch_Click(object sender, EventArgs e)
{
    // Creates the variable part of the custom query
    List<string> whereClausesList = new List<string>();

    if (CHK_enableGameName.Checked == true)
    {
        Connectqry(customwhereclause);

        whereClausesList.Add("Game.GameName LIKE '%" + TXT_addGame.Text + "%'");
    }
    ...
    string.Join(" AND ", whereClausesList);

as suggested by Rob

edited May 23 '17 at 11:51

Community

1
1

answered Sep 23 '15 at 12:10

Thomas Ayoub

29,063
15
95
142

Just add ! before string.IsNullOrEmpty to negate this statement. – Tjasun Sep 23 '15 at 12:11
Shouldn't that be `if (!string.IsNullOrEmpty(customwhereclause))` since you want to add it if you already have a condition in there... – komodosp Sep 23 '15 at 12:11
1

Didn't the OP want commas, not AND? – juharr Sep 23 '15 at 12:13
1

A tidier way (and most likely more performant) would be to store the clauses in a list, and at the end return `string.Join(" AND ", clauses);` - rather than checking if the expression is empty or not each time – Rob Sep 23 '15 at 12:14
@juharr that's true, but I doubt that sql condition can be concatenated with comas – Thomas Ayoub Sep 23 '15 at 12:18

score 0 · Answer 2 · answered Sep 23 '15 at 12:25

Your code is not working because string is immuteable. When you do string concatenation like s = s + " , "; this is not updating the object that s references. It's creating a new string and assigning the reference to s. And because you don't pass s as a ref you are only updating a local copy of the reference and not the original. The correct way to fix that is to return the new string and assign it.

private string Connectqry(string s)
{
    if (s == "")
    {
        Console.WriteLine("the query is blank");
    }
    else
    {
        s = s + " , ";
        Console.WriteLine(s);
    }

    return s;
}

and use it like

customwhereclause = Connectqry(customwhereclause);

As other's have mentioned you probably want to use "AND" instead of commas, and using string.Join or StringBuilder would likely be more efficient, but string being immutable and string concatenation creating a new string is why your current code doesn't do what you expect.

Missing commas from a string

2 Answers2