Created
February 11, 2018 21:58
-
-
Save naimrajib07/4e85347755cfeea7164e7f993c8375ba to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
def rot13(secrete_messages) | |
alphabetSet = {1 => 'a', 2 => 'b', 3 => 'c', 4 => 'd', 5 => 'e', 6 => 'f', 7 => 'g', | |
8 => 'h', 9 => 'i', 10 => 'j', 11 => 'k', 12 => 'l', 13 => 'm', 14 => 'n', | |
15 => 'o', 16 => 'p', 17 => 'q', 18 => 'r', 19 => 's', 20 => 't', 21 => 'u', | |
22 => 'v', 23 => 'w', 24 => 'x', 25 => 'y', 26 => 'z'} | |
secrete_messages.map do |elm| | |
if (elm == '' || !alphabetSet.has_value?(elm.downcase)) | |
if(elm.length > 1) | |
rot13(elm.split('')).join() | |
else | |
elm | |
end | |
else | |
alpha_index = alphabetSet.key(elm.downcase) | |
alpha_msg_index = alpha_index % 13 | |
if (alpha_index < 13) | |
msg = alphabetSet.values_at(alpha_msg_index + 13).first | |
else | |
msg = alphabetSet.values_at(roundedIndex(alpha_msg_index)).first | |
end | |
msg = msg.upcase if elm == is_uppercase?(elm).to_s | |
msg | |
end | |
end | |
end | |
def roundedIndex(index) | |
index == 0 ? index+ 1 : index | |
end | |
def is_uppercase?(elm) | |
/[[:upper:]]/.match(elm) | |
end |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Your primary issue is the translation of
m
andz
. They should be translating to each other, but instead they are both translating toa
.They get
alpha_index
13 and 26, and both getalpha_msg_index
0. That means that they are both retrievingThere's a two-step fix to this: Note that the expression
x % 13
will give a number from 0-12, never 13. So if you adjust your alphabetSet toYou will find that that bug is mostly fixed. The other issue, then is
roundedIndex
. You should have heard some warning bells in your head when you wrote this function, because the formula you are using to calculate index in therot13
function gives the correct result most of the time... but some of the time, it just needs to arbitrarily change from 0 to 1? Examining this a little more closely might have led you to make the 0..25 indexing change I've just suggested.At any rate, you should no longer have need of the
roundedIndex
function at all, so get rid of it and I believe your code will be functioning. correctly. To verify, try using it to translate the string"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
and confirm you get"NOPQRSTUVWXYZABCDEFGHIJKLM"
.Beyond getting the code functioning, there are a few other changes I'd recommend, if you're interested. I'd encourage you to consult the ruby-docs as you read through these.
Range
andEnumerable#zip
.(0..25)
will give you the Range of numbers from 0 to 25,('a'..'z')
will give you the range of letters from a to z, andzip
will transform the two ranges into an array of pairs. This can be passed directly theHash
class for pain-free initialization:alphabetSet = Hash[(0..25).zip('a'..'z')]
Hash#values_at
, but you built the hash, so you know for a fact that each key has only one value. Save some time and just accessalphabetSet[alpha_msg_index + 13]
oralphabetSet[alpha_msg_index]
.elm == elm.upcase ? msg.upcase : msg
. This would let you remove theis_uppercase
function as a trivial one-liner.alphabetSet
. Hashes are made to be very fast in one direction (key -> value, or has_key), but they're no better than any other collection (and perhaps worse) at looking the other way (value -> key, or has_value). So if you want your code to be very fast, it's a good idea to avoid that. So you could make one hash mapping letters to their indexes, and another hash mapping indexes back to letters.indexes = {'a'=>0, 'b'=>1 ...}
andletters={ 0=>'a', 1=>'b' ...}
. Then the main part of your code would look something like'a'
always translates to'n'
, and that'b'
always translates to'm'
, and so on. So you might as well make a hashtranslation = {'a'=>'n', 'b'=>'m' ...}
, so that your main code is onlymsg = translation[elm]
. The only question is, what is the best way to initialize that hash? I recommend usingArray#rotate
:Hash#default_proc
. You can tell the hash, "If I ask about a key you don't have, just give me the key back instead of looking for a value". Here's how:translation.default_proc = proc{|hash, key| key }
Now we've got a nice readable function, that handles rot13 translation on an array of strings, while preserving whitespace and leaving non-alpha characters unchanged. Can we do it in less code? Absolutely. Can we make it faster? You bet. But I'd argue that we would need to sacrifice readability & beauty in order to do either, so I think we're at a pretty happy place.