Which is faster vs proper

vining

Which incarnation of the below function would be consider proper and which would be more efficient or faster? I understand changing things via functions by reference is frowned upon but in most cases it just seems to make more sense and might be faster but is it?

What I consider the preferred method:

sMyFB.cFB_Tag[nIndx] = fnJSON_TrimLeadChars('"{}[],',sMyFB.cFB_Tag[nIndx]);

DEFINE_FUNCTION CHAR[MAX_15999]fnJSON_TrimLeadChars(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the begining of a string and pass in the string
     STACK_VAR INTEGER nCheck;
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLen;
     STACK_VAR INTEGER i;
    
     nCheck++;
     nLen = LENGTH_STRING(iStr);
     nLenT = LENGTH_STRING(iTrimChars);
     WHILE(nCheck && nLen)
      {
      nCheck = 0;
      for(i = nLenT; i; i--)
           {
           if(iTrimChars[i] == iStr[1])
            {
            GET_BUFFER_CHAR(iStr);
            nCheck++;
            nLen--;
            BREAK;
            }
           }
      }

     RETURN iStr;
     }

The simpler and possibly faster method:

fnJSON_TrimLeadChars('"{}[],',sMyFB.cFB_Tag[nIndx]);

DEFINE_FUNCTION nJSON_TrimLeadChars(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the begining of a string and pass in the string
     STACK_VAR INTEGER nCheck;
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLen;
     STACK_VAR INTEGER i;
    
     nCheck++;
     nLen = LENGTH_STRING(iStr);
     nLenT = LENGTH_STRING(iTrimChars);
     WHILE(nCheck && nLen)
      {
      nCheck = 0;
      for(i = nLenT; i; i--)
           {
           if(iTrimChars[i] == iStr[1])
            {
            GET_BUFFER_CHAR(iStr);
            nCheck++;
            nLen--;
            BREAK;
            }
           }
      }
     }

is it even worth thinking about? I'm using the 1st method but I'm thinking what's the point of returning the value of itself to itself other than some proper coding etiquette BS.

PhreaK

vining wrote: »

...what's the point of returning the value of itself to itself other than some proper coding etiquette BS.

"Coding etiquette BS" is what (can) separate horrible, unreadable, bug prone code from something that going to be nice for both yourself, and anyone else working on it in the future.

Think about the following scenario.

stack_var float area

area = MATH_PI * square(r)

vs

stack_var float area
stack_var integer x

square(r)
area = MATH_PI * r
...
x = r * 5    // did you mean to use the original r, or r-squared?

Obviously, this is an oversimplified example. There will be use cases where each are applicable (e.g. if you are attempting to return a complex type, which is not allowed in NetLinx). As a rule of thumb, I'll try to make sure that a function or parameter name, as well as the documentation, make it clear it is modifying external state. One thing I religiously adhere to though is if I am dicking with external state, anything being affected is passed in as a parameter. Modifying global state is just plain evil.

In general, avoiding side effects wherever possible helps to make it easier to reason about your logic and construct bug-free code quicker.

a_riot42

The logic seems muddled. You increment nCheck to get into a while loop then set it to 0 before starting a for loop? It isn't often I see a for loop inside a while loop. You check nCheck in the while loop but it will always be true, since you are always incrementing it. Only nLen will stop the while loop. Seems loopy.
Paul

vining

a_riot42 wrote: »

The logic seems muddled. You increment nCheck to get into a while loop then set it to 0 before starting a for loop? It isn't often I see a for loop inside a while loop. You check nCheck in the while loop but it will always be true, since you are always incrementing it. Only nLen will stop the while loop. Seems loopy.
Paul

Actually nCheck or nLen can stop the while loop and while the logic may be loopy the function does do what it is supposed to using loops and I can't see anything that would significantly improve it or make it faster or more effiecient other than what I brought up in the OP.

edit: nCheck only increments in the for loop if there's a char match in which case it removes the char and repeats the loop to check the next char but looking at it this morning it might benefit from a break after the nCheck++ and nLen-- so that it stops the for and goes back to the while to check the next char to run the for loop from the beginning otherwise it looks like it will finish the for loop on the next char. Of course letting it finish the for loop could be beneficial and more effecient if it does find something but it won't be going through the entire array of chars to look for and it will repeat when it goes back to the while. I think a break would be proper.

vining

PhreaK wrote: »

As a rule of thumb, I'll try to make sure that a function or parameter name, as well as the documentation, make it clear it is modifying external state. One thing I religiously adhere to though is if I am dicking with external state, anything being affected is passed in as a parameter. Modifying global state is just plain evil.

In general, avoiding side effects wherever possible helps to make it easier to reason about your logic and construct bug-free code quicker.

In this case I'm passing an element of a stack structure which later gets copied to the global so it's not modifying a global in the function posted above and in both examples the function name clearly states it's intended mission so I don't think there would be any confusion later in life by me or others so etiquette BS aside are there performance considerations doing it one way vs the other. I have no formal code training other than AMX and a few weekend courses and I don't read code books so in my simple mind the 2nd method just seems it would be easier/faster for the processor.

GregG

I tend to write my library functions so that they never touch the incoming parameters, because anyone else using my functions later may not have time to read through them all and check them.

I also have a similar function that goes like this:

