Skip to main content
deleted 58 characters in body
Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that. Another option would be to open the file in main() and pass the file handle to your functions.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

If you cannot assume that a char has 8 bits then use 1 << CHAR_BIT as the array size, not 256.

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

can be simplified toshould be (as I understand it from https://en.wikipedia.org/wiki/Entropy_(information_theory)#Definition)

    shan = shan + log2(vector[i]) * vector[i];

because generally, $$ \ln(a^b) = \ln (e^{b \ln a}) = b \ln a \quad \text{ for } a, b > 0 \, . $$ That save one call into the math library.

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that. Another option would be to open the file in main() and pass the file handle to your functions.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

If you cannot assume that a char has 8 bits then use 1 << CHAR_BIT as the array size, not 256.

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

can be simplified to

    shan = shan + log2(vector[i]) * vector[i];

because generally, $$ \ln(a^b) = \ln (e^{b \ln a}) = b \ln a \quad \text{ for } a, b > 0 \, . $$ That save one call into the math library.

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that. Another option would be to open the file in main() and pass the file handle to your functions.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

If you cannot assume that a char has 8 bits then use 1 << CHAR_BIT as the array size, not 256.

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

should be (as I understand it from https://en.wikipedia.org/wiki/Entropy_(information_theory)#Definition)

    shan = shan + log2(vector[i]) * vector[i];
added 98 characters in body
Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that. Another option would be to open the file in main() and pass the file handle to your functions.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

If you cannot assume that a char has 8 bits then use 1 << CHAR_BIT as the array size, not 256.

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

can be simplified to

    shan = shan + log2(vector[i]) * vector[i];

because generally, $$ \ln(a^b) = \ln (e^{b \ln a}) = b \ln a \quad \text{ for } a, b > 0 \, . $$ That save one call into the math library.

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

can be simplified to

    shan = shan + log2(vector[i]) * vector[i];

because generally, $$ \ln(a^b) = \ln (e^{b \ln a}) = b \ln a \quad \text{ for } a, b > 0 \, . $$ That save one call into the math library.

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that. Another option would be to open the file in main() and pass the file handle to your functions.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

If you cannot assume that a char has 8 bits then use 1 << CHAR_BIT as the array size, not 256.

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

can be simplified to

    shan = shan + log2(vector[i]) * vector[i];

because generally, $$ \ln(a^b) = \ln (e^{b \ln a}) = b \ln a \quad \text{ for } a, b > 0 \, . $$ That save one call into the math library.

Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92

In addition to what @pacmaninbw already said:

The return value of fopen() is not checked. It will be NULL if the file does not exist or the caller has no read permission. You have to decide how to handle that.

A simpler method to get the size of a file is using stat() (or fstat() if you already have a handle to the file). This is also much faster, because it only queries the filesystem, without reading the entire file.

The cmp() function can be simplified to

int cmp(double foo, double bar)
{
  return foo >= bar;
}

because in C, the result of a comparison operator is either 0 or 1. In fact that is so simple that you probably don't need a dedicated function for that purpose.

The return value of getc() is either EOF, or an unsigned char converted to an int. In other words, if a char has 8 bit (which is the case on all Posix-compatible platforms) then the return value can only be EOF or an integer in the range 0 ... 255, so there is no need for a check

if (c >= 0 && c <= 256)

(and you probably meant c < 256 here).

In the freq() function, at

    vector[i] = (vector[i] / ccount(foo));

the file size it computed for each loop iteration. Better compute it once and store it in a local variable.

In the shannon() function

    shan = shan + (log2(pow(vector[i], vector[i]))); 

can be simplified to

    shan = shan + log2(vector[i]) * vector[i];

because generally, $$ \ln(a^b) = \ln (e^{b \ln a}) = b \ln a \quad \text{ for } a, b > 0 \, . $$ That save one call into the math library.