Improve Project Euler problem 014 solution 2 #5744
Conversation
algorithms-keeper
bot
added
awaiting reviews
algorithms-keeper
bot
added
tests are failing
Nice work overall. You replaced the loop with recursion, possibly making the code faster. Also, having the sequence lengths as a global dictionary would save a lot of time computing.
Maybe note such changes in the commit message next time (E.g. loop to recursion).
Sincerely,
IdoErel
| sequence_length = collatz_sequence_length(next_n) + 1 | ||
| COLLATZ_SEQUENCE_LENGTHS[n] = sequence_length |
There was a problem hiding this comment.
Unnecessary variable assignment.
This can be shortened to one line.
There was a problem hiding this comment.
But then we should return COLLATZ_SEQUENCE_LENGTHS[n] instead of sequence_length. I don't know Python perfectly - whether there will be an extra search in the dictionary or not?
There was a problem hiding this comment.
No, you're right. I missed line 43.
Disregard my previous comment.
I originally meant, that assigning the result to sequence_length - and immediately after assigning it again to COLLATZ_SEQUENCE_LENGTHS, without any further usage is unnecessary. (of sequence_length).
However, you're returning sequence_length as the result. Saving another lookup - in short, keep it this way. I was wrong.
Sorry for my ignorance.
Sincerely,
IdoErel
algorithms-keeper
bot
removed
the
awaiting reviews
Describe your change:
Improve Project Euler problem 014 solution 2 - the top 1 slowest solution on Travis CI logs (under
slowest 10 durations:14.20s call scripts/validate_solutions.py::test_project_euler[problem_014/sol2.py]):Improve solution (locally 10+ times - from 15+ seconds to ~1.5 seconds)
Uncomment code that has been commented due to slow execution affecting Travis (now it should be quite fast execution and not affect Travis)
Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?
Checklist:
Fixes: #{$ISSUE_NO}.