tags:

views:

160

answers:

7

i made a program to convert lowercase strings to upper case like strupr(); in strings.h ..its printing some ascii code when ever i run the program

#include<stdio.h>
#include<conio.h>

void xstrupr(char string[]);

void main(void)
{
    char string[40];
    puts("Enter string:");
    gets(string);
    xstrupr(string);
    printf(" %s ",string);
    getch();
}

void xstrupr(char string[])
{
    int i;
    for(i=0;;i++)
    {
        if ((string[i]>='a')&&(string[i]<='z') )
            string[i]+=64;
        else
            if(string[i]=='\0')
                break;
    }
}
+2  A: 

You're returning an integer (either 0 or i-64) and treating it like it was a pointer to a null terminated string. I'm not surprised it prints junk.

Paul Tomblin
It's only 0 if there are no lower case letters in `string`. Otherwise it's `i-64` where `i` is the index of the first lower case letter.
sepp2k
+1  A: 

The function exits when a lowcase character is encountered, instead it shall replace the current character.

Also the type returned by strupr() is wrong, it shouldn't be char. And it does not return the right thing. So it certainly outputs garbage.

[I don't give you all the details since you seem to be learnig, feel free to ask for more].

philippe
can any 1 help me this code :(
fahad
@fahad: I tried to help, without giving you the solution, explaining you errors in your code. Why did you our that return statement when a lowercase char in met ? The function shall be able to convert several chars. Do you know how to make the replacement ? Also do you know you're supposed to vote for the [good] answers you're getting ? You never voted.
philippe
+6  A: 

This is horrible, if you don't mind me saying. Don't take this personally, of course.

  • Don't rely on ASCII values, never hardcode actual character values as literal numbers.
  • Don't use char variables as loop indices; they're typically too small.
  • Use islower() to check if a character is lower-case.
  • Use toupper() to convert it to upper-case.
  • Realize that code like this doesn't work for the majority of languages
  • Using wide characters is not enough either; some languages have words that change length (charactercount) when going from lowercase to upper.

My suggested implementation in this style would be:

#include <ctype.h>

void xstrup(char *string)
{
  for(; *string; string++)
    *string = toupper((unsigned char) *string);
}

Note that this is just a reasonable (in my opinion) implementation of what it looked like you wanted to achieve, it's not by any means perfect and does not address all of my complaints.

unwind
+1 on not using raw ASCII values.
casablanca
-1 on using toupper() with (possibly) signed char.
Roland Illig
Why do you cast *string to (unsigned char)? That makes no sense. toupper is designed to take a char so give it a char.
Zan Lynx
@Because of Roland's comment. And no, toupper() is not designed to take a char as I remembered, it takes int.
unwind
Yes, it takes an `int` with a value in the range of `unsigned char`.
caf
Thumbs down (tho I'll skip the -1) on not using raw ASCII values. This is 2010 not 1980. EBCDIC is dead and UTF-8 (a superset of ASCII) is the future.Further, whether it's wrong to do your own case conversion depends on the application. For instance if you're implementing case-insensitive, identifiers in a programming language interpreter that might have called `setlocale` for localized messages, using `toupper`/`tolower` will fail in Turkish locale! Here you **must** roll your own.
R..
To elaborate, I have worked on projects where important functionality broke in Turkish locale due to this issue, as well as projects that refused to call `setlocale` for fear of breaking code that's using std lib case mapping functions, numeric formatting, etc. The solution (until POSIX 2008 `locale_t` is part of ISO C...) is to write your own ASCII-only functions for interpreting non-human-language data (and using an advanced Unicode-aware library for natural-language data).
R..
A: 
  1. First, your function returns char. This is irrelevant.
  2. return would break from the function, not replace a character.
  3. This is completely wrong. Upr cannot be done on per-character basis. Example. Upr of the german eszett 'ß' is two characters, aka "SS". Assumping your char* input does not contain utf-8 is a 'text crime'.
Pavel Radzivilovsky
While you're completely correct on your third point, that's out of scope. The C string and character functions were intended to work on a single-byte character set that didn't include characters like the eszett. (Even wide characters are fixed-width, and in many implementations 16-bit, meaning you have the inadequate UCS-2 implementation.) `toupper()` and `tolower()` operate on individual characters, and have no ability to either accept or emit two characters. This is a problem, but dealing with it is really outside the scope of the C standard library.
David Thornley
I don't agree with the "out of scope" claim. UTF-8 should be considered the default for char*, and there's nothing in C standard against it.
Pavel Radzivilovsky
+1  A: 

First, the difference in ASCII value between uppercase and lowercase letters is 32, not 64.

Second, and most important, the C library provides tolower() and toupper() functions, which work not only on ASCII, but in whatever encoding you are currently using.

ninjalj
+1  A: 

It looks to me like you're trying to do this at the most basic level. That being said, you're making one erroneous assumption.

You don't get the uppercase version of a letter by adding 64 to it. Moreover, just supplying a magic number is unclear and may be wrong on another character set.

Try changing string[i] += 64; to string[i] += 'A' - 'a';. That will work on all character sets where there's a constant difference between uppercase and lowercase letters.

Now, this will fail in some cases. For example, in EBCDIC, the letters are not contiguous, so the range from 'a' to 'z' is not all alphabetic. This is why, in real code, you use standard features like isalpha() and toupper(), but this is good as an exercise.

David Thornley
+1  A: 

Actually in ASCII Chart Ranges are as follows:

  • A to Z: 65 to 90
  • a to z: 97 to 122

Your edited program is like that:

#include<stdio.h>
#include<conio.h>

void xstrupr(char string[]);

void main(void)
{
    char string[40];
    puts("Enter string:");
    gets(string);
    xstrupr(string);
    printf(" %s ",string);
    getch();
}

void xstrupr(char string[])
{
    int i;
    for(i=0;;i++)
    {
    if ((string[i]>='a')&&(string[i]<='z') )
        string[i]-=32;
    else
        if(string[i]=='\0')
        break;
    }
}

See the ascii table below:

ASCII Table


Sources:

Image is taken from http://www.unitechnical.info/

chanchal1987