Learning pointers

[MathStudent]

Can someone take a look at my code and let me know if you see any
mistakes?

========================

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work
#include <string.h>

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

// Ensure that the string terminator is no more than the last
// element in the array.
// -----------------------------------------------------------

txtptr = text;
*(txtptr + strlen(txtptr) - 1) = '\0';

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;
while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\n\nThe line of text in lowercase is:\n");

txtptr = text;
while ( *txtptr != '\0')
putchar( tolower(*txtptr++));
putchar('\n');

printf("\n");

getch(); // Pauses output

return 0;

} // end main

 

[Jens.Toerring]

Can someone take a look at my code and let me know if you see any
mistakes?

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

// Ensure that the string terminator is no more than the last
// element in the array.
// -----------------------------------------------------------

txtptr = text;
*(txtptr + strlen(txtptr) - 1) = '\0';

This won't work. If 'text' does not alrealy contain a '\0' at the
end you can't use strlen() because all it does is count the number
of chars until it hits '\0', so if there isn't one strlen() won't
be able to figure out where the string ends. I understand that you
wrote it this way to use pointers but here a simple

*(txtptr + sizeof text - 1) = '\0';

is what you need.
Regards, Jens
--
_ _____ _____
| ||_ _||_ _| (e-mail address removed)-berlin.de
_ | | | | | |
| |_| | | | | | http://www.physik.fu-berlin.de/~toerring
\___/ens|_|homs|_|oerring

 

[MathStudent]

txtptr = text;

This won't work. If 'text' does not alrealy contain a '\0' at the
end you can't use strlen() because all it does is count the number
of chars until it hits '\0', so if there isn't one strlen() won't
be able to figure out where the string ends. I understand that you
wrote it this way to use pointers but here a simple

*(txtptr + sizeof text - 1) = '\0';

is what you need.
Regards, Jens

=========================

Hey Jens,

When I add that code I get the following warning:

local variable 'txtptr' used without having been initialized

How should I initialize the variable? Just set it equal to zero?

Here is my updated code:

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work
#include <string.h>

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

// Ensure that the string terminator is no more than the last
// element in the array.
// -----------------------------------------------------------

*(txtptr + sizeof text - 1) = '\0';

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;
while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\n\nThe line of text in lowercase is:\n");

txtptr = text;
while ( *txtptr != '\0')
putchar( tolower(*txtptr++));
putchar('\n');

printf("\n");

getch(); // Pauses output until the user presses a button on
the keyboard.

return 0;

} // end main

 

[Richard Bos]

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work
#include <string.h>

