2
\$\begingroup\$

Julius Caesar protected his confidential information by encrypting it in a cipher. Caesar's cipher rotated every letter in a string by a fixed number, K, making it unreadable by his enemies. Given a string,S , and a number, K , encrypt S.

Note: The cipher only encrypts letters; symbols, such as -, remain unencrypted.

func cipher(messageToCipher: String, k: Int)-> String{
  var  eMessage = ""
  let arr = messageToCipher.characters.map { String($0) }
  for ch in arr {
    for code in String(ch).utf8 { 
        if (65<=code && code<=90) || (97<=code && code<=122) {
              if k > 26{
                    let value =  k % 26 == 0 ? k / 26 : k % 26 
                    var pCode  = ((Int(code))  + (value))  

                    if (pCode  > 122 && (97<=code && code<=122)) || (pCode  > 90 && (65<=code && code<=90)) {
                        pCode = pCode - 26
                    }

                   let s = String(UnicodeScalar(UInt8(pCode)))
                   eMessage = eMessage + s
                }else
                {
                    var pCode  = Int(code) + k

                    if (pCode  > 122 && (97<=code && code<=122)) || (pCode  > 90 && (65<=code && code<=90)) {
                        pCode = pCode - 26
                    }

                   let s = String(UnicodeScalar(UInt8(pCode)))
                   eMessage = eMessage + s
                }

        }
        else{        
            eMessage = eMessage + ch
        }
    }   
  }
  return eMessage
}


print(cipher(messageToCipher: "abc", k: 2) )

I solved this way and it was passed all test cases.

More information on this problem: Caesar Cipher

\$\endgroup\$
1
\$\begingroup\$

Enumerating UTF-8 characters: Your code uses two nested loops to encrypt letters (based on the UTF-8 code):

var  eMessage = ""
let arr = messageToCipher.characters.map { String($0) }
for ch in arr {
    for code in String(ch).utf8 {
        if (65<=code && code<=90) || (97<=code && code<=122) {
            // ... translate `code` and append to `eMessage` ...
        }
        else{
            eMessage = eMessage + ch
        }
    }
}

This is unnecessary complicated and has an unwanted side effect if the message contains non-ASCII characters (which are encoded as 2 or more UTF-8 code units):

print(cipher(messageToCipher: "Γ€ € πŸ˜€ πŸ‡§πŸ‡©", k: 2) )
// ÀÀ €€€ πŸ˜€πŸ˜€πŸ˜€πŸ˜€ πŸ‡§πŸ‡©πŸ‡§πŸ‡©πŸ‡§πŸ‡©πŸ‡§πŸ‡©πŸ‡§πŸ‡©πŸ‡§πŸ‡©πŸ‡§πŸ‡©πŸ‡§πŸ‡©

This could be solved by adding a break in the else-case. The better solution is to enumerate the UTF-8 code units directly:

var encoded: [UInt8] = []
for code in messageToCipher.utf8 {
    if (65<=code && code<=90) || (97<=code && code<=122) {
        // ... translate `code` and append to `encoded` ...
    }
    else{
        encoded.append(code)
    }
}
return String(bytes: encoded, encoding: .utf8)!

Code duplication: There is some duplicate code in

            if k > 26{
                let value =  k % 26 == 0 ? k / 26 : k % 26
                var pCode  = ((Int(code))  + (value))
                if (pCode  > 122 && (97<=code && code<=122)) || (pCode  > 90 && (65<=code && code<=90)) {
                    pCode = pCode - 26
                }
                let s = String(UnicodeScalar(UInt8(pCode)))
                eMessage = eMessage + s
            }else
            {
                var pCode  = Int(code) + k
                if (pCode  > 122 && (97<=code && code<=122)) || (pCode  > 90 && (65<=code && code<=90)) {
                    pCode = pCode - 26
                }
                let s = String(UnicodeScalar(UInt8(pCode)))
                eMessage = eMessage + s
            }

which is not necessary. The if-part works for the case k <= 26 as well.

Exceptional key values: The special handling of the case k % 26 == 0 causes unexpected results:

print(cipher(messageToCipher: "abc xyz ABC XYZ", k: 26) ) // abc xyz ABC XYZ
print(cipher(messageToCipher: "abc xyz ABC XYZ", k: 52) ) // cde zab CDE ZAB

and is not needed. Negative key values are not handled at all:

print(cipher(messageToCipher: "abc xyz ABC XYZ", k: -2) )   // _`a vwx ?@A VWX
print(cipher(messageToCipher: "abc xyz ABC XYZ", k: -200) )
// Fatal error: Negative value is not representable

Both issues can be solved by computing the remainder of k modulo 26, as a number in the range 0...25:

var value = k % 26
if value < 0 { value += 26 }

In addition, this computation can be done once, before entering the loop.

Other issues:

  • I would use some different argument/variable names, e.g. message instead of messageToCipher (it is clear from the context and the function name that this string should be encrypted), or key instead of k.
  • There are unnecessary parentheses, e.g. in

    var pCode  = ((Int(code))  + (value))
    
  • The usage of white space is not consistent, e.g. in

    if (pCode  > 122 && (97<=code && code<=122)) || (pCode  > 90 && (65<=code && code<=90)) {
    

Summarizing the suggestions so far, we have

func cipher(message: String, key: Int) -> String {

    var offset = key % 26
    if offset < 0 { offset += 26 }

    var encoded: [UInt8] = []
    for code in message.utf8 {
        if (65 <= code && code <= 90) || (97 <= code && code <= 122) {
                var pCode  = Int(code) + offset
                if (pCode > 122 && (97 <= code && code <= 122)) || (pCode > 90 && (65 <= code && code <= 90)) {
                    pCode -= 26
                }
                encoded.append(UInt8(pCode))
        } else {
            encoded.append(code)
        }
    }
    return String(bytes: encoded, encoding: .utf8)!
}

Make it functional: If the encryption of a single character is moved to a separate function then one can apply this to the UTF-8 view with map():

func cipher(message: String, key: Int) -> String {

    var offset = key % 26
    if offset < 0 { offset += 26 }

    func utf8encrypt(code: UInt8) -> UInt8 {
        // ... compute and return encrypted character ...
    }

    return String(bytes: message.utf8.map(utf8encrypt), encoding: .utf8)!
}

Further suggestions:

  • Use a switch-statement with range patterns instead of the if-conditions:

    switch code {
    case 65...90:
        // ...
    case 97...122:
        // ...
    default:
        // ...
    }
    
  • Use constants instead of the literal numbers, e.g.

    let letter_A = 65
    let letter_Z = 90
    let letter_a = 97
    let letter_z = 122
    

so that a future reader of your code does not have to guess what the numbers stand for.

\$\endgroup\$
1

Your Answer

By clicking β€œPost Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.