Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward @Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

added 1 character in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ccapecap`, ecap are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap` are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

added 432 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap` are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap` are really not intuitive, and your code could become much more readable if you gave these better names.

This is unnecessarily complicated:

 if (argc != '0円' && argc == 2 ) 

This is exactly the same thing but simpler:

 if (argc == 2) 

Instead of the hard-coded ASCII code 65 of "A", you could use 'A' to make the code easier to read. So instead of this:

//converting capitalized chars
char acap = (a[i] - 65);
int ccap = (acap+f)%26;
//final capitalized chars loop
char ecap = ccap + 65;
printf("%c", ecap);

You can write like this:

//converting capitalized chars
char acap = (a[i] - 'A');
int ccap = (acap + f) % 26;
//final capitalized chars loop
char ecap = ccap + 'A';
printf("%c", ecap);

I also added spaces around operators in (acap+f)%26 to make it more readable, which is a good practice.

The same goes for the number 97, it's better to use 'a' instead.


The code for handling upper case and lower case letters is practically the same, except for the constant 'A' and 'a'. Avoid such code duplication, repetition and copy-paste coding as much as possible. Extract the common logic to a helper function, and reuse it by passing 'A' and 'a' as parameter.


Use more descriptive variable names. f, acap, ccap, ecap` are really not intuitive, and your code could become much more readable if you gave these better names.


@Edward made an excellent remark in comments that I'm just going to quote verbatim:

It's worth noting that both original and suggested changes assume a character encoding with a contiguous encoding for the alphabet. This is not actually guaranteed by the C standard. For example, EBCDIC systems are admittedly rare but not yet extinct.

added 178 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396
Loading
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396
Loading
lang-c

AltStyle によって変換されたページ (->オリジナル) /