4
\$\begingroup\$

I have solved the coding problem written here.

In summary:

  • Input: str = "red2 blue5 black4 green1 gold3"
  • Output: "green red gold black blue"

Here is my code:

def order_color(str1):
    #Split string using the whitespaces and send to a list
    #Sort elements within the list by the number
    str2 = sorted(str1.split(), key=lambda color: color[-1])

    #Convert the list to a string and erase the number
    str3 = ' '.join([str(colo[: -1]) for colo in str2])

    return str3

Is there a way to make the code more elegant, "pythonic" (Ex. To impress a MAANG inteviewer).

Thanks

\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Comments

Don't use comments to describe what your code does. Any developer can see that by reading the code itself. Comments are there to clarify why your codes does certain things if that is not immediately clear.

Docstrings

In order to document your code, use docstrings on modules, classes and functions.

Type hints

Using type hints can simplify determining the parameter and return types of functions.

Use existing library code

It might not be a big deal in your case, but lambda color: color[-1] can be substituted by using operator.itemgetter.

Useless casting

The casting to a string at str(colo[: -1]) is unnecessary, since slicing a str will return a str again.

Useless comprehension

You don't need to create a list in a comprehension at ' '.join([str(colo[: -1]) for colo in str2]). str.join will happily take a generator expression.

Useless local variable

Assigning an expression to str3 and then immediately returning it is useless. Just return the expression instead.

Naming

I hope we can all agree, that str{1,2,3} are suboptimal names. Call variables for what they represent.

Putting it together:

from operator import itemgetter


def order_colors(colors: str) -> str:
    """Order a whitespace separated string of colors
    ending with an single digit index by that index and
    return a string of the ordered colors without that index.
    """
    
    return ' '.join(
        color[:-1] for color in
        sorted(colors.split(), key=itemgetter(-1))
    )
\$\endgroup\$
2
\$\begingroup\$

This won't work if there are more than 9 numbers. To fix this write a function to get the number at the end of the string, e.g.:

def number_split(string):
    color = string.rstrip('0123456789')
    number = string[len(color):]
    return color, number

So the code becomes

def sorted_colors(input_line):
    return ' '.join(
        color for color, number in sorted(
            map(number_split, input_line.split()),
            key=lambda pair: pair[1]
        )
    )

The problems a MAANG interviewer would notice in your code are:

  • Lack of meaningful names: none of them mean anything and str2 is not even a string. Meaningful names are extremely important since they allow you to read and understand code much faster. It is an absolute requirement for MAANG since your code there will be read by many people and nobody wants to work with obscure code.
  • Lack of abstraction in the code: you use the fact that all strings in the list end with a single-digit number. This solution is not scalable since the code has to be overhauled when the number of colors increases.
  • Comments should explain not what the code is doing (it should be clear by reading the code itself) but why it has to be done. In this case any comments are redundant.
\$\endgroup\$
2
  • \$\begingroup\$ "This won't work if there are more than 9 numbers" - it's specified in the description that there won't be more than 9. \$\endgroup\$ Nov 11, 2022 at 9:51
  • \$\begingroup\$ @GrajdeanuAlex it's not mentioned in the question. \$\endgroup\$ Nov 11, 2022 at 10:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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