/////////////////////////////////////////////////////////////
// Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
// takes an input string and returns a version with all characters listed in cCharsToStrip removed from it
//
// eg- StripChars('File/Name?.txt',':/\?') returns 'FileName.txt'
//
/////////////////////////////////////////////////////////////
Define_Function Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
{
Stack_Var Char cInput[16000]
Stack_Var Char cOutput[16000]
Stack_Var Char t;
    cInput = cString
    t = GET_BUFFER_CHAR(cInput)
    While(t)
    {
        If(!Find_String(cCharsToStrip,"t",1))
        {
            cOutput = "cOutput,t"
        }
        t = GET_BUFFER_CHAR(cInput)
    }
    Return cOutput
}

vining

GregG wrote: »

I tend to write my library functions so that they never touch the incoming parameters, because anyone else using my functions later may not have time to read through them all and check them.

I also have a similar function that goes like this:

/////////////////////////////////////////////////////////////
// Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
// takes an input string and returns a version with all characters listed in cCharsToStrip removed from it
//
// eg- StripChars('File/Name?.txt',':/\?') returns 'FileName.txt'
//
/////////////////////////////////////////////////////////////
Define_Function Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
{
Stack_Var Char cInput[16000]
Stack_Var Char cOutput[16000]
Stack_Var Char t;
cInput = cString
t = GET_BUFFER_CHAR(cInput)
While(t)
{
If(!Find_String(cCharsToStrip,"t",1))
{
cOutput = "cOutput,t"
}
t = GET_BUFFER_CHAR(cInput)
}
Return cOutput
}

I like that method, simple and accomplishes the same thing. Might even be faster since it runs the loop in the netlinx function instead of your function.

What did you do for its counterpart, stripping from the end of a string? Mine looks almost identical to my TrimLeadChar function:

DEFINE_FUNCTION CHAR[MAX_15999]fnJSON_TrimEndChars(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the end of a string and pass in the string
     STACK_VAR INTEGER nCheck;
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLen;
     STACK_VAR INTEGER i;
    
     nCheck++;
     nLen = LENGTH_STRING(iStr);
     nLenT = LENGTH_STRING(iTrimChars);
     WHILE(nCheck && nLen)
      {
      nCheck = 0;
      for(i = nLenT; i; i--)
           {
           if(iTrimChars[i] == iStr[nLen])
            {
            nLen--;
            nCheck++;
            SET_LENGTH_STRING(iStr,nLen);
            BREAK;
            }
           }
      }


RETURN iStr;
}

GregG

Pretty much the same thing with an extra IF statement:

/////////////////////////////////////////////////////////////
// Char[16000] StripCharsFromTail(Char cString[], Char cCharsToStrip[])
// takes a long string and removes any and all ascii characters listed in
// cCharsToStrip from the last "nFromLastN" characters.
//
// eg- StripCharsFromTail('Thing12 Thing13','0123456789',2) gives 'Thing12 Thing'
//
/////////////////////////////////////////////////////////////
Define_Function Char[16000] StripCharsFromTail(Char cString[], Char cCharsToStrip[], Integer nFromLastN)
{
Stack_Var Char cInput[16000]
Stack_Var Char cOutput[16000]
Stack_Var Integer nLen;
Stack_Var Integer nPos;
Stack_Var Char t;
    cInput = cString
    nLen=LENGTH_STRING(cInput)
    t = GET_BUFFER_CHAR(cInput)
    While(t)
    {
        nPos++
        If(nFromLastN>nLen-nPos)
        {
            If(!Find_String(cCharsToStrip,"t",1))
            {
                cOutput = "cOutput,t"
            }
        }
        Else
        {
            cOutput = "cOutput,t"
        }
        t = GET_BUFFER_CHAR(cInput)
    }
    Return cOutput
}

a_riot42

vining wrote: »

Actually nCheck or nLen can stop the while loop and while the logic may be loopy the function does do what it is supposed to using loops and I can't see anything that would significantly improve it or make it faster or more effiecient other than what I brought up in the OP.

edit: nCheck only increments in the for loop if there's a char match in which case it removes the char and repeats the loop to check the next char but looking at it this morning it might benefit from a break after the nCheck++ and nLen-- so that it stops the for and goes back to the while to check the next char to run the for loop from the beginning otherwise it looks like it will finish the for loop on the next char. Of course letting it finish the for loop could be beneficial and more effecient if it does find something but it won't be going through the entire array of chars to look for and it will repeat when it goes back to the while. I think a break would be proper.

I think GregG's implementation is likely faster and much more readable. I think the only changes I would make is to do less copying and use less memory if possible. IE:

Define_Function Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
{
  Stack_Var Char cOutput[16000]
  stack_var integer i, len
  len = length_array(cString)
 
  for(i = 1;i <= len;i++)
  {
    If(!Find_String(cCharsToStrip, "cInput[i]",1))
    {
      cOutput = "cOutput, cInput[i]"
    }
  }
  Return cOutput
}

vining

a_riot42 wrote: »

I think GregG's implementation is likely faster and much more readable. I think the only changes I would make is to do less copying and use less memory if possible. IE:

Define_Function Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
{
  Stack_Var Char cOutput[16000]
  stack_var integer i, len
  len = length_array(cString)
 
  for(i = 1;i <= len;i++)
  {
    If(!Find_String(cCharsToStrip, &quot;cInput[i]&quot;,1))
    {
      cOutput = &quot;cOutput, cInput[i]&quot;
    }
  }
  Return cOutput
}

