Exiting early

[Stephen_Thomas]

[quote=153886:@Rick Araujo]Seems obvious for some, but let’s go. Your example:

[code]
Function f() As Integer

Dim ResultCode As Integer

If Condition = True Then
ResultCode = 0 // here it continues after “End If”
Else
ResultCode = 1 // here it continues after “End If”
End If

Return ResultCode // We still “in” the function after the IF THEN.
End Function
[/code][/quote]

This is not what I asked. This runs through to the end of the If… Then… Else. What I asked is if I could leave the If… Then… Else but without leaving the function.

In a system I used some time ago (Can’t remember which now), it had an ‘Exit If’ statement. It looks misleading because of the If part of it, which suggests it was a conditional exit, when in fact it didnt use any condition at all. It simply made the system jump outside of the If… Then… Else unconditionally.

Your code above still completes the If… Then… Else as normal.

[Thomas_ROBISSON]

I never write more than one Return in my code. Return is always the last instruction.
Then I always use a variable, and return TheVariable.

I do like that, I don’t say it’s the best way, but it’s mine.

[Rick_Araujo]

@153939:@Stephen Thomas

This is not what I asked. This runs through to the end of the If… Then… Else. What I asked is if I could leave the If… Then… Else but without leaving the function.

If you can write an example of a problem, even using gotos, I can rewrite it as it should be expected.
Usually it involves just refactoring it in a better logical structure, possibly using a nested IF.

[Stephen_Thomas]

Thankyou for the offer Rick, I’ve changed it already now and everything is good.

The original question came about because i was coding to add a new UserID in a database of existing users. It included doing a check to see if the new UserID already existed.

Having supplied the requested UserID to the function, I then used the standard database method to fetch all records from the Users table and do a compare; the ResultCode was set depending on the following conditions :

1. Does the database open correctly (db.Connect)
2. Does the SQL work OK (db.Error)
3. Do any users exist at all (Different action if this is the first ever user to be added)
4. If Users exist, do any of them match the newly supplied User ID

Using Return worked for the most part, but failed in the While… Wend, which itself was within an If… Then… Else.

So I moved the Return statement outside of the While… Wend but it was still within the If… Then… Else. Thats where my original question came from. The code I posted here originally was poor and I put ‘Exit’ in the code without having checked it properly first. Note to self : Check code before posting in future

Having said all that, I have changed the code anyway to be more efficient.

I query the database first using ‘Select Count(*)’, which answers number 3 above. If the result here is zero then no further checking is needed since we know no users exist. At this point I close the RecordSet and Database, then exit gracefully with a ResultCode.

If that result is a non-zero value, I then setup a simple SQLitePreparedStatement, using the newly requested UserID as the search parameter.

A zero result here tells me some users do exist but do not match the supplied one. Of course a non-zero result here means the UserID does indeed exist already.

If either of the above two conditions are met, I can still close the database and recordset then specify the ResultCode cleanly.

Searching the database for the supplied UserID was more efficient overall than looping through every record and doing a compare, even if it means 2 queries instead of one.

The function is currently defined as :

CheckUserNameExists(NewUserID As String) As Integer

And the code :

[code] Dim ResultCode As Integer

Dim UserDatabase As New SQLiteDatabase
UserDatabase.DatabaseFile = GetFolderItem(“MyDatabase.sqlite”)

If UserDatabase.Connect Then

Dim UserCount As RecordSet
UserCount = UserDatabase.SQLSelect("Select Count(*) From Users")

If UserCount.IdxField(1).IntegerValue = 0 Then ' No users currently exist
  
  UserCount.Close
  UserDatabase.Close
  ResultCode = 202
  
Else ' We have users so check if any match our supplied input
  
  Dim CheckMatchingUsers As SQLitePreparedStatement = UserDatabase.Prepare("SELECT * FROM Users WHERE Username =?")
  CheckMatchingUsers.BindType(0, SQLitePreparedStatement.SQLITE_TEXT)
  Dim UserExists As RecordSet = CheckMatchingUsers.SQLSelect(NewUserID)
  
  If UserExists.RecordCount = 0 Then ' The UserID does not already exist
    ResultCode = 203
  Else ' The UserID already exists
    ResultCode = 204
  End If
  
  UserCount.Close
  UserExists.Close
  UserDatabase.Close
  
End If

Else
ResultCode = 201 ’ Unable to find/connect to database
End If

Return ResultCode[/code]

Here I have used ResultCode’s starting at 201 and increasing, so they do not conflict with the SQLite error codes. The only thing left to do is add error checks after the 2 SQL statements. If either of these are non-zero then the SQLite error code will be returned as the ResultCode.

