The first method appears to do nothing as it returns v no matter what type v is. It used all over the place in this package when function returns anything that isn’t converted to a string. Like there is actually a case of return SOME(false). I think the author was coming from Java or some dogmatically OO language and became very worried about Julia types.
The second method does something but seems to wrap what it does in SOME unnecessarily. “Parser” is a struct that contains a character buffer for an IO stream (which is also a member of the struct). Putting a type declaration as the first argument of the method is a usage I’ve never seen before. The author seems to want to make sure this method is only called when the second argument isa Parser type with a member that can be converted to String. But, the first thing is unnecessary as the type specifier of the second argument would do the job. Further the ::Type{String} is unnecessary as SOME(::String, p::Parser) ensures that the first and unnamed argument has type String (any string does as well as the type itself).
Wrapping the thing in SOME seems again, unnecessary. The only 2 instances it is called the purpose is to return the contents of the char buffer as a string, which String(take!(p.charbuffer)) does perfectly well. I am not even sure the charbuffer is needed as it is readonly and you can set a counter to an IO Stream and read at a specific position of the IOStream.
Unless I am missing something, which I often do, SOME and its two methods result from a misunderstanding about types in Julia. I am creating a pull request to clean up this package.
My plan is 1) to get rid of the first SOME method and every instance of calling it; and 2) to replace the second SOME with the obvious string conversion, which is simply the function body in the 2 places it is needed.
Obviously, I’ll find out if I’m off when I try it, but can you see any reasons to keep these SOME methods?
The code in question (which I recognise in TOML.jl in Pkg) was upgraded (in a frenzy I might add) from 0.6 where it used Nullables. I did it as mechanically as I could just to get it working so I could move on and keep working on the package manager.
Some refactoring of the code would likely improve it.
I was referring to the external package, not the version in Pkg—which fixes some bugs and addresses 1.x compatibility. It is very hard to understand someone else’s code. If it mostly works and there are only a few things
to do to get it working, that is what anyone would do. I looked at a few other parsers, especially the Python implementations. They are very hard for me to understand even in entirely idiomatic Python. Please take no offense. I was asking if I am interpreting
a couple of these methods correctly—I could be missing something. But, a few little tests on the side suggest that the code could be simplified a bit. I will still be far from understanding it all unless I spend more time than I intended to. I meant no
offense, which is why I isolated the examples from the rest to be sure if my hunches were right. I am going to fork it and simplify a bit and I’ll find out.
Thanks for responding. You did right. TOML is much easier to hand author and programmatically parse than most other alternative formats. Pkg just needs basic TOML to work—and it mostly does. Job done with the right priorities.
I found a bug, posted and saw Karpinski’s and others’ discussion of it. Before putting TOML in the repository they wanted some more work done, which was not high on anyone’s priority list—appropriately. I’ll try to move it a little further along, for what
it’s worth. You are a better man than I for all that you’ve contributed.
Singleton types are in fact very common in Julia code, and help you dispatch on types as values.
This is not equivalent at all. The first would match
SOME(String, ...)
the second
SOME("abc", ...)
Furthermore, it is quite difficult to say whether something is necessary without any context. Simple interface functions are frequently one-lines, but are useful to convey intent and define an extensible API.
You are right about the difference. I did some experiments. In the way this function is used, just qualifying the data argument would have been sufficient. There is no added safety, in the sense of preventing incorrectly typed arguments.