Reading from a binary file

Question

I am trying to count the no of zeroes and ones from a binary file. The problem is, I am getting correct no. of zeroes but ones are coming out to be equal to no. of zeroes. What I am doing is reading the file char by char . Since there can be max 256 chars I am storing the results in a temporary array for both zeroes and ones, and retrieve from there if a character occurs again.

#include<iostream>
#include<cstdio>
#include<cstdlib>
using namespace std;
void func(int* a1 ,int* a2)
{
    for(int i=0;i<256;i++)
    for(int j=0;j<8;j++)
    {
        if( (i & 1) ==1 )
        {
            a1[i]+=1;
        }
        else if( (i & 1) ==0 )
        {
            a2[i]+=1;   
        }
        i>>1;
    }
}
int main()
{
    int zero[256];
    int one[256];
    int tzero[256];
    int tone[256];
    for(int i=0;i<256;i++)
    {
        zero[i]=0;
        one[i]=0;
        tzero[i]=0;
        tone[i]=0;
    }
    func(tone,tzero);
    FILE* input;
    FILE* output;
    output=fopen("ascii.txt","w");
    input=fopen("one.bin","r");
    int c;
    while((c=fgetc(input))!=EOF)
    {
        fprintf(output,"%d\n",c);
        zero[c]+=tzero[c];
        one[c]+=tone[c];
    }
    int zeroes=0;
    int ones=0;
    for(int i=0;i<=255;i++)
    {
        zeroes+=zero[i];
        ones+=one[i];
    }
    cout<<"zeroes:"<<zeroes<<endl;
    cout<<"ones:"<<ones<<endl;
    fclose(input);

    fclose(output);

}

Answer 1

If your goal is just to bitwise count 1 and 0 bits in a file, you can simplify things greatly by using C++ file streams and not using lookup tables:

#include <iostream>
#include <fstream>

int main(int argc, char** argv)
{
  std::ifstream fpInput("Input.txt");
  unsigned unOnes = 0;
  unsigned unZeros = 0;
  char chTemp;

  chTemp = fpInput.get();
  while (fpInput.good())
  {
    for (unsigned i = 0; i < 8; ++i)
    {
      if (chTemp & 0x1<<i) unOnes++;
      else unZeros++;
    }

    chTemp = fpInput.get();
  }

  fpInput.close();

  std::cout << "Found " << unOnes << " ones." << std::endl;
  std::cout << "Found " << unZeros << " zeros." << std::endl;

  return 0;
}

A good compiler should expand the middle loop out if you pass it the right optimisation flags.

Answer 2

The loop that counts zeros and ones destroys the value of c by doing

c >>= 1;

After all eight shifts are complete, c is always zero, so the following code increments wrong counts:

// The value of c is always zero
tzero[c]=z;
tone[c]=o;
one[c]+=tzero[c];
zero[c]+=tzero[c];

You should save the value of c before the bit-counting loop, and restore it after the loop is over.

Better yet, compute the values of tzero[] and tone[] upfront, without waiting for them to occur in the file. This would make the body of your main loop very short and clean:

while((c=fgetc(input))!=EOF) {
    one[c] += tzero[c];
    zero[c] += tzero[c];
}