Re: Is this code totaly a ***?

lovecreatesbeautifulgirls@xxxxxxxxx writes:
[original code snipped]

Here's a version of your code reformatted for legibility; I've
made no other changes. Code is prefixed by "| "; my comments are
interspersed.

| #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];

Txt1 and Txt2 are variable-length arrays. Since the value of n at
this point is always 40, there's no good reason not to use ordinary
fixed-length arrays:

#define MAX 40
char Txt1[MAX];
char Txt2[MAX];

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

Why do you print new-lines before each prompt?

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

This is dangerous. scanf with a "%s" format reads an arbitrarily long
whitespace-delimited string. If the user types more than 40
consecutive non-whitespace characters, you'll have a buffer overflow
and undefined behavior.

Furthermore, the prompt asks for a string, but you only accept a
single whitespace-delimited word. If I enter "Hello world", the "
world" will be silently ignored. (Well, not silently; it remains in
the input stream, waiting to be read.)

| n = strlen(Txt1);

You originally used n to determine the size of your arrays. Now
you're using it to store the current length of the input string. Use
a different variable with a more meaningful name.

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

Normally when you iterate over a string, the index would run from 1 to
n-1, where n is the length. Here your loop covers the terminating
'\0' as well. It's difficult to tell whether that's right or wrong.

| if ((Txt1[i] <= 'a') || (Txt1[i] >= 'z')) {
| printf("\nThere are no lowercase letters "
| "in this string !\n\n");

You perform this check once for every character of the input string.
If I enter "HELLO", I get the "There are no lowercase letters" message
6 times.

| }
| else {
| UppStrg(&Txt1[i], &Txt2[i], n);

And again, your calling UppStrg multiple times for each input string.

| }
| }
| } while ((Txt1[i] <= 97) && (Txt1[i] >= 123));

What is the significance of the numbers 97 and 123?

97 happens to be the ASCII code for 'a'. 123 happens to be the ASCII
code for '{'; 'z' is 122. Use character constants, not integer
constants.

You're also assuming that the representations of the characters
'a' through 'z' are contiguous. The language doesn't support
this assumption.

Most of what you're doing here could be made vastly easier if you
used <ctype.h>. If your intent is to reproduce the functionality
of <ctype.h> as a learning exercise, please say so.

What is the value of i at this point? You've already iterated over
your string; you probably shouldn't even be using i. As far as I can
tell, i is the index just past the terminating '\0' of Txt1. It could
be uninitialized, or it could point past the end of the array. And
your program will terminate only when this uninitialized or
nonexistent character is within the range 'a' .. '{' (assuming an
ASCII system). I don't believe that's what you intended.

|
| for (i = 0; i <= n; i++) {
| putchar(Txt2[i]);
| }

It's unlikely this code will ever execute.

| printf("\n\n");

Why the double new-line?

| 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';

*(Upp + i) is more clearly written as Upp[i].
*(Low + i) is more clearly written as Low[i].

Again, you're making unwarranted assumptions about the representations
of characters.

| }
| }

Did you try running this program before you posted it?

If you didn't, you are wasting our time (and yours).

If you did, then you should be aware of some of the ways in which
it doesn't behave correctly -- and yet you didn't bother to tell us.
Again, you are wasting our time and yours.

There's nothing wrong with posting non-working code. But if you
want help with it, you should tell us, as precisely as possible,
what it's supposed to do, what it actually does, and how those
things differ. If you have a specific question, ask it. And if
people have already been offering you advice, don't just re-post
code that makes it clear you've ignored it all.

--
Keith Thompson (The_Other_Keith) kst-u@xxxxxxx <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
.