CInput should be cString and it might be faster since it's offloading the bulk of the work to a system function which just do things faster when my function does all the work with out calling in any other functions. They're also not really the same function since mine only trims chars at the beginning of the string where Gregs strips chars out of the entire string. I could use a while loop and use find_string and eliminate the for loop but that's just because find_string will use one instead.

My main concern/question was whether to change the var by reference or return the string to itself. Even netlinx functions do it both ways like set_length_string(var,lenght), it's not var = set_length_string(var,length) so that changes by reference too.

a_riot42

vining wrote: »

CInput should be cString and it might be faster since it's offloading the bulk of the work to a system function which just do things faster when my function does all the work with out calling in any other functions. They're also not really the same function since mine only trims chars at the beginning of the string where Gregs strips chars out of the entire string. I could use a while loop and use find_string and eliminate the for loop but that's just because find_string will use one instead.

My main concern/question was whether to change the var by reference or return the string to itself. Even netlinx functions do it both ways like set_length_string(var,lenght), it's not var = set_length_string(var,length) so that changes by reference too.

Yes, it should be:

 Define_Function Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
{  
  Stack_Var Char cOutput[16000]  
  stack_var integer i, len  
  len = length_array(cString)    
  for(i = 1;i <= len;i++)  
  {    
    If(!Find_String(cCharsToStrip, "cString[i]", 1))    
    {      
      cOutput = "cOutput, cString[i]"    
    }  
  }  
  Return cOutput
}

set_length_string alters the string length so it has to modify the string to do so, so passing the array back to the calling function doesn't really make much sense. I don't think there is a hard and fast rule regarding whether to modify arguments or not. It depends on what the function does, and other things. For this type of function, modifying it rather than creating a new string saves 16000 bytes of memory so it can be done that way as well, leaving the option to the caller whether or not a original copy is needed.
Paul

GregG

Modifying and reducing even more...

Define_Function Char[16000] StripChars(Char cString[], Char cCharsToStrip[])
{
Stack_Var Char cOutput[16000]
Stack_Var Integer nLen
Stack_Var Integer nPos

    nLen = Length_String(cString)
    For(nPos = 1;nPos <= nLen;nPos++)  
    {    
        If(!Find_String(cCharsToStrip, "cString[nPos]", 1))    
        {      
            cOutput = "cOutput, cString[nPos]"    
        }  
    }
    Return cOutput
}

Define_Function Char[16000] StripCharsFromHead(Char cString[], Char cCharsToStrip[], Integer nUpTo)
{
Stack_Var Integer nLen
    nLen = Length_String(cString)
    
    If(nUpTo>=nLen)
        Return StripChars(cString, cCharsToStrip)
    Else
        Return "StripChars(Left_String(cString,nUpTo),cCharsToStrip),Right_String(cString,nLen-nUpTo)"
}

Define_Function Char[16000] StripCharsFromTail(Char cString[], Char cCharsToStrip[], Integer nFromLastN)
{
Stack_Var Integer nLen
    nLen = Length_String(cString)
    
    If(nFromLastN>=nLen)
        Return StripChars(cString, cCharsToStrip)
    Else
        Return "Left_String(cString,nLen-nFromLastN),StripChars(Right_String(cString,nFromLastN),cCharsToStrip)"
}

vining

GregG wrote: »

Modifying and reducing even more...

Define_Function Char[16000] StripCharsFromHead(Char cString[], Char cCharsToStrip[], Integer nUpTo)
Define_Function Char[16000] StripCharsFromTail(Char cString[], Char cCharsToStrip[], Integer nFromLastN)

Those are better function names, when I made mine I gave up trying to think of a proper names, head and tail never came to mind.

vining

Here's 3 version that use find_string instead of running the for loops inside the function. They're not tested but look good, although I've said that before and was wrong. The only different between your TrimCharsFromString and mine is I ran mine backwards since for loops are supposedly more efficient backwards not that it would really matter, just more ways to skin a live cat.

DEFINE_FUNCTION CHAR[MAX_15999]fnTrimCharsFromHead_(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the begining of a string and pass in the string
     WHILE(find_string(iTrimChars,"iStr[1]",1))
      {
      GET_BUFFER_CHAR(iStr);
      }
     RETURN iStr;
     }

DEFINE_FUNCTION CHAR[MAX_15999]fnTrimCharsFromTail_(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the end of a string and pass in the string
     STACK_VAR INTEGER nLenS;
     nLenS = LENGTH_STRING(iStr);
     WHILE(find_string(iTrimChars,"iStr[nLenS]",1))
      {
      nLenS--;
      SET_LENGTH_STRING(iStr,nLenS);
      }
     RETURN iStr;
     }
    
DEFINE_FUNCTION CHAR[MAX_15999]fnTrimCharsFromString_(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the end of a string and pass in the string
     STACK_VAR INTEGER i;
     STACK_VAR CHAR cStr[15999];
     for(i=LENGTH_STRING(iStr);i;i--)
      {
      if(!find_string(iTrimChars,"iStr[i]",1))
           {
           cStr = "iStr[i],cStr";
           }
      }
     RETURN cStr;
     }

a_riot42

What's with the 15999?
Paul

vining

a_riot42 wrote: »

What's with the 15999?
Paul

