Jump to content

Code Refactoring, and IIF() - Why not?


Love Zhaoying
 Share

You are about to reply to a thread that has been inactive for 734 days.

Please take a moment to consider if this thread is worth bumping.

Recommended Posts

Hey all,

I am working on some scripting, which I wanted to reduce to fewer code lines. 

So, I tried a few code patterns in LSL which I don't remember trying before.

First, I was surprised that I could a) assign a variable and b) use its value in the same statement ("#2" below).

Finally, I added "IIF()" (which is "?" in some languages), reducing the code to a single line ("#3" below). So far as I know, there is no standard IIF() in LSL.

While the IIF() use adds a function call, it does prevent repeated code, and so reduces the overall number of lines..

The use-case here is a project where most of my cases use a single line of code; following the example in #3 reduces many more cases to a single line of code.

Please let me know your opinion. 

string GetThingName() {
    return "X";   
}

integer GetThingValue(string name) {
    return 1;   
}

SetThing(string name, integer value) {
// Do stuff
    llOwnerSay("Setting thing:" + name + ", value=" + (string)value);
    return;   
}

integer IIF(integer condition, integer truevalue, integer falsevalue) {
    if (condition)
        return truevalue;
    else
        return falsevalue;
}

default
{
    state_entry()
    {

// 1) Standard coding method
        integer method = TRUE;
        // Get Name of the thing
        string sThingName = GetThingName(); 
        // Get Value of the thing
        integer iThingValue = GetThingValue(sThingName);  
        if (method)  // Increment if Method#1, else decrement
            ++iThingValue; 
        else 
            --iThingValue;  
        // Save thing value
        llOwnerSay("1) Old method: method=" + (string)method);
        SetThing(sThingName, iThingValue);
        // Output: 
        // 1) Old method: method=1
        // Tester: Setting thing:X, value=2

// 2) Checking if code above can be refactored 
        method = FALSE;
        integer factor;
        if (method)
            factor = 1;
        else
            factor = -1;
        llOwnerSay("2) New method: method=" + (string)method);
// The next line surprised me - it works!                
        SetThing(sThingName = GetThingName(),  GetThingValue(sThingName) + 1*factor );
        // Output: 
        // 2) New method: method=0
        // Setting thing:X, value=0

// 3) Finally, try using IIF()
        method = TRUE;
        llOwnerSay("3) IIF() method: method=" + (string)method);
        SetThing(sThingName = GetThingName(), GetThingValue(sThingName) + 1*IIF(method, 1, -1) );
        // Output: 
        // 3) IIF() method: method=1
        // Tester: Setting thing:X, value=2
    }

}
 

Link to comment
Share on other sites

Personally, I would avoid assignment inside a function call, purely for maintainability sake - Think about the you in a couple of years time who has to edit the program, now it's not obvious if it was meant to be == or =. It's harder to spot mistakes

The conditional assignment idea seems solid to me.

  • Thanks 1
Link to comment
Share on other sites

This "IIF" function is better known as a ternary. The common syntax is:

condition ? true : false

It's definitely a useful tool, but it can also make code harder to read, especially once you start nesting them. There's plenty of articles for and against it, so it's not an entirely settled debate. I don't mind it.

What I would ask is what's the reason you're trying to reduce the number of lines? Sure, refactoring the logic to something simpler and reducing the amount of code is good, but I don't think there's a benefit to doing an assignment within a function call, for example.

The amount of code doesn't change, it's arguably harder to read, and now you can't debug the value going in without having to move that code again.

  • Thanks 1
Link to comment
Share on other sites

is always pretty interesting to me these kinds of things


a way to do IFF without doing a function is to use a list

example

integer result = llList2Integer([falsevalue, truevalue], condition);

 

other things we can do

say we want to return a range bound result

if (value < low)
   return low
else if (value > high)
   return high
else
   return value

value = llList2Integer([low, value, high], (value > low) * (-(value > high) | 1));

 

then there is when we have multiple bounded ranges

if (v <= a)
   return a
else if (v <= b)
   return b
else if (v <= c)
   return c
else if (v <= d)
   return d
else
   return e

