C or C++?

[jyb]

Originally Posted by chalsall

OK, OS was the wrong term. The point I was trying to make is why call a function (say 70) times to read a line when you can call another once to get the same result?

Perhaps because he finds getchar to be simpler and more intuitive in usage than fgets. He can't count on only being able to call fgets once if he wants the whole line, because as he points out he has to guess the size in advance and then wait to see if it was enough. So if he really wants the whole line, he'll have to put it in a loop anyway. And handling that situation ends up being slightly more complicated than using getchar, the number of function calls notwithstanding. I.e. he's optimizing programmer time vs. running time.

But let's face it, even the running time difference is negligible. He's doing I/O, which means the extra function call overhead involved in using getchar will be lost in the noise. Not to mention that while getchar may be a macro that expands to getc(stdin), getc itself is pretty much always a macro that doesn't result in a function call.

[Dubslow]

Well, even with the getchar() for loop, I still have to allocate the array to store the chars ahead of time, so I still have to guess.

[jyb]

Originally Posted by Dubslow

Well, even with the getchar() for loop, I still have to allocate the array to store the chars ahead of time, so I still have to guess.

Yes and no. The whole point chalsall was making when talking about realloc was that you need to build the version which does what you want yourself. I.e. it is perfectly possible to make a function that returns an array of characters of exactly the size needed to hold a line of input. It's just that such a function is not part of the standard library; you have to write it.

Doing so is a decent exercise. What strategy do you think makes sense for allocating space when there are more characters on the line than fit in your buffer? Just add a constant amount each time? Double the size? These are interesting things to consider, and give a good introduction to some issues that come up quite a lot in C. Of course, they also seem like a pain to have to consider if you're a novice to the language. But that's C for you.

[bsquared]

Originally Posted by Dubslow

Well, even with the getchar() for loop, I still have to allocate the array to store the chars ahead of time, so I still have to guess.

No, you don't, not with realloc. Get a char, store it in a char variable, realloc your string one bigger, copy in the char, repeat.

To make it slightly more efficient, you could always realloc to the next higher power of 2, say, once the number of characters read exceeds the current allocation, and then to a final realloc to the final string size. I do storage via binary growth of arrays all the time for problems in which I don't know exactly how much data is going to be stored.

[Dubslow]

Yeah, I forgot about realloc. I put that aside so I can get the rest of the prog working first.

[jyb]

Originally Posted by Dubslow

strlen, and also removed the redundant string = fgets

		char * string = (char *)malloc( MAXSTRLEN * sizeof(char));
		fgets(string, MAXSTRLEN, stdin);
		i = (int)strlen(string);
		if( i == (MAXSTRLEN - 1) ) {	// Then the string is longer than MAXSTRLEN - 1, and so the rest is lost.
			printf("Source is longer than %d characters. The rest is lost.\n", MAXSTRLEN-1); // will add in realloc later once the behavior is correct
		}
		if ( string[0]==0 ) {	// The entire string is null, or rather, the only character was \n, so no input.
			printf("Bye.\n");
			return 7;
		}

However, the behavior is still as above, which is to say, not correct.

The problem here is that fgets does something rather annoying. If there is room for it, it puts the '\n' character from the line into the array. What that means is that you *always* have at least one character in string. Even if you just hit return, then string will be exactly 1 character long instead of 0. So your test will never be true.

To fix this you could of course just change the test to "if (i == 1)" or something like that. But more generally, you will usually not want that newline character in there when you want to do stuff with the line. So your best bet is to kill it. I.e. right after your call to strlen, you can put something like "if (string[i-1] == '\n') string[i-1] = '\0';"

If you do that, I think you'll find things work as expected. Though of course you still have an egregious memory leak, and you still can't handle lines larger than the max size you chose.

[Dubslow]

That's among the reasons I initially implemented it myself, i.e. I knew exactly how it would behave. Thanks for the tip, I'll go try it now. (Although I don't think that will solve the problem of it not waiting for input the first time through the loop -- we'll see.)

Edit: Yep -- now it catches blanks correctly, but of course now the problem is that for whatever reason it doesn't wait to read the user's input the first time. So when I ran it I got

Please enter the source text (empty line to quit)
Bye.

[jyb]

Originally Posted by Dubslow

That's among the reasons I initially implemented it myself, i.e. I knew exactly how it would behave. Thanks for the tip, I'll go try it now. (Although I don't think that will solve the problem of it not waiting for input the first time through the loop -- we'll see.)

