Presumably, if array[i] == '-'
is meant to check if the number is negative. Having it like you have though doesn't make a lot of sense. If it's in the loop, that suggests that a -
could be expected at any point within the number. In reality though, it will only ever occur as the first character in the string. I'd just check N
the number with if N < 0
to see if it's negative. You can streamline this check right in the definition of mod
:
mod = 1 if N >= 0 else -1
Unfortunately, that slightly complicates adding -
to result
. The simplest way to deal with it I think is just to change the definition of result
to be similar to that of mod
:
result = [] if n >= 0 else ['-']
Since the checks are the same though, you could combine then into returning a tuple instead then unpacking it:
mod, result = (1, []) if n >= 0 else (-1, ['-'])
And, array
will need to have the -
cleaved off if we're doing it before the loop:
mod, result, checked_digits = (1, [], digits) if n >= 0 else (-1, ['-'], digits[1:])
Normally, I don't think a triple variable definition like this should be encouraged, but there's nothing too fancy or complicated going on here to make the line confusing. I think it's fine.
I'd rename array
and N
:
You're iterating over the indices of digits
and using digits[i]
all over the place even though you don't even need the index in most cases. I'd iterate over the digits themselves, and use enumerate
to get the index in the couple cases that you actually need it for:
for i, digit in enumerate(digits):
. . .
if int(digit) * mod > 5 * mod:
result.append(digit)
I'd move the if i == len(array):
check out of the loop as well. Really, it's just checking if at no point a break
was encountered. That's exactly the use case for for
's else
clause (yes, for
can have an else
):
for i, digit in enumerate(checked_digits):
. . .
else:
result.append(5)
The else
will only execute if no break
was encountered. In the case of this code, that means that no digit greater than 5
was found.
Honestly, this is the first time I've ever found a useful case for for...else
. It worked out nice here though.
I'd add 5
and '5'
as constants at the top. Having the magic strings and numbers floating your code makes your code harder to change latter, and doesn't indicate what the 5
and '5'
actually are for:
TARGET_NUM = 5
TARGET_DIGIT = str(TARGET_NUM)
def solution(n):
Or, just make that number a parameter with a default to make it easy to use different target numbers:
def solution(n: int, target_num: int = 5) -> int:
target_digit = str(target_num)
I also added some type hints in there, because why not?
In the end, this is what I ended up with:
def solution(n: int, target_num: int = 5):
target_digit = str(target_num)
digits = str(n)
mod, result, checked_digits = (1, [], digits) if n >= 0 else (-1, ['-'], digits[1:])
for i, digit in enumerate(checked_digits):
if int(digit) * mod > TARGET_NUM * mod:
result.append(digit)
else:
result.append(TARGET_DIGIT)
result.append(checked_digits[i:])
break
else:
result.append(TARGET_DIGIT)
return int("".join(result))