How to fix this code to bring all items from database ?

Question

1.00/5 (1 vote)

See more:

VB

Dim btnsender As Button = sender
           Dim lbl As String = btnsender.Name
            Try
                da = New SqlDataAdapter("Select Item_id, Item_menu_description From tbl_item where Item_Cat = " & lbl & "", cn)
                da.Fill(dt1)

                keycount = -1
                For Each btn As Button In FlowLayoutPanel2.Controls()
                    keycount += 1
                    btn.Text = dt1.Rows(keycount)(1).ToString
                   ' btn.Tag = dt1.Rows(keycount)(0).ToString
                Next
            Catch ex As Exception
            Finally
                cn.Close()
                da = Nothing
                dt1.Clear()
            End Try

What I have tried:

I have 5 buttons named : btn1,btn2,btn3,btn4,btn5 and i trying to bring all of the Item_menu_description to the buttons 
The table in database : Item_id,Item_menu_description,Item_Cat,btnName
    Item_id | Item_menu_description | Item_Cat | btnName
-----------------------------------------------------------
       1          keyboard            computer     btn3
       2          Mouse               computer     btn1
       3          screens             computer     btn4

I want the Item_menu_description to appear on the same names as the buttons in the database, for example the 'keyboard' appears on the button Named 'btn3' and 'Mouse' appears on the button Named 'btn1'


Please help me discover the error in the code

Thank you so much

Posted 24-Sep-17 0:15am

Member 13187018

Updated 26-Sep-17 10:59am

Add a Solution

Comments

Richard MacCutchan 24-Sep-17 9:35am

"I have 5 buttons named : btn1,btn2,btn3,btn4,btn5"
That is really lazy programming; you should give all your controls meaningful names so anyone who has the job of maintaining your code can get an idea of what they are for.

You are also using string concatenation to build your SQL statements which leaves you vulnerable to SQL injection, and the destruction of your database.

Member 13187018 24-Sep-17 13:34pm

Thank you Richard for your advice, but do you have a solution to the problem ?

Richard MacCutchan 24-Sep-17 13:41pm

What problem? You have not described what the problem actually is. I notice also that you catch Exceptions but then ignore them - another bad practice.

Richard MacCutchan 24-Sep-17 18:32pm

Maybe you could try a proper question.

3 solutions

Solution 1

There is several ways you could fix this, the simplest is to order your results

VB

da = New SqlDataAdapter("Select Item_id, Item_menu_description From tbl_item where Item_Cat = '" & lbl & "' order by btnName", cn)

You will run into issues if you have more than 9 buttons and use btn1 to btn10 as their names - the query will sort the results via the text value, hence btn10 would come before btn2 in the result set.
An alternative is to do as follows;

VB

' Edit Query to include the btnName field
da = New SqlDataAdapter("SELECT Item_id, Item_menu_description, btnName FROM tbl_item WHERE Item_Cat = '" & lbl & "'")
For Each btn As Button In FlowLayoutPanel2.Controls()
    Dim foundRows() AS DataRow 
    ' Locate the specific row
    foundRows = dt1.Select("btnName = '" & btn.Name & "'")
    If foundRows.Length > 0 Then
        btn.Text = dt1.Rows(keycount)(1).ToString
        ' btn.Tag = dt1.Rows(keycount)(0).ToString
    End If
Next

Please take the time to appreciate comments by senior members of the community - they are trying to help

Kind Regards

Posted 25-Sep-17 14:49pm

an0ther1

Updated 25-Sep-17 18:13pm

v2

Comments

Member 13187018 25-Sep-17 21:24pm

Thank you, Sir, I did not mean to insult Mr.Richard, but his answer contained some insult and contempt.
Anyway I apologize and I've deleted my comments
Thank you very much

an0ther1 26-Sep-17 0:11am

No problem, just trying to assist.
If this answers your question please accept it as a solution

Kind Regards

Member 13187018 26-Sep-17 9:46am

Thanks a lot but the code does not work well
I think he needs some modification
Please if you have some time to modify it

thank you again

Solution 2

VB

da = New SqlDataAdapter("Select Item_id, Item_menu_description From tbl_item where Item_Cat = " & lbl & "", cn)

Not a solution to your question, but another problem you have.
Never build an SQL query by concatenating strings. Sooner or later, you will do it with user inputs, and this opens door to a vulnerability named "SQL injection", it is dangerous for your database and error prone.
A single quote in a name and your program crash. If a user input a name like "Brian O'Conner" can crash your app, it is an SQL injection vulnerability, and the crash is the least of the problems, a malicious user input and it is promoted to SQL commands with all credentials.
SQL injection - Wikipedia[^]
SQL Injection[^]
SQL Injection Attacks by Example[^]
PHP: SQL Injection - Manual[^]
SQL Injection Prevention Cheat Sheet - OWASP[^]

Posted 25-Sep-17 16:14pm

Patrice T

Updated 25-Sep-17 16:19pm

v2

Comments

Member 13187018 26-Sep-17 9:43am

Dear Sir, I often use stored procedures in my projects to avoid SQL Injection Attacks but this code is only an experiment then it will be modified to stored procedures

Thank u very much for advice

Richard Deeming 26-Sep-17 15:26pm

Stored procedures offer zero defence against SQL Injection unless you use properly parameterized queries!

da = New SqlDataAdapter("Select Item_id, Item_menu_description From tbl_item where Item_Cat = @Item_Cat", cn)
da.SelectCommand.Parameters.AddWithValue("@Item_Cat", lbl)

Member 13187018 27-Sep-17 15:04pm

Thank you so much Mr.Richard Deeming, I will try to learn more about the protection from SQL Injection .

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Member 13187018 · Accepted Answer · 2017-09-26T10:59:00

Thank god
Problem solved, thanks for everyone who helped me solve the code problem
Thanks to all

VB

<pre>    dt1.Clear()
        Try
            da = New SqlDataAdapter("SELECT Item_id, Item_menu_description, btnName FROM tbl_item WHERE Item_Cat = '" & 1 & "'", cn)
            For Each btn As Button In FlowLayoutPanel2.Controls()
                Dim foundRows() As DataRow
                da.Fill(dt1)
                foundRows = dt1.Select("btnName = '" & btn.Name & "'")
                If foundRows.Length > 0 Then
                    keycount += 1
                    btn.Text = foundRows(0).Item("Item_menu_description").ToString
                End If
            Next
        Catch ex As Exception
        Finally
            cn.Close()
            da = Nothing
            dt1.Clear()
        End Try

How to fix this code to bring all items from database ?

3 solutions

Solution 1

Solution 2

Solution 3

Add your solution here

Preview 0

Existing Members

...or Join us