3
\$\begingroup\$
public  delegate DataTable loadDataTable();

DataTable shops = (cmbShop.Text == "All Shops") ? 
      new loadDataTable(() =>
       {
       Program.con.GET_Table_From_DataBase("sh", "select * from shops ");
       return Program.con.dst.Tables["sh"];
       }
      ).Invoke()
     :
       new loadDataTable(() =>
       {
        Program.con.GET_Table_From_DataBase("sh", "select * from shops where shopname='" +  cmbShop.Text + "' ");
        return Program.con.dst.Tables["sh"];
       }
  ).Invoke();

I am just setting the value of DataTable shops here.

I am learning about lambda expressions, so just for learning purposes, I want to know if this code can be shortened while using lambda expressions.

\$\endgroup\$
6
  • 4
    \$\begingroup\$ Why on earth would you use a lambda expression here? \$\endgroup\$
    – SLaks
    Jun 14 '11 at 13:28
  • 7
    \$\begingroup\$ You have a SQL injection vulnerability. \$\endgroup\$
    – SLaks
    Jun 14 '11 at 13:28
  • 2
    \$\begingroup\$ Any code which has combobox.Text near GetTableFromDatabase can be optimized \$\endgroup\$
    – Snowbear
    Jun 14 '11 at 18:06
  • \$\begingroup\$ @Slakes will you please explain(You have a SQL injection vulnerability) \$\endgroup\$
    – Zain
    Jun 15 '11 at 5:25
  • 2
    \$\begingroup\$ xkcd.com/327 en.wikipedia.org/wiki/SQL_injection \$\endgroup\$
    – SLaks
    Jun 15 '11 at 13:10
11
\$\begingroup\$

You don't need any lambda expressions at all.

You can write

if (cmbShop.Text == "All Shops")
    Program.con.GET_Table_From_DataBase("sh", "select * from shops ");
else
    Program.con.GET_Table_From_DataBase("sh", "select * from shops where shopname='" +  cmbShop.Text + "' ");

DataTable shops = Program.con.dst.Tables["sh"];
\$\endgroup\$
2
  • 1
    \$\begingroup\$ You can move everything except second parameter out of the if statement. \$\endgroup\$
    – Snowbear
    Jun 14 '11 at 18:04
  • \$\begingroup\$ no {} - this needs a review ;-P \$\endgroup\$
    – t3chb0t
    Aug 28 '16 at 7:34
1
\$\begingroup\$

Using lambda expressions here are a bit of overkill.

Though I think this goes overboard (imho) on the ternary operator:

 Program.con.GET_Table_FromDataBase("sh", string.Format("select * from shops{0}", 
                 (cmbShop.Text == "All Shops") ? string.Empty : string.Format(" where shopname='"+cmbShop.Text+"'"));

As others stated, I would reconsider how you are fetching your data. The current method might leave you exposed for a world of hurt. :) Changing your approach on data access will, more than likely, also change how you are building your queries so a lot of this would be moot.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.