Bad Logic in IF statements?

willydude · Jan 12, 2008

By using PRINT, I have verifed that values are going into the variables. The variables and their values are:

@PrevailingWage 1.00 --PW. Values from 1.00 to 12.00 tested.
@StateMinimumWage 6.65 --SM
@FederalMinimumWage 5.85 --FM

User enters the PW value during data entry. sproc pulls value from MinWage table for SM and FM. In the MinWage table, I have used both State > Fed and vice versa.

For a record to be valid, the PW must be > the higher of the SM or the FM. With the above #’s, an @Message error should tell me that the PW is < than the SM. If the FM was > than SM, then the @Message should tell me that the PW < FM. Or at least <> in both situations.

This one produces a NULL in the record ID that I am trying to enter plus in @Message I get “PW Must Be >= State Minimum Wage”. No matter the value of the SM or FM, this one always bypasses the first IF stmnt and uses the second IF stmnt.

Code:

IF @StepCode = 'PR' OR @StepCode = 'TS' AND 
	 @StateMinimumWage < @FederalMinimumWage AND 
		@PrevailingWage < @FederalMinimumWage
	Begin
	  Set @Message = 'PWage Must Be >= Federal Minimum Wage'
	  Set @Continue = 0
	  end

IF @StepCode = 'PR' OR @StepCode = 'TS' AND 
	@StateMinimumWage >= @FederalMinimumWage AND
			@PrevailingWage < @StateMinimumWage
	  Begin
	  Set @Message = 'PW Must Be >= State Minimum Wage'
	  Set @Continue = 0
	  end

This one produces a NULL in the record ID that I am trying to enter but there is no msg in @Message.

Code:

IF @StepCode = 'PR' OR @StepCode = 'TS' AND 
	 @StateMinimumWage < @FederalMinimumWage  
	Begin
	IF @PrevailingWage < @FederalMinimumWage
	  Set @Message = 'PWage Must Be >= Federal Minimum Wage'
	  Set @Continue = 0
	  end

IF @StepCode = 'PR' OR @StepCode = 'TS' AND 
	@StateMinimumWage >= @FederalMinimumWage
	  Begin
	  IF @PrevailingWage < @StateMinimumWage
	  Set @Message = 'PW Must Be >= State Minimum Wage'
	  Set @Continue = 0
	  end

I’m close, but not close enough.

Thanks,

Bill

gmmastros · Jan 12, 2008

Ooooppppssss.

You failed to adhere to my number 1 rule concerning OR's. My rule is...

[tt][blue]If you have a condition that includes an OR, then you must use parenthesis. [/blue][/tt]

Obviously, this is not a T-SQL rule. It's mine. I do recommend that you also adopt this rule, because it will save you a lot of trouble. This is my rule, and it's one that you do not need to follow, but it is a good rule and is strongly recommended (by me, at least).

The problem with your code is that AND's are evaluated before OR's, so this statement....

[tt][blue]
IF @StepCode = 'PR' OR @StepCode = 'TS' AND
@StateMinimumWage < @FederalMinimumWage AND
@PrevailingWage < @FederalMinimumWage
[/blue][/tt]

Is evaluated as...

[tt][blue]
IF @StepCode = 'PR' OR [!]([/!]@StepCode = 'TS' AND
@StateMinimumWage < @FederalMinimumWage AND
@PrevailingWage < @FederalMinimumWage[!])[/!]
[/blue][/tt]

I suspect what you really want is...

Code:

IF [!]([/!]@StepCode = 'PR' OR @StepCode = 'TS'[!])[/!] AND
     @StateMinimumWage < @FederalMinimumWage AND
        @PrevailingWage < @FederalMinimumWage

Alternatively, you can remove the OR condition and use IN instead.

Code:

IF @StepCode In('PR','TS') AND
     @StateMinimumWage < @FederalMinimumWage AND
        @PrevailingWage < @FederalMinimumWage

