Thread: Frequencies of characters in a file

Hi. I just wrote this program that's supposed to read from a file, count the frequency of each character, sort them, then output any characters in a file along with their frequencies:

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define FILEPATH_MAX FILENAME_MAX*15

int main(int argc, char * argv[])
{
	char filePath[FILEPATH_MAX];
	FILE *file;
	int i, k, maxchar;
	char charcount[256] = {0};				/* array to know how many of a specific character have been seen */
	char charcount2[256] = {0};				/* for sorting */
	char charcount2_identifiers[256] = {0};			/* for sorting (identifiers for characters in charcount2 */

	if (argc == 2)
		strcpy(filePath, argv[1]);
	else
	{
		fputs("Wrong arguments. Press enter to terminate...", stderr);
		getchar();
		exit(EXIT_FAILURE);
	}

	if ((file = fopen(filePath, "rt")) == '\0')
	{
		fputs("Error opening file. Press enter to terminate...", stderr);
		getchar();
		exit(EXIT_FAILURE);
	}
	printf("File opened successfully\n");

	/* start counting */
	while ((i=fgetc(file)) != EOF)
		++charcount[i];
	
	fclose(file);

	/* sort */
	for (i=0; i<256; i++)
	{
		for (k=0, maxchar=0; k<256; k++)
			if (charcount[k] > maxchar)
			{
				maxchar = charcount[k];
				charcount[k] = 0;
				charcount2_identifiers[i] = k;
			}

		charcount2[i] = maxchar;
	}



	/* output results */
	for (i=0; i<256; i++)
	{
		if (charcount2[i] != 0)
		{
			if (charcount2_identifiers[i] >= 33 && charcount2_identifiers[i] <= 126)
				printf("&#37;c: %d\n", charcount2_identifiers[i], charcount2[i]);				/* print the identifier */
			else
				printf("0x%x: %d\n", charcount2_identifiers[i], charcount2[i]);
		}
	}

	printf("Finished. Press enter to continue...");
	getchar();

	return 0;
}

The problem is, not all the characters in the file are output.
E.g. I tried the following text in a file:
UJTUQJ FQQ FWTZSI YMJ BTWQI QNPJ YT XTQAJ UZEEQJX
when I run the program it does not display the letter J or its frequencey.

I think the problem is in the sorting. I'm pretty sure the output part is right.

Thanks,

I'm a bit confused about your sorting algorithm. Have you tried printing the values BEFORE you sort them [so you print in alphabetical order]?

Edit: I see the problem. You are setting you charcount[i] = 0 whenever you find something that is greater than the current maxchar - but you don't know yet if this is actually the HIGHEST SO FAR. You need to just remember the position of the highest one, _THEN_ set it to zero at the same time as you set the charcount2[i] to maxchar.

--
Mats

Did you see my edit?

--
Mats

Oh sorry, I just saw it. I'll try implementing it now (shouldn't take any time).
but do you have any better sorting algorithms that I can use (just a simple one)? or does mine seem OK?

EDIT: I just implemented it now. but I don't like the look of this statement:

Code:

/* sort */
	for (i=0; i<256; i++)
	{
		for (k=0, maxchar=0; k<256; k++)
			if (charcount[k] > maxchar)
			{
				maxchar = charcount[k];
				charcount2_identifiers[i] = k;
			}
		
		charcount[charcount2_identifiers[i]] = 0;
		charcount2[i] = maxchar;
	}

Is it illegal in C? because it works completely fine.

That's perfectly legal, if a bit complex.

I would probably just store maxchar's "k" in a temp variable, and set both charcount2_identifier and the charcount using that temp variable.

--
Mats

This is the C forum, and as I don't see anything C++-ish about that code -- it looks like plain, vanilla C -- you probably shouldn't be recommending std::map.

Code:

if ((file = fopen(filePath, "rt")) == '\0')

'\0' is usually used as a character. When you're dealing with pointers, which is what fopen() returns, it's standard practise to use NULL instead -- or just plain 0. Or even

Code:

if (!(file = fopen(filePath, "rt")))

Code:

			if (i >= 33 && i <= 126)
				printf("&#37;c: %d\n", i, charcount[i]);

Consider isprint() from <ctype.h>. It's more portable and easier on the eyes, too.

Instead of using the "magic number" 256 everywhere, you could use UCHAR_MAX from <limits.h>. Or not; 256 is pretty common.

Rather than declare a large array with FILENAME_MAX*15 elements, which still might not be enough, you could just declare filePath as a pointer and have it point to argv[1]. That wouldn't work if you wanted to add support for fgets()-ing the filename from the user at some point in the future, however.

Of course, I forgot about that. Maybe something like this, then:

Code:

#define CHARS (UCHAR_MAX + 1)