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.