Thread: output screen isn't giving proper result

I have a program that I'm working on for class, I have coded all but the last function. It compiles and links without any problem. Although it gives me a scanf and fopen warning. I'm able to get the menu to appear and get user input. Once I input number the black screen goes away. I put a scanf in it to keep open but it doesn't work. Can someone look it over to see if there are logical errors (I'm sure there are, or else it would work properly) I appreciate any input or advice you may have.

Code:

/* This program has a report attached to it and asks user if they want to count lines, words, characters, sentences or all of the above.*/

#include <stdio.h>


int countLines();//file declarations
int countWords();
int countChars();
int countSents();
int all();

FILE* spReport;//report in resource file

int main()
{
	int selection, key;

	printf("Please select item from the menu:\n");//menu
	printf("1. count the lines \n");
	printf("2. Count the words\n");
	printf("3. Count the characters \n");
	printf("4. Count the sentences \n");
	printf("5. All of the above \n");

	scanf("%d", &selection);
	
	spReport = fopen("Report1.txt", "r");
		     

	switch (selection)
	{
	case 1: printf("You have chosen count lines!");
		int countLines();
		break;
	case 2: printf("You have chosen to count words!");
		int countWords();
		break;
	case 3: printf("You have chosen to count characters!");
		int countChars();
		break;
	case 4: printf("You have chosen to count sentences!");
			int countSents();
			break;
	case 5: printf("You have chose all of the above!");
			int countAll();
			break;
	}//switch

	fclose(spReport);
	scanf("%c", &key);
	return 0;
}//main
/*++++++++++++++++countLines+++++++++++++++++*/
int countLines()
{
	int curCh; //current character
	int countLine = 0;

	while (curCh = fgetc(spReport)!= EOF)
	{if (curCh == '\n' )
		countLine++;
	}//while
	printf("The number of lines is %d", countLine);

	return(countLine);
}//countLine
/*===================CountWords=================*/
int countWords()
{
#define WHT_SPC\
		(curCh == ' ' || curCh == '\n' || curCh == '\t')
	
	int curCh;//current character
	int countWord = 0;
	char word = 'O';

	while(curCh = fgetc (spReport) != EOF)
	{
	if (WHT_SPC)
		word = 'O';

	else 
		if(word == 'o')
		{countWord++;
		word = 'I';
	}//else

	}//while
	printf("The number of words are %d\n", countWord);

	return 1;
}//count Words
/*=================countchars===================*/
int countChars()
{
	int countchar = 0;
	int curch, preCh;
	int countline = 0;

	while ((curch = fgetc(spReport)) != EOF)
	{
	if (curch != '\n')
			countchar++;
	else 
		countline++;

	preCh = curch;
	}//while

	if (preCh != '\n')
		countline++;

	printf("The number of characters in the the report are %d\n", countchar);
	return 1;
}//countchars
/*================countsents==========================*/
int countSents()
{
	int curch;
	int sentences = 0;

while ((curch = fgetc(spReport)) != EOF)
{
	if (curch == '.')
		sentences++;
}//while
	printf("The number of sentances are %d.\n", sentences);

return 1;
}//countSents

I assume you're working in Windows? Read this FAQ: Cprogramming.com FAQ > Stop my Windows Console from disappearing everytime I run my program?.

Also, don't write off compiler warnings. Heed them and correct your bad code. You need to pass the right type to scanf. You also should be getting a warning from your compiler about lines 59ish and 77ish:

Code:

    while (curCh = fgetc(spReport)!= EOF)

That doesn't do what you think. It checks if fgetc is not equal to EOF, and stores that value in curCh. Given that != is a boolean operator, curCh will probably only be a 0 or 1, not the actual character that fgetc read. Try the following instead:

Code:

    while ((curCh = fgetc(spReport)) != EOF)

Add those green parentheses to make sure curCh holds the character read by fgetc. That value will then be checked against EOF to stop your loop.

EDIT: and the reason your scanf doesn't work is because your menu choice only reads in a number, so the newline you enter after typing your menu choice sits in the input buffer. When you scanf("%c", &key), the character it reads is the leftover newline.

thanks for the heads up on the parenthesis. The warning for the scanf suggested that I use scanf_s. I haven't heard of it before so I googled it. I ended up putting it in the code and it doesn't give me a warning anymore. but it still doesn't perform the functions yet.

it does give me the printf statements inside the switch statements. I removed the type on the function call, but nothing changes.

Read the FAQ I linked in my previous post, as well as the FAQ it linked: Cprogramming.com FAQ > How do I get my program to wait for a keypress?.

Also read the EDIT line in my previous post. I explain the issue you're having, but here's another FAQ for you to read: Cprogramming.com FAQ > Flush the input buffer.

