4
\$\begingroup\$

Task is simply to write the classic isPalindrom-function.

Here's my implementation:

func isStringPalindrom(str: String) -> Bool {
    let lcStr = str.lowercased()
    var i = 0
    let aChars = Array(lcStr)
    
    while i < (lcStr.count / 2) {
        if aChars[i] != aChars[aChars.count - (i + 1)] {
            return false
        }
        i += 1
    }
    
    return true
}

// Samples (provided with the exercise)
print(isStringPalindrom(str: "rotator")) // Should return true
print(isStringPalindrom(str: "Rats live on no evil star")) // Should return true
print(isStringPalindrom(str: "Never odd or even")) // Should return false
print(isStringPalindrom(str: "Hello, world")) // Should return false

// Own ideas
print(isStringPalindrom(str: "Anna"))
print(isStringPalindrom(str: "Otto"))
print("Otxto: \(isStringPalindrom(str: "Otxto"))")
print(isStringPalindrom(str: "Tacocat"))
print(isStringPalindrom(str: "Tacoxcat"))

All tests work as expected. Therefore I think it's formally correct.

But could it become improved concerning algorithm and naming?

\$\endgroup\$
3
  • \$\begingroup\$ Maybe the algorithm should not be concerned about lowercase. Let caller decide if they lowercase the input isPalindrome(str.lowercased()), remove whitespace, commas, etc. None of that is concern of the core of the algorithm. \$\endgroup\$
    – slepic
    Commented May 20, 2023 at 10:48
  • \$\begingroup\$ You don't need the intermediate string lcStr, and you don't have to lowercase all letters. lcStr.count is O(n). \$\endgroup\$
    – ielyamani
    Commented May 20, 2023 at 11:32
  • \$\begingroup\$ slepic, ielyamani : Thanks for the hints. \$\endgroup\$ Commented May 21, 2023 at 5:22

1 Answer 1

6
\$\begingroup\$

Nitpicking: In English it is “palindrome” and not “palindrom”, so the function name should be isStringPalindrome.

The parameter name str does not convey any information to the caller, I would use an empty argument label instead

func isStringPalindrome(_ str: String) -> Bool

or simply

func isPalindrome(_ str: String) -> Bool

Another option is to make it an extension method of the String type

extension String {
  func isPalindrome() -> Bool {
        // ...
    }
}

which is then called as

"someString".isPalindrome()

As said in the comments, you might want to leave it to the caller whether all characters should be converted to lower case before the comparison.

You convert the string to an array first. That allows efficient (\$ O(1) \$) random access to all characters. But then you should use aChars.count in the while-condition and not lcStr.count. The latter is \$ O(N) \$ where \$ N \$ is the number of characters in the string, whereas the former is \$ O(1) \$.

Instead of creating an array with all characters you can use two indices into the string, one traversing from the start of the string forward, and the other traversing from the end of the string backwards:

func isPalindrome(_ str: String) -> Bool {
    if str.isEmpty {
        return true
    }
    
    var front = str.startIndex
    var back = str.endIndex
    str.formIndex(before: &back)

    while front < back {
        if str[front] != str[back] {
            return false
        }
        str.formIndex(after: &front)
        str.formIndex(before: &back)
    }
    
    return true
}

But actually this can be shortened and simplified to

func isPalindrome(_ str: String) -> Bool {
    str.elementsEqual(str.reversed())
}

reversed() creates a view presenting the elements of the string in reverse order, so no additional string or array is allocated, and elementsEqual compares the two strings (or in general, sequences) one by one until a difference is found.

\$\endgroup\$

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