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

WIP - Document privacy of type fields #12083

Closed
wants to merge 3 commits into from

Conversation

ssfrr
Copy link
Contributor

@ssfrr ssfrr commented Jul 9, 2015

This is a first cut documenting Julian style regarding direct field access.

I'm not sure we have a consensus on this, but my hope is that if we can nail
down the exceptions to these guidelines then we'll come to some understanding.

When merged this should close #12064 and #7561.

@ScottPJones
Copy link
Contributor

Looks very good to me 👍

@nalimilan
Copy link
Member

+1

@jdlangs
Copy link
Contributor

jdlangs commented Jul 9, 2015

Should people be advised to use the python-style underscore prefix for non-public fields? Has that been decided to be proper style?

@ssfrr
Copy link
Contributor Author

ssfrr commented Jul 9, 2015

I'm not sure how to square "use underscores to mark private fields" with "fields are assumed to be private". They don't seem to jive together because the latter implies that fields without underscores are part of the interface.

I suppose in the case where it doesn't make sense to follow this guideline and you have a mix of public and private fields then you should mark the private ones with an underscore.

@jdlangs
Copy link
Contributor

jdlangs commented Jul 9, 2015

Yes, that's tough because there's definitely a lot of code that encourages interfacing through direct field access. The best example might be Expr, where .head and .args appear prominently in the manual.

At any rate, leading underscores for fieldnames are currently not very prevalent right now, correct? No need to bring it up until a more formal system for handling mixed public/private fields is introduced then.

@kshyatt kshyatt added docs This change adds or pertains to documentation types and dispatch Types, subtyping and method dispatch labels Jul 9, 2015
- Package developers are free to change the implementation without breaking
user code
- Methods can be passed to higher-order constructs like `map` (e.g.
`map(length, items))` rather than `[i.length for i in items]`)
Copy link
Member

Choose a reason for hiding this comment

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

this feels like a strawman example, and the takeaway (for me) is more "don't use fields for things which could be methods".... personally I'm not sure I agree that direct field access should be discouraged in general.

BUT a better example is for complex numbers: use imag(z) over z.im. The point is they lower to the same for Complex, but you could use imag on a complex number say in polar form.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, that's a better example. Thanks, I'll update the PR.

@mauro3
Copy link
Contributor

mauro3 commented Jul 10, 2015

An exception to the privacy of fields should be for storage-only types, say:

type Phys
    g::Float64 # acc. due to gravity
    L::Float64 # latent heat of fusion
    ...
end

I think in this case Phys does not represent some entity itself, it is merely a collection of values.

@ssfrr
Copy link
Contributor Author

ssfrr commented Jul 10, 2015

@hayd I think your takeaway is more or less what I'm trying to get across. Basically that when in doubt, prefer methods over direct field access for the reasons listed. There are times when it's the right move to encourage your users to access your type's fields directly, but people should think twice and be aware of the trade-offs. In those cases a package's documentation would communicate the expectation. In the cases where the documentation doesn't show any direct field access users should assume that they're risking future breakage by accessing the fields in their code.

@StefanKarpinski
Copy link
Member

I don't really want to start using underscores for indicating privacy. I'm
sure we can come up with something better than that. I just don't think
that marking some fields as private in the C++ style is the right way to
go. I also think it's too early to make this sort of design decision. We
need to see how the precompilation and interfaces/protocols stuff plays out
first and then we'll be in a position to understand what the privacy model
in the language should be.

@tknopp
Copy link
Contributor

tknopp commented Jul 10, 2015

@mauro3: These "storage only" could actually be what usually immutables are used for. For that reason be vague feeling is that immutables are an exception here.

I fully agree with Stefan that here is no urgent need to make any sort of decision. Interfaces are the much more important point.

@StefanKarpinski
Copy link
Member

It would be a bad situation if we rushed into a privacy model only to find that it conflicts with how we end up doing interfaces or separate compilation (which may end up being desirable for precompilation).

@ssfrr
Copy link
Contributor Author

ssfrr commented Jul 11, 2015

My goal here is that the idea seems to pop up from time to time that "proper Julian style" says to avoid direct field access, but it hasn't been documented anywhere. My hope with this PR is to document current community standards in the spirit of an FAQ answer, not so much to re-define a privacy model and certainly not to advocate for language features. (just to be clear).

My other hope was that having something concrete to decide on would help direct the various discussions around this topic into something useful.

