search

Shopify / schmooze

Schmooze lets Ruby and Node.js work together intimately.
MIT License
72 stars 16 forks source link

Reduce Schmooze instance variable footprint #5

Closed rmacklin closed 8 years ago

rmacklin commented 8 years ago

The change made in https://github.com/Shopify/schmooze/commit/c4a5d35239b9214ff0f4a10471959c17dabb0d02 is a great improvement, but I noticed that we could pretty easily reduce the number of instance variables reserved by Schmooze::Base to just two (one at the class level and one at the instance level) if we scope everything inside two internal objects. That's what I set out to do in commits https://github.com/Shopify/schmooze/compare/960d26f9cbffa3c6860758d8017aac272fc3c670~...4035e45ac8bd50f4469f7b8ee6029871fd16bfe2. After doing that, the structure of the code revealed that some of the existing methods were a better fit in the new Bridge class, so I pushed them down (commits https://github.com/Shopify/schmooze/compare/686b0142652f4b62f6c41c5579f3d23aa61f486d~...9506e67e19f1d9d37d8a8a08e355d967f977ffb7).

Let me know what you think of this and thanks for creating this gem.

rmacklin commented 8 years ago

cc @bouk

bouk commented 8 years ago

While I appreciate the effort, I don't think there's really any benefit to the added complexity this PR brings, so I'm going to close it.

Are you using Schmooze for anything?