Is there a better way of implementing this control structure?

Question

I have the following control structure that I would like to improve:

if(number >= 100)    then doSetInc(id, 8, 20)
elseif(number >= 91) then doSetInc(id, 8, 30)
elseif(number >= 81) then doSetInc(id, 8, 50)
elseif(number >= 7)  then doSetInc(id, 8, 70)
elseif(number >= 1)  then doSetInc(id, 8, 100)
end

Answer 1

Kinda hard to know what you mean with better since you don't specify in what regard.

In regards to cleaner code:

• FunctionName doSetInc is a bit weird, what does it do?

• You are sending in 3 variables into the function, it's common practice to use as few as possible (rarely more than 2, 0 or 1 prefered).

• You are always sending in "id" and "8" into the function, thus this function might not need those attributes and could be set in another way.

Other than that, since I find no easy mathematical similarities between these checks and what to do, I would say no, you can't improve it given this small piece of code and no other context.

Answer 2

1.
What does 'doSetInc' do? Its not a descriptive name.
- Where does that 8 come from?
- Where does that 20 come from?

Bad code good code: http://commadot.com/wp-content/uploads/2009/02/wtf.png


2.
You have a set of ranges and for each a specific value is to be derived. Depending on how many integers there are to be evaluated, you might want to make a hash containing what each integer turns into.

It would look something like this:

derived_id{1}->20
derived_id{2}->20
...
derived_id{64}->70
...

At that you can replace the entire block with a single line.

doSetInc(id, 8, derived_id{number})

And if you would improve the readability like I suggested above, it would become a single easily readable line.

Be careful though, at this point people may start to feel like you haven't really done anything.

Answer 3

1)

To simplify the if else conditions, use < instead of >=. Note: This only works correctly if number isn't a decimal. Be sure to round.

Old Code:

if(number >= 100)    then doSetInc(id, 8, 20)
elseif(number >= 91) then doSetInc(id, 8, 30)
elseif(number >= 81) then doSetInc(id, 8, 50)
elseif(number >= 7)  then doSetInc(id, 8, 70)
elseif(number >= 1)  then doSetInc(id, 8, 100)
end

New Code:

if( 99 < number )    then doSetInc(id, 8, 20)
elseif(90 < number ) then doSetInc(id, 8, 30)
elseif(80 < number ) then doSetInc(id, 8, 50)
elseif( 6 < number )  then doSetInc(id, 8, 70)
elseif( 0 < number )  then doSetInc(id, 8, 100)
end 

2)

Create a function that retrieves the value of the third parameter, since that's the only thing that changes for the doSetInc calls.

3)

Rename doSetInc to setInc.

4)

Declare variables as local, unless global is required.

Final Code

-- core function
local getGrade = function(x)
    local val       
    if ( 99 < number ) then val = 20
    elseif (90 < number ) then val = 30
    elseif (80 < number ) then val = 50
    elseif ( 6 < number ) then val = 70
    elseif ( 0 < number ) then val = 100
    return val;
end 

setInc(id, 8, getGrade( number ) )

Use case:

local number = 10
local id = "283"
local setInc = function(a,b,c) 
    print( "setInc("..a..", " .. b .. ", " .. c .. ")" )
end

setInc(id, 8, getGrade( number ) )