And yes, you needed to remove the types from the functions in your code. With the return type, you have a function prototype:

Code:

int foo();  // don't actually call foo, just tell the compiler there is a function named foo, that takes any parameters and returns int
bar();  // actually call bar

Yeah, actually. It's all kinds of crazy. Your #define is wonky, and should just be in the if check like so:
if (curCh == ' ' || ...)
But better yet, look at functions like isalpha() in ctype.h for determining whether curCh is a letter.

I don't know what you're doing with word and 'O', 'o' and 'I', but it's definitely wrong. You should note that 'O' and 'o' are two separate characters and most definitely not equal.

But I agree, this is the toughest of the functions you have to write, so here's the gist of it in pseudo code:

Code:

for each character you read
    if you are currently in a word and the character is not a letter
        set your in word flag to false
    else, if you are not currently in a word and character is a letter
        set your in word flag to true
        increment your word count

the 'O' means out of a word and 'I' means in a word. I guess I should comment that.

Code:

int countWords()
{
#define WHT_SPC\
		(curCh == ' ' || curCh == '\n' || curCh == '\t')
	
	int curCh;//current character
	int countWord = 0;
	char word = 'O';

	while(curCh = fgetc (spReport) != EOF)
	{
	if (WHT_SPC)
		word = 'O';

	else 
		if(word == 'o')
		{countWord++;
		word = 'I';
	}//else

	}//while
	printf("The number of words are %d\n", countWord);

the define statement is used to see if the curCh is outside of a word. Although it looks different (way different) from the code you use, it is similar in pseudocode. When I googled the count word function I actually saw that code, but I can't remember why I didn't use it. I think I was just looking for the overall logic. I try to work the code myself if I can so I can learn it better.

I'm not familiar with 'flag' also. I'm not sure if it is a keyword or not.

"flag" is not a reserved word. It's usually a simple 1 or 0 (boolean), value, frequently done on a simple "flag" integer. (Boolean wasn't supported early on by C, people just used int's).

Other times, it's a variable with a larger range. Think of compiler errors and their several numbers.

thanks Adak,

another question, I'm not at my computer with my compiler program on it, but would it work for my last menu option, countAll() to just put the function call or do I have to include each step from each function.

Code:

int countAll()
{
countLine()
countWord()
countChars()
etc....

Think of your functions like little "black boxes" that you make up once, and then never mess with the innards again.

In real life, we need to change the black boxes from time to time, but it should be a rare occasion. Ideally, you want to make your functions so the only thing you need to change is the specifics of the parameter list.

For example, I wrote a program that did the same work with two different strings. The answer was to have the function "getstring()", rename the string that was passed to it, to mystring[], like this:

Code:

getstring(len, string1);

void getstring(char len, char mystring[]) {

   //string manipulation code in here
}

//~~~~~~~ later ~~~~~~
getstring(len, string2);

Now I could send getstring(len, string1), and I could send it getstring(len, string2), and encapsulate all my string manipulation code, into just the one function.

If you are repeating the same code in your program, think whether it could be combined into just one function, and treated as a "black box" that you do not open. It doesn't always make sense for just a few simple lines of code, but for more extensive code, and for code that has to be repeated three times or more, it's the way you want to go.

Originally Posted by time4f5

the 'O' means out of a word and 'I' means in a word. I guess I should comment that.

I kind of figured that's what you mean, but wasn't totally sure. One reason I like the boolean/flag method is that since there are only two values you care about (true/nonzero and false/zero), it's easy to switch between them and check for truth. You can just do:

Code:

#include <stdbool.h>
bool foo = false;  // foo is false
foo = !foo;  // now foo is true
if (foo) {
    printf("foo is true!\n");
}

the define statement is used to see if the curCh is outside of a word. Although it looks different (way different) from the code you use, it is similar in pseudocode. When I googled the count word function I actually saw that code, but I can't remember why I didn't use it. I think I was just looking for the overall logic. I try to work the code myself if I can so I can learn it better.

Yeah, I knew what it did, and that it worked for it's purpose -- by wonky, I meant it's just not a very clean way to do it. Wherever you found that code, never go there again for advice. That is a horrible example of when to use a #define and whoever wrote it is not somebody you should listen to. Generally, macros, like the goto statement, should be used as a last resort.

Adak's got you on the right direction about functions. It wont be the quickest way to get your file statistics, but making countAll() call the other functions is reusing code, which is generally a very good thing, and the whole reason we have functions -- small chunks of reusable code we write once and maintain in a single place.

wonky is a good word. I'll defninately use the function call, it does make is so much cleaner. thanks