CODEWITHSUNDEEP
vb.netif-statement
gchq
gchq

Reputation: 1775

If statement is always true

Have the following If statement that should only allow the sub to continue if User_ID = 1 or the User_ID = the author ID.

When debugging I know that the first part is true (the user ID is 1), but it still trips the popup.

Stared at this for a while - must have brain fog as it looks like it should work as intended

If (Not User_ID = 1 OrElse Not User_ID = WL_UserID) Then
    AppBoxValidation("Only the original author can edit this item!")
    Exit Sub
End If

Upvotes: 1

Views: 273

Answers (5)

hunch_hunch
hunch_hunch

Reputation: 2331

If the user ID is 1, Not User_ID =1 will evaluate to false, and the code to the right of the OrElse will be evaluated. See this page on the OrElse operator, specifically the Remarks section.

Upvotes: 0

user2864740
user2864740

Reputation: 62005

If Not User_ID = 1 is false (because User_ID = 1 is false), then Not User_ID = WL_UserID is true. That's all there is to it .. but, the logic error can be seen as follows.

The initial expression,

Not User_ID = 1 OrElse Not User_ID = WL_UserID

can be rewritten as 

Not (User_ID = 1 AndAlso User_ID = WL_UserID)

by application of De Morgan's Law. This reads as: "It is not the case that User_ID is 1 and User_ID is WL_UserID". Assuming that 1 <> WL_UserID, then the inner AndAlso expression is always false (as User_ID cannot be equal to two different values at once) and the entire expression is true - always.

On way to solve the original, which is what I recommend, is to have one Not on the outside:

Not (User_ID = 1 OrElse User_ID = WL_UserID)

The changed expression reads as: "It is not the case that User_Id is 1 or User_Id is WL_UserID".

However, taking the now correct expression, it is trivial to transform it back to an equivalent with the Not's inside, again by application of DM. (I'm not a fan of this construct and avoid it most of the time.)

Not User_ID = 1 AndAlso Not User_ID = WL_UserID

Or, with transformation of the comparison operators (Not x = y -> x <> y),

User_ID <> 1 AndAlso User_ID <> WL_UserID

Upvotes: 2

T McKeown
T McKeown

Reputation: 12857

Readability is an important attribute of code:

If (User_ID = 1 Or User_ID = WL_UserID) Then
   'continue
Else 
   AppBoxValidation("Only the original author can edit this item!")
   Exit Sub
End If

Alernatively, you could write:

If (User_ID <> 1 AndAlso User_ID <> WL_UserID) Then
   AppBoxValidation("Only the original author can edit this item!")
   Exit Sub
End If

Upvotes: 3

Blorgbeard
Blorgbeard

Reputation: 103575

I'd write it this way: "if the user is not the admin, and the user is not the author", disallow the action.

If (User_ID <> 1 AndAlso User_ID <> WL_UserID) Then
    AppBoxValidation("Only the original author can edit this item!")
    Exit Sub
End If

This clearly shows the logic, and is shorter.

Upvotes: 1

jormigo
jormigo

Reputation: 41

As it is if the current User_ID is not 1 or if the User_ID is not equal to WL_USERID then the popup will appear. But from your description it sounds like it should be if the User_ID = 1 or User_ID = WL_UserID then show the pop-up

If (User_ID = 1 OrElse User_ID = WL_UserID) Then
    AppBoxValidation("Only the original author can edit this item!")
    Exit Sub
End If

Upvotes: 1

Related Questions