Closed lbstr closed 3 years ago
If we want to move forward this idea, we should document this in the README. I just want to get some feedback first before we do that.
This looks good to me. I think this makes this package a little more robust. The small memory leak shouldn't be an issue, but like you said, after some usage it'll be easier to measure if it has any impact.
I like this way of doing it.
OK I added some info in the README. Let me know if you would like to see any changes here.
At what point do we feel a merge into master is ready? I'm going to test the current dev changes tomorrow on paper. Is there much more we want to add before a 2.0 release?
@117 My hunch is next week we should be good. There's a little more to do on the number parsing and I have a few other things. I'll get all of my notes into Issues so we can discuss which ones we want for 2.0. I think they are all quick things.
This is a proof of concept of @KalebMills 's comment here. Let me know what you think!
Summary
We return parsed entities, but they include a method,
raw()
, which you can call to get the raw data.Pros
raw()
is a method, not a field, so we aren't cluttering the data if you want to shove it in a data storeCons
raw()
in our parser methods, we create a closure. We ruin garbage collection, so we technically have a small memory leak here.Example