don't load JSON at runtime

[stevengj]

If you just define a

export elements
const elements = ...load elements into a Dict{Int,Element} or similar....

then the contents of your element dictionary can be precompiled in, and you won't need to load the JSON data at runtime.

Then you can just do

using PeriodicTable
elements[8]

to get e.g. the element with atomic number 8.

No need for a get_element function, either, then.

[stevengj]

Since there are no gaps any more in the periodic table, it is perhaps better for elements to simply be a Vector{Element}, since this is faster to access than a dictionary.

[stevengj]

Actually, you might want element to be some custom type, like:

struct Elements
    bynumber::Vector{Element}
    byname::Dict{String,Element}
    Elements(data::Vector{Element}) = new(data, Dict{String,Element}(lowercase(d.name)=>d for d in data))
end
Base.getindex(e::Elements, i::Integer) = e.bynumber[i]
Base.getindex(e::Elements, i::AbstractString) = e.byname[lowercase(i)]
Base.show(io::IO, e::Elements) = print(io, "Elements table of ", length(e.bynumber), " elements")

so that you can look up the elements by number or by name, for convenience, with elements[8] or elements["oxygen"].

[carstenbauer]

First of all, thanks for the package.

I was going to open a similar issue. Just to complement steven's point, also from a users perspective it seems unnecessary to have to create a periodic table object to look up an element. It should be a constant in your package that I can access immediately, as steven showed in his first post.

[rahulkp220]

Thanks a lot, @stevengj and @crstnbr for taking out time and opening up the issues. It means a lot to me and makes me really happy.

I 100% agree with you on this one and will make the changes. Btw, if you would love, would you like to contribute? 🙂