OpenStruct Pull Request [WIP]
Hello Ruby Core,
In this Pull Request I have made a series of changes that I would like to be added to the Ruby stdlib library ostruct.
Before getting into anything I want to stress one thing: All changes are mostly backwards compatible with the previous ostruct library. Any non-compatible changes are what I consider semi-bug fixes.
First a list of problems with the current OpenStruct class:
-
OpenStruct
is a class, but often is only used to give the flexible behavior to children classes. TheOpenStruct
behavior should be mixed into other classes as a Module. However there are still use cases of just wanting a plainOpenStruct
object. -
OpenStruct
behaves similarly to aHash
.OpenStruct
can be built from, updated with, and dump aHash
. It even has a similar method toHash#delete()
, theOpenStruct#delete_field()
method. Yet theOpenStruct
is missing a few key methods likemerge
ormerge!
. These methods would work exactly like the (already defined)OpenStruct#marshal_load()
. I believe thatOpenStruct
would benefit from aliases that make it behave likeHash
and abide by the Principle Of Least Surprise. - All of the methods on the
OpenStruct
are open to easy overwriting. In addition some of the methods use other methods that are likely to be overwritten like the call toOpenStruct#object_id
in theOpenStruct#inspect
method. It also causes problems like this: https://gist.github.com/3460906
I started working on an alternative to OpenStruct after having some problems with it.
Instead of modifying OpenStruct it was simply easier to build my own version.
After a while I abandoned it until recently when I needed OpenStruct again.
I decided to finish my work, upgraded with what I've learned since I started.
I've done a few things in this pull request:
- I've fixed at least 3 bugs:
- Using the
OpenStruct#delete_field
method wouldn't actually remove the method given, only the table key/value. This creates a confusing mismatch in the object and the table. - When using
OpenStruct#marshal_load
it would correctly create getter and setter methods for each key/value. However it would also delete the entire table. - One of the tests freezes an
OpenStruct
object and then redefinesOpenStruct#frozen?
and it works. Freezing should stop changing the object, in any way.
- Using the
- I've made a series of important improvements as well:
- To fulfill the requirements of the first problem I have with
OpenStruct
I have created a module nested in theOpenStruct
class calledM
. That module contains all of the behavior and can be included when needed. - I've designed that all the public methods are wrapped in double underscores in the same way that
Object#__send__
and many other important methods are named. In addition I have aliased many methods to their "underscore" forms (exOpenStruct#inspect
aliased toOpenStruct#__inspect__
) to avoid confusion. Finally I've aliased "underscore" methods to the important methods likeOpenStruct#__freeze__
toOpenStruct#freeze
. Because of the nature of aliasmethod you can now define a key/value "objectid" and still have access to the originalOpenStruct#object_id
withOpenStruct#__object_id__
. This avoids the bug listed in the gist: https://gist.github.com/3460906 - I have added the ability to only dump a specific set of keys with the
OpenStruct#__dump__
method.
- To fulfill the requirements of the first problem I have with
- Finally I have improved the tests for this library by either breaking them up into manageable pieces or refined them to read better.
There are now test for other features of
OpenStruct
as well as tests for the new features added in this pull request.
Finally with those things listed I only have the benchmarks to show.
I used two types of benchmarking, both Benchmark#bmbm
and the benchmark-ips gem's Benchmark#ips
.
Both show favorable results for my version of ostruct.
You will notice that my version is labeled AltStruct
, because it is also a gem
Here are links to the results: https://gist.github.com/3462357