initial POC for 'as' API addition

[nsauro]

I had a longer than expected plane ride the other day, and decided to take a stab at #8 .

This PR is a more of a POC than an actual formal PR, to generate some discussion.

Codewise, there is not much to this, however the larger discussion is had the use case for this feature. The issue is that 99% of the (default) use cases don't need to read in multiple values from a Config object(and thus a path parameter makes sense), since its one field per value usually. However, case classes are the very large exception to that rule AND also one of this libraries greater conveniences, so there is a bit of a weird relationship going on there. They are a bit at odds with each other.

That being said, I still decided to go ahead with this POC, just to show what a potential fix could look like:

I added a new function to ValueReader

def read(config : Config) : A 

In an effort to not break compatibility, I opted to give it a default implementation of..throwing 🥇 🤕 🤒 . (which I totally do not like, but there really is no good default for this).

The ArbitraryTypeReader macro was updated to give an implementation to this new function, which allows it to take a Config object and directly pull values from it, without the need for the optional path parameter.

I updated some tests to make sure it all works, which so far seem to be a go.

So...outstanding issues:

• I don't like the default throwing behavior at all, it was plane food. One other option would be to have a separate trait which houses this new function definition, and then have the ArbitraryTypeReaders macro generate an impl for that. Then, the as function on the FicusConfig instance could match on the ValueReader type and throw if the ValueReader is not of that type. Not a 100% better, but at least a bit more obvious and not a throwing mess.
• should there be an equivalent getAs function? If so, should the catching behavior catch the above mismatch, or is that something that should still be thrown? I lean towards having it throw if the ValueReader is not of the appropriate type, as this could mean a programming error, not a configuration error(potentially). I feel like either side could be argued successfully.

[coveralls]

Coverage increased (+0.2%) to 91.367% when pulling 3bc0174 on nsauro:as_fix into 05e6d47 on iheartradio:master.

[nsauro]

I just reread:
#8
And the last comment:

update: Thinking about this again, we can add another trait like value reader that has a method read(config: Config), and get that trait implicitly (instead of ValueReader[A]) in config.as[X]

is in the same vein of what i was thinking.

[kailuowang]

The difficulty of deciding what to do here is an indicator that it doesn't belong to this trait. A new trait (possibly inheriting this one) makes sense.

[coveralls]

Coverage decreased (-0.5%) to 90.714% when pulling f70d04a on nsauro:as_fix into 05e6d47 on iheartradio:master.

[nsauro]

So, I updated this to use a separate trait. It's better than the first commit, for sure, but I'm still not super happy because:

• the name CompositeValueReader isn't great, so I'm open to suggestions :)
• This is a workaround because I don't want to introduce a major breaking change to the existing API(as I stated in my initial PR comments)
• There is a weird relationship between ArbitraryTypeReader, ValueReader and CompositeValueReader. ArbitraryTypeReader now generates 2 readers, one CompositeValueReader (for the new use case/API) and one ValueReader(for backwards compat, and for handling nested case classes, etc), They are both pretty much equivalent, sans that one takes a path parameter, and one doesn't..which I guess is just a side effect of not wanting to change the API.
• I actually tried to have CompositeValueReader extend ValueReader but that broke all sorts of tests b/c of ambiguous implicits being found all over. I also tried having ArbitraryTypeReader mixin CompositeValueReader with ValueReader, but to the same effect. I could try again and paste in those errors if anyone is curious. I could have done it wrong also :).

Anyways I think this current impl is what was suggested in those previous comments I read.

[kailuowang]

let's close this for now. we can reopen if anyone is interested in restart the work