It's 1 less than 16,000. I also seem to recall that's it's the max length of a string in a variable unless maybe for created buffers. It's been so long since topics like this have been discussed on the forums so I really don't know anymore and don't care enough to look it up.

vining

Here's yet another version of fnTrimCharsFromHead that seems to work well.

sTa,g.cTag = fnTrimCharsFromHead('}],"{['sTa,gTemp.cTag);

DEFINE_FUNCTION CHAR[MAX_15999] fnTrimCharsFromHead(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the begining of a string and pass in the string
     STACK_VAR INTEGER i; 
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLenS;
     STACK_VAR CHAR cCopy[15999];
     
     cCopy = iStr;
     nLenT = LENGTH_STRING(iTrimChars);
     nLenS = LENGTH_STRING(cCopy);
     
     for(i=nLenT;i&&nLenS;i--)
      {
      if(cCopy[1] == iTrimChars[i])
           {
           GET_BUFFER_CHAR(cCopy);    //remove 1st char
           nLenS--;            //update your string length
           i=(nLenT+1);        //force it to start at the begining again
           }
      }
     RETURN cCopy;
     }

I've decided that if you're going to return the value you should always copy the incoming string 1st so that you maintain the original otherwise returning the value is pointless and should be done like this similar to set_length_string which is fairly similar in purpose.

fnTrimCharsFromHead('}],"{['sTa,gTemp.cTag);

DEFINE_FUNCTION fnTrimCharsFromHead(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the begining of a string and pass in the string
     STACK_VAR INTEGER i; 
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLenS;
         
     nLenT = LENGTH_STRING(iTrimChars);
     nLenS = LENGTH_STRING(iStr);
     
     for(i=nLenT;i&&nLenS;i--)
      {
      if(iStr[1] == iTrimChars[i])
           {
           GET_BUFFER_CHAR(iStr);    //remove 1st char
           nLenS--;            //update your string length
           i=(nLenT+1);        //force it to start at the begining again
           }
      }
     }

I prefer the last version. it's just easier to use and since it's usually accompanied by set_length_string in through out my parser it just looks better.

vining

Ok, here's yet another variant of the two functions, these are probably faster than the earlier version since they don't actually change the string during a match pass of the loop, it just counts and trims the total when done. These also don't pass back, they affect the change via reference just like set_string_length so if you're a purest and want a proper function just add another stack_var char to copy the string and make some mods to the function, I just didn't see the point especially if you're not copying the incoming string. These are likely the versions I'm going to stick with unless I find a flaw in them.

fnTrimCharsFromHead('}],"{[',sTagTemp.cTag);

DEFINE_FUNCTION fnTrimCharsFromHead(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the begining of a string and pass in the string
     STACK_VAR INTEGER i; 
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLenS;
     STACK_VAR INTEGER nCnt;
          
     nLenS = LENGTH_STRING(iStr);
     nLenT = LENGTH_STRING(iTrimChars);
         
     for(i=nLenT,nCnt=1;i&&(nCnt<nLenS);i--)
      {
      if(iStr[nCnt] == iTrimChars[i])
           {
           nCnt++;                   //count chars to remove
           i=(nLenT+1);        //force it to start at the begining again
           }
      }
     if(nCnt)
      {
      GET_BUFFER_STRING(iStr,nCnt-1);    //remove chars if any
      }
     }

fnTrimCharsFromTail(',}]',sJLoop[i].cItem[nCnt]);

DEFINE_FUNCTION fnTrimCharsFromTail(CHAR iTrimChars[],CHAR iStr[MAX_15999])

     {//pass in a string of all the chars you want trimmed from the end of a string and pass in the string
     STACK_VAR INTEGER i; 
     STACK_VAR INTEGER nLenT;
     STACK_VAR INTEGER nLenS;
     STACK_VAR INTEGER nCnt;
     
     nLenS = LENGTH_STRING(iStr);
     nLenT = LENGTH_STRING(iTrimChars);
     
     for(i=nLenT;i&&(nCnt<nLenS);i--)
      {
      if(iStr[nLenS-nCnt] == iTrimChars[i])
           {
           nCnt++;                //count chars to remove
           i=(nLenT+1);               //force it to start at the begining again
           }
      }
     if(nCnt)
      {
      SET_LENGTH_STRING(iStr,nLenS-nCnt);
      }
     } 
     

GregG

I'd probably boil those down to this:

Define_Function fnTrimCharsFromHead(CHAR iTrimChars[],CHAR iStr[])
{
    While(Find_String(iTrimChars,"iStr[1]",1))
    {
        Get_Buffer_Char(iStr);
    }
}

Define_Function fnTrimCharsFromTail(CHAR iTrimChars[],CHAR iStr[])
{
Stack_Var nLen
    nLen = Length_String(iStr);
    While(Find_String(iTrimChars,"Right_String(iStr,1)",1))
    {
        nLen--;
        Set_Length_String(iStr,nLen);
    }
}

vining

GregG wrote: »

I'd probably boil those down to this:

Define_Function fnTrimCharsFromHead(CHAR iTrimChars[],CHAR iStr[])
{
While(Find_String(iTrimChars,"iStr[1]",1))
{
Get_Buffer_Char(iStr);
}
}