Does this make sense?

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

r937 · Jan 12, 2008

my rule is better than your rule ;-) because yours will generate parens when only ORs are involved

my rule says: [blue]any time you mix ANDs and ORs, use parentheses to ensure you get what you want[/blue]

r937.com | rudy.ca

bborissov · Jan 12, 2008

That is because of OR and AND, You SHOULD round the or statements with brackets:

Code:

IF (@StepCode = 'PR' OR @StepCode = 'TS')    AND
     @StateMinimumWage < @FederalMinimumWage AND
     @PrevailingWage   < @FederalMinimumWage
    Begin
      Set @Message = 'PWage Must Be >= Federal Minimum Wage'
      Set @Continue = 0
    end

IF (@StepCode = 'PR' OR @StepCode = 'TS')    AND
    @StateMinimumWage >= @FederalMinimumWage AND
    @PrevailingWage   < @StateMinimumWage
    Begin
      Set @Message = 'PW Must Be >= State Minimum Wage'
      Set @Continue = 0
    end

or

Code:

IF   @StepCode IN ('PR', 'TS')                AND
     @StateMinimumWage < @FederalMinimumWage AND
     @PrevailingWage   < @FederalMinimumWage
    Begin
      Set @Message = 'PWage Must Be >= Federal Minimum Wage'
      Set @Continue = 0
    end

IF  @StepCode IN ('PR', 'TS')                AND
    @StateMinimumWage >= @FederalMinimumWage AND
    @PrevailingWage   < @StateMinimumWage
    Begin
      Set @Message = 'PW Must Be >= State Minimum Wage'
      Set @Continue = 0
    end

or

Code:

IF  @StepCode IN('PR','TS')                  AND
    @PrevailingWage   < @FederalMinimumWage
    BEGIN
       IF @StateMinimumWage < @FederalMinimumWage
          Begin
             Set @Message = 'PWage Must Be >= Federal Minimum Wage'
           end
       ELSE 
          Begin
              Set @Message = 'PW Must Be >= State Minimum Wage'
          end
       Set @Continue = 0
    END

Borislav Borissov
VFP9 SP2, SQL Server 2000/2005.
Microsoft MVP VFP

gmmastros · Jan 12, 2008

Well, obviously, I need to work on the wording, because that is not what I intended with my rule. My intention was that ANY condition that includes an OR (regardless of other logic conditions), you must use parenthesis. This will occasionally cause you to use un-needed parenthesis, but that's not necessarily a bad thing.

For example, if I have a condition the ONLY includes OR, I would still use a parenthesis. While this situation does not require parenthesis to produce the expected results, I still use them because I may re-visit the code at a later time, and then forget to put the parenthesis in there.

Ex.

I would write....

[tt][blue]If [!]([/!]@A = 1 or @A = 2[!])[/!]
Print 'A is 1 or 2'[/blue][/tt]

I guess my point is.... even if you are not [blue]mixing[/blue] ANDs and ORs, you should still use parenthesis. While it's not necessary, it can save you problems later when modifying the code to include more conditions.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

r937 · Jan 12, 2008

what if there is only one condition? you might still come back and add another, right?

so by your own logic, in order to prevent you from "forgetting to put the parenthesis in there," you use parentheses in this case too, right? to "save you problems later when modifying the code to include more conditions"

;-)

also, unneeded parentheses are necessarily a bad thing -- you've obviously nevcer had to read/understand a query produced by microsoft access

r937.com | rudy.ca

gmmastros · Jan 12, 2008

If there is only 1 condition, then there cannot be an OR, so no parenthesis. If, later, I add another condition that includes an OR, then I will add parenthesis.

I've have used Access, and I do hate the parenthesis that it likes to add to the query. Primarily, my dislike stems from the use of parenthesis in the join clause, which is something else entirely different. Truth is... I've never had to use parenthesis in a join clause with SQL Server. Not even once.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