v = llList2Integer([a, b, c, d, e], (v > a) + (v > b) + (v > c) + (v > d));
 

  • Like 1
  • Thanks 1
Link to comment
Share on other sites

25 minutes ago, Wulfie Reanimator said:

What I would ask is what's the reason you're trying to reduce the number of lines? Sure, refactoring the logic to something simpler and reducing the amount of code is good, but I don't think there's a benefit to doing an assignment within a function call, for example.

I agree on most all points. Unfortunately, the "readability" issue is secondary to the "standardization" issue. 

Standardization will result in overall "normalization" - assume a few hundred uses for this.  Before the suggested change, some would be multi-line. After the change, most all would be single-line.  This would actually make the code "more readable" - because the differences can be easily spotted: what's being passed to the "IIF()", is there an "IFF()" or not, etc.

The reason for reducing the number of lines of code: a few hundred cases starts to add up over time.  Reducing the code lines has a net-positive overall impact.

To put it simply: This is the 4th or 5th time I've written this project, and this time I'm following the KISS rules as much as possible.

  • Like 1
Link to comment
Share on other sites

6 minutes ago, Mollymews said:

is always pretty interesting to me these kinds of things


a way to do IFF without doing a function is to use a list

example

integer result = llList2Integer([falsevalue, truevalue], condition);

 

other things we can do

say we want to return a range bound result

if (value < low)
   return low
else if (value > high)
   return high
else
   return value

value = llList2Integer([low, value, high], (value > low) * (-(value > high) | 1));

 

then there is when we have multiple bounded ranges

if (v <= a)
   return a
else if (v <= b)
   return b
else if (v <= c)
   return c
else if (v <= d)
   return d
else
   return e

v = llList2Integer([a, b, c, d, e], (v > a) + (v > b) + (v > c) + (v > d));
 

Believe it or not - this time I am accomplishing the same goals as the previous 4 or 5 times writing this project - without using lists at all. 

So, "not using lists" has become a goal in itself.  Although, I like the way you used the list instead of IFF()! 

Edited by Love Zhaoying
Link to comment
Share on other sites

1 minute ago, Xiija said:

@Love Zhaoying

you could even get rid of the function call?

 

 string sThingName ; 
 integer   method = TRUE;      
SetThing(sThingName = GetThingName(), GetThingValue(sThingName) + (1 * (method > 0 )) );

 

Good catch! I think that only works in this case where I am specifically wanting "+1" or "-1", because I am either incrementing or decrementing.  

In most other cases, it will be string comparisons for now (integer values later).  So, for example now vs. later..

Now - using strings

string method="FirstMethod"; // In this instance, the "first method" is being used

// Only 1 parameter for the llFunctionCall() changes depending on the method - it is random depending on the llFunctionCall(), so using a list would just take up space - since it is only used in this ONE line of code!

llFunctionCall(parm1, parm2, parm3, IIF(method=="FirstMethod", somevalue, someothervalue) ); 

Later - using integers

integer method=134; // In this instance, the value happens to be 134

// Only 1 parameter for the llFunctionCall() changes depending on the method - it is random depending on the llFunctionCall(), so using a list would just take up space - since it is only used in this ONE line of code!

llFunctionCall(parm1, parm2, parm3, IIF(method==134, somevalue, someothervalue) ); 

Of course, this would seem to support @Mollymews suggestion..

 

Link to comment
Share on other sites

5 minutes ago, Love Zhaoying said:

Believe it or not - this time I am accomplishing the same goals as the previous 4 or 5 times writing this project - without using lists at all. 

So, "not using lists" has become a goal in itself.

 

i use lists a lot. I am influenced tho by the kinds of apps I write for myself, mostly wearables.  Things like dance hud, my tail, AO, wearable MOAP player, etc. With these kinds of apps then I just want to change out the dataset to effect changes and not have to mod the script itself

 

  • Thanks 1
Link to comment
Share on other sites

22 minutes ago, Mollymews said:

 

i use lists a lot. I am influenced tho by the kinds of apps I write for myself, mostly wearables.  Things like dance hud, my tail, AO, wearable MOAP player, etc. With these kinds of apps then I just want to change out the dataset to effect changes and not have to mod the script itself

 