If ayone can suggest further improvements to the above, then please feel free to post them.

[Rick_Araujo]

Not tested. Check how this performs.

  Dim ResultCode As Integer = 0
  
  Dim UserDatabase As New SQLiteDatabase
  UserDatabase.DatabaseFile = GetFolderItem("MyDatabase.sqlite")
  
  If Not UserDatabase.Connect Then
    ResultCode = 201 // Can't connect/find DB
  Else
    
    If UserDatabase.SQLSelect("SELECT 1 FROM Users LIMIT 1").EOF Then
        ResultCode = 202 // There are no users
      
    Else ' We have users so check if any match the NewUserID
      
      Dim CheckMatchingUsers As SQLitePreparedStatement = UserDatabase.Prepare("SELECT 1 FROM Users WHERE Username = ? LIMIT 1")
      CheckMatchingUsers.BindType(0, SQLitePreparedStatement.SQLITE_TEXT)
      
      If CheckMatchingUsers.SQLSelect(NewUserID).EOF Then
        ResultCode = 203 //  The UserID does not already exist
      Else 
        ResultCode = 204 // The UserID already exists
      End If
      
    End If
      
    UserDatabase.Close

  End If
  
  Return ResultCode

[Will_Shank]

I like to keep code ‘flat’ if possible. This mostly applies when there’s one success case but multiple linear checks for invalidity. It relies on returning when you have the necessary info of invalidity and allows you to read down instead of down and in. Compare these psuedo methods…

[code]Function foo() As integer //Single Return

dim result As integer

calcA
if (a) then
calcB
if (b) then
calcC
if (c) then
calcD
if (d) then
result = 0
else
result = -1
end
else
logCfailed
result = -2
end
else
result = -3
end
else
result = -3
end

return result

End Function[/code]

vs

[code]Function foo() As integer //Multiple Returns nested

calcA
if (a) then
calcB
if (b) then
calcC
if (c) then
calcD
if (d) then
return 0
else
return -1
end
else
logCfailed
return -2
end
end
end

return -3

End Function[/code]

vs

[code]Function foo() As integer //Multiple Returns flat

calcA
if (not a) then return -3

calcB
if (not b) then return -3

calcC
if (not c) then
logCfailed
return -2
end

calcD
if (d) then
return 0
else
return -1
end

End Function[/code]

Notice that last block breaks the flat convention which would be

[code] calcD
if (not d) then return -1

return 0[/code]

Either way is fine as long as it’s easy to scan down and see what, where, why.

So I’d write your method like this… (not a db guy so maybe missed something but you can get the idea)

[code]Function CheckUserNameExists(NewUserID As String) As integer

//get db
Dim UserDatabase As New SQLiteDatabase
UserDatabase.DatabaseFile = GetFolderItem(“MyDatabase.sqlite”)

//couldn’t connect
If not UserDatabase.Connect Then return 201

//get users
Dim UserCount As RecordSet
UserCount = UserDatabase.SQLSelect(“Select Count(*) From Users”)

//no users
If UserCount.IdxField(1).IntegerValue = 0 Then
UserCount.Close
UserDatabase.Close
return 202
end

//We have users so check if any match our supplied input
Dim CheckMatchingUsers As SQLitePreparedStatement = _
UserDatabase.Prepare(“SELECT * FROM Users WHERE Username =?”)
CheckMatchingUsers.BindType(0, SQLitePreparedStatement.SQLITE_TEXT)
Dim UserExists As RecordSet = CheckMatchingUsers.SQLSelect(NewUserID)

UserCount.Close
UserExists.Close
UserDatabase.Close

If UserExists.RecordCount = 0 Then return 203 //The UserID does not already exist

return 204 //The UserID already exists

End Function[/code]

[Kem_Tekinay]

I prefer something like this:

if not verifyParams then
  return
end if
if errorCondition then
  return
end if

// If we get here, the params are good and there are no other
// errors that would hinder processing

dim proceed as boolean = true

if proceed then
  // Do some stuff that may change proceed
end if

if proceed then
  // More stuff that may change proceed
end if

…

// Clean up code
if proceed then
  // Things are going swimmingly 
else
  // Not so much
end if

// Finally
return

Nested if’s are avoided, and I know where I am exiting. At each point, I can handle matters according to the state of things.

[Stephen_Thomas]

There are definitely some interesting approaches here. Thankyou to everyone who has replied.

Apologies with regard to the original code; it was late, i was tired, and did it from my head without thinking.