Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExpressionValueCollector is too lazy #5

Closed
jimwhite opened this issue Feb 23, 2014 · 2 comments
Closed

ExpressionValueCollector is too lazy #5

jimwhite opened this issue Feb 23, 2014 · 2 comments

Comments

@jimwhite
Copy link
Contributor

ExpressionValueCollector is lazy in how it converts values to strings. Therefore if a value is mutable then it may change before it gets converted to a string to be sent to LT.

@rundis
Copy link
Owner

rundis commented Feb 23, 2014

Fixed by #6

Maybe a better solution would be to change the collector to be an emitter and let the observer(s) of the emitter to deal with stringifying.

This is another case of mutability making solutions more complex to reason about (:

@rundis rundis closed this as completed Feb 23, 2014
@jimwhite
Copy link
Contributor Author

Yes, the transform & collector needs some adjustment, especially because we would like it to work for classes declared within the script too. One way to do that would be to make ExpressionValueCollector an observer interface which gets set on a ThreadLocal which is given as an argument to ScriptTransform. That's kind of what I had in mind to start with but needed to see how the plugin works for the right path to be clearer.

Not precisely sure of the how the variable will get to the transformed code, but injecting a synthetic field is one way. The tradeoff in using ThreadLocal is that it adds overhead but keeps ordering correct in the event folks spawn threads. OTOH, maybe it is a good to leave that indeterminacy in.

Actually I think the only thing I would change at the moment (until the class-within-script stuff gets done) is change ExpressionValueCollector to have a CollectedValue class instead of using maps and to possibly use a larger-than-default size for the ArrayList (the default is 10 - something like 100 would avoid a few expansions). I think the ExpressionValueCollector is a sufficient abstraction since it is the observer and can do whatever it likes. Being in Java rather than Groovy is good here since we like the speed. Probably should use something smarter than Object.toString though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants