POC: de-duplicate code via Result setter methods#884
Conversation
|
Hm... So one of my eventual goals for the parser is having a representation of the result that retains a mapping from string to datetime to hopefully allow us to do things like cache the inference step, and to allow people to modify some of the parsing logic once they have the full mapping. I don't really have a problem with the deduplication part of it or the idea that we should probably have a cleaner representation of the current state of the parse while we're parsing things, but if this is intended to be a public interface and it's not consistent with that goal, I'm a bit afraid that we'll introduce a lot of churn by immediately switching to a different representation of the parse state. In any case, you have marked this as "POC", what do you want to happen with this PR? Do you want a thorough review of the interface? The implementation? Or is it just part of a gallery of possible approaches for when we finally get around to revamping the parser interface? |
This is a big part of what the
This closely matches the design I landed on downstream as being roughly optimal, so I'd be happy to see this merged as-is. But since historically class-structure/interface issues have been the ones we have been least on-the-same-page about, I labelled this Proof Of Concept in the hopes that there would be a few rounds of discussion before strong opinions were formed.
Can you clarify the latter part of this, i.e. "it's not consistent with that goal"? |
Two main goals here:
e.g. at the moment we have 3 places where we in principal should* be setting
res.ampm, but we are only actually doing so in one of them.* For some value of "should". I don't think we ever actually use it...
Removes a couple of TODO comments to check that attributes are not already set.