Define_Function fnTrimCharsFromTail(CHAR iTrimChars[],CHAR iStr[])
{
Stack_Var nLen
nLen = Length_String(iStr);
While(Find_String(iTrimChars,"Right_String(iStr,1)",1))
{
nLen--;
Set_Length_String(iStr,nLen);
}
}

I​ agree using find_string and even right_string does make the function look cleaner but that also means that every pass of the while loop you have to make function calls which adds more code to the functions all be it kept in another location and if the find_string returns true it has to do a string manipulation each true pass of the loop where as the last two examples I posted don't need to call any functions during the loop or manipulate the string during the loop, it just counts and when done it does a single string manipulation. If the beginning or end of the string contained each one of the chars I wanted removed the examples you posted would have to make 5 function calls and manipulate the string 5 times, not to mention how many loops that happen inside the find_string function. While I'm for cleaner and shorter code the whole purpose of a function is to make it as efficient as possible and never look at it again, just call it when needed so does it matter which version is sexier, shorter or cleaner? Besides for me the fun is coming up with my own logic even if it is slower or not as elegantly written. It's like when you have children, they might not be the smartest, best athletes or best looking kids around but you love them more than anything else because they are yours, you and your spouse created them.

GregG

I'm taking this whole concept as a fun mental exercise. I like reading the forums here to see how other people do things and I picked up the "for(i=MAX;i;i--)" thing from a post here and have used it since.

I had a summer job once at in a materials testing lab, and one of the test machines had a sticker on the side that I have remembered for 25+ years: "One test is worth a thousand expert opinions"
So on that track, I just had to write up a short test program, which is included as attachment here.

I checked the time on 200 loops parsing both normal and worst case strings through the last 2 sets of head and tail functions we posted.

The "normal" string for the test was '"]},{["String Data","More Data"]}'
The "worst case" string was '"]},{[""]},{[""]},{[""]},{["Data"]},{[""]},{[""]},{[""]},{["'
And the character list to trim was '}],"{['

The results (using GET_TIMER deltas) are:

**NI700**

fnhead on normal data using character compares: 173
fntail on normal data using character compares: 165
fnhead on worst case data using character compares: 178
fntail on worst case data using character compares: 172

fnhead on normal data using string function calls: 60
fntail on normal data using string function calls: 87
fnhead on worst case data using string function calls: 63
fntail on worst case data using string function calls: 93

**NX1200**

fnhead on normal data using character compares: 45
fntail on normal data using character compares: 42
fnhead on worst case data using character compares: 46
fntail on worst case data using character compares: 45

fnhead on normal data using string function calls: 55
fntail on normal data using string function calls: 60
fnhead on worst case data using string function calls: 59
fntail on worst case data using string function calls: 65


So your concept runs faster on the new masters, and mine runs faster on the old ones, hehe.

Overall, I'll probably stick with readability, since it will make future maintenance easier (also makes it easier for me to copy and modify into other functions).

vining

GregG wrote:

I'm taking this whole concept as a fun mental exercise. I like reading the forums here to see how other people do things and I picked up the "for(i=MAX;i;i--)" thing from a post here and have used it since.

Isn't that the point of these forums, unfortunately it doesn't happen nearly enough anymore but it used to be done regularly here.

Even though I was trying to avoid using find_string it is optimized as your test on the NI-700 showed. A few years ago after someone mentioned this optimization I ran a test between Netlinx's find_string and a version that I wrote that I thought would be comparable to how find_string functions internally and the netlinx find_string blew myfind_string out of the water. Granted my code probably wasn't as efficient as it could be so I kinda want to find it just to see if it could be done better or should I say if I could do it better and try the test again if I found some significant shortcuts.

I am surprised with your test comparing the find_string in the NI vs NX, not big difference and I would have thought the NX would have been much faster. The difference between the char compare and find_string on the NI was huge which makes me wonder what they do to optimize those netlinx functions in the NI's and why it doesn't seem to be a factor in the NX processor.

I think it was Spire_Jeff who first posted "for(i=MAX;i;i--)" with a lot of test data. That was back in the day when the forum was fun and there were a lot of folks wanting to learn, share and willing test their or some one else's theories. I was planning on running some tests too just cuz some times what you think might be faster, isn't. I also think my latest version might throw an error if the string only contains chars to trim so nothing remains in the string. That might happen 1 in a 1000 times so I want to verify and fix if that's the case.

Like you said it's nice to see another persons approach, I never would of thought about using find_string it what I would consider a backwards fashion putting the chars to remove in as the string to search in. In my brain that's backwards but I guess as Spire_Jeff posted several years ago, some times backwards is the better way.

GregG

So I gave optimization some more thought tonight and came up with an idea that can be used in situations where the character list you are trying to strip is frequently the same. It is similar to a log table lookup I did a while back for translating linear audio taper into a log taper.

First pre-populate a 255 char boolean flag array with the characters you want to kill using a prep function, then do a character compare via table lookups. Multiple prep tables can be kept around in global vars. The test program is included, but the prep and trim functions look like this:

Define_Function Char[255] InitTable(Char cStrip[])
{
Stack_Var Char cCompareTable[255]
Stack_Var Integer nPos
    cCompareTable = "0,0<....255 total zeros in here....>0,0,0,0"
    For(nPos=Length_String(cStrip);nPos;nPos--)
        cCompareTable[cStrip[nPos]] = 1
    Return cCompareTable
}

