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

Adding feature clarity of memory management #86

Closed
wivlaro opened this Issue Jan 23, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@wivlaro
Contributor

wivlaro commented Jan 23, 2015

Adding a non-inherited feature to an object requires doing this at the moment:

new MyFeature(*containingObject);

This looks like the application programmer has to memory-manage the new feature theirself. While it is possible (i.e. you can manually destruct the feature before object, but not before) and sometimes convenient, the API itself does not suggest that containingObject will destroy its features on destruction.

I would suggest adding a method T& AbstractObject::addFeature<T>(arg1,arg2,...argn) that calls constructor new T(*this, arg1, arg2... argn), which gives the impression of the object managing the feature, as it does.

As features always require an object on construction, it would be a logical addition.

@mosra mosra added feature and removed feature labels Jan 23, 2015

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Jan 23, 2015

Owner

👍 yup, good idea.

If I remember correctly, someone suggested doing similar thing also for object children, because (in terms of memory management) they are behaving the same -- T& Object::addChild<T>(args...). Can't find the original discussion, though.

I'll think that through in regards to planned "data-oriented" improvements to see if these two ideas can play well together. The ultimate goal is to get rid of naked pointers and news completely.

Owner

mosra commented Jan 23, 2015

👍 yup, good idea.

If I remember correctly, someone suggested doing similar thing also for object children, because (in terms of memory management) they are behaving the same -- T& Object::addChild<T>(args...). Can't find the original discussion, though.

I'll think that through in regards to planned "data-oriented" improvements to see if these two ideas can play well together. The ultimate goal is to get rid of naked pointers and news completely.

@LB--

This comment has been minimized.

Show comment
Hide comment
@LB--

LB-- Jan 23, 2015

Contributor

I second the suggestion 👍

Contributor

LB-- commented Jan 23, 2015

I second the suggestion 👍

@mosra mosra self-assigned this Feb 4, 2015

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Feb 4, 2015

Owner

One potential problem came across my mind:

  • The AbstractObject::addFeature<F>(args...) function would call new F{*this, args...} and thus assume that F's constructors always have AbstractObject& (or some derived type) as first argument.
  • Similarly, the Object::addChild<O>(args...) function would call new O{args..., this} and thus assume that O's constructors always have Object* (or some derived type) as last, possibly optional argument.

Although the above argument order is probably common practice (is it?), these functions would add constraints on feature and object constructors, which might be annoying in some cases.

Looking for inspiration into the STL, std::tuple has a special constructor taking std::allocator_arg_t tag as first argument to indicate that the user wants to specify also an allocator in addition to initializers for all the tuple elements.

Thoughts?

Owner

mosra commented Feb 4, 2015

One potential problem came across my mind:

  • The AbstractObject::addFeature<F>(args...) function would call new F{*this, args...} and thus assume that F's constructors always have AbstractObject& (or some derived type) as first argument.
  • Similarly, the Object::addChild<O>(args...) function would call new O{args..., this} and thus assume that O's constructors always have Object* (or some derived type) as last, possibly optional argument.

Although the above argument order is probably common practice (is it?), these functions would add constraints on feature and object constructors, which might be annoying in some cases.

Looking for inspiration into the STL, std::tuple has a special constructor taking std::allocator_arg_t tag as first argument to indicate that the user wants to specify also an allocator in addition to initializers for all the tuple elements.

Thoughts?

@wivlaro

This comment has been minimized.

Show comment
Hide comment
@wivlaro

wivlaro Feb 4, 2015

Contributor

Maybe it would be easier if the Feature's object was not required to be set in the constructor. I had thought that a good practise was to have the constructor should do as little as possible. But this would break the contract that a Feature always has an object.

As an example, if we ever added some kind of callbacks or hooks for when features are added/removed it would open up a world of pain whereby application code could be run (and throw exceptions from) constructors.

The alternative is requiring a sub-classed feature's constructor to conform to some kind of contract, or require an allocator interface.

I think I'd personally prefer to be able to attach Features to Objects later than construction, as with the Object parent->child relationship. It opens up the possibilities of: more flexible Feature pooling; effectively enabling/disabling features; remove logic from my own constructors.

Googling around for some precedent, maybe this would provide some inspiration:

https://github.com/alecthomas/entityx#creating-components

Contributor

wivlaro commented Feb 4, 2015

Maybe it would be easier if the Feature's object was not required to be set in the constructor. I had thought that a good practise was to have the constructor should do as little as possible. But this would break the contract that a Feature always has an object.

As an example, if we ever added some kind of callbacks or hooks for when features are added/removed it would open up a world of pain whereby application code could be run (and throw exceptions from) constructors.

The alternative is requiring a sub-classed feature's constructor to conform to some kind of contract, or require an allocator interface.

I think I'd personally prefer to be able to attach Features to Objects later than construction, as with the Object parent->child relationship. It opens up the possibilities of: more flexible Feature pooling; effectively enabling/disabling features; remove logic from my own constructors.

Googling around for some precedent, maybe this would provide some inspiration:

https://github.com/alecthomas/entityx#creating-components

@LB--

This comment has been minimized.

Show comment
Hide comment
@LB--

LB-- Feb 4, 2015

Contributor

and thus assume that F's constructors always have AbstractObject& (or some derived type) as first argument.

The classes are being designed to implement an interface. Even the compiler will let people know when they have done something wrong. If someone is making such a class they have already agreed to adhere to the design pattern.

Contributor

LB-- commented Feb 4, 2015

and thus assume that F's constructors always have AbstractObject& (or some derived type) as first argument.

The classes are being designed to implement an interface. Even the compiler will let people know when they have done something wrong. If someone is making such a class they have already agreed to adhere to the design pattern.

@mosra mosra added this to the Last pre-Vulkan release milestone Mar 22, 2015

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Apr 12, 2015

Owner

Done in 7fc3113. Sorry about everything taking so long.

Owner

mosra commented Apr 12, 2015

Done in 7fc3113. Sorry about everything taking so long.

@mosra mosra closed this Apr 12, 2015

@mosra mosra added this to the 2015.05 milestone Feb 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment