Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
1
 
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.

Actually i am learning Lambda expression.So just for learning purpose i want to know that this code can be shortened while using lambda expression.a

share|improve this question
4  
Why on earth would you use a lambda expression here? – SLaks Jun 14 '11 at 13:28
7  
You have a SQL injection vulnerability. – SLaks Jun 14 '11 at 13:28
2  
Any code which has combobox.Text near GetTableFromDatabase can be optimized – Snowbear Jun 14 '11 at 18:06
@Slakes will you please explain(You have a SQL injection vulnerability) – Zain Jun 15 '11 at 5:25
2  

2 Answers

up vote 8 down vote

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"];
share|improve this answer
1  
You can move everything except second parameter out of the if statement. – Snowbear Jun 14 '11 at 18:04
up vote 1 down vote

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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