Skip to content

Commit 458a2c4

Browse files
committed
Extend sign to Factorization. Improve divisors
Implement `sign(::Factorization)` to easily check if the number represented by a Factorization is lt, gt, or eq to 0, and use it when identifying special cases in `divisors` and `totient`. In the `divisors` function: - No more wasteful allocations from pointlessly `collect`ing factors. - Make `divisors(0)` return an empty vector instead of throwing. - Remove unnecessary type annotations and let return types be inferred.
1 parent 7e968b5 commit 458a2c4

File tree

3 files changed

+41
-27
lines changed

3 files changed

+41
-27
lines changed

src/Primes.jl

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# This file includes code that was formerly a part of Julia. License is MIT: http://julialang.org/license
22
module Primes
33

4-
using Base.Iterators: repeated
4+
using Base.Iterators: repeated, rest
55

66
import Base: iterate, eltype, IteratorSize, IteratorEltype
77
using Base: BitSigned
@@ -591,7 +591,7 @@ given by `f`. This method may be preferable to [`totient(::Integer)`](@ref)
591591
when the factorization can be reused for other purposes.
592592
"""
593593
function totient(f::Factorization{T}) where T <: Integer
594-
if !isempty(f) && first(first(f)) == 0
594+
if iszero(sign(f))
595595
throw(ArgumentError("ϕ(0) is not defined"))
596596
end
597597
result = one(T)
@@ -908,7 +908,7 @@ prevprimes(start::T, n::Integer) where {T<:Integer} =
908908
collect(T, Iterators.take(prevprimes(start), n))
909909

910910
"""
911-
divisors(n::T) -> Vector{T}
911+
divisors(n::Integer) -> Vector
912912
913913
Return a vector with the positive divisors of `n`.
914914
@@ -918,6 +918,8 @@ lexicographic (rather than numerical) order.
918918
919919
`divisors(-n)` is equivalent to `divisors(n)`.
920920
921+
For convenience, `divisors(0)` returns `[]`.
922+
921923
# Example
922924
923925
```jldoctest
@@ -935,46 +937,49 @@ julia> divisors(60)
935937
15 # 5 * 3
936938
30 # 5 * 3 * 2
937939
60 # 5 * 3 * 2 * 2
940+
941+
julia> divisors(-10)
942+
4-element Vector{Int64}:
943+
1
944+
2
945+
5
946+
10
947+
948+
julia> divisors(0)
949+
Int64[]
938950
```
939951
"""
940-
function divisors(n::T)::Vector{T} where {T<:Integer}
941-
n::T = abs(n)
952+
function divisors(n::T) where {T<:Integer}
953+
n = abs(n)
942954
if iszero(n)
943-
throw(ArgumentError("divisors function is not defined for 0"))
955+
return T[]
944956
elseif isone(n)
945-
return T[n]
957+
return [n]
946958
else
947959
return divisors(factor(n))
948960
end
949961
end
950962

951963
"""
952-
divisors(f::Factorization{T}) -> Vector{T}
964+
divisors(f::Factorization) -> Vector
953965
954966
Return a vector with the positive divisors of the number whose factorization is `f`.
955967
Divisors are sorted lexicographically, rather than numerically.
956968
"""
957-
function divisors(f::Factorization{T})::Vector{T} where {T<:Integer}
958-
factors = collect(f)
959-
960-
if isempty(factors) # n == 1
961-
return T[one(T)]
962-
elseif iszero(first(first(factors))) # n == 0
963-
throw(ArgumentError("divisors function is not defined for 0"))
964-
elseif first(first(factors)) == -1 # n < 0
965-
if length(factors) == 1 # n == -1
966-
return T[one(T)]
967-
else
968-
deleteat!(factors, 1)
969-
end
969+
function divisors(f::Factorization{T}) where {T<:Integer}
970+
sgn = sign(f)
971+
if iszero(sgn) # n == 0
972+
return T[]
973+
elseif isempty(f) || length(f) == 1 && sgn < 0 # n == 1 or n == -1
974+
return [one(T)]
970975
end
971976

972-
i::Int = 1
973-
m::Int = 1
974-
divs = Vector{T}(undef, prod(x -> x.second + 1, factors))
977+
i = m = 1
978+
fs = rest(f, 1 + (sgn < 0))
979+
divs = Vector{T}(undef, prod(x -> x.second + 1, fs))
975980
divs[i] = one(T)
976981

977-
for (p, k) in factors
982+
for (p, k) in fs
978983
i = 1
979984
for _ in 1:k
980985
for j in i:(i+m-1)

src/factorization.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,5 @@ Base.length(f::Factorization) = length(f.pe)
6565

6666
Base.show(io::IO, ::MIME{Symbol("text/plain")}, f::Factorization) =
6767
join(io, isempty(f) ? "1" : [(e == 1 ? "$p" : "$p^$e") for (p,e) in f.pe], " * ")
68+
69+
Base.sign(f::Factorization) = isempty(f.pe) ? one(keytype(f)) : sign(first(f.pe[1]))

test/runtests.jl

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,13 @@ end
236236

237237
@test factor(1) == Dict{Int,Int}()
238238

239+
# correctly return sign of factored numbers
240+
@test sign(factor(100)) == 1
241+
@test sign(factor(1)) == 1
242+
@test sign(factor(0)) == 0
243+
@test sign(factor(-1)) == -1
244+
@test sign(factor(-100)) == -1
245+
239246
# factor returns a sorted dict
240247
@test all([issorted(collect(factor(rand(Int)))) for x in 1:100])
241248

@@ -319,7 +326,7 @@ divisors_brute_force(n) = [d for d in one(n):n if iszero(n % d)]
319326
@testset "divisors(::$T)" for T in [Int16, Int32, Int64, BigInt]
320327
# 1 and 0 are handled specially
321328
@test divisors(one(T)) == divisors(-one(T)) == T[one(T)]
322-
@test_throws ArgumentError @inferred(divisors(T(0)))
329+
@test divisors(zero(T)) == T[]
323330

324331
for n in 2:1000
325332
ds = divisors(T(n))
@@ -333,7 +340,7 @@ end
333340
# We just need to verify that the following cases are also handled correctly:
334341
@test divisors(factor(1)) == divisors(factor(-1)) == [1] # factorizations of 1 and -1
335342
@test divisors(factor(-56)) == divisors(factor(56)) == [1, 2, 4, 8, 7, 14, 28, 56] # factorizations of negative numbers
336-
@test_throws ArgumentError @inferred(divisors(factor(0))) # factorizations of 0
343+
@test isempty(divisors(factor(0)))
337344
end
338345

339346
# check copy property for big primes relied upon in nextprime/prevprime

0 commit comments

Comments
 (0)