Define_Function fnTrimCharsFromHead(CHAR cKillTable[255],CHAR iStr[])
{
Stack_Var Integer nCnt
Stack_Var Integer nPos
Stack_Var Integer nLen
    nLen = Length_String(iStr)
    For(nPos=1;nPos<=nLen;nPos++)
    {
        If(cKillTable[iStr[nPos]])
            nCnt++
        Else
            Break
    }
    If(nCnt)
        Get_Buffer_String(iStr,nCnt)
}

The results using this method are:

** NI700 **
Normal tests on head total time: 63
Normal tests on tail total time: 63
Worst case tests on head total time: 64
Worst case tests on tail total time: 64

** NX1200 **
Normal tests on head total time: 25
Normal tests on tail total time: 25
Worst case tests on head total time: 25
Worst case tests on tail total time: 25

GregG

I realize I had a partial mix of 2 different ideas for loops as part of my test routines, so I fixed that and re-ran all the tests:

               |     Normal Data    |  Worst Case Data  |
---------------------------------------------------------
Function       |  Head NI | Head NX | Head NI | Head NX |
---------------------------------------------------------
Vining         |    137   |    26   |   398   |    68   |
Greg strfn     |     82   |    79   |   274   |   289   |
Greg table     |     45   |    11   |    98   |    22   |

This really emphasizes the poor string function speed on the NX

vining

GregG wrote: »

So I gave optimization some more thought tonight and came up with an idea that can be used in situations where the character list you are trying to strip is frequently the same. It is similar to a log table lookup I did a while back for translating linear audio taper into a log taper.

First pre-populate a 255 char boolean flag array with the characters you want to kill using a prep function, then do a character compare via table lookups. Multiple prep tables can be kept around in global vars. The test program is included, but the prep and trim functions look like this:

Define_Function Char[255] InitTable(Char cStrip[])
{
Stack_Var Char cCompareTable[255]
Stack_Var Integer nPos
cCompareTable = "0,0<....255 total zeros in here....>0,0,0,0"
For(nPos=Length_String(cStrip);nPos;nPos--)
cCompareTable[cStrip[nPos]] = 1
Return cCompareTable
}

Define_Function fnTrimCharsFromHead(CHAR cKillTable[255],CHAR iStr[])
{
Stack_Var Integer nCnt
Stack_Var Integer nPos
Stack_Var Integer nLen
nLen = Length_String(iStr)
For(nPos=1;nPos<=nLen;nPos++)
{
If(cKillTable[iStr[nPos]])
nCnt++
Else
Break
}
If(nCnt)
Get_Buffer_String(iStr,nCnt)
}

The results using this method are:

** NI700 **
Normal tests on head total time: 63
Normal tests on tail total time: 63
Worst case tests on head total time: 64
Worst case tests on tail total time: 64

** NX1200 **
Normal tests on head total time: 25
Normal tests on tail total time: 25
Worst case tests on head total time: 25
Worst case tests on tail total time: 25

Ok, I'm feeling kinda stupid right now and I'm hoping it's your code and not me but I'm not following this logic, I don't see how it's doing anything so maybe I just need a good night's sleep and I can figure it out in the morning.

Now that I just admitted being stupid I think I got it and it's pretty cool the way it works, now I just have to reason what's making it so much faster than my version, visually it appears to be pretty much the same work load except I'm doing an extra comparison each pass and an additional simple math assignment. I never would have thought about making a comparison in this manner, very cool! I just hope others look at it and don't get it at first either.

GregG

The theory behind the speed difference comes from the differences in the algorithmic complexity between the two loops.

We have 2 inputs to the algorithm:

The list of strip characters which, we'll say has 'm' items
and
The actual input string which has 'n' items.

In your loop you go through each character in the strip list comparing it to the current character in the input. In a worst case scenario, where the whole input string contains nothing but the last character in the strip list, your algorithm would have to make (n*m) passes in the for loop. This is called 'polynomial time' in algorithmic analysis: https://en.wikipedia.org/wiki/Time_complexity#Polynomial_time

In the version where I pre-populated a lookup table, there is only ever one comparison made for any given input character, so at most it would need to make (n) passes through the loop. Which is 'linear time': https://en.wikipedia.org/wiki/Time_complexity#Linear_time

https://en.wikipedia.org/wiki/Lookup_table

vining

Yep that makes perfect sense and looks like the clear logical winner, nice job!

vining

After being enlighten by GregG to usage and the speed of using look up tables for string manipulation I decided to play around with some other functions I had it this module and converted them to use these look up tables instead of additional loops so here they are along with some code to make it easy to understand and play with them. I didn't do any speed comparison like Greg did on the TrimcharFromHead test but given those lightning fast speeds I would expect these to be similarly faster than their loopy counterparts. Of course GregG didn't run the Table test on an NI master that I know of so maybe he'll do that since he has the code already to do that.

DEFINE_VARIABLE //GENERAL

VOLATILE INTEGER nRunTest = 0;
VOLATILE CHAR cSomeVar_1[128];
VOLATILE CHAR cSomeVar_2[128];
VOLATILE CHAR cSomeVar_3[128];
VOLATILE CHAR cSomeVar_4[128];

VOLATILE CHAR cTrimCharTable_JSON[255];
VOLATILE CHAR cTrimCharTable_URL[255];
VOLATILE CHAR cReplaceCharTable_1[510];

