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

Next bigger number with the same digits

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

Problem

I have to create a function that takes a positive integer number and returns the next bigger number formed by the same digits: 59853 -> 83559

Please let me know if there are any better solutions. I would like feedback how to improve this.

def next_bigger(num):
    numlist = [int(i) for i in str(num)]

    index_of_replace_num = -1
    i = len(numlist) - 1
    while i > 0:
        if numlist[i] > numlist[i-1]:
            index_of_replace_num = i - 1
            break
        i -= 1

    if index_of_replace_num == -1:
        return -1
    else:
        firstlist = numlist[:index_of_replace_num]
        replaced_num = numlist[index_of_replace_num]
        secondlist = numlist[index_of_replace_num+1:]
        new_replaced_num = 9
        i = 0
        delindex = 0
        while i  replaced_num and secondlist[i] < new_replaced_num:
                new_replaced_num = secondlist[i]
                delindex = i
            i += 1

        secondlist.pop(delindex)
        secondlist.append(replaced_num)
        firstlist.append(new_replaced_num)
        output = firstlist + sorted(secondlist)
        return int(''.join(str(x) for x in output))

Solution

No need to convert to int

Since for all digits '0'...'9', comparison works consistently as you would expect from integers, no need to convert to integers.
So instead of this:

numlist = [int(i) for i in str(num)]


You can simplify to:

numlist = list(str(num))


(I would also rename numlist to digits.)

Use range(...)

Instead of manually decrementing loop indexes,
for example with i -= 1,
it's more idiomatic to use a range(...).
For example, instead of:

i = len(numlist) - 1
while i > 0:
    if numlist[i] > numlist[i-1]:
        index_of_replace_num = i - 1
        break
    i -= 1


I recommend this writing style:

for i in range(len(numlist) - 1, 0, -1):
    if numlist[i] > numlist[i-1]:
        index_of_replace_num = i - 1
        break


Use early returns to reduce nesting

Instead of this:

if index_of_replace_num == -1:
    return -1
else:
    # many many lines of code ...


It becomes a bit more readable if you drop the else statement:

if index_of_replace_num == -1:
    return -1

# many many lines of code ...


Decompose to smaller functions

Instead of putting all the logic inside the next_bigger function,
it would be good to decompose to smaller logical steps,
which can be extracted into their own functions.
Decomposing to many smaller functions increases modularity of your programs,
makes testing easier,
and often leads to reduced duplication.

Alternative implementation

Consider this alternative implementation,
with the main method decomposed to smaller logical steps that are unit tested.
You can run these doctests with the command:

python -mdoctest next_bigger.py


Empty output means all tests pass (or that there are no tests).

In addition to doctests, it's of course recommended to write proper docstrings too, which I omitted for brevity.

def swap_first_with_higher(digits):
    """
    >>> swap_first_with_higher(list('59853'))
    ['8', '9', '5', '5', '3']

    """
    for pos in range(len(digits) - 1, 0, -1):
        if digits[0] >> reversed_tail(list('89553'))
    ['8', '3', '5', '5', '9']

    """
    return [digits[0]] + digits[1:][::-1]

def next_biggest(num):
    """
    >>> next_biggest(59853)
    83559

    >>> next_biggest(111)
    -1

    >>> next_biggest(11211)
    12111

    """
    digits = list(str(num))
    for pos in range(len(digits) - 1, 0, -1):
        if digits[pos-1] < digits[pos]:
            left = digits[:pos-1]
            right = reversed_tail(swap_first_with_higher(digits[pos-1:]))
            return int(''.join(left + right))

    return -1

Code Snippets

numlist = [int(i) for i in str(num)]
numlist = list(str(num))
i = len(numlist) - 1
while i > 0:
    if numlist[i] > numlist[i-1]:
        index_of_replace_num = i - 1
        break
    i -= 1
for i in range(len(numlist) - 1, 0, -1):
    if numlist[i] > numlist[i-1]:
        index_of_replace_num = i - 1
        break
if index_of_replace_num == -1:
    return -1
else:
    # many many lines of code ...

Context

StackExchange Code Review Q#115609, answer score: 5

Revisions (0)

No revisions yet.