Code Optimization

ID:1669814
 
Aug 30 2014, 1:26 pm
(See the best response by Sir Lazarus.)
How do I go about making this all neat and optimized? When trying to do this, only the generic way came to mind but I'm sure there are better ways to do it.

mob/proc
    train()
        if(tfocus == "bal")     
            if((rpu == 1) && (rewardpoints > 0) && (traing == 0))
                TrainingBonus()
                traing = 1
                str += (strmod * tb) * 0.4000
                end += (endmod * tb) * 0.4000
                dex += (dexmod * tb) * 0.4000 
                apn += (strmod * tb) * 0.4000
                apn += (endmod * tb) * 0.4000
                apn += (dexmod * tb) * 0.4000
                ustats()
                rewardpoints -= 1
                spawn(100) traing = 0
            else if(traing == 0)
                TrainingBonus()
                traing = 1
                str += (strmod * tb) * 0.1667
                end += (endmod * tb) * 0.1667
                dex += (dexmod * tb) * 0.1667
                apn += (strmod * tb) * 0.1667
                apn += (endmod * tb) * 0.1667
                apn += (dexmod * tb) * 0.1667
                ustats()
                spawn(100) traing = 0
                
        if(tfocus == "str")
            if((rpu == 1) && (rewardpoints > 0) && (traing == 0))
                TrainingBonus()
                traing = 1
                str += (strmod * tb) * 0.8000
                apn += (strmod * tb) * 0.8000
                ustats()
                rewardpoints -= 1
                spawn(100) traing = 0
            else if(traing == 0)
                TrainingBonus()
                traing = 1
                str += (strmod * tb) * 0.3334
                apn += (strmod * tb) * 0.3334
                ustats()
                spawn(100) traing = 0
                
                
        if(tfocus == "end")
            if((rpu == 1) && (rewardpoints > 0) && (traing == 0))
                TrainingBonus()
                traing = 1
                end += (endmod * tb) * 0.8000
                apn += (endmod * tb) * 0.8000
                ustats()
                rewardpoints -= 1
                spawn(100) traing = 0
            else if(traing == 0)
                TrainingBonus()
                traing = 1
                end += (endmod * tb) * 0.3334
                apn += (endmod * tb) * 0.3334
                ustats()
                spawn(100) traing = 0
                
        if(tfocus == "dex")
            if((rpu == 1) && (rewardpoints > 0) && (traing == 0))
                TrainingBonus()
                traing = 1
                dex += (dexmod * tb) * 0.8000
                apn += (dexmod * tb) * 0.8000
                ustats()
                rewardpoints -= 1
                spawn(100) traing = 0
            else if(traing == 0)
                TrainingBonus()
                traing = 1
                dex += (dexmod * tb) * 0.3334
                apn += (dexmod * tb) * 0.3334
                ustats()
                spawn(100) traing = 0



Aug 30 2014, 1:44 pm
To sum it up, you could start by removing duplicate code, look into while(), and perhaps switch(). Also, you don't need var == 1 to see if it's true, if(var) will do. Likewise, var == 0 should be if(!var).

I sat here debating on whether or not to provide code, but I'd rather explain it to you and hope you put forth effort into fixing your issue as opposed to providing copy/paste snippets.
Aug 30 2014, 1:53 pm (Edited on Aug 30 2014, 2:11 pm)
Best response
 
// Danger! Untested code.

mob/proc
    train()
        if (!traing && rpu == 1 && rewardpoints > 0 && (tfocus == "bal" || tfocus == "str" || tfocus == "end" || tfocus == "dex"))
            var/cond = rpu == 1 && rewardpoints > 0

            TrainingBonus()
            traing = TRUE

            var/multiplier

            switch (tfocus)
                if ("bal")
                    if (cond)
                        multiplier = 0.4000
                    else
                        multiplier = 0.1667

                    str += (strmod * tb) * multiplier
                    end += (endmod * tb) * multiplier
                    dex += (dexmod * tb) * multiplier
                    apn += (strmod * tb) * multiplier
                    apn += (endmod * tb) * multiplier
                    apn += (dexmod * tb) * multiplier

                if ("str")
                    if (cond)
                        multiplier = 0.8000
                    else
                        multiplier = 0.3334

                    str += (strmod * tb) * multiplier
                    apn += (strmod * tb) * multiplier

                if ("end")
                    if (cond)
                        multiplier = 0.8000
                    else
                        multiplier = 0.3334

                    end += (endmod * tb) * multiplier
                    apn += (endmod * tb) * multiplier

                if ("dex")
                    if (cond)
                        multiplier = 0.8000
                    else
                        multiplier = 0.3334

                    dex += (dexmod * tb) * multiplier
                    apn += (dexmod * tb) * multiplier

            ustats()

            if (cond) rewardpoints--

            spawn (100) traing = FALSE


Beware what Lige said: The fact that I am crazy does not mean that others will be so keen to help you in the future.
Aug 30 2014, 1:59 pm
Thanks, I have a question though. Isn't the priority of ">" low? I usually put brackets around ">" because it messes up my if conditions. I never thought about putting conditions into vars either. Also I need to practice using !var and var more often for checks. Also the same with -- or ++ for adding onto vars. Thanks for the help.
Aug 30 2014, 2:10 pm
In response to Gluscap
Gluscap wrote:
Thanks, I have a question though.

No problem.

Isn't the priority of ">" low? I usually put brackets around ">" because it messes up my if conditions.

Brackets are typically employed when you're performing mathematical operations inside an if statement.

Take the following condition:

if (1 + 2 * 3 == 9) // true or false?


If you enter into a case such as the above then you should employ brackets to enforce one operation above the other. For instance:

if ((1 + 2) * 3 == 9) // true: do 1 + 2 first (= 3), then * 3 (= 9)


I'm too lazy to remember what the order of operations is, and I consider it a fools errand to memorize it. My reasoning is that your code must be readable by anyone you share it with, even your future self who may have forgotten the order of operations.

In short: when in doubt, use brackets. It's better to be clear than to leave things open to interpretation.

I never thought about putting conditions into vars either.

By putting it in a variable you have a reference to your (complex) condition. If the condition ever changes, you'll only need to change a single line of code.

Do give it a meaningful name though. I named it "cond" (= "condition") because I don't know what it means. You'll want to rename it to something that makes more sense and/or add a comment that says what the condition is for.

Also I need to practice using !var and var more often for checks. Also the same with -- or ++ for adding onto vars. Thanks for the help.

If you're dealing with booleans you should use !var and var in your checks. I'd also recommend using TRUE and FALSE and I've modified my original post accordingly.

Most of the time I don't even bother using shorthand. My personal preference is to write code like dex = dex + (dexmod * tb) * multiplier instead of dex += (dexmod * tb) * multiplier because to me it's more clear what happens to the variable than by using the shorthand version.