willydude · Jan 12, 2008

Guys and/or Gals,

I have tried a number of variations on the suggested code with no success. The record is added to the JobSteps table when an error msg should happen instead. I’m beginning to wonder if I haven’t done something else wrong. The code is syntax correct, but logic wrong. I’ve posted the complete sproc below with notes. Thanks for all the help.

Code:

set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
GO
ALTER procedure [dbo].[Jobs_Insert_NewJobSteps] 
(@CustomerNumber varchar (4),
@JobNumber varchar (5),
@StepNumber varchar (3),
@LocationNumber varchar (3), 
@StepCode varchar (3),
@StepDescription varchar (50),
@PrevailingWage numeric (6,3) = NULL, 
@Standard Numeric (10,1) = NULL,
@GuaranteedStepWage numeric (5,2) = NULL, 
@MinimumWageCap varchar (2) = NULL, --need "Y" or "N" constraint
	--needed only for a PR step. need code to calc pay if "Y".
--state and fed mw's stored ONLY in the GlobalValues table.
@StateMinimumWage numeric (5,2) = NULL,
@FederalMinimumWage numeric (5,2) = NULL,
@StepNumber_PK_ID int output,
@JobNumber_PK_ID int output,
@Message varchar (30) output)
as
--declare variables
declare @Continue bit

set @Continue = 1
set @JobNumber_PK_ID = 0 
--set @JobNumber = 0  --<<DO NOT DO THIS
--set @LocationNumber = 0  --<<DO NOT DO THIS
set @Message = ''