DEFINE_FUNCTION fnInit_Stuff()//called in define_start, your virtual online or somewhere else prior ro needing

     {                                            
     cReplaceCharTable_1 = fnInit_ReplaceCharTable('",{}[]','gsesTt');
     cTrimCharTable_JSON = fnInit_TrimCharTable('",{}[]');
     cTrimCharTable_URL = fnInit_TrimCharTable('/\:%');
     }

DEFINE_FUNCTION CHAR[510]fnInit_ReplaceCharTable(CHAR iReplace[],CHAR iReplacement[])
//initialize a table of chars to replace and the chars to replace them with    
     {
     if(length_string(iReplace) == length_string(iReplacement))
      {
      STACK_VAR CHAR cTable[510];
      STACK_VAR INTEGER i;
      
      for(i=510;i;i--)//ensure a zero table
           cTable = "cTable,0";
      for(i=Length_String(iReplace);i;i--)
           {
           cTable[iReplace[i]] = 1;
           cTable[255 + iReplace[i]] = iReplacement[i];
           }
      RETURN cTable;
      }
     else
      SEND_STRING 0, 'ERROR, Array lengths do not match!!!';
     RETURN '';
     }    

DEFINE_FUNCTION CHAR[255] fnInit_TrimCharTable(Char iTrimStr[])
//initialize a table of chars to remove from head, tail or anywhere in between    
     {
     STACK_VAR CHAR cTable[255];
     STACK_VAR INTEGER i;
    
     for(i=255;i;i--)//ensure a zero table
      cTable = "cTable,0";
     for(i=Length_String(iTrimStr);i;i--)
           cTable[iTrimStr[i]] = 1;
          
     RETURN cTable;
     }

DEFINE_FUNCTION fnTrimCharsFromHead(CHAR iTrimTable[255],CHAR iStr[])
//remove chars set in a trim table from the head of a string
     {
     STACK_VAR INTEGER i;
     STACK_VAR INTEGER nCnt;
     STACK_VAR INTEGER nLenS;
        
     nLenS = Length_String(iStr);
     for(i=1;i<=nLenS;i++)
      {
      if(iTrimTable[iStr[i]])
           nCnt++;
      else
           BREAK;
      }
     if(nCnt)
      GET_BUFFER_STRING(iStr,nCnt);
     }
    
DEFINE_FUNCTION fnTrimCharsFromTail(CHAR iTrimTable[255],CHAR iStr[MAX_15999])
//remove chars set in a trim table from the tail of a string
     {
     STACK_VAR INTEGER i;
     STACK_VAR INTEGER nCnt;    
     STACK_VAR INTEGER nLenS;
      
     nLenS = LENGTH_STRING(iStr);
     for(i=nLenS;i;i--)
      {
      if(iTrimTable[iStr[i]])
           nCnt++;
      else
           BREAK;
      }
     if(nCnt)
      SET_LENGTH_STRING(iStr,nLenS-nCnt);
     }
 
DEFINE_FUNCTION CHAR[MAX_15999]fnTrimCharsFromString(CHAR iTrimTable[255],CHAR iStr[MAX_15999])
//remove chars set in a trim table from anywhere in a string
     {
     STACK_VAR INTEGER i;
     STACK_VAR CHAR cStr[15999];
    
     for(i=LENGTH_STRING(iStr);i;i--)
      {
      if(!iTrimTable[iStr[i]])
           cStr = "iStr[i],cStr";
      }
     RETURN cStr;
     }
  
DEFINE_FUNCTION CHAR[MAX_15999]fnReplaceChars(CHAR iReplaceTable[510],CHAR iStr[MAX_15999])
//remove chars set in a replace table and replace with another char in the same table from anywhere in the string        
     {
     STACK_VAR INTEGER i;
     STACK_VAR CHAR cStr[15999];

     for(i=LENGTH_STRING(iStr);i;i--)
      {
      if(!iReplaceTable[iStr[i]])
           cStr = "iStr[i],cStr";
      else
           cStr = "iReplaceTable[255+iStr[i]],cStr";
      }
     RETURN cStr;
     }      

DEFINE_PROGRAM

WAIT 10
     {
     if(nRunTest)
      {
      nRunTest = 0;
      cSomeVar_1 = fnReplaceChars(cReplaceCharTable_1,'Thi, [{}] Strin"');
      cSomeVar_2 = fnTrimCharsFromString(cTrimCharTable_URL,'%Thi:\\s ://Test Stri%ng');  
      cSomeVar_3 = '",}]{[This Test String{},"][}'
      fnTrimCharsFromHead(cTrimCharTable_JSON,cSomeVar_3);
      cSomeVar_4 = cSomeVar_3;
      fnTrimCharsFromTail(cTrimCharTable_JSON,cSomeVar_4);
      }
     }

To test and change just drop these vars into the debug window and set nRunTest to 1.

GregG

Actually in post #24 ( http://www.amxforums.com/forum/technical-forum/general-discussion/120562-which-is-faster-vs-proper?p=120613#post120613 ) I made a comparison table of all 3 remove from head function variations for normal and worst cases on both the NI and the NX.

I like the replace character idea, I often replace "CR,LF"s from http results with spaces (then strip the extra spaces) so that I can reformat the text into a nicer display block.

vining

