Extract constant expression to outside of loop
In this code, (decode ? 26 - key : key)
is evaluated for each character in text
:
text .chars .select { |char| ALPHABET.include?(char) } .map {|char| shift(char, (decode ? 26 - key : key) )} .join
It would be better to move it outside the expression so that it's only evaluated once:
shift_key = decode ? 26 - key : key
text
.chars
.select { |char| ALPHABET.include?(char) }
.map {|char| shift(char, shift_key)}
.join
The same goes for the other cipher method, where you do the same.
Minor formatting issues
The code formatting style is not consistent, for example compare these two lines:
.select { |char| ALPHABET.include?(char) } .map {|char| shift(char, shift_key)}
In the first one, there's a space after {
and before }
,
but in the second there isn't.
Both ways seem fine to me,
but it would be better to use one of these ways, consistently everywhere.
Similarly here,
there's a space after [
but not before ]
.
For the sake of consistency,
it would be better to remove the space after [
or add one before ]
.
ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]
Overall
This looks nice, a pleasure to read, and I don't even know Ruby ;-)
Extract constant expression to outside of loop
In this code, (decode ? 26 - key : key)
is evaluated for each character in text
:
text .chars .select { |char| ALPHABET.include?(char) } .map {|char| shift(char, (decode ? 26 - key : key) )} .join
It would be better to move it outside the expression so that it's only evaluated once:
shift_key = decode ? 26 - key : key
text
.chars
.select { |char| ALPHABET.include?(char) }
.map {|char| shift(char, shift_key)}
.join
The same goes for the other cipher method, where you do the same.
Minor formatting issues
The code formatting style is not consistent, for example compare these two lines:
.select { |char| ALPHABET.include?(char) } .map {|char| shift(char, shift_key)}
In the first one, there's a space after {
and before }
,
but in the second there isn't.
Both ways seem fine to me,
but it would be better to use one of these ways, consistently everywhere.
Similarly here,
there's a space after [
but not before ]
.
For the sake of consistency,
it would be better to remove the space after [
or add one before ]
.
ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]
Overall
This looks nice, a pleasure to read, and I don't even know Ruby ;-)
Extract constant expression to outside of loop
In this code, (decode ? 26 - key : key)
is evaluated for each character in text
:
text .chars .select { |char| ALPHABET.include?(char) } .map {|char| shift(char, (decode ? 26 - key : key) )} .join
It would be better to move it outside the expression so that it's only evaluated once:
shift_key = decode ? 26 - key : key
text
.chars
.select { |char| ALPHABET.include?(char) }
.map {|char| shift(char, shift_key)}
.join
Minor formatting issues
The code formatting style is not consistent, for example compare these two lines:
.select { |char| ALPHABET.include?(char) } .map {|char| shift(char, shift_key)}
In the first one, there's a space after {
and before }
,
but in the second there isn't.
Both ways seem fine to me,
but it would be better to use one of these ways, consistently everywhere.
Similarly here,
there's a space after [
but not before ]
.
For the sake of consistency,
it would be better to remove the space after [
or add one before ]
.
ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]
Overall
This looks nice, a pleasure to read, and I don't even know Ruby ;-)
Extract constant expression to outside of loop
In this code, (decode ? 26 - key : key)
is evaluated for each character in text
:
text .chars .select { |char| ALPHABET.include?(char) } .map {|char| shift(char, (decode ? 26 - key : key) )} .join
It would be better to move it outside the expression so that it's only evaluated once:
shift_key = decode ? 26 - key : key
text
.chars
.select { |char| ALPHABET.include?(char) }
.map {|char| shift(char, shift_key)}
.join
The same goes for the other cipher method, where you do the same.
Minor formatting issues
The code formatting style is not consistent, for example compare these two lines:
.select { |char| ALPHABET.include?(char) } .map {|char| shift(char, shift_key)}
In the first one, there's a space after {
and before }
,
but in the second there isn't.
Both ways seem fine to me,
but it would be better to use one of these ways, consistently everywhere.
Similarly here,
there's a space after [
but not before ]
.
For the sake of consistency,
it would be better to remove the space after [
or add one before ]
.
ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]
Overall
This looks nice, a pleasure to read, and I don't even know Ruby ;-)