I use lists plenty. This type of script is both slow and a memory eater so - less lists means less memory, quicker speed, and less llFunction() calls.

Link to comment
Share on other sites

44 minutes ago, Love Zhaoying said:

So, "not using lists" has become a goal in itself.  Although, I like the way you used the list instead of IFF()! 

I realize this isn't about optimizing efficiency. Nonetheless, library list functions incur some overhead, so a worthwhile goal in general. What I'm not sure about, though, is whether that overhead is greater than calling a user-defined function such as IIF(). (Empirically knowable, I realize.) In either case, though, we can be pretty sure there's a runtime price to pay for the reduced code length compared to in-lining some conditionals.

I've certainly used the llList2Integer([falsevalue, truevalue], condition) method myself when it just feels ugly to have the conditional sprawled out in the code, but I've never used @Mollymews' longer-list cases. For me, these raise the discomfort of having all the conditions evaluated, none skipped by an else between conditions. (I feel a similar annoyance within conditions, when for example all the logical-or clauses get exhaustively evaluated when just the first TRUE would do.) On the other hand, there's certainly some virtue in the tidy closed-form calculation of an index into the list.

It would be fun to know the efficiency tradeoffs, less fun to measure them. 

(FWIW, I do occasionally make an assignment within a function call, but more often with ++, --, +=, etc., when it just "feels" part of the meaning of the statement doing the function call.)

  • Thanks 1
Link to comment
Share on other sites

5 minutes ago, Qie Niangao said:

I realize this isn't about optimizing efficiency. Nonetheless, library list functions incur some overhead, so a worthwhile goal in general. What I'm not sure about, though, is whether that overhead is greater than calling a user-defined function such as IIF(). (Empirically knowable, I realize.) In either case, though, we can be pretty sure there's a runtime price to pay for the reduced code length compared to in-lining some conditionals.

Guess I misstated earlier - one of my main purposes to avoid lists is greater efficiency.

I assume a user-defined function, if very small, incurs very little overhead. In this case, if the same function (or 2 versions perhaps) is called each time, at least it will be "predictable".

7 minutes ago, Qie Niangao said:

(FWIW, I do occasionally make an assignment within a function call, but more often with ++, --, +=, etc., when it just "feels" part of the meaning of the statement doing the function call.)

Between readability, "will it work?!?", and "are we sure the operands are always evaluated left-to-right", I wasn't so sure.

I definitely wouldn't normally write that way just due to "good practices", although coming from C/C++ you know people often write utter garbage - "good" code that is vastly unreadable.  I don't want to "be that guy"; but in taking that stance, I miss out on some more common tricks.

  • Like 1
Link to comment
Share on other sites

15 minutes ago, Qie Niangao said:

I've certainly used the llList2Integer([falsevalue, truevalue], condition) method myself when it just feels ugly to have the conditional sprawled out in the code

I wonder why this would seem to be efficient, unless lists (small lists?) are really efficient?

It has to:

1) Construct the list

2) Pass the list to llList2Integer(), along with the second parameter (index of list item to return)

3) llList2Integer() has to do it's thing and return the result

That's a lot of overhead!

Link to comment
Share on other sites

I couldn't wait, hehehe.

I'd hide this code behind a spoiler tag if I remembered how to do that. It's three different sequences for no good reason, plus some extra testing superstition. Sorry 'bout this:


integer IIF(integer condition, integer truevalue, integer falsevalue)
{
    if (condition)
        return truevalue;
    else
        return falsevalue;
}

integer REPS = 50000;

