Member Avatar for MrDiaz

Hey everyone,

I have a series of loops that read through a text file and then output the occurrences for each letter on the file. This seems to be fine when using small text only. When providing a lot of text it just takes too many CPU resources and that's no good for me.

Here's the code that gets the job done

for(i = 0; i < 26; i++) {
   letters[i] = 0; 
   for(j = 0; j < strlen(textStr); j++) {
       if(textStr[j] == i + 'a')
       letters[i]++;
   }
   if(letters[i] > 0)
	printf("%c - %d\n", (i + 'a'), letters[i]);
}

Some additional details:

letters[26] contains the count of occurrences. Example: {24242,2323,424,.....} where each position corresponds to a char of the alphabet.

textStr[] contains the whole text

Edited by MrDiaz because: forgot some details
  • 2 Contributors
  • 3 Replies
  • 135 Views
  • 6 Hours Discussion Span
  • Latest Post Latest Post by Tellalca

Dani AI

Generated

Good catch by : calling strlen repeatedly inside the inner loop is costly. The loop in Post #1 ends up doing a lot more work than necessary. A faster approach is a single linear scan of the text that updates a 26-slot histogram once per character. That reduces work and avoids the quadratic behavior caused by repeated length scans.

A compact, efficient single-pass example (counts both cases and avoids signed-char pitfalls):

size_t counts[26] = {0};
for (const unsigned char *p = (const unsigned char *)textStr; *p; ++p) {
    unsigned char c = *p;
    if (c >= 'a' && c <= 'z') counts[c - 'a']++;
    else if (c >= 'A' && c <= 'Z') counts[c - 'A']++;
}

For very large files, do not hold the whole file in memory. Read fixed-size buffers and process each block with the same inner loop. Example pattern:

char buf[1<<16];
FILE *f = fopen("file.txt","rb");
size_t n;
while ((n = fread(buf,1,sizeof buf,f)) > 0) {
    for (size_t i = 0; i < n; ++i) {
        unsigned char c = (unsigned char)buf[i];
        if (c >= 'a' && c <= 'z') counts[c - 'a']++;
        else if (c >= 'A' && c <= 'Z') counts[c - 'A']++;
    }
}

Extra tips and cautions:

  • Use size_t (or 64-bit) for counters if files can be huge.
  • Casting to unsigned char before using ctype.h functions or array indexes prevents undefined behavior on signed char.
  • If maximum speed matters, a 256-entry table (count every byte value) and then fold A-Z/a-z into 26 buckets avoids repeated range checks.
  • On POSIX, mmap can be faster than fread for very large files; on Windows, use file mapping. These are platform-specific and require careful cleanup.

These changes keep the logic simple and drop costly repeated scans while preserving correct counts.

Recommended Answers

All 3 Replies

Member Avatar for Tellalca

You can assign the value returned by the function "strlen(textStr)" so you avoid calling this function strlen*26 times :)

int a = strlen(textStr);
 for(j = 0; j < a; j++) {
...
}

Maybe you should use binary files instead. This will decrease the encoding and decoding of the string that will be placed into the buffer, reducing the time your program consume dramatically.

Member Avatar for MrDiaz

Damn wow, such a simple fix yet so important! Thanks so much Tellalca :)

Member Avatar for Tellalca

You are very welcome :)

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.