Simplification of setconfig! methods #1
Conversation
|
Just looking at this now @tisztamo , sorry for the delay. I have my head down building a Julia Websocket library. Comments to follow on your pull request. Generally, thanks for pointing out the messy code factoring! |
| end | ||
| function setconfig!(path::String, value::Array) | ||
| value = json(value) |> JSON.parse | ||
| function setconfig!(path::String, value) | ||
| value = pathtodict(path, value) |
There was a problem hiding this comment.
I have pathtodict as configpathtodict in utils.jl. Have you changes there not in this PR?
There was a problem hiding this comment.
Actually....
I think that your fork is behind my master, I am seeing that pathtodict is also in the replaced lines.
I am happy with all your proposals if your PR is aligned with the latest master. Just to verify some Julia priorities:
setconfig!(path::String, value::String)
setconfig!(path::String, value)
function setconfig!(path::String, value::Union{Tuple, AbstractArray, NamedTuple, Dict})
Is Julia going to prioritise functions with declared types, or is there meaning in the line ordering of function declarations?
Other than that, if you would bump the patch version and add yourself as a contributer, I will merge the request.
| setconfig!(value) | ||
| end | ||
| function setconfig!(path::String, value::Dict) | ||
| function setconfig!(path::String, value::Union{Tuple, AbstractArray, NamedTuple, Dict}) | ||
| value = json(value) |> JSON.parse | ||
| value = pathtodict(path, value) |
|
As this is now in the code I close it. To your question "Is Julia going to prioritise functions with declared types, or is there meaning in the line ordering of function declarations?": The first one. Julia selects the most specific method, ordering is not considered. No type annotation means Any. Would be great to hear about your websocket library when it gets usable! I am currently working with HTTP.jl but not very happy with it. |
|
@tisztamo https://github.com/citkane/Websocket.jl - All done, just busy with documentation before I publish it to JuliaHub. I am happy that HTTP.jl is there, it is a big effort, but I had to work around loads of issues to get my Websockets working with it... |
Eliminated some code duplication. vscode also removed a few trailing spaces.