6
\$\begingroup\$

I am trying to solve the common "print a character after every item in a list, except the last one" problem.

I do not want to use list indexes. There are a couple of simple ways to do this in Python, but I am not sure how to apply them to my code.

Is there a more readable/couple liner way to do this?

def echo(*args):
    for data in args:
        if data is args[-1]:
            ender = ''
        else:
            ender = ' '

        if isinstance(data, six.string_types) \
                    or isinstance(data, six.integer_types):

            print_(data, end=ender)
        else:
            op = getattr(data, '__str__', None)
            if op:
                print_(data.__str__(), end=ender)
            else:
                LOG.error("echo called with an unsupported data type")
    print_('')
\$\endgroup\$
  • \$\begingroup\$ (I am not able to solve the "except the last one") , Is there a way to do this without the trailing space. -> sounds like you want us to fix it. This is not within the scope of the site. Members can only review working code. And working code means it gives you what you expect in the end or try to achieve. \$\endgroup\$ – bhathiya-perera Aug 18 '14 at 21:52
  • \$\begingroup\$ @Bhathiya-JaDogg-Perera Fixed. My code now works. But is ugly \$\endgroup\$ – Humdinger Aug 18 '14 at 22:01
  • \$\begingroup\$ @Bhathiya-JaDogg-Perera Yes. It must work on both versions, hence why i am using six \$\endgroup\$ – Humdinger Aug 18 '14 at 22:07
7
\$\begingroup\$

I might be misreading your code, but it looks like all you want is:

def echo(*args):
    six.print_(*args, sep=" ", end="")

For something a bit more manual:

def echo(*args):
    six.print_(" ".join(map(str, args)), end="")

In terms of reviewing your actual code, on top of the issues Morwenn raises, note that:

op = getattr(data, '__str__', None)
if op:

simplifies to:

if hasattr(data, '__str__'):

However, it is rare to call magic methods directly in Python; normally you'd just do str(data) and let Python call __str__. This is safer, as although an old-style class may not implement __str__, you can still get e.g. '<foo.bar object at 0x...>' via str(...).


I would include the filtering as a separate function:

def valid(arg):
    """Include a docstring to explain your filtering rules."""
    if (isinstance(data, (six.string_types, six.integer_types)) or
        hasattr(data, '__str__')):
        return True
    LOG.error("echo called with an unsupported data type")
    return False

Then echo can use filter:

def echo(*args):
    six.print_(*filter(valid, args), sep=" ", end="")
\$\endgroup\$
  • \$\begingroup\$ It seems that there is also the "type handling" part. I don't know what it really adds to print_ though. \$\endgroup\$ – Morwenn Aug 18 '14 at 22:34
  • \$\begingroup\$ @Morwenn I couldn't figure out exactly what cases the OP was trying to cover with that either. Specifically trying to find and call __str__ seems a bit odd. \$\endgroup\$ – jonrsharpe Aug 18 '14 at 22:46
  • \$\begingroup\$ I have been trying to figure it out for 20 minutes too, but I am as clueless as you are :/ \$\endgroup\$ – Morwenn Aug 18 '14 at 22:50
  • \$\begingroup\$ @Morwenn the main thing is that If I call echo on an object that doesnt have a __str__ method, or is a scalable, I want to log it, and not print it. \$\endgroup\$ – Humdinger Aug 18 '14 at 23:29
  • \$\begingroup\$ @Humdinger I have added this to the end of my answer. \$\endgroup\$ – jonrsharpe Aug 19 '14 at 6:15
3
\$\begingroup\$

You could probably write a nested function that does the print and then you can handle the number of the iteration in the main function:

def echo(*args):

    # print a value with regard to its type
    def do_echo(data, ender):
        if isinstance(data, six.string_types) \
                    or isinstance(data, six.integer_types):

            print_(data, end=ender)
        else:
            op = getattr(data, '__str__', None)
            if op:
                print_(data.__str__(), end=ender)
            else:
                LOG.error("echo called with an unsupported data type")

    # print all the values but the last one
    for arg in args[:-1]:
        do_echo(arg, ender=' ')

    # print the last value
    do_echo(args[-1], ender='\n')

That was for the refactoring of the code logic. Now, I also have a couple of remarks:

  • If args is empty, args[-1] will raise an IndexError. Therefore, it would be good to write this instead:

    # print the last value
    if args:
        do_echo(args[-1], ender='\n')
    
  • The PEP8, the style guide for Python does not like explicit line continuations with the backslash and prefers implicit line continuations with parenthesis:

    if (isinstance(data, six.string_types) or
        isinstance(data, six.integer_types)):
    

    And as pointed by @jonrsharpe in the comments, isinstance can take a tuple of types instead of a simple type. Therefore, you can rewrite this condition as:

    if isinstance(data, (six.string_types, six.integer_types)):
    
  • Why did you fully qualify six.string_types and six.integer_types but not six.print_? Being consistent with regard to the use of the six. prefix would be good.

\$\endgroup\$
  • 2
    \$\begingroup\$ Note you can pass a tuple to isinstance; if isinstance(data, (six.string_types, six.integer_types)): \$\endgroup\$ – jonrsharpe Aug 18 '14 at 22:48

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.