Why does my for loop always return 'False'?

Question

Here is the given task: Write a function that receives two words, in string format, s1 and s2.

It should return True, if s1 and s2 are palindromes, and False otherwise.

I started by converting the strings to lists. Then, I check if the 1st string (now converted to a list) is a palindrome, followed by the second string (also converted to a list). Eventually, I form a boolean that returns True if both lists are palindromic, and False otherwise. However, when I input strings with both words being palindromic (kayak and madam), it returns False. Could you provide some guidance on this?

Here is my code:

def palindrome(s1, s2):
    convert_s1 = list(s1)
    convert_s2 = list(s2)
    for letter in [convert_s1]:
        n = 0
        if convert_s1[n] == convert_s1[-(n+1)]:
            is_s1_palindrome = True 
        else:
            is_s1_palindrome = False 
    for letter in [convert_s2]:
        m = 0
        if convert_s2[m] == convert_s1[-(m+1)]:
            is_s2_palindrome = True
        else:
            is_s1_palindrome = False
    if (is_s1_palindrome == True) and (is_s2_palindrome == True):
        both_pals = True
    else: 
        both_pals = False
    return both_pals

Answer 1

The reason you're getting false all the time is almost certainly due to the fact that your second comparison is using s2 and s1:

if convert_s2[m] == convert_s1[-(m+1)]:

That means you will always get false except, I think, for very rare cases like the collections being both palindromic and identical.

Also, you seem to set is_s2_palindromic for the true case and is_s1_palindromic for the false case. I'm not convinced that was intended.

---

If your intent is just to fix that specific problem, you can probably stop reading now.

But there are other issues as well that you may want to consider. First, this code is not correct:

if convert_s1[n] == convert_s1[-(n+1)]:
    is_s1_palindrome = True 
else:
    is_s1_palindrome = False 

The instant you detect a difference in equivalent character positions, it ceases to be a palindrome. But you continue checking and, if positions start matching again, you re-flag it as a palindrome.

That means it's only the final comparison that matters, the one that compares the first and last elements (for the second time).

So something like the non-palindromic abca will be considered a palindrome, due to the following checks:

• a == a, flag true.
• b != c, flag false.
• c != b, flag false.
• a == a, flag true, this is the final result.

Also take note of that parenthetical "for the second time". There is no reason to go through the entire list since, if a[0] == a[-1], you can be reasonably certain that a[-1] == a[0]. Unless you've suborned the __eq__ dunder method, which I doubt :-)

You only really need to go to the halfway point.

---

As you appear to be learning algorithms rather than a specific language (Python can actually reverse lists and compare them very succinctly), your best bet taking all that into account would be something like:

left = 0                           # left, moving right.
right = len(coll) - 1              # right, moving left.
is_palindrome = True               # assume palindrome to start.
while left < right:                # if different, flag so and stop.
    if coll[left] != coll[right]:
        is_palindrome = False
        break
    left += 1                      # otherwise, move toward center.
    right -= 1

In fact, in terms of well structured code, you should look into the DRY principle ("Don't repeat yourself").

You can refactor out the palindrome check for a single collection and re-use that code for each. And keep in mind, if they're strings, you don't have to turn them into a list first. The snippet for letter in "abc" works fine.

Well structured code would be something like:

# This function evaluates one collection by
# starting indexes at the left and right,
# then moving toward the center. It
# compares each character, returning false
# as soon as a mismatch occurs. If it gets
# to the middle without a mismatch, it can
# safely return true.

def is_palin(coll):
    left = 0
    right = len(coll) - 1
    while left < right:
        if coll[left] != coll[right]:
            return False
            break
        left += 1
        right -= 1
    return True

# Returns true if and only if both
# collections are palindromic.

def palindrome(coll_1, coll_2):
    return is_palin(coll_1) and is_palin(coll_2)

---

And, although moot in light of this answer, you should be aware that code like:

some_list = list("abc")
for letter in [some_list]: ...

will actually give you a nested list like [['a', 'b', 'c']].

Hence letter will iterate once, with the single value ['a', 'b', 'c'], not three times with the individual values 'a', 'b', and 'c'.

To iterate over the letters, it would be for letter in some_list: ....

---

And just one final note. The actual specifications for the code you need to write could be considered ambiguous:

Write a function that receives two words, in string format, s1 and s2. It should return True, if s1 and s2 are palindromes, and False otherwise.

It's possible to interpret that as (so you should probably clarify) asking if s1 and s2 are reversals of each other, rather than both being palindromes (so something like s1 == "blub" and s2 == "bulb").

Technically, that wouldn't make the individual words palindromes, and I base this possible interpretation on little more than the fact the use-case of checking that two strings are palindromic is ... unusual.

So it's something you may want to follow up.

Answer 2

You can try using slicing:

def palindrome(s1, s2):
    is_s1_palindrome = s1 == s1[::-1]
    is_s2_palindrome = s2 == s2[::-1]
    
    return is_s1_palindrome and is_s2_palindrome