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

allocate -> Class #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

danielpclark
Copy link
Contributor

@danielpclark danielpclark commented Feb 20, 2018

new_instance on class produces an AnyObject which is okay but not preferred. As this is already baked in and depended upon I'd recommend changing that at 1.0 to Class. (Or if it really goes the direction I would like it would be Result<Class, AnyException>).

I've chosen to implement the allocate method to produce the Class type as output as it's a convenience method. If it were just AnyObject one could simply use send("allocate", None) and get what they want. But with AnyObject the worst case code would look like

Class::from_existing("String").allocate().try_convert_to::<Class>()?

Since send("allocate", None) is simple enough for anyone who really wants to protect their specific scenario then it becomes a simple choice for the to choose that or allocate() -> Class.

These are my thought on it. A convenience method which wouldn't make sense to implement as AnyObject as the convenience of send is already there.

new_instance goes through both Class.new and Class.initialize which both have been known to be re-written by software developers. Class.allocate can be overwritten but I believe that's unheard of as it's sole purpose is to give you a raw instance of the class and not do anything else… so no exceptions are to be raised. Since allocate is a raw instance of a class by definition I believe this form of implementation for allocate in ruru just makes sense.

Resolves #83

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

Successfully merging this pull request may close these issues.

1 participant