default
{
    state_entry()
    {
        integer true = TRUE; // superstitious attempt to prevent an optimization?
        integer false = FALSE;
        integer result;
        float ignoreTime;
        float totalIIFtime;
        float totalListTime;
        float totalInlineTime;
        integer countdown;
//  ORDER 1
        ignoreTime += llGetAndResetTime();   // for consistency?
        for (countdown = REPS; countdown; --countdown)
        {
           result = IIF(true, TRUE, FALSE);
           result = IIF(false, TRUE, FALSE);
        }
        totalIIFtime += llGetAndResetTime();
        for (countdown = REPS; countdown; --countdown)
        {
           result = llList2Integer([TRUE, FALSE], true);
           result = llList2Integer([TRUE, FALSE], false);
        }
        totalListTime += llGetAndResetTime();
        for (countdown = REPS; countdown; --countdown)
        {
           if (true)
                result = TRUE;
            else
                result = FALSE;
            if (false)
                result = TRUE;
            else
                result = FALSE;
        }
        totalInlineTime += llGetAndResetTime();
        llOwnerSay("ORDER 1: Total times:\n\t"
            +(string)totalIIFtime+" : IIF (user function call)\n\t"
            +(string)totalListTime+" : llList2Integer indexing\n\t"
            +(string)totalInlineTime+" : inline conditionals");
//  ORDER 2
        totalIIFtime = 0.0;
        totalListTime = 0.0;
        totalInlineTime = 0.0;
        ignoreTime += llGetAndResetTime();   // for consistency?
        for (countdown = REPS; countdown; --countdown)
        {
           if (true)
                result = TRUE;
            else
                result = FALSE;
            if (false)
                result = TRUE;
            else
                result = FALSE;
        }
        totalInlineTime += llGetAndResetTime();
        for (countdown = REPS; countdown; --countdown)
        {
           result = IIF(true, TRUE, FALSE);
           result = IIF(false, TRUE, FALSE);
        }
        totalIIFtime += llGetAndResetTime();
        for (countdown = REPS; countdown; --countdown)
        {
           result = llList2Integer([TRUE, FALSE], true);
           result = llList2Integer([TRUE, FALSE], false);
        }
        totalListTime += llGetAndResetTime();
        llOwnerSay("ORDER 2: Total times:\n\t"
            +(string)totalIIFtime+" : IIF (user function call)\n\t"
            +(string)totalListTime+" : llList2Integer indexing\n\t"
            +(string)totalInlineTime+" : inline conditionals");
//  ORDER 3
        totalIIFtime = 0.0;
        totalListTime = 0.0;
        totalInlineTime = 0.0;
        ignoreTime += llGetAndResetTime();   // for consistency?
        for (countdown = REPS; countdown; --countdown)
        {
           result = llList2Integer([TRUE, FALSE], true);
           result = llList2Integer([TRUE, FALSE], false);
        }
        totalListTime += llGetAndResetTime();
        for (countdown = REPS; countdown; --countdown)
        {
           if (true)
                result = TRUE;
            else
                result = FALSE;
            if (false)
                result = TRUE;
            else
                result = FALSE;
        }
        totalInlineTime += llGetAndResetTime();
        for (countdown = REPS; countdown; --countdown)
        {
           result = IIF(true, TRUE, FALSE);
           result = IIF(false, TRUE, FALSE);
        }
        totalIIFtime += llGetAndResetTime();
        llOwnerSay("ORDER 3: Total times:\n\t"
            +(string)totalIIFtime+" : IIF (user function call)\n\t"
            +(string)totalListTime+" : llList2Integer indexing\n\t"
            +(string)totalInlineTime+" : inline conditionals");
    }
}

Results:

[07:48] Object: ORDER 1: Total times:
    0.910954 : IIF (user function call)
    10.178350 : llList2Integer indexing
    0.176679 : inline conditionals
[07:48] Object: ORDER 2: Total times:
    0.800232 : IIF (user function call)
    9.489452 : llList2Integer indexing
    0.221642 : inline conditionals
[07:48] Object: ORDER 3: Total times:
    0.776226 : IIF (user function call)
    10.380840 : llList2Integer indexing
    0.179195 : inline conditionals

  • Thanks 2
Link to comment
Share on other sites

2 minutes ago, Qie Niangao said:

Results:

[07:48] Object: ORDER 1: Total times:
    0.910954 : IIF (user function call)
    10.178350 : llList2Integer indexing
    0.176679 : inline conditionals
[07:48] Object: ORDER 2: Total times:
    0.800232 : IIF (user function call)
    9.489452 : llList2Integer indexing
    0.221642 : inline conditionals
