Running a for loop only after an if condition has been met

Question

I'm currently trying to make a very simply testing function that first checks to see if the users input is a multiple of 3, then tests each individual character to see if they are valid characters. Here's my code below:

def is_dna(string):

    string.upper() 
    if(len(string) % 3 == 0):
        print("LENGTH CORRECT")
        for n in string:
            if(n == "A" or n == "T" or n == "C" or n == "G"):
                print("Valid")
            else:
                print("Invalid character")
                break
        return True
    else:
        print("Too many/little characters")
        return False

When run, the bottom section will run fine, and if a correct amount of characters is used this will also successfully print the debug "LENGTH CORRECT" string. The issue is that the for loop will not initialize, and I haven't the foggiest why. Testing just the loop shows it to work fine; what is wrong with this function?

`string.upper() ` won't change the string. A *new string* with the characters converted is returned. — Martijn Pieters, Mar 21 '17 at 13:14
The loop initialises just fine, but you still return `True` anyway, regardless of the outcome. — Martijn Pieters, Mar 21 '17 at 13:14
Thank you Martijn, you identified the issue. The string.upper was the key fault here. Thank you! — pixel rain, Mar 21 '17 at 13:16
FWIW, you can validate the letters more efficiently using set operations, eg `{'A', 'T', 'C', 'G'}.issuperset(string.upper())`. — PM 2Ring, Mar 21 '17 at 13:25
What do you meant with "not initialize", not even "Invalid character" gets printed? If you want to check the validity of a string content, I suggest using regular expressions, something like: `re.match("[ATCG]{3}", string)` — Rolando Urquiza, Mar 21 '17 at 13:29

Infinidoge · Accepted Answer · 2017-03-21T13:27:51.553

2

simple fix you just need to have the string.upper() move into a variable then act upon the variable

code that is fixed

def is_dna(stri):
    string = stri.upper() 
    if(len(string) % 3 == 0):
        print("LENGTH CORRECT")
        for n in string:
            if(n == "A" or n == "T" or n == "C" or n == "G"):
                print("Valid")
            else:
                print("Invalid character")
                break
        return True
    else:
        print("Too many/little characters")
    return False
is_dna("ATCGCTATC") #this works and tests it perfectly

edited Mar 21 '17 at 13:27

answered Mar 21 '17 at 13:21

Infinidoge

56
7

Adding onto this, if anyone in future has this issue place a "return False" in the loop break section otherwise you will return True regardless. Thanks for the answer, this is the error! – pixel rain Mar 21 '17 at 13:26
Also, this code returns `True` even if `"Invalid character"` is printed. – PM 2Ring Mar 21 '17 at 13:41
@FragilePixel Why do you want the `is_dna` test to return `True` if it has the correct length even when the string contains invalid characters? That seems strange to me. – PM 2Ring Mar 21 '17 at 14:02

score 0 · Answer 2 · answered Mar 21 '17 at 13:27

0

string = string.upper()

the function upper() just return the uppercase of the character,the string self is not change.

answered Mar 21 '17 at 13:27

hankym

149
2

Rolando Urquiza · Answer 3 · 2017-03-21T21:11:19.993

0

You don't need the cycle, just use regular expressions:

import re

def is_dna(string)
    return re.match("([CAGT]{3})+", string.upper()) is not None

edited Mar 21 '17 at 21:11

answered Mar 21 '17 at 13:34

Rolando Urquiza

1,404
3
17
29

1

Use `is not None` instead of `!= None`. – Martijn Pieters Mar 21 '17 at 13:36

Running a for loop only after an if condition has been met

3 Answers3