9
\$\begingroup\$

I'm brand new and been going through some beginner code problems on checkIO. I finished this one today and am posting because I'm sure there is a much more elegant way to accomplish this, and I think this particular problem has a lot of useful applications. I would like to know what are the 'clean and efficient' ways to do it, or maybe what built-ins I haven't learned yet that I should explore. My version seems pretty ugly. Thanks!

Problem: take a tuple of phrases, anytime the word 'right' appears, replace it with 'left'. Create a new string combining all the phrases separated with a comma.

def left_join(phrases):
    """
        Join strings and replace "right" to "left"
    """
    phrases = list(phrases)
    newstr = ''
    for j, phrase in enumerate(phrases):
        if 'right' in phrase:
            mod = dict(enumerate(phrase))
            for key in mod:
                if mod[key] == 'r' and mod[key+4] == 't':
                    i = key
                    mod[i] = 'l'
                    mod[i+1] = 'e'
                    mod[i+2] = 'f'
                    mod[i+3] = 't'
                    mod[i+4] = ''
            switch = (list(mod.values()))
            phrase = ''.join(switch)
            if j == (len(phrases) - 1) or len(phrases) == 1:
                newstr += phrase
            else:
                newstr += phrase + ','
        else:
            if j == (len(phrases) - 1) or len(phrases) == 1:
                newstr += phrase
            else:
                newstr += phrase + ','
    return newstr

test1 = ("bright aright", "ok", "brightness wright",)
test2 = ("lorem","ipsum","dolor", "fright", "nec","pellentesque","eu",
         "pretium","quis","sem","nulla","consequat","massa","quis",)
\$\endgroup\$
  • \$\begingroup\$ Try your code on test3=(“crash”,). \$\endgroup\$ – Carsten S Oct 27 at 9:32
  • \$\begingroup\$ It returned "crash". I was expecting it to fail. Trying to find what you were getting at I did see another error my code would introduce, if a a five letter sub-string begins with r and ends with t that is not the word right, it would modify that too. That's not good. What did you see about the test ("crash",)? \$\endgroup\$ – spence Oct 27 at 16:36
  • \$\begingroup\$ sorry, I had missed a line in your code. Try “rightcrash”. \$\endgroup\$ – Carsten S Oct 27 at 16:39
  • 1
    \$\begingroup\$ Aha, I kind of thought that's what you were getting at, lucky guess on my part. I see now. KeyError: 10. Originally, I had a break statement so it would pop out after it found the word, but then it would miss another instance of 'right' in the phrase if there was one. I'm going to play around with a fix. \$\endgroup\$ – spence Oct 27 at 16:41
  • \$\begingroup\$ I think I got it, I added a try statement after for key and before the if statement: try: mod[key] == 'r' and mod[key+4] == 't' except KeyError: break \$\endgroup\$ – spence Oct 27 at 16:56
6
\$\begingroup\$

Welcome to code review, good job using a docstring and trying to make your code readable.

  • Space after a comma:

    test2 = ("lorem","ipsum","dolor", "fright", "nec","pellentesque","eu", "pretium","quis","sem","nulla","consequat","massa","quis",)

    According to PEP0008 which I recommend checking for being the official Python style guide: a space should be left after a comma for readability.

    Which should look like:

    test2 = ('lorem', 'ipsum', ...)

  • Use descriptive names: names like left_join() newstr j are not describing the objects they represent ex: your function name can be def replace_word(words, word, new_word):
  • Type hints: You may use type hints to indicate what are the inputs and outputs in the following way:

    def your_function_name(phrases: tuple) -> str:

  • Docstrings: Good job using a docstring for your function, however the description is not very clear here's a better description and btw docstrings should indicate what the input and the output is (what the function parameters are and what it returns):

    def your_function_name(phrases: tuple, old_word: str, new_word: str) -> str:
        """
        Replace the old_word by the new_word if found in phrases.
        Args:
            phrases: Tuple of words.
            old_word: The word to replace.
            new_word: The replacement.
     Return:
         Joined string after replacements.
        """
    
  • str.replace() and str.join() You can use the replace method to achieve the same results in a shorter code and since you're familiar with join():

    Instead of doing this for every letter in every word:

    else:
        newstr += phrase + ','
    

    You can do:

    ', '.join(words)
    

    Code might look like:

    And Note: You usually wouldn't be creating a function for such a simple task but use replace() and join() directly, I'm creating a function just for the sake of the example.

    def replace_word(phrases: tuple, old_word: str, new_word: str) -> str:
        """
        Replace the old_word by the new_word if found in phrases.
        Args:
            phrases: Tuple of words.
            old_word: The word to replace.
            new_word: The replacement.
     Return:
         Joined string after replacements.
        """
        return ', '.join(word.replace(old_word, new_word) for word in phrases)
    
    
    if __name__ == '__main__':           
        test1 = ("bright aright", "ok", "brightness wright",)
        test2 = ("lorem","ipsum","dolor", "fright", "nec","pellentesque","eu",
     "pretium","quis","sem","nulla","consequat","massa","quis",)
        print(replace_word(test1, 'right', 'left'))
        print(replace_word(test2, 'right', 'left'))
    

    Output:

    bleft aleft, ok, bleftness wleft

    lorem, ipsum, dolor, fleft, nec, pellentesque, eu, pretium, quis, sem, nulla, consequat, massa, quis

\$\endgroup\$
  • \$\begingroup\$ wow, exactly what I was looking for. so concise. thanks for taking the time. (not supposed to thank but it won't show my upvote) \$\endgroup\$ – spence Oct 27 at 1:59
  • \$\begingroup\$ You're welcome and I guess the upvotes will work when you're past reputation of 15 \$\endgroup\$ – bullseye Oct 27 at 2:21
0
\$\begingroup\$

To replace the "right" instances from your phrases you could do it in a more elegant and quicker way using regular expressions.

#Import Regex Library
import re 
#Function to replace all "right" instances from your phrases
def replace_right(phrase): 
    phrase=list(phrase)
    for i in range(len(phrase)):
        #If the search of "right" is succesful
        if re.search(r"right",phrase[i]):
           #Substitutes "right" with "left"
           phrase[i]=re.sub(r"right","left",phrase[i])
    return phrase
\$\endgroup\$
  • 3
    \$\begingroup\$ welcome to code review, as per regulations of this website: codereview.stackexchange.com/help/how-to-answer you should make at least one insightful observation about the code in the question and answers that provide an alternative solution without explaining their reasoning are invalid answers and may be deleted. \$\endgroup\$ – bullseye Oct 27 at 19:01

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.