The replace char function doesn't really need to use 510 char table, it could 255 chars and instead setting a flag to 1 and putting the replacement at that index + 255 you could instead of setting the flag to 1 just it set to the replacement char value. I was initially going to do that but for some reason changed it.

I remember seeing that NI comparisons, I think my short term memory is failing. . Looking at the results changing to a look up table can be anywhere from 2 to 8 times faster depending what's presently being used so it's worth updating to this method in any code that has to do a lot of string manipulations.

Edit: a 255 char table works unless you want to replace a char with a decimal 0 in which case you'd need the 510 char table with the flag set to 1 and then the value of 0 and that index + 255 but I don't ever see a need to change a char to a decimal 0.

vining

I ran some tests to check the speed of these table based string manipulating function on my NI3100 and here's want I got. Later after I do some real work I'll try some comparisons to other versions of these function and maybe load it onto my NX2200 for further comparisons. GregG has already done the TrimCharHead/Tail so we really only need to see if there are significant benefits to using the table version of RepalceChars and TrimCharsFromString.

Results:

Line 1 (09:55:36):: String table test "ReplaceChars" completed....... 200 loops in 207 milliseconds.
Line 2 (09:55:36):: String table test "TrimCharsFromString" completed 200 loops in 237 milliseconds.
Line 3 (09:55:36):: String table test "TrimCharsFromHead" completed...200 loops in 82 milliseconds.
Line 4 (09:55:36):: String table test "TrimCharsFromTail" completed...200 loops in 85 milliseconds.
Line 5 (09:55:49):: String table test "ReplaceChars" completed....... 1000 loops in 1036 milliseconds.
Line 6 (09:55:50):: String table test "TrimCharsFromString" completed 1000 loops in 1213 milliseconds.
Line 7 (09:55:50):: String table test "TrimCharsFromHead" completed...1000 loops in 403 milliseconds.
Line 8 (09:55:51):: String table test "TrimCharsFromTail" completed...1000 loops in 407 milliseconds.

TestCode: (I just dropped this into a running timeline or define_program after a var flag that resets to zero when run and I change that var flag in NS's debug window to 1 to run it)

//in vars, can also change on the fly in debug
VOLATILE INTEGER nNumTestLoops = 200;

//in time_line or define_program
LOCAL_VAR CHAR cSomeVar_1[128];
      LOCAL_VAR CHAR cSomeVar_2[128];
      LOCAL_VAR CHAR cSomeVar_3[128];
      LOCAL_VAR CHAR cSomeVar_4[128];
      
      LOCAL_VAR LONG nSomeVar_1;
      LOCAL_VAR LONG nSomeVar_2;
      LOCAL_VAR LONG nSomeVar_3;
      LOCAL_VAR LONG nSomeVar_4;
      STACK_VAR LONG nTL_TIME_[1];
      
      STACK_VAR INTEGER i;
      
      i=0;//allows compile,
      nTL_TIME_[1] = 1000000;
                  
      if(!TIMELINE_ACTIVE(TL_TIMER_TEST))
           {
           TIMELINE_CREATE(TL_TIMER_TEST,nTL_TIME_,1,TIMELINE_ABSOLUTE,TIMELINE_REPEAT);
           }
      
      TIMELINE_SET(TL_TIMER_TEST,0);
      for(i=nNumTestLoops;i;i--)
           {
           cSomeVar_1 = fnReplaceChars(cReplaceCharTable_1,'Thi, [{}] Strin"');
           }
      nSomeVar_1 = TIMELINE_GET(TL_TIMER_TEST);
      SEND_STRING 0, "'String table test "ReplaceChars" completed....... ',itoa(nNumTestLoops),' loops in ',itoa(nSomeVar_1),' milliseconds.'";
      
      TIMELINE_SET(TL_TIMER_TEST,0);
      for(i=nNumTestLoops;i;i--)
           {
           cSomeVar_2 = fnTrimCharsFromString(cTrimCharTable_URL,'%Thi:\\s ://Test Stri%ng');
           }
      nSomeVar_2 = TIMELINE_GET(TL_TIMER_TEST);
      SEND_STRING 0, "'String table test "TrimCharsFromString" completed ',itoa(nNumTestLoops),' loops in ',itoa(nSomeVar_2),' milliseconds.'";
      
      TIMELINE_SET(TL_TIMER_TEST,0);
      for(i=nNumTestLoops;i;i--)
           {
           cSomeVar_3 = '",}]{[This Test String{},"][}'
           fnTrimCharsFromHead(cTrimCharTable_JSON,cSomeVar_3);
           }
      nSomeVar_3 = TIMELINE_GET(TL_TIMER_TEST);
      SEND_STRING 0, "'String table test "TrimCharsFromHead" completed...',itoa(nNumTestLoops),' loops in ',itoa(nSomeVar_3),' milliseconds.'";
      
      TIMELINE_SET(TL_TIMER_TEST,0);
      for(i=nNumTestLoops;i;i--)
           {
           cSomeVar_4 = cSomeVar_3;
           fnTrimCharsFromTail(cTrimCharTable_JSON,cSomeVar_4);
           }
      nSomeVar_4 = TIMELINE_GET(TL_TIMER_TEST);
      SEND_STRING 0, "'String table test "TrimCharsFromTail" completed...',itoa(nNumTestLoops),' loops in ',itoa(nSomeVar_4),' milliseconds.'";
      TIMELINE_KILL(TL_TIMER_TEST);
      }