Code review needed :)

Hi,
I have a window with a search text field, and a search button.
When the search button is pressed - I need to search my database for matches, then erase previous listbox entries, and then show all the matching results in the same listbox.

I have somehow managed to scrape up the code below, and I would appreciate it if someone could just tell me if the code is correct, or if I am on the right path.

Thank you all in advance.

[code]// ENSURE THE SEARCH FIELD IS NOT EMPTY
if NSSearchField1.text<>"" Then

dim sql as String = "SELECT * FROM Snippets WHERE Title LIKE "+NSSearchField1.text+" order by Title Asc"
dim rs as RecordSet = db.SQLSelect(sql)

if rs <> Nil then
  // EMPTY LISTBOX ROWS
  MainWindow1.Listbox1.deleteAllRows
  
  // LOOP THROUGH RESULTS AND POPULATE LISTBOX
  while not rs.EOF
    MainWindow.Listbox1.AddRow(rs.Field("Title").StringValue)
    rs.MoveNext
  wend
  
  // CLOSE THE RECORDSET
  rs.Close
  
  // CLOSE THE SEARCH WINDOW
  SearchWindow.Close
  
else
  // DISPLAY ERROR MESSAGE
  MsgBox("No matching records found!")
  
  // CLOSE THE SEARCH WINDOW
  SearchWindow.Close
  
end if

else
// DISPLAY ERROR MESSAGE
MsgBox(“Please enter some text before searching!”)

end if[/code]

Hi,
looks effective so far, but I would use PreparedStatements instead and bind the value you’re looking for. By this you would avoid SQL injections.

Also, it looks like you’re only deleting the entries from the listbox if the query was successful. I would do that prior the SQL command. If the result is nil, you should have a nil result, not an old one. Here, you close the window but if you don’t…

Another thing to bear in mind (and it may or not may not be important in this application)… but SQL statements such as “LIKE” (and other comparisons) are CASE SENSITIVE…

Andreas,
you advised me to empty the listbox BEFORE the sql. Does this now look correct?

Dave,
I need case insensitive - is there any way to do this?

I will look into converting this to a prepared statement (after I at least have a working solution) :slight_smile:

[code]// ENSURE THE SEARCH FIELD IS NOT EMPTY
if NSSearchField1.text<>"" Then

// EMPTY LISTBOX ROWS
  MainWindow1.Listbox1.deleteAllRows

// DEFINE THE SQL STATEMENT
dim sql as String = “SELECT * FROM Snippets WHERE Title LIKE “+NSSearchField1.text+” order by Title Asc”
dim rs as RecordSet = db.SQLSelect(sql)

if rs <> Nil then

  // LOOP THROUGH RESULTS AND POPULATE LISTBOX
  while not rs.EOF
    MainWindow.Listbox1.AddRow(rs.Field("Title").StringValue)
    rs.MoveNext
  wend
  
  // CLOSE THE RECORDSET
  rs.Close
  
  // CLOSE THE SEARCH WINDOW
  SearchWindow.Close
  
else
  // DISPLAY ERROR MESSAGE
  MsgBox("No matching records found!")
  
  // CLOSE THE SEARCH WINDOW
  SearchWindow.Close
  
end if

else
// DISPLAY ERROR MESSAGE
MsgBox(“Please enter some text before searching!”)

end if[/code]

I would expect LIKE to be non case sensitive.
And you may want to put % before and after search string as wildcards.

Wildcards are described as missing letters before and after the search string?
I don’t quite understand what is meant by missing letters?

Does that mean if a person wanted to search for Window, but typed in indow by mistake - the word Window would still be found?

Thanks.

yes. Find things without knowing exactly what you want.

Test%

find all words starting with Test

%ing

Find all words ending with ing.

%test%

Find all words containing test.

Ahh - thanks Christian :slight_smile:

Ok - here is what I now have:

[code]// ENSURE THE SEARCH FIELD IS NOT EMPTY
if NSSearchField1.text<>"" Then

// EMPTY LISTBOX ROWS
MainWindow1.Listbox1.deleteAllRows

// DEFINE THE SQL STATEMENT
dim sql as String = “SELECT * FROM Snippets WHERE Title LIKE %”+NSSearchField1.text+"% order by Title Asc"
dim rs as RecordSet = db.SQLSelect(sql)

if rs <> Nil then

  // LOOP THROUGH RESULTS AND POPULATE LISTBOX
  while not rs.EOF
    MainWindow.Listbox1.AddRow(rs.Field("Title").StringValue)
    rs.MoveNext
  wend
  
  // CLOSE THE RECORDSET
  rs.Close
  
  // CLOSE THE SEARCH WINDOW
  SearchWindow.Close
  
else
  // DISPLAY ERROR MESSAGE
  MsgBox("No matching records found!")
  
  // CLOSE THE SEARCH WINDOW
  SearchWindow.Close
  
end if

else
// DISPLAY ERROR MESSAGE
MsgBox(“Please enter some text before searching!”)

end if[/code]

Richard you are going to run into the same problems you had yesterday with '. Change your select to a prepared statement.

Wayne - I am initially trying to learn how to do it this way, and then when I know it is all working correctly and the code is efficient - I will then also convert it as a prepared statement in order to prevent the apostrophe catastrophe :slight_smile:

This way I learn both ways. It’s more of a learning exercise.

After you helped me yesterday, I managed to create 2 other prepared statements alone - which edited and deleted database entries :slight_smile:

I have never tried to search a database yet, so I started it this way and will then proceed to a prepared statement.
Your help was fantastic yesterday :slight_smile:

First of all however, I need to know this looks correct.

Ok.

Put an error check after getting your recordset, this will at least tell you if the db error’s and what the message is.

Wayne,
I will put error checking in, but I now have a strange problem.

How can I possibly get 3 THIS ITEM DOES NOT EXIST errors?

The screenshot below clearly shows that:

A) There is a NSSearchField1 in the SearchWindow

B) There is a listbox1 in the MainWindow SOLVED THIS ERROR - THERE WAS A ROGUE NUMBER 1 AFTER THE WINDOW NAME

C) Once again -There is a NSSearchField1 in the SearchWindow

Screenshot

You can remove SearchWindow. from before nssearchfield1, and take the 1 out after MainWindow, then try again.

By adding error checking I meant

If db.Error Then MsgBox db.Errormessage End If

After dim rs as RecordSet = db.SQLSelect(sql)

Wayne:
I had already noticed the typo so I changed MainWindow1 to MainWindow - that is now fixed.

I had already tried to remove the word SearchWindow - but that made no difference?

I now just get the following errors - all of which involve the NSSearchField in some way?

Screenshot

I don’t have OSX or NSSearchField, but could it be that text is not the correct property?

Oh noooooo :slight_smile:

Guess I will have to hope that Kem or one of the other MacOSLib guys read my post.

Replacing the NSSearchField with a normal TextField now works :slight_smile:

Just need to find out now why the NSSearchField fails?