Skip to main content
Code Review

Return to Answer

edited body
Source Link
Martin R
  • 24.2k
  • 2
  • 37
  • 95

A Ruby string literal can be enclosed in single or double quotes. The difference is that the formerlatter does string interpolation. Examples (from Double vs single quotes on Stack Overflow):

A Ruby string literal can be enclosed in single or double quotes. The difference is that the former does string interpolation. Examples (from Double vs single quotes on Stack Overflow):

A Ruby string literal can be enclosed in single or double quotes. The difference is that the latter does string interpolation. Examples (from Double vs single quotes on Stack Overflow):

added 661 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 37
  • 95

A Ruby string literal can be enclosed in single or double quotes. The difference is that the former does string interpolation. Examples (from Double vs single quotes on Stack Overflow):

a = 2
puts "#{a}" # prints the value of a
puts 'a\nb' # just print a\nb 
puts "a\nb" # print a, then b at newline 

That may be wanted (e.g. to include newline characters), or not:

puts rot13_string "#{Wot?}"
// rot13.rb:100:in `<main>': undefined method `Wot?' for main:Object (NoMethodError)

so generally you should enclose the string literals in single-quotes.


The regular expression /[[:upper:]]/ matches all "uppercase alphabetical characters" (Unicode characters in the "Lu" general category, to be precise), and not just the ASCII uppercase letter A...Z.

The regular expression /[[:upper:]]/ matches all "uppercase alphabetical characters" (Unicode characters in the "Lu" general category, to be precise), and not just the ASCII uppercase letter A...Z.

A Ruby string literal can be enclosed in single or double quotes. The difference is that the former does string interpolation. Examples (from Double vs single quotes on Stack Overflow):

a = 2
puts "#{a}" # prints the value of a
puts 'a\nb' # just print a\nb 
puts "a\nb" # print a, then b at newline 

That may be wanted (e.g. to include newline characters), or not:

puts rot13_string "#{Wot?}"
// rot13.rb:100:in `<main>': undefined method `Wot?' for main:Object (NoMethodError)

so generally you should enclose the string literals in single-quotes.


The regular expression /[[:upper:]]/ matches all "uppercase alphabetical characters" (Unicode characters in the "Lu" general category, to be precise), and not just the ASCII uppercase letter A...Z.

added 772 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 37
  • 95

The regular expression /[[:upper:]]/ matches all "uppercase alphabetical characters" (Unicode characters in the "Lu" general category, to be precise), and not just the ASCII uppercase letter A...Z.

Therefore your method can produce garbage, e.g. for German "umlauts", or even crash, e.g. for Greek letters:

puts rot13_string "ÄÖÜ"
puts rot13_string "ΔΘΣ"

Output:

???
rot13.rb:42:in `chr': 903 out of char range (RangeError)
 from rot13.rb:42:in `rot13_char'
 from rot13.rb:62:in `block in rot13_string'
 from rot13.rb:61:in `each'
 from rot13.rb:61:in `rot13_string'
 from rot13.rb:86:in `'

That might not be relevant for this specific programming challenge (which probably works with ASCII only).

But generally, to substitute only ASCII letters and ignore all other input, the patterns should be /[A-Z]/ and /[a-z]/, respectively.


The rot13_string can be shortened to

def rot13_string str
 # Apply ROT13 cipher to all characters in a string
 #str+:: the string to apply ROT13 to
 str.each_char.map { |ch| rot13_char ch }.join("")
end

Instead of creating an array with all characters and appending each mapped character to the result array, each_char returns an enumerator for the characters, and map returns an array with the transformed characters.

In the same way, the rot13 function becomes

def rot13 messages
 # Apply ROT13 to an array of "messages" i.e. strings
 # +messages+:: the array of strings to apply ROT13 to
 messages.map { |msg| rot13_string msg }
end 

As an alternative, you can use the tr method which takes two string arguments (with possible ranges) and transforms the given string by replacing each character in the first argument by the corresponding character in the second argument:

def rot13_string str
 # Apply ROT13 cipher to all characters in a string
 # +str+:: the string to apply ROT13 to
 str.tr('A-Za-z', 'N-ZA-Mn-za-m')
