-
Notifications
You must be signed in to change notification settings - Fork 0
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
Shrink MappingTable interface #4
Comments
It looks like the interface I have settled on is:
|
The current implementation for If we really wanted a reduced interface, I can't see a good way to do this other than S4, and I think that's probably not worth it. |
Ah that's a shame. I don't think it is a big issue as there are plenty of packages building objects in S3 where everything is just kept in a big list wrapped in an S3 class and the user just ignores the elements they don't need 😄 An alternative could be to create the 'public' options as intended, and create one last public called |
The interface for mapping tables exposes a lot of implementation details, and I don't want to support them publicly.
Public
initialize()
definitely needs to be publicprint()
must be publiccount_aggregate()
must be public, because that's the functionality we want!preprocess()
is likely only to be used bycount_aggregate()
, so I am leaning towards making this private.Active bindings
map
is only important forcount_aggregate()
. It might need to be extended when implementingdbplyr
backendsmtab
should be available to create an output table skeleton (and allow sorting sonullspace
is in the right order).matrix
is to getnullspace
table_cols
will already be available frommtab
('s replacement)join_clause
is an implementation detail (and I want to change it back to a character type for dtplyr compatibility), so can be made privatenullspace
is a major part of what we need for cell suppression (the only other part being what we want to suppress)Interestingly, private superclass methods are available to subclasses as
private$method()
orsuper$method()
, but private fields are not.The text was updated successfully, but these errors were encountered: