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

Placeholder accessors can be default constructed #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Ruyk
Copy link
Contributor

@Ruyk Ruyk commented Jul 10, 2017

  • Placeholder accessor constructor can optionally not take memobj

  • Handler can specify the buffer later on

* Placeholder accessor constructor can optionally not take memobj

* Handler can specify the buffer later on
@Ruyk Ruyk self-assigned this Jul 10, 2017
@Ruyk Ruyk requested a review from AerialMantis July 10, 2017 09:50
Copy link
Contributor

@AerialMantis AerialMantis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal looks good, though I think this would benefit from having a member function for querying whether a placeholder accessor requires a buffer to be provided to require(), for example accessor::has_buffer().

placeholder_accessors/index.md Show resolved Hide resolved
Copy link
Contributor

@AerialMantis AerialMantis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some additional comments.

The accessor API features constructors that don't take the handler parameter
and/or memory object as a constructor. Accessors can then be default
constructed, and the memory object can be assigned later when registering
the accessor in the command group.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should describe the additional constructors which need to be added to allow deferred initialization of the memory object and handler.

@@ -192,6 +195,12 @@ The handler gains a new method,
that registers the requirements of the placeholder accessor on the given
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wish to support the option for constructing a regular accessor from a placeholder accessor (i.e. get_access) then this interface should also be extended to allow you to specify a memory object. We should also specify that an exception should be thrown if get_access is called without providing a memory object on a placeholder accessor which doesn't have a memory object already.

On a side note, perhaps we should consider changing the naming of this member function, get_access could be confused with the regular get_access member functions.

#21)

* Providing explicit documentation for throwing an exception when no buffer exists.

* Removed implementation details and provided interface specification only.

* Applied fixes to Gordon's review

* Changed has_buffer to be a const function

* Changed wording for the placeholder accessor buffer
@ProGTX
Copy link
Contributor

ProGTX commented Apr 17, 2020

I believe #124 offers a suitable alternative to this.

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.

4 participants