end

(From https://rosettacode.org/wiki/Rot-13#Ruby. )

This also works "Unicode-correct" in the sense that all other Unicode characters are preserved. It is less flexible than your approach however, since you cannot simply change the "offset" from 13 to any other number.

The regular expression /[[:upper:]]/ matches all "uppercase alphabetical characters" (Unicode characters in the "Lu" general category, to be precise), and not just the ASCII uppercase letter A...Z.

Therefore your method can produce garbage, e.g. for German "umlauts", or even crash, e.g. for Greek letters:

puts rot13_string "ÄÖÜ"
puts rot13_string "ΔΘΣ"

Output:

???
rot13.rb:42:in `chr': 903 out of char range (RangeError)
 from rot13.rb:42:in `rot13_char'
 from rot13.rb:62:in `block in rot13_string'
 from rot13.rb:61:in `each'
 from rot13.rb:61:in `rot13_string'
 from rot13.rb:86:in `'

That might not be relevant for this specific programming challenge (which probably works with ASCII only).

But generally, to substitute only ASCII letters and ignore all other input, the patterns should be /[A-Z]/ and /[a-z]/, respectively.


The rot13_string can be shortened to

def rot13_string str
 # Apply ROT13 cipher to all characters in a string
 #str+:: the string to apply ROT13 to
 str.each_char.map { |ch| rot13_char ch }.join("")
end

Instead of creating an array with all characters and appending each mapped character to the result array, each_char returns an enumerator for the characters, and map returns an array with the transformed characters.

In the same way, the rot13 function becomes

def rot13 messages
 # Apply ROT13 to an array of "messages" i.e. strings
 # +messages+:: the array of strings to apply ROT13 to
 messages.map { |msg| rot13_string msg }
end 

The regular expression /[[:upper:]]/ matches all "uppercase alphabetical characters" (Unicode characters in the "Lu" general category, to be precise), and not just the ASCII uppercase letter A...Z.

Therefore your method can produce garbage, e.g. for German "umlauts", or even crash, e.g. for Greek letters:

puts rot13_string "ÄÖÜ"
puts rot13_string "ΔΘΣ"

Output:

???
rot13.rb:42:in `chr': 903 out of char range (RangeError)
 from rot13.rb:42:in `rot13_char'
 from rot13.rb:62:in `block in rot13_string'
 from rot13.rb:61:in `each'
 from rot13.rb:61:in `rot13_string'
 from rot13.rb:86:in `'

That might not be relevant for this specific programming challenge (which probably works with ASCII only).

But generally, to substitute only ASCII letters and ignore all other input, the patterns should be /[A-Z]/ and /[a-z]/, respectively.


The rot13_string can be shortened to

def rot13_string str
 # Apply ROT13 cipher to all characters in a string
 #str+:: the string to apply ROT13 to
 str.each_char.map { |ch| rot13_char ch }.join("")
end

Instead of creating an array with all characters and appending each mapped character to the result array, each_char returns an enumerator for the characters, and map returns an array with the transformed characters.

In the same way, the rot13 function becomes

def rot13 messages
 # Apply ROT13 to an array of "messages" i.e. strings
 # +messages+:: the array of strings to apply ROT13 to
 messages.map { |msg| rot13_string msg }
end 

As an alternative, you can use the tr method which takes two string arguments (with possible ranges) and transforms the given string by replacing each character in the first argument by the corresponding character in the second argument:

def rot13_string str
 # Apply ROT13 cipher to all characters in a string
 # +str+:: the string to apply ROT13 to
 str.tr('A-Za-z', 'N-ZA-Mn-za-m')
end

(From https://rosettacode.org/wiki/Rot-13#Ruby. )

This also works "Unicode-correct" in the sense that all other Unicode characters are preserved. It is less flexible than your approach however, since you cannot simply change the "offset" from 13 to any other number.

added 480 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 37
  • 95
Loading
Source Link
Martin R
  • 24.2k
  • 2
  • 37
  • 95
Loading
lang-rb

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