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

Struct Public Members #4

Closed
jeremylt opened this issue Mar 24, 2022 · 2 comments
Closed

Struct Public Members #4

jeremylt opened this issue Mar 24, 2022 · 2 comments

Comments

@jeremylt
Copy link

It seems like many structs have pub fields that are not used in the examples. Do you intent for users to have access to all of this data, or could some of it be made pub(crate)?

For example, the BasisFn struct exposes

t: Raw transformation matrices at each sample point. Describes transformation from real space to sampled parametric space.

The word 'raw' makes me think this is perhaps something that users should not have access to. I try to make as many fields as possible pub(crate) over pub so I have freedom in future refactors. Anything declared pub that is changed may result in more invasive changes than intended in user code.

I recommend double checking which struct fields need to be pub and which can be made pub(crate).

@jeremiah-corrado
Copy link
Owner

Thank you for the suggestion! I agree that the names of the fields in BasisFn were pretty undescriptive. The naming and documentation for that structure have been updated now. In that case, the "raw" simply meant "not inverted". Those fields are now called "jac_inv" and "jac".

In many cases the public fields provide information that I think could be useful for developing code around the Crate. For example, I have used the h_levels field in Elem to develop some refinement algorithms that set the polynomial expansion orders on an Elem to be a function of its h-refinement levels.

To make a more general comment on the high degree of publicness: as I described in the updated version of the paper, this package is really intended to be used in a research context, so generally speaking, I erred on the side of over-exposing internal data rather than under-exposing.

I will also note that much of the library's documentation has been updated recently, so hopefully other confusing public fields that you may have noticed also received improved documentation, making them more usable. Please let me know if you see other specific cases where a public field is poorly described or seems like it should definitely be private.

@jeremylt
Copy link
Author

The documentation updates look good to me.

I've seen two schools of thought on exposing strut members. On one hand, exposing the strut members makes them part of your API and any changes to your internal design break other people's code that uses your crate. On the other hand, writing all the accessors to all the data you want to expose can be tedious. As long as it's an intentional design decision to expose the internals and make those strut members part of the API, it can work fine.

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

No branches or pull requests

2 participants