jackfirth / resyntax

A Racket refactoring engine
Apache License 2.0
56 stars 10 forks source link

Suggest using `dict-ref` instead of `hash-ref`/`assoc` etc. #165

Closed dannypsnl closed 1 year ago

dannypsnl commented 1 year ago

As the title described, dict-ref can handle more kinds of associated data structures, using it as default can relax the code for future changes, should be a good idea?

jackfirth commented 1 year ago

If the code is already using hash-ref, then making it use dict-ref doesn't change anything (except make it a little slower) so users won't see much reason to accept the suggestion. I think this could make more sense in cases where a dict is converted to a hash just so someone can use hash-ref, but I've never seen code like that in the wild. Have you got a link to some code that would benefit from this suggestion?

dannypsnl commented 1 year ago

The situation I have is quite specific. The outer scope use hash to collect bindings, but when type checking local terms, it would be nice if it didn’t try to modify the parent scope. Hence, convert it to list by hash->list is a nice choice. But if the lookup function would like to behavior normally for both kind of environment context, use dict-ref is more proper choice, performance here don’t get huge impact.

But this maybe not general enough to get performance issue everywhere then?

jackfirth commented 1 year ago

That sounds like a problem that's outside of Resyntax's domain You might be interested in the ongoing effort in Rhombus to provide better collection APIs. I'm working on a generic collections framework that would provide a way to wrap collections with read-only views, which sounds like it fits your use case.