Skip to main content
added 1 character in body
Source Link
Emanuele Paolini
  • 2.1k
  • 1
  • 14
  • 14

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reasonreasons for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.

  4. i would always enclose with braces the blocks of an if statement even when there is a single line. This is to avoid the risk to forget the braces when a line is later added. As an alternative I would put the single line on the same line as the if statement.

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reason for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.

  4. i would always enclose with braces the blocks of an if statement even when there is a single line. This is to avoid the risk to forget the braces when a line is later added. As an alternative I would put the single line on the same line as the if statement.

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reasons for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.

  4. i would always enclose with braces the blocks of an if statement even when there is a single line. This is to avoid the risk to forget the braces when a line is later added. As an alternative I would put the single line on the same line as the if statement.

added 272 characters in body
Source Link
Emanuele Paolini
  • 2.1k
  • 1
  • 14
  • 14

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reason for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.

  4. i would always enclose with braces the blocks of an if statement even when there is a single line. This is to avoid the risk to forget the braces when a line is later added. As an alternative I would put the single line on the same line as the if statement.

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reason for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reason for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.

  4. i would always enclose with braces the blocks of an if statement even when there is a single line. This is to avoid the risk to forget the braces when a line is later added. As an alternative I would put the single line on the same line as the if statement.

Source Link
Emanuele Paolini
  • 2.1k
  • 1
  • 14
  • 14

The code is very well written in my opinion. I have only very marginal questions:

  1. in the main function you have a small repetition when checking argv to determine if the FILE should be closed or not. I would prefer checking (in != stdin) which is telling the real reason for the check. Alternatively I would simply neglect closing the files... since you are in the main function, at the end all file descriptors are guaranteed to be closed anyway.

  2. i would prefer for(;;){...} instead of do {...} while(1);. I see three reason for this: it is shorter, you don't introduce the arbitrary constant 1, it is clear from the beginning that the loop has no termination condition.

  3. i would expect less invasive error checking. Since your encode and decode functions are perfect candidates to be reused in a library, you should not write errors to stderr and should not terminate the execution with exit. A better approach would be to use return codes as signals for errors and let the main function write the messages and terminate.