Help a beginner - simple lowercase to uppercase and so on function

Discussion in 'C Programming' started by bpascal123, Jul 26, 2009.

  1. bpascal123

    bpascal123 Guest

    Hi,

    I'm posting quite a lot, i'm learning c without any solid programming
    experience. So when my code doesn't work after looking why if i can't
    get to know why, I ask here and i keep on coding as well.

    So below is a simple lowercase to uppercase code from a to z that
    doesn't fully work. I use gcc on linux and i've tried on gcc on
    Windows and the output is quite the same (however nothing is return in
    djgpp windows).

    In gcc linux, It takes the input but it returns it after some delay
    and it adds some characters i have never seen at this stage of
    learning and nowwhere since i use a computer...

    I don't know if it's the right way to do this, i wish pointers
    wouldn't be involved for this. I can't think of a way to do this with
    functions to split actions. Once this part below works, i'd like to
    add some more string functions on the model of the function (UppStrg)
    below :


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

    void UppStrg(char *Low, char *Upp, int cnt) ;

    int main(void)
    {
    int n = 40 ;
    char Txt1[n] ;
    char Txt2[n] ;

    int i ;

    printf("\n\nThis program reads and converts : 1- a lower case string
    into upper case : \n") ;

    do
    {
    printf("\nEnter a lowercase string : \n") ;
    scanf("%s", Txt1) ;

    n = strlen(Txt1) ;

    for ( i = 0 ; i <= n ; i++ )
    {
    if ( (Txt1 <= 'a' ) || (Txt1 >= 'z') ) /* IF checks if there
    are any characters other than lowercase letters */
    printf("\nThere are no lowercase letters in this string !\n\n") ;
    else
    UppStrg(&Txt1, &Txt2, n) ;
    }

    } while ( (Txt1 <= 97 ) && (Txt1 >= 123) ) ;

    for ( i = 0 ; i <= n ; i++ )
    putchar(Txt2) ;
    printf("\n\n") ;

    return 0 ;
    }

    /* Function that should move a lowercase letter into uppercase */

    void UppStrg(char *Low, char *Upp, int cnt)
    {
    int i ;

    for ( i = 0 ; i <= cnt ; cnt++ )
    *(Upp+i) = *(Low+i) - 'a' + 'A';
    }
     
    bpascal123, Jul 26, 2009
    #1
    1. Advertisements

  2. bpascal123

    bartc Guest

    You're using variable arrays here. Not causing your problems but take a bit
    more care to use.



    These 2 nested loops are a mess. Once you've detected a character is not
    lower case, you want to break out of the inner loop.

    The Uppstrg() functions converts a single character, or a whole string? If a
    whole string, it should be outside the inner loop, with Tx1 and Txt2
    arguments. If a single character, you shouldn't use "&", and you need to
    rewrite UppStrg! This is mainly why the program is doing strange things.

    The conditional to the outer do-while loop is meaningless. 'i' is undefined
    at this point. (I'm not familiar with how scanf forces a new input line, so
    I'd would read the input a little differently myself, perhaps using fgets()
    or getchar())

    Your for-loop is also looking at the 0-terminator of the string, which is
    not necessary; make i count from 0 to <n.
    If you're going to assume Ascii code, then just subtract 32 from the
    character code...
    Usually case conversion code will work with any character, and ignore
    anything not a..z or A..Z, depending which way it's going. (There are anyway
    standard C functions for case-converting characters or strings.)
     
    bartc, Jul 26, 2009
    #2
    1. Advertisements

  3. bpascal123

    Doug Miller Guest

    Is there some reason you don't simply use toupper() ?
     
    Doug Miller, Jul 26, 2009
    #3
  4. bpascal123

    luserXtrog Guest

    When that happens, it means the ends of your strings (the terminating
    nul) is getting snipped. Step one: check loop conditions and array
    lengths.
    I'd make a function to transpose one character (int tr(int)).
    And then one to loop across the string calling that function
    for each character. And I'd probably do it in place instead of
    copying at to a new array (why would you need both?).


    Your spacing makes this very difficult to read.
    I don't understand why you need all that space.
    I also don't understand why you're calling UppStrg
    in this loop. You're incrementing the offset and
    asking it to modify 40 elements starting at each position.
    Every time through this loop after the first, you're
    asking UppStrg to modify elements that are outside of
    the bounds of the array.
    UppStrg should be called once for each string.
    If you need to verify that the string consists
    only of lowercase characters, you can use some
    kind of flag value that's set in the loop when
    a bogus character is encountered and then check
    after the loop whether it was set. Or you can
    use some standard library functions. I recently
    discovered this one:

    if (strspn(s,"abcdefghijklmnopqrstuvwxyz") == strlen(s))
    printf("char *s consists wholy of lowercase letters\n");



    Dude, use the array notation for strings. It's prettier.
    Upp = Low - 'a' + 'A';
    I don't like the arithmetic here, either. It would make more
    sense to me to add the difference rather than subtracting the
    sum. But better that all these (IMHO) is

    char *al="abcdefghijklmnopqrstuvwxyz";
    char *AL="ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    ....
    Upp = AL[strchr(al, Low) - al];

    This way doesn't assume ASCII, but it assumes that you've already
    made sure the string consists only of lowercase characters.
    Otherwise strchr will return NULL and NULL - al is not a valid
    index into the AL array.
     
    luserXtrog, Jul 26, 2009
    #4
  5. bpascal123

    bartc Guest

    Leave him be; he's single-handedly keeping this group alive it seems to me.
     
    bartc, Jul 26, 2009
    #5
  6. the only way to get the experience is to practice programming.
    Though you could be a little more sytematic...
    you could try debugging it. Looking isn't enough.
    you could use arrays rather than pointers.
    imagine you had a friend looking over you shoulder and you explaining
    what the different steps of the program does. Or add comments to
    explain
    what blocks of code (not lines!) do.

    So your program does the following

    read a line
    check the line is all lower case letters
    convert each character to upper case
    print the new line

    that gives you four possible functions. The middle two
    can also be tested on their own.

    #include <assert.h>

    /*returns true(1) if all the characters are lower case
    and false(0) otherwise */
    int is_all_lower (const char s[], int size);

    /* terminates the program if is_all_lower() doesn't return the
    expected
    answer */
    void test_is_all-lower (const char s[], int size, int expected_result)
    {
    /* this crashes if we don't get the expected answer */
    assert (is_all_lower (s,size) == expected_answer);
    }

    void test (void)
    {
    test_is_all_lower ("abc", 3, 1);
    test_is_all_lower ("Abc", 3, 0);
    test_is_all_lower ("abC", 3, 0);
    test_is_all_lower ("@bc", 3, 0);
    test_is_all_lower ("a@c", 3, 0);
    test_is_all_lower ("a", 3, 1);
    }

    int main (void)
    {
    test();
    return 0;
    }

    your posts would look better on the internet if you used a smaller
    indentation. Maybe 4 chatacters (my favourite) or even less.
    beginning identifiers with an upper case less is an odd convention.
    And using a constant like n won't work on older versions of C.

    scanf() is tricky to use correctly (fgets() followed by sscanf() is
    better).
    But if you do use it you *must* check its return value.
    re-using variables for multiple purposes is a bad idea. Modern
    computers
    have *billions* of bytes of RAM.
    I really *hate* this. Could you use less blank space?

    for (i = 0; i < n; i++)

    You also had an off by one error. Suppose n is 10 your loop runs 11
    times.

    [code reformatted]


    really? What about "abcdefghijklmnopqrstuvwxyZ"? Any lower case
    letters?



    you know about character constants ('a' etc) why start using integers?


    <snip>
     
    Nick Keighley, Jul 26, 2009
    #6
  7. bpascal123

    bartc Guest

    It wasn't relevant to my point.
    I assumed the OP's code was a learning exercise.
     
    bartc, Jul 26, 2009
    #7
  8. bpascal123

    Richard Bos Guest

    This is not the best idea to start with. Your own nick should give you a
    hint in a more advisable direction.
    This, by contrast, is a very bad idea. _First_ get it right, then
    complicate it.


    Comparing against unencoded ASCII values is a bad idea for more reasons
    than just portability.
    Your spacing doesn't make your code any easier to read.

    Richard
     
    Richard Bos, Jul 26, 2009
    #8
  9. Even that isn't sufficent. scanf with a "%s" format is inherently
    unsafe; it will attempt to read an arbitrarily long space-delimited
    word, and store that word in an object that cannot possibly be large
    enough to store all possible inputs.

    [...]
    Another style point: don't put blanks in front of your semicolons.

    [...]
     
    Keith Thompson, Jul 26, 2009
    #9
  10. [...]
    ___________________________________________________________
    #include <stdio.h>
    #include <ctype.h>


    char*
    strtoupper(
    char* const buf
    ) {
    char* cur = buf;
    while (*cur) {
    *cur = toupper(*cur);
    ++cur;
    }
    return buf;
    }


    int main(void) {
    char str[] = "1 plus 1 = two";
    puts(strtoupper(str));
    return 0;
    }
    ___________________________________________________________
     
    Chris M. Thomasson, Jul 27, 2009
    #10
  11. bpascal123

    bpascal123 Guest

    Hi,

    I'd first say thanks for your replies.
    I'd study carefully what was said here, at this time i'm not sure i
    can understand everything but it should take shape with more practice
    of C.
    I don't spend too much time thinking before i post because it's not
    certain i'll keep studying at that pace.

    For the code, I think (a little late) the mistake that made it not
    work (compile and return something) is about thses loops where <= n
    was replaced with < n (the '\0' can't be uppercase):


    main :
    for ( i = 0 ; i < n ; i++ )
    {
    if ( (Txt1 <= 'a' ) || (Txt1
    printf("\nThere are no lowercase
    letters in this string !\n\n") ;
    else
    UppStrg(&Txt1, &Txt2, n) ;
    }

    void UppStrg :

    for ( i = 0 ; i < cnt ; i++ )
    *(Upp+i) = *(Low+i) - 'a' + 'A';

    With these modification, it compiles and returns uppercase letters
    however not regarding if true or not "There are no lowercase letters
    in this string !" in gcc linux.

    In Windows djgpp it just doesn't compile unless the loop in main looks
    like :

    for ( i = 0 ; i < n ; i++ )
    UppStrg(&Txt1, &Txt2, n) ;

    Some posts above seem to tell why. I'll read them carefully again.

    I'd expect this code to make upppercase a whole string and not just
    one charactere entered at a time if i answer right your question. Ok,
    this code is not taking many consideration such as non ascii letters
    or uppercase letters... Also the non-official purpose is to put at
    work my understanding of loops and functions which needs more work
    regarding the time i have spent on this i think.

    About the spacing : I first find it easier to spot mistakes and i have
    read spacing would play a role in other languages such as python. I'm
    not preparing to switch to python this way yet, i'd like a good
    understanding of c maybe to program with linux os...i don't know what
    i'll be able to do and when.

    Thx,
    Pascal Baro
     
    bpascal123, Jul 28, 2009
    #11
  12. Pascal, I think that you're probably using lots of tab
    characters in your source, have your tab stops set to
    something other than "every eight columns", and don't
    realize what your code looks like when
    viewed by other people. It looks like this:


    Notice that there is a huge amount of whitespace around your
    ';' and "||", which makes the code extremely hard to read for
    anyone used to any other C code. That's what people are complaining about.


    Assuming ASCII, or some other character set where all of the lowercase
    letters are contiguous, with 'a' having the lowest numeric value
    and 'z' the highest,


    For EVERY CHARACTER IN Txt1 that is not a lowercase letter, this will
    print "There are no lowercase letters". Even if the call to UppStrg made
    sense, and even if UppStrg were coded properly, if Txt1 contained the
    string "Pascal Baro" then the loop would print, THREE TIMES, the message
    "\nThere are no lowercase letters in this string !\n\n". I don't think
    this
    is what you want.
     
    Morris Keesan, Jul 28, 2009
    #12
  13. Why? Do you really find it more readable to be
    modifying your parameters?
     
    Morris Keesan, Jul 28, 2009
    #13
  14. This does have a problem in that plain char may be signed.
    #include <ctype.h>
    char *str_toupper(char *s)
    {
    unsigned char *r = (void *) s;
    while (*r = toupper(*r)) r++;
    return s;
    }
     
    Peter Nilsson, Jul 28, 2009
    #14
  15. The value of arguments won't be modified.
    I think this was discussed here before, and you obviously know it.
    I also prefer pete's methold.
    The signedness of char isn't specified.
    toupper() accepts an int, so I don't see problems here :)
     
    lovecreatesbeauty, Jul 28, 2009
    #15
  16. I'm not sure why. Am I missing some subtlety?
    why? I modify a parameter only if necessary.
    not in my world
    a good time to start!

    <snip>
     
    Nick Keighley, Jul 28, 2009
    #16
  17. do more thinking

    don't think, test! By which I mean, don't guess that your code works;
    test it.


    sorry I didn't understand that



    please post the complete coce that compiles under gcc Linux but not
    under Windows djgpp. At the core it's the same compiler so it *ought*
    to compile on both.
    the question was (or should have been) "does the function UppStrg()
    convert a single character to upper case or does it convert the whole
    string?". If it converts the entire string how many times would you
    expect your code to call it? How many times does your program call it?

    whilst making it harder for every other C programmer in the galaxy.
    Please switch to a more conventional layout. It will become readable
    with practice.
    well you aren't programming in Python at the moment.
    In english we have the expression "don't change horses in mid-stream".
    So stick with one language until you're reasonably happy with it.
    Then learn another one. The first language is the hardest.


    --
    Nick Keighley

    "Programs must be written for people to read, and only
    incidentally for machines to execute."
    - Abelson & Sussman, Structure and Interpretation of Computer Programs
     
    Nick Keighley, Jul 28, 2009
    #17
  18. bpascal123

    bartc Guest

    How about here:

    int main(int argc, char **argv) ?

    Parameters are always input data to a C function. You can modify them if
    they are not quite right, or if they are doing double duty as local
    variables. But usually they are left alone.

    I suspect you weren't being serious however..
     
    bartc, Jul 28, 2009
    #18
  19. bpascal123

    James Kuyper Guest

    It's a variable, for which space has already been allocated - why waste
    it?. If at some point in the function I no longer need to retain it's
    original value, I feel perfectly free to change that value, I only
    declare it const if I see no reason to change it. However, I only change
    the value if I can reasonably say that the new and old values are in
    some sense different values of the same thing.

    That's the same rule I apply to any other variable. I don't re-use
    variables for different purposes in the same block of code; that just
    leads to confusion. I count on the compiler to notice that usage of one
    variable never overlaps usage of another variable, and to save space by
    using the same location in memory for both variables - it's better (and
    far more reliable) about noticing such things than I am.
    Nor mine.
     
    James Kuyper, Jul 28, 2009
    #19
  20. That's your _personal_ experience; so be it.



    struct foo {
    int x;
    };


    void
    foo_foo(
    struct foo* const self
    ) {
    /* [...]; */
    }




    What's wrong with that?
     
    Chris M. Thomasson, Jul 28, 2009
    #20
    1. Advertisements

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.