--No leading zeroes req'd for CustomerNumber.
--validate that the customer number exists in the parameter (not 
	--NULL or empty 
if @CustomerNumber is null or @CustomerNumber = 0 
	or Len(@CustomerNumber) > 3 
Begin  --In cust table, varchar (4). constraint > 0 and < 1000.
set @Message = 'Valid Customer Number Required'
set @Continue = 0
end
--
--validate the job number by checking its length. 
--Must be 4, include leading 0, i.e. 0786.
if @JobNumber is null or Len(@JobNumber) <> 4 
begin  --varchar (5). In JobNumber tbl, constraint > 0 and < 10000
set @Message = 'Valid Job Number Required'
set @Continue = 0
end
--
--validate the Step number. Length Must be 2, including 
-- leading 0, i.e. 07.  Must be #'s, not letters.
if @StepNumber is null or len(@StepNumber) <> 2
begin --varchar (3). In JobStep tbl, constraint > 0 and < 100. correct?
set @Message = 'Valid Step Number Required'
set @Continue = 0
end
--
--validate Location. May be 1 or 2 digits in length. 
--Leading zeroes NOT required.
if @LocationNumber is null or len(@LocationNumber) > 2
begin  --constraint in JobNumber table > 0 and < 100
set @Message = 'Valid Location Number Required'
set @Continue = 0
end

--validate Step Code. StepCode must be entered.
--Letters such as PR, HR, etc. Not numbers. Must be Len of 2
if @StepCode is null or Len(@StepCode) <> 2
begin --constraint in Jobstep tbl Len(StepCode) = 2
set @Message = 'Valid Step Code Required'
set @Continue = 0
end

--validate description. Can be up to 50 characters long.
if @StepDescription is null  --must have some sort of descr entered.
begin --In JobStep tbl, constraint b/w 0 and 31
set @Message = 'Description Required'
set @Continue = 0
end

After looking over the suggestions on Tek-Tips, I decided that
I could put the PR and TS code parts into their own areas.
An attempt with the new code is at the end of this block.

Checking value of StepCode. PR first.

Code:

IF @StepCode = 'PR'
Begin
IF @PrevailingWage is null or @PrevailingWage = 0
	begin   --must have a prevailingWage value
	set @Message = 'Prevailing Wage Required'
	set @Continue = 0
	end
IF @Standard is null or @Standard = 0
	begin --must have a Standard value
	set @Message = 'Standard Required'
	set @Continue = 0
	end
IF @GuaranteedStepWage is NOT null or @GuaranteedStepWage > 0
	begin 
	set @Message = 'Can Not Use Guaranteed Step Wage With This Step Code'
	set @Continue = 0
	end
IF @MinimumWageCap is NULL   --
	begin  --'N' is the default value wanting to use
	set @Message = 'Capped? Y or N '
	set @Continue = 0
	end

--Following is still part of PR block.
--Tried the following with and without Par's.
--This checks the value of the PW, SM and FM
--only when the StepCode is PR (IF statement from above).
-- Same result. PW slips past.
-- The TS StepCode does it own checking a bit further down.

Code:

IF (@StateMinimumWage < @FederalMinimumWage) AND
        (@PrevailingWage < @FederalMinimumWage)
		Begin
		Set @Message = 'PWage Must Be >= Federal Minimum Wage'
		Set @Continue = 0
		end
IF (@StateMinimumWage > @FederalMinimumWage)  AND
		(@PrevailingWage < @StateMinimumWage)
	Begin
	  Set @Message = 'PW Must Be >= State Minimum Wage'
	  Set @Continue = 0
	  end
END

--PR block ends.

-- TS block begins.
--check value of StepCode.

Code:

IF @StepCode = 'TS'
Begin
IF @PrevailingWage is null or @PrevailingWage = 0
	begin   --must have a prevailingWage value
	set @Message = 'Prevailing Wage Required'
	set @Continue = 0
	end
IF @Standard is NOT null or @Standard > 0
	begin --must have a Standard value
	set @Message = 'Standard Not Used With Step Code "TS" '
	set @Continue = 0
	end
--Can Not Use Guaranteed Step Wage With This Step Code
IF @GuaranteedStepWage is NOT null or @GuaranteedStepWage > 0
	begin 
	set @Message = 'Can Not Use Guaranteed Step Wage With This Step Code'
	set @Continue = 0
	end
IF @MinimumWageCap is NULL   --
	begin  --'N' is the default value wanting to use
	set @Message = 'Capped? Y or N '
	set @Continue = 0
	end

TS block continues. This checks the value of the PW, SM and FM.

Code:

IF (@StateMinimumWage < @FederalMinimumWage) AND
     (@PrevailingWage  < @FederalMinimumWage)
	Begin
	 Set @Message = 'PWage Must Be >= Federal Minimum Wage'
	 Set @Continue = 0
    end
IF (@StateMinimumWage >= @FederalMinimumWage) AND
    (@PrevailingWage   < @StateMinimumWage)
	  Begin
	  Set @Message = 'PW Must Be >= State Minimum Wage'
	  Set @Continue = 0
	  end
END

--the following has both PR and TS in the same block of code.
-- Doesn't work either.

Code:

/*  --REM’d because other is used above.
IF @StepCode In('PR','TS') 
BEGIN
IF (@StateMinimumWage < @FederalMinimumWage) AND
        (@PrevailingWage < @FederalMinimumWage)
	Begin
	  Set @Message = 'PWage Must Be >= Federal Minimum Wage'
	  Set @Continue = 0
	end
IF @StateMinimumWage > @FederalMinimumWage  AND
			@PrevailingWage < @StateMinimumWage  
	Begin
	  Set @Message = 'PW Must Be >= State Minimum Wage'
	  Set @Continue = 0
    end
END
*/

End of TS and PR blocks which are the only blocks with any comparison of the PW, SM and FM.

The rest of the sproc.

Code:

IF @StepCode = 'AW'
Begin
IF @PrevailingWage is NOT null or @PrevailingWage > 0
	begin   --must have a prevailingWage value
	set @Message = 'Prevailing Wage Not Used With Step Code "AW" '
	set @Continue = 0
	end
IF @Standard is NOT null or @Standard > 0
	begin --must have a Standard value
	set @Message = 'Standard Not Used With Step Code "AW" '
	set @Continue = 0
	end
--Can Not Use Guaranteed Step Wage With This Step Code
IF @GuaranteedStepWage is NOT null or @GuaranteedStepWage > 0
	begin 
	set @Message = 'Guaranteed Step Wage Not Used With Step Code "AW" '
	set @Continue = 0
	end

end

IF @StepCode = 'GS'
Begin
IF @PrevailingWage is NOT null or @PrevailingWage > 0
	begin   --must have a prevailingWage value
	set @Message = 'Prevailing Wage Not Used With Step Code "GS" '
	set @Continue = 0
	end
IF @Standard is NOT null or @Standard > 0
	begin --must have a Standard value
	set @Message = 'Standard Not Used With Step Code "GS" '
	set @Continue = 0
	end
IF @GuaranteedStepWage is null or @GuaranteedStepWage = 0
	begin 
	set @Message = 'Guaranteed Step Wage Required '
	set @Continue = 0
	end
IF @MinimumWageCap is NULL   --
	begin  --'N' is the default value wanting to use
	set @Message = 'Capped? Y or N '
	set @Continue = 0
	end
end

--all data is validated at this point.
if @Continue = 0
begin
return 1
end
else

if @StepNumber_PK_ID is null or
	@StepNumber_PK_ID = 0
begin

Could this be out of place. I’ve tried it in different areas.

Code:

select @StateMinimumWage = StateMinimumWage,
		@FederalMinimumWage = FederalMinimumWage
	from tbl_Admin_GlobalValues

The above Select combined with the following?

Code:

SET @JobNumber_PK_ID  =
(select JobNumber_PK_ID 
from
tbl_Jobs_JobNumber
join
tbl_Customers_CustomerBasicInfo
ON  
tbl_Jobs_JobNumber.Customer_PK_ID = 
		tbl_Customers_CustomerBasicInfo.Customer_PK_ID
where
tbl_Customers_CustomerBasicInfo.CustomerNumber = @CustomerNumber
and
tbl_Jobs_JobNumber.JobNumber = @JobNumber
and
tbl_Jobs_JobNumber.LocationNumber = @LocationNumber)

Code:

print @JobNumber_PK_ID 
print 'Customer # ' + COALESCE(@CustomerNumber,'')
print 'Job # ' + COALESCE(@JobNumber,'') 
print 'Step # ' + COALESCE(@StepNumber,'')
print 'Location #' + COALESCE(@LocationNumber,'')
print 'Prevailing ' + cast(@PrevailingWage as varchar)
print 'State MW ' + cast(@StateMinimumWage as varchar)
print 'Fed MW ' + cast(@FederalMinimumWage as varchar)

--the following puts all the info into the JobsJobSteps table
insert into tbl_Jobs_JobSteps
(JobNumber_PK_ID, 
StepNumber,
StepDescription,
StepCode,
PrevailingWage,
Standard,
GuaranteedStepWage,
MinimumWageCap
)
values
(@JobNumber_PK_ID, 
@StepNumber, 
@StepDescription,
@StepCode,
@PrevailingWage,
@Standard,
@GuaranteedStepWage,
@MinimumWageCap
)
--assigns new StepNumber_PK_ID to new record just added.
set @StepNumber_PK_ID = scope_identity ()
end

Code:

------ In results pane.
2
Customer # 36
Job # 0786
Step # 08
Location #1
Prevailing 1.000
State MW 4.00
Fed MW 5.85

(1 row(s) affected)

(1 row(s) affected)

(1 row(s) affected)

Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

Bad Logic in IF statements?

willydude

Programmer

gmmastros

Programmer

r937

Technical User

bborissov

Programmer

gmmastros

Programmer

r937

Technical User

gmmastros

Programmer

willydude

Programmer

Similar threads

Part and Inventory Search

Sponsor