LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Programming (http://www.linuxquestions.org/questions/programming-9/)
-   -   please optimize this C function (http://www.linuxquestions.org/questions/programming-9/please-optimize-this-c-function-352121/)

ewt3y 08-11-2005 04:56 AM

please optimize this C function
 
i, I launched myself into writing a function used to remove symbols within a string. It is not very robust, can you improve ?

char *RmSymbols ( const char *str ) {
char *ptr = str;
while (*ptr != '\0') {
if (*ptr >= 65 && *ptr <=90) {
*RmSymbols++ = *ptr;
}
ptr++;

}
}

theYinYeti 08-11-2005 05:33 AM

Is that homework, or what? That's two strange posts I've seen from you...

Yves.

itsme86 08-11-2005 11:36 AM

Not very robust? Optomize? If it doesn't even work I'd say optomization should be the last thing you should be worrying about. You're trying to dereference and increment a function. You're going to need to create another buffer.

lowpro2k3 08-11-2005 01:24 PM

Please use [ code ] tags. Quote me if you dont know how. You might also want to try using a COMPILER which will tell you this function doesn't work unless RmSymbols is some sort of global variable, and if thats the case why are you returning it. This isn't bash, you need to declare your variables before you use them.

Code:

char *RmSymbols ( const char *str ) {
  char *ptr = str;
  while (*ptr != '\0') {
    if (*ptr >= 65 && *ptr <=90)
      *RmSymbols++ = *ptr; /* what are you doing??? */
    ptr++;
  }
}


vladmihaisima 08-11-2005 03:27 PM

For sure seems like a homework :-p.

Anyway I vaguely rember that in some language (Pascal?) the return value of a function was a variable with the function name.

In C this is not the case. You must declare the variable (and allocate if it is a pointer) which you want to return and return it explicitly.

But anyway I think you should read first a C tutorial...

orgcandman 08-11-2005 03:34 PM

Code:

char *RmSymbols( const char *str)
{
    static char cRmSymBuf[8192] = {0};
    char *pStrIt = str;
    int iPos = 0;
   
    while((*pStrIt != 0) && (iPos < 8192))
    {
        if(*pStrIt >= 'A' && *pStrIt <= 'Z')
        {
          cRmSymBuf[iPos++] = *pStrIt;
        }
        pStrIt++;
    }

    return cRmSymBuf;
}

would be a better example of something for people to optimize.

Personally, I'd make it look like this:

Code:

int fRemoveSyms(char *pSource, char *pDest)
{
    while((*pSource++ = *pDest) != 0)
    {
        if((*pDest <= 'Z') && (*pDest >= 'A'))
        {
            pDest++;
        }
    }
    return strlen(pDest);
}

ymmv (And note, I haven't tested either of these functions, but I'm pretty sure they work).

itsme86 08-11-2005 04:10 PM

Aren't you copying the wrong way here (dest to source, instead of source to dest)?
Code:

while((*pSource++ = *pDest) != 0)
Also, the call to strlen() can be eliminated (since it's slow) so I would write it like this:
Code:

int fRemoveSyms(char *pSource, char *pDest)
{
    char *d = pDest;

    while((*d = *pSource++))
    {
        if((*d <= 'Z') && (*d >= 'A'))
        {
            d++;
        }
    }
    return d - pDest;
}


orgcandman 08-11-2005 04:25 PM

You got me. Sorry...I had been eating cake, so I wasn't looking for program corectness...more of a general idea. Plus, leaving bugs in would help him/her with debugging anyway.

Good catch w.r.t. strlen(), also. I should probably spend less time writing code while eating cake, I think ;)

JanusPaul 08-12-2005 02:32 AM

char *RmSymbols ( const char *str ) {
char *ptr = str;
while (*ptr != '\0') {
if (*ptr >= 65 && *ptr <=90) {
*RmSymbols++ = *ptr;
}
ptr++;
}
}

I suggest ditching 'str' since you really aren't using it in the first place. 'str' isn't byref so altering it wouldn't be a bad thing persay here... You can reduce the size of the code with increasing/reducing number figures around >= and <= to do the exact same thing only with > and <! Using the function name *( RmSymbols++ ), as a variable itself, is very indicative this guy is a Visual Basic user. As this is one feature to the language I would imagine is Microsoft's attempt at making people more dependent on their products.

char *RmSymbols ( char *ptr )
{
while ( *ptr )
{
if ( *ptr > 64 && *ptr < 91 )
*( RmSymbols++ ) = *ptr;
ptr++;
}
}

ewt3y 08-12-2005 05:25 AM

My heartfelt thanks to you all !

Hitler's follower.

theYinYeti 08-12-2005 06:48 AM

Quote:

Originally posted by ewt3y
My heartfelt thanks to you all !

Hitler's follower.

:eek: :eek: I'll consider you don't grasp the meaning of those words!


All times are GMT -5. The time now is 11:34 AM.