int main(void)
{

// Declare variables
// -----------------

Be careful with this kind of comment. They add nothing to the code
except that they restate the obvious; they can confuse the reader into
thinking something non-obvious is going on (after all, why else
comment?) when there isn't; and because they tend to get ignored, they
can easily get out of sync with the code.
It's much better not to comment on _what_ is done, but on _why_ it is
done. Everybody can figure out that you declare variables here, but it's
sometimes useful to know what a particular variable is intended to be
used for.

char text[70];
char *txtptr;

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

Note that this doesn't prevent the user from entering more than 69
characters; the rest will remain in the input stream, waiting for your
next input function.

// Ensure that the string terminator is no more than the last
// element in the array.

This comment does not even make sense. You do nothing below with the
last element in the array. You do something near the last element of the
_string_, which is a different thing. And the last character of a string
is _always_ the null terminator - that's what makes it a string, not an
unterminated char array.

txtptr = text;
*(txtptr + strlen(txtptr) - 1) = '\0';

What you do here, in a roundabout way, is overwrite the last character
before the string terminator. You do this regardless of whether there is
a last character at all; moreover, you do this regardless of what that
character actually is.
What I suspect you really wanted to do is to remove the newline
character which fgets() tends to leave in the string. If so, note that
this newline is not necessarily there: it is only there if your user
entered it. It could be missing if, e.g., the user enters a string of
more than seventy characters - the first seventy will be put into text,
and the rest, including the newline, will remain in the input stream.
It's also possible that you get your input piped from a file, and the
line you read happens to be the last line in the file - but does not end
in a newline. In that case, there won't be a newline in the resulting
string, either.
So what you want to do is:
- check that there is a newline in text;
- if so, overwrite it, but if not, overwrite nothing.

One way to do this is

if ((txtptr=strchr(text, '\n')) *txtptr='\0';

// Converts and outputs the text in uppercase characters.

This converts nothing. It merely prints the upper-case equivalent of the
input.

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;
while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

First of all, toupper() takes an int, requires that its input is in the
range of _unsigned_ char, or EOF. If your char happens to be signed, and
you enter a character with a negative value, this call can result in
some... creative values.
Second, this is the perfect place for a for-loop.

for (txtptr=text; *txtptr!='\0'; txtptr++)
putchar(toupper((unsigned char)*txtptr);

Ditto for the lowercase loop.

putchar('\n');

printf("\n");

Why these two different calls? They do exactly the same thing...

Richard

 

[Darrell Grainger]

This won't work. If 'text' does not alrealy contain a '\0' at the
end you can't use strlen() because all it does is count the number
of chars until it hits '\0', so if there isn't one strlen() won't
be able to figure out where the string ends. I understand that you
wrote it this way to use pointers but here a simple

*(txtptr + sizeof text - 1) = '\0';

is what you need.
Regards, Jens

=========================

Hey Jens,

When I add that code I get the following warning:

local variable 'txtptr' used without having been initialized

How should I initialize the variable? Just set it equal to zero?

Here is my updated code:

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work
#include <string.h>

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");

Minor enhancement:

printf ("\nEnter a line of text (up to %d characters):\n",
sizeof(text)-1);

By doing this, the number 69 changes automatically when you change the
size of the array.

fgets(text,sizeof text,stdin);

// Ensure that the string terminator is no more than the last
// element in the array.
// -----------------------------------------------------------

*(txtptr + sizeof text - 1) = '\0';

The variable txtptr is not initialized. This line of code is saying, "take
the address in txtptr, add the size of the array text then subtract 1, go
to that address and store the null character." If we have not assigned an
address to txtptr then it is unclear what address will we get. You need to
give txtptr an address.

I believe what you are doing here is attempt to set text[69] to null
character. Why you are using the txtptr is not clear to me. I would just
use:

text[sizeof(text)-1] = '\0';

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;
while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

This seems pretty clear. A good use of txtptr. If there is no newline
character it might cause a problem with your display. You might want to
add a putchar('\n');

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

I'm guessing that this comment is wrong. The code below prints the string
as all lowercase.

printf ("\n\nThe line of text in lowercase is:\n");

txtptr = text;
while ( *txtptr != '\0')
putchar( tolower(*txtptr++));
putchar('\n');

Looks good.

printf("\n");

Odd that you use putchar('\n') and then printf("\n"); I would have
preferred two putchar('\n'); calls, one printf("\n\n"); or possibly one
puts("\n");

 

[Bertrand Mollinier Toublet]

Can someone take a look at my code and let me know if you see any
mistakes?

========================

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work
#include <string.h>

int main(void)
{

getch(); // Pauses output

return 0;

} // end main

In addition to other people's comments, I'd like to add that calling
getch() (a non-standard function) just before your main returns is
merely a _useless_ *kludge*.

If the aim is that the execution window doesn't disappear as soon as
execution ends, then learn how to use your IDE and how to modify that
behaviour. This problem does not have to, and thus should not have to,
be solved in your code.

 

[Ed Morton]

<snip>

When I add that code I get the following warning:

local variable 'txtptr' used without having been initialized

How should I initialize the variable? Just set it equal to zero?

Here is my updated code:

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work
#include <string.h>

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

Test the return code.

// Ensure that the string terminator is no more than the last
// element in the array.
// -----------------------------------------------------------

*(txtptr + sizeof text - 1) = '\0';

You shouldn't have deleted the preceding "txtptr = text" line, hence your
compiler warning.

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;
while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

Indent this so it's obvious that the putchar is within the loop. Also, although
you don't syntactically NEED an { ... } in the while loop since you're just
doing one thing, adding them anyway makes the code slightly more future-proof.
For example, if we have the above code written as:

a) while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

b) while (*txtptr != '\0') {
putchar( toupper(*txtptr++) );
}

then if you or somone else comes along in future and adds, say, a debugging
print statement like:

a') while (*txtptr != '\0')
fprintf(stderr,"writing a char\n");
putchar( toupper(*txtptr++) );

b') while (*txtptr != '\0') {
fprintf(stderr,"writing a char\n");
putchar( toupper(*txtptr++) );
}

then in the case of "a'", they've just broken the code by pushing the "putchar"
outside of the loop, whereas in "b'" it'll work as intended. Ditto for any other
loops and "if" statements.

Ed.

 

[Emmanuel Delahaye]

#include <stdio.h>
#include <ctype.h>
#include <conio.h> // Used to make the getch() work

You don't this not portable extension at the moment.

#include <string.h>

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

Good.

// Ensure that the string terminator is no more than the last
// element in the array.
// -----------------------------------------------------------

txtptr = text;
*(txtptr + strlen(txtptr) - 1) = '\0';

For this behaviour, you don't need the pointer:

text[strlen(test) - 1] = 0;

is fine, but not the best way of doing it. If you are not scared by data
recycling, you can use the pointer to search and kill the '\n'.

txtptr = strchr (text, '\n');
if (txtptr)
{
*txtptr = 0;
}

// Converts and outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;

This is what I call 'data recycling'...

while (*txtptr != '\0')
putchar( toupper(*txtptr++) );

Your indentation is scary. Also, I find confusing to have an parameter with a
unary operator. I prefer to make things crystal clear for the reader.
Nowadays compilers are smart enough to provide the required optimization.

Try:

while (*txtptr != '\0')
{
putchar (toupper((unsigned) *txtptr));
txtptr++;
}

getch(); // Pauses output

getchar ();

 

[Emmanuel Delahaye]

This won't work. If 'text' does not alrealy contain a '\0' at the