Thoughts on whether this seems to capture what we're going for? Maybe with some softening as per my earlier comment?

@mauro3
Copy link
Contributor

mauro3 commented Jul 11, 2015

This recent discussion following comment #12064 (comment) would suggest guidelines like:

  • generally fields are considered private and access should be through methods only
  • however, direct field access is preferred over trivial getter and setter methods
  • unless there is an established function for it

Example:

type MyT
    prop1
    len
end
# no getter/setter for prop1
# getter for len:
Base.length(a::MyType) = a.len

@nalimilan
Copy link
Member

@mauro3 Well, I'd say it's a bit more complex than that. Adding a trivial getter should be encouraged if it can allow for more generic programming, i.e. the user wouldn't need to rely on the internals of the type anyway to perform an operation. That's why length or step were added.

"unless there is an established function for it" doesn't sound like a sufficient rule to me: why were these added in the first place? I argue that's because they suit the criterion I highlighted above.

@malmaud
Copy link
Contributor

malmaud commented Jul 12, 2015

I'm sympathetic to having trivial getters, especially since they could just
generated with a simple macro, but it could make name collisions amongst
packages a lot more common; if packages A and B both define a type with a
field named f, then using A; using B becomes less convenient.

On Sun, Jul 12, 2015 at 9:11 AM, Milan Bouchet-Valat <
notifications@github.com> wrote:

@mauro3 https://github.com/mauro3 Well, I'd say it's a bit more complex
than that. Adding a trivial getter should be encouraged if it can allow for
more generic programming, i.e. the user wouldn't need to rely on the
internals of the type anyway to perform an operation. That's why length
or step were added.

"unless there is an established function for it" doesn't sound like a
sufficient rule to me: why were these added in the first place? I argue
that's because they suit the criterion I highlighted above.


Reply to this email directly or view it on GitHub
#12083 (comment).

@mauro3
Copy link
Contributor

mauro3 commented Jul 12, 2015

The reason length and step are established is because they feature with many/several types. Once that is the case you'd add a getter/setter. But you wouldn't add one for a field which features just in one type.

@nalimilan
Copy link
Member

Yes, of course, except if it might make sense at some point for another type to offer the same interface to be able to replace the other one. That probably depends on the context.

@tknopp
Copy link
Contributor

tknopp commented Jul 12, 2015

It should also be mentioned that trivial getters are of course not(!) to be generated for fields that are not public.

When designing a type one has to think at some point about the public interface. This is the point where one will do getters/setters for those fields that should be exposed. Doing the getter will be the least work. Much more important is to:

  • add the function to the export list
  • add documentation!

If one uses public fields one cannot do the export and documentation is (with the new documentation system) also not possible. One issue for me would be further that a user needs to read the type definition (i.e. the place where all my private/dirty things happen). If we look at the sparse matrix type one then has to discuss why n and m are private but colptr is public.

For me this is all much to complicated and I therefore believe that the fields are private rule could help distinguishing between interface and implementation.

@ScottPJones
Copy link
Contributor

@tknopp I wonder, with Julia's very good in-lining capabilities, if using trivial getters/setters for public fields would generate exactly the same code as if the fields were accessed directly?
I think you've made very good points about documentation also.

@tknopp
Copy link
Contributor

tknopp commented Jul 13, 2015

@ScottPJones: Yes, this is an area where Julia shines. There will be no computational cost.

You may have observed that the language is still to much in flux in this area. If as indicated here #12064 (comment) field overloading is set in stone (#1974), please forget all what I have said about fields are private. It will have major influence on the interface topic.

@vtjnash
Copy link
Member

vtjnash commented Jun 26, 2019

I think this is an OK statement of the status quo for the typical type. Perhaps needs some update to clarify that it is acceptable for types (AbstractDataFrame, HTTP.Response, NamedTuple, PyCall, Regex, etc.) to define their API in terms of . access? And needs a rebase (the file was renamed).

@ViralBShah
Copy link
Member

Do we want to get this in? And would it close the 2 old issues in the OP?

@tknopp
Copy link
Contributor

tknopp commented May 2, 2020

What would be interesting how much the getproperty introduction has influenced common practice for interface designs. So has that been massively adopted or is it used more for special cases.

@vtjnash
Copy link
Member

vtjnash commented Apr 19, 2021

Replaced by #40533

@vtjnash vtjnash closed this Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document privacy of type-fields