Fix partitions(0), closes #143

[FedericoStra]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.23%. Comparing base (e005ee0) to head (87f7e23).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
+ Coverage   97.22%   97.23%   +0.01%     
==========================================
  Files           8        8              
  Lines         829      832       +3     
==========================================
+ Hits          806      809       +3     
  Misses         23       23              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[oameye]

Could we not instead use xs = Int[] and do isempty(xs)?

[FedericoStra]

I don't think so, and that's exactly the point.

collect(partitions(0)) must return Vector{Int}[[]].

This means that on the first call iterate(partitions(0)) must return (Int[], Int[]) and on the second call iterate(partitions(0), Int[]) must return nothing. Therefore the default value of xs (which is used in the first call) cannot be Int[] (which is used in the second call), because the behavior must be different.

Of course we could swap the role of nothing and Int[] and have iterate(partitions(0)) return (Int[], nothing) and iterate(partitions(0), nothing) return nothing, but this doesn't really work around the necessity of having an extra sentinel value which is different from Int[].

Leaving the code as it currently is preserves the "symmetry" that iterate always returns (xs, xs). Swapping nothing and Int[] instead has the benefit that the sentinel value nothing is used only for partitions(0) and not partitions(n) with n>0.

[FedericoStra]

Just for reference, the "swapped" code would look something like this:

function Base.iterate(p::IntegerPartitions, xs = Int[])
    if p.n == 0
        if xs === nothing # sentinel value
            return nothing
        else
            return (Int[], nothing) # sentinel value
        end
    end
    length(xs) == p.n && return
    xs = nextpartition(p.n, xs)
    return (xs, xs)
end

In this code it is maybe clearer that the nothing in the two lines marked with # sentinel value can be replaced by anything else we like as long as it is different from Int[]. For instance, we could use [0], which makes the code more type stable. I tried benchmarking this and it is slightly faster for collect(permutations(5)) (thanks to type stability), at the expenses of collect(permutations(0)) which is slower because Int[] === nothing is faster than Int[] == [0].

[oameye]

Thanks for the clear explanation. I would be more fan of the latter approach. @inkydragon What do you think?