That's wrong. 'text' has to contain a zero. This is guaranteed by the
standard, because it has been set by fgets(). (Well, assuming the second
parameter of fgets() is > 0, and the return of fgets() is not NULL)

end you can't use strlen() because all it does is count the number
of chars until it hits '\0', so if there isn't one strlen() won't
be able to figure out where the string ends. I understand that you
wrote it this way to use pointers but here a simple

*(txtptr + sizeof text - 1) = '\0';

is what you need.

No. I think you have not got the point. fgets() grabs all the possible
characters, including the final '\n' when there is room enough. What the OP
wanted to do was to get rid of this '\n'. As I have indicated in another post
of this thread, there is a better way to do it.

 

[student]

Hello Again,

Thanks to everyone that helped me to clean up my code! Especially
Emmanuel Delahaye. I spent time understanding and implementing all of
your suggestions. I am re-submitting the final product to the group
in case anyone would like to take a look at the finished product and
alert me to any obvious errors. I know that the comments are
overkill, but my instructor likes us to comment on everything. I
think it is to make sure that we understand what it going on.

Mahalo!

==============================================

/*
---------------------------------------------------------------------------------------------

Write a C program that prompts the user to enter a line of text (up to
69 characters).

It should then convert the text entered to uppercase letters and then
to lowercase letters.

Your program should use a character array: text[70]. It should use the
gets function to prompt
the user for the line of text.

Your program must use pointer notation.

The dialog with the user must look like:

Enter a line of text (up to 69 characters):
ThiS is A LiNe of teXt with UPPERCASE anD lowercase LeTterS!

The line of text in uppercase is:
THIS IS A LINE OF TEXT WITH UPPERCASE AND LOWERCASE LETTERS!

The line of text in lowercase is:
this is a line of text with uppercase and lowercase letters!

----------------------------------------------------------------------------------------------
*/

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

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

// Uses the pointer to search and delete the \n from fgets
// -----------------------------------------------------------

txtptr = strchr (text, '\n');
if (txtptr)
{
*txtptr = 0;
}

// Outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;

while(*txtptr != '\0')
{
putchar (toupper((unsigned) *txtptr));
txtptr++;
}

// Outputs the text in lowercase characters.
// ----------------------------------------------------

printf ("\n\nThe line of text in lowercase is:\n");

txtptr = text;

while(*txtptr != '\0')
{
putchar (tolower((unsigned) *txtptr));
txtptr++;
}

printf("\n");

getchar(); // Pauses output until the user presses a button on
the keyboard.

return 0;

} // end main

 

[Emmanuel Delahaye]

Thanks to everyone that helped me to clean up my code! Especially
Emmanuel Delahaye.

You are welcome.

I spent time understanding and implementing all of
your suggestions. I am re-submitting the final product to the group
in case anyone would like to take a look at the finished product and
alert me to any obvious errors. I know that the comments are
overkill, but my instructor likes us to comment on everything. I
think it is to make sure that we understand what it going on.

You are on the tracks.

/*
-------------------------------------------------------------------------
--------------------

Write a C program that prompts the user to enter a line of text (up to
69 characters).

It should then convert the text entered to uppercase letters and then
to lowercase letters.

Your program should use a character array: text[70]. It should use the
gets function to prompt
the user for the line of text.

Your program must use pointer notation.

The dialog with the user must look like:

Enter a line of text (up to 69 characters):
ThiS is A LiNe of teXt with UPPERCASE anD lowercase LeTterS!

The line of text in uppercase is:
THIS IS A LINE OF TEXT WITH UPPERCASE AND LOWERCASE LETTERS!

The line of text in lowercase is:
this is a line of text with uppercase and lowercase letters!

-------------------------------------------------------------------------
--------------------- */

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

int main(void)
{

// Declare variables
// -----------------

char text[70];
char *txtptr;

// Prompt user for line of text
// ----------------------------

printf ("\nEnter a line of text (up to 69 characters):\n");
fgets(text,sizeof text,stdin);

You could test the return of fgets() against NULL. It denotes some input
error.

// Uses the pointer to search and delete the \n from fgets
// -----------------------------------------------------------

txtptr = strchr (text, '\n');
if (txtptr)
{
*txtptr = 0;
}

Good.

// Outputs the text in uppercase characters.
// ----------------------------------------------------

printf ("\nThe line of text in uppercase is:\n");

txtptr = text;

while(*txtptr != '\0')
{
putchar (toupper((unsigned) *txtptr));
txtptr++;
}

A '\n' is missing.

// Outputs the text in lowercase characters.
// ----------------------------------------------------

printf ("\n\nThe line of text in lowercase is:\n");

.. ^^--------------------------------------------------+
I think that the first '\n' belongs to the previous bloc of code. --+

txtptr = text;

while(*txtptr != '\0')
{
putchar (tolower((unsigned) *txtptr));
txtptr++;
}

printf("\n");

getchar(); // Pauses output until the user presses a button on
the keyboard.

return 0;

} // end main

Apart these nits, sounds good to me.