[07:48] Object: ORDER 3: Total times:
    0.776226 : IIF (user function call)
    10.380840 : llList2Integer indexing
    0.179195 : inline conditionals

Thanks for saving me the trouble!

Sorry I de-emphasized the importance of "efficiency" earlier, it is actually my main driver (more than the code readability, etc.).

That being said, what do you think now that you've run the tests?  If "IIF()" is still 10x faster than lists, seems worthy to me.

Odd that the order makes a noticeable difference..

Thanks for all your help!

Link to comment
Share on other sites

4 minutes ago, Love Zhaoying said:

Odd that the order makes a noticeable difference..

Oh, I don't think the order actually made a difference. This was run on a Mainland sim with god only knows what else adding noise to the measurement, so a little fluctuation is expected and I find the results consistent enough across orders to not suspect any significant sequence artifact in the results.

I pretty much expected the rank-order of results, but not the magnitude of differences. And the user function call (IIF) was less costly than I expected, only slower than inline by a factor of four or so. And the list was substantially worse than I expected, especially for constant two-element lists. (Now I'm wondering how lame this compiler could be, and whether those lists could possibly be cons'd-up each time the expression is encountered at runtime.)

Incidentally, it's Mono that makes the user function call tolerable. Compiling with "LSL" instead of Mono, and with REPS cut to 5000:

[08:18] Object: ORDER 1: Total times:
    2.370628 : IIF (user function call)
    2.649557 : llList2Integer indexing
    0.800808 : inline conditionals
[08:18] Object: ORDER 2: Total times:
    2.655021 : IIF (user function call)
    2.667178 : llList2Integer indexing
    0.800890 : inline conditionals
[08:18] Object: ORDER 3: Total times:
    2.548527 : IIF (user function call)
    2.727801 : llList2Integer indexing
    0.872940 : inline conditionals

  • Thanks 2
Link to comment
Share on other sites

35 minutes ago, Love Zhaoying said:

Dumb question, I probably knew at one point - how to you paste the code "prettily" (formatted for not just font, but the color etc.)?

Click the little <> icon for a code textbox:

image.png.ebb72078ce4a6441562526c7a13467a3.png

Pick a language from the dropdown:

image.png.f282d651abeaf8aabd2dbd75abab6513.png

"C Languages" seems to work best for most LSL code.

I don't think there's a way to create hidden text anymore, since the forum uses a what-you-see-is-what-you-get editor.

 

And on topic: The IIF-way seems like the most conductive to both less code, less typing (even if lists were faster), and 💋.

  • Like 1
  • Thanks 1
Link to comment
Share on other sites

20 minutes ago, Qie Niangao said:

And the list was substantially worse than I expected, especially for constant two-element lists. (Now I'm wondering how lame this compiler could be, and whether those lists could possibly be cons'd-up each time the expression is encountered at runtime.)

Yeah, over the years I've gotten more and more disappointed in the list implementation, in general. 

I may use a little JSON, which is supposed to be even WORSE than lists, but I am not dependent on it at all.  Optional stuff.

Ultimately I will have to add some list stuff for llFunction() calls that take lists as parameters.

21 minutes ago, Qie Niangao said:

Incidentally, it's Mono that makes the user function call tolerable. Compiling with "LSL" instead of Mono, and with REPS cut to 5000:

[08:18] Object: ORDER 1: Total times:
    2.370628 : IIF (user function call)
    2.649557 : llList2Integer indexing
    0.800808 : inline conditionals
[08:18] Object: ORDER 2: Total times:
    2.655021 : IIF (user function call)
    2.667178 : llList2Integer indexing
    0.800890 : inline conditionals
[08:18] Object: ORDER 3: Total times:
    2.548527 : IIF (user function call)
    2.727801 : llList2Integer indexing
    0.872940 : inline conditionals

That's interesting. LSO was sure a um, dog!

Since I started in 2006, I don't remember LSO days very clearly.

  • Like 1
Link to comment
Share on other sites

You are about to reply to a thread that has been inactive for 734 days.

Please take a moment to consider if this thread is worth bumping.

Please sign in to comment

You will be able to leave a comment after signing in



Sign In Now
 Share

×
×
  • Create New...