JuliaMath/Primes.jl

factor modifies its argument

Closed this issue · 1 comments

I noticed something:

julia> h = Dict{Int,Int}(1=>10, 4=>6)
Dict{Int64,Int64} with 2 entries:
  4 => 6
  1 => 10

julia> factor(100, h)  # Undocumented usage
Dict{Int64,Int64} with 4 entries:
  4 => 6
  2 => 2
  5 => 2
  1 => 10

Despite the fact that such usage is undocumented, a user can still easily see that the method exists with methods(factor). Here's what I propose we do: follow the convention of using ! at the end of the function name, e.g. factor!, then define factor to call factor! on an empty dictionary, export factor, and not export factor!. That approach seems to be common in other packages. Thoughts?

cc @rfourquet

This makes sense. I'm not 100% convinced yet but will think about it. Then should the dict be the first argument? I should probably put up a PR with the change I was mentioning there, at least as a WIP (I'm not really ready and needs more thinking), this could help deciding on this issue.