Code for substring & int conversion

Discussion in 'C Programming' started by qazmlp, Oct 26, 2004.

  1. qazmlp

    qazmlp Guest

    I have written the following code to extract the last 3 digits from a string.
    Is there any improvement needed for this code?

    -------------------
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>

    int main()
    {
    const char* nameWithID = "SOME_NAME_261" ;

    char bufToHold3Digits[4] = {0} ;

    strncpy( bufToHold3Digits , nameWithID + strlen( nameWithID ) - 3 , 3 ) ;

    printf( "%d", atoi(bufToHold3Digits) ) ;
    }
    -------------------
    qazmlp, Oct 26, 2004
    #1
    1. Advertising

  2. On Tue, 26 Oct 2004 12:03:33 -0700, qazmlp wrote:

    > I have written the following code to extract the last 3 digits from a string.
    > Is there any improvement needed for this code?

    Not really. There would be amazingly many ways of achieving the same
    though. If needed you might anticipate there to be more or less than 3
    digits, e.g. an implementation would find the last '_' and take everything
    from there to eol.

    for just 3 digits, a shorter might be:
    const char* nameWithID ="SOME_NAME_261";
    const char *last3; last3 = nameWithID +(strlen(nameWithID)-3);
    printf( "%d",atoi(last3));


    > -------------------
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > int main()
    > {
    > const char* nameWithID = "SOME_NAME_261" ;
    >
    > char bufToHold3Digits[4] = {0} ;
    >
    > strncpy( bufToHold3Digits , nameWithID + strlen( nameWithID ) - 3 , 3 ) ;
    >
    > printf( "%d", atoi(bufToHold3Digits) ) ;
    > }
    > -------------------
    =?iso-8859-1?q?Nils_O=2E_Sel=E5sdal?=, Oct 26, 2004
    #2
    1. Advertising

  3. qazmlp

    Eric Sosman Guest

    qazmlp wrote:
    > I have written the following code to extract the last 3 digits from a string.
    > Is there any improvement needed for this code?
    >
    > -------------------
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > int main()
    > {
    > const char* nameWithID = "SOME_NAME_261" ;
    >
    > char bufToHold3Digits[4] = {0} ;
    >
    > strncpy( bufToHold3Digits , nameWithID + strlen( nameWithID ) - 3 , 3 ) ;
    >
    > printf( "%d", atoi(bufToHold3Digits) ) ;
    > }
    > -------------------


    You are doing both too little and too much work.

    The "too little" part refers to defensive coding:
    your technique will fail on input like "SOME_NAME_26"
    and will fail even worse on input like "42". To guard
    against such possibilities, you should check that
    strlen(nameWithID) >= 3, and you should replace atoi()
    with strtod() and use the error-checking it provides.

    The "too much" refers to all that silly copying.
    You *know* you're copying a three-character string, so
    why not use strcpy() instead of strncpy(), avoiding the
    need to pre-initialize bufToHold3Digits[] (using a form
    C.B.F. thinks ought not to work). Better yet, why not
    get rid of bufToHold3Digits[] altogether, and simply
    apply strtod() to the final three characters of the
    input string, right where they are?

    A still more general approach might convert an
    arbitrary number of trailing digits, not necessarily
    three, enabling you to make sense of "SOME_NAME_1234".
    Whether the greater generality is an "improvement" or
    not depends on the wider context of the problem.

    --
    Eric Sosman, Oct 26, 2004
    #3
  4. qazmlp

    Malcolm Guest

    "qazmlp" <> wrote
    > I have written the following code to extract the last 3 digits from a

    string.
    > Is there any improvement needed for this code?
    >
    > -------------------
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > int main()
    > {
    > const char* nameWithID = "SOME_NAME_261" ;
    >
    > char bufToHold3Digits[4] = {0} ;
    >
    > strncpy( bufToHold3Digits , nameWithID + strlen( nameWithID ) - 3 , 3 )

    ;
    >
    > printf( "%d", atoi(bufToHold3Digits) ) ;
    > }
    >

    The most important thing is that code should be reusable. So you need to
    make the process into a function.

    /*
    gets the last three digits of a string
    Params: out - pointer to output (must be at least 4 characters long)
    in - pointer to input string
    Notes: As Eric Sosman pointed out, what happens if the input has less than
    three characters? What if the last three characters are not decimal
    digits.
    What happens if you are passed NULL?
    There are not necessarily any right answers to these questions, but
    you
    need to think about them.
    */
    void last3digits(char *out, const char *in)
    {
    /* code goes here */
    }

    The other problem is that the test is lousy. You need to recompile for every
    test.

    Try this

    int main(int argc, char **argv)
    {
    char buff[4];
    if(argc == 2)
    {
    last3digits(buff, argv[1]);
    printf("Last 3 digits %s\n", buff);
    }
    else
    printf("Call with an argument string\n");

    return 0;
    }
    Malcolm, Oct 26, 2004
    #4
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Schnoffos
    Replies:
    2
    Views:
    1,190
    Martien Verbruggen
    Jun 27, 2003
  2. Hal Styli
    Replies:
    14
    Views:
    1,605
    Old Wolf
    Jan 20, 2004
  3. arun
    Replies:
    8
    Views:
    426
    Dave Thompson
    Jul 31, 2006
  4. aling
    Replies:
    8
    Views:
    927
    Jim Langston
    Oct 20, 2005
  5. Replies:
    3
    Views:
    182
    Sherm Pendley
    Aug 3, 2005
Loading...

Share This Page