HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Append nones to list if is too short

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
nonesshorttooappendlist

Problem

Is it possible to improve this bit?

def append_nones(length, list_):
    """
    Appends Nones to list to get length of list equal to `length`.
    If list is too long raise AttributeError
    """
    if len(list_)  length:
        raise AttributeError('Length error list is too long.')
    else:
        return list_

Solution


  1. Comments on your code



-
The documentation does not match the implementation. The docstring says "append Nones to list" but it does not append: if the list does not have the required length, it returns a newly allocated list.

-
It seems wrong to return the original list if it was the right length, but return a newly allocated list if it was too short. For consistency, the function should either always return a new list, or always (update and) return the old list.

-
It's a loss of generality to support only the addition of None elements. Why not let the caller pass in their preferred placeholder element.

-
If you run help(AttributeError) you'll see that its description is "Attribute not found." This does not seem appropriate here. ValueError would be a better choice, or better still, create your own.

-
There's no need to name the variable list_: this avoids shadowing the built-in function list, but you don't use that function, so it doesn't matter if you shadow it.

-
You call len(list_) three times. Simpler to call it once and store the value.

-
The case len(list_) == length could be combined with the case `len(list_)

-
The error message "Length error list is too long" makes no sense in English.

-
The error message would be more useful if it contained the actual numbers.

-
This kind of function is ideal for doctests.

  1. Revised code



I've chosen to make the function always update its argument, and fixed all the problems noted above.

class TooLongError(ValueError):
    pass

def pad(seq, target_length, padding=None):
    """Extend the sequence seq with padding (default: None) so as to make
    its length up to target_length. Return seq. If seq is already
    longer than target_length, raise TooLongError.

    >>> pad([], 5, 1)
    [1, 1, 1, 1, 1]
    >>> pad([1, 2, 3], 7)
    [1, 2, 3, None, None, None, None]
    >>> pad([1, 2, 3], 2)
    ... # doctest: +IGNORE_EXCEPTION_DETAIL
    Traceback (most recent call last):
      ...
    TooLongError: sequence too long (3) for target length 2

    """
    length = len(seq)
    if length > target_length:
        raise TooLongError("sequence too long ({}) for target length {}"
                           .format(length, target_length))
    seq.extend([padding] * (target_length - length))
    return seq

Code Snippets

class TooLongError(ValueError):
    pass

def pad(seq, target_length, padding=None):
    """Extend the sequence seq with padding (default: None) so as to make
    its length up to target_length. Return seq. If seq is already
    longer than target_length, raise TooLongError.

    >>> pad([], 5, 1)
    [1, 1, 1, 1, 1]
    >>> pad([1, 2, 3], 7)
    [1, 2, 3, None, None, None, None]
    >>> pad([1, 2, 3], 2)
    ... # doctest: +IGNORE_EXCEPTION_DETAIL
    Traceback (most recent call last):
      ...
    TooLongError: sequence too long (3) for target length 2

    """
    length = len(seq)
    if length > target_length:
        raise TooLongError("sequence too long ({}) for target length {}"
                           .format(length, target_length))
    seq.extend([padding] * (target_length - length))
    return seq

Context

StackExchange Code Review Q#40272, answer score: 11

Revisions (0)

No revisions yet.