Edit: Yep -- now it catches blanks correctly, but of course now the problem is that for whatever reason it doesn't wait to read the user's input the first time. So when I ran it I got

Please enter the source text (empty line to quit)
Bye.

Nothing in the code you've shown should cause that behavior. There must be something in the code you haven't shown, or something about the way you're invoking the program.

[LaurV]

Originally Posted by chalsall

OK, OS was the wrong term. The point I was trying to make is why call a function (say 70) times to read a line when you can call another once to get the same result?

yeah, why sending/receiving thousands of network packages, when you could send/receive the whole terabyte in a single tcpip package... :P

[Dubslow]

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

#define MAXSTRLEN 100


/* A program to take a string, and create its Ceasar Cypher using a shift value input by the user.
 * (Entering 999 or -999 uses a letter's position as its shift value instead.)
 */

int isULetter(char c) {	// boolean
	if ('A' <= c && c <= 'Z')
		return 1;
	else
		return 0;
}

int isLLetter(char c) {	// boolean
	if ('a' <= c && c <= 'z')
		return 1;
	else
		return 0;
}

int main(void) {
	int shift;
	int badin; 	// boolean
	int secret = 0;	// boolean
	int usecret = 0;// boolean
	int i; // index
	char c;
	do {    // get user input
		printf("Please enter the shift value (between -25..-1 and 1..25)\n");
		scanf("%d", &shift);
		badin = !(((-25 <= shift) && (shift <= -1)) || ((1 <= shift) && (shift <= 25)));
		if (shift == 999) {
			secret = 1;
			badin = 0;
		} else if (shift == -999) {
			usecret = 1;
			badin = 0;
		}
		if (badin)
			printf("%d is not a valid shift value.\n", shift);
	} while (badin);                      // done getting input
	if (secret || usecret)
		printf("Using position shift\n");
	else
		printf("Using shift value of %d\n", shift);
	if (shift < 0)
		shift += 26;
	while (1) {                            // the cypher stage
		printf("Please enter the source text (empty line to quit)\n");
		char * string = (char *)malloc( MAXSTRLEN * sizeof(char));
		fgets(string, MAXSTRLEN, stdin);
		i = (int)strlen(string);
		if( i == (MAXSTRLEN - 1) ) {	// Then the string is longer than MAXSTRLEN - 1, and so the rest is lost.
			printf("Source is longer than %d characters. The rest is lost.\n", MAXSTRLEN-1); // will add in realloc later once the behavior is correct
		}
		if ( string[0]=='\n' ) {	// The entire string is null, or rather, the only character was \n, so no input.
			printf("Bye.\n");
			return 7;
		}
		printf("Source   : ");
		puts(string); // put string; accepts pointer to char array as argument.
		char * code = (char *)malloc( MAXSTRLEN * sizeof(char));
		for ( i=0; ((c=string[i])!=0) && (i<(MAXSTRLEN-1)) ; i++) {
			if ( isULetter(c) ) {
				if (secret) {
					c = (char) ('A' + ((c - 'A' + i) % 26));
				} else if (usecret) {
					c = (char) ('A' + ((c - 'A' + (26 - (i % 26))) % 26));
				} else {
					c = (char) ('A' + ((c - 'A' + shift) % 26));
				}
			} else if ( isLLetter(c) ) {
				if (secret) {
					c = (char) ('a' + ((c - 'a' + i) % 26));
				} else if (usecret) {
					c = (char) ('a' + ((c - 'a' + (26 - (i % 26))) % 26));
				} else {
					c = (char) ('a' + ((c - 'a' + shift) % 26));
				}
			}
			code[i] = c;
		}
		printf("Processed: ");
		puts(code);
	} // while
} // main

bill@Gravemind:~/bin/c∰∂ cypher
Please enter the shift value (between -25..-1 and 1..25)
-999
Using position shift
Please enter the source text (empty line to quit)
Bye.
bill@Gravemind:~/bin/c∰∂

Compiled with gcc -Wall. I can provide a plaintext webpage version if you want (will use more screen, easier to read). As I said before, when it couldn't catch blank input, it cycled once then stopped the second time, but not the first time through the while(1) loop.

For context, this is evolved from a Java assignment that was due on Monday. I'm trying to fulfill the specifications using C, so that my C doesn't fall behind Java.

[Dubslow]

I think I've got at least part of the problem. I disabled the scanf and hardcoded in a shift value; then the string prompt paused just fine for me without looping or exiting.