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

Document preferred naming convention for getters/setters in style guide #16770

Open
nalimilan opened this issue Jun 5, 2016 · 25 comments
Open
Labels
docs This change adds or pertains to documentation

Comments

@nalimilan
Copy link
Member

I think it would be useful to add a recommendation regarding the naming of getters and setters to the style guide here: http://docs.julialang.org/en/latest/manual/style-guide/#use-naming-conventions-consistent-with-julia-s-base

For getters it looks like the rule is pretty clear: just use the name of the field, e.g. names(x). For setters, I'm not sure we're clear on whether names!(x) or setnames!(x) is preferred. Cf. this thread. Could we agree on a convention? If that's still undecided, probably better make a quick survey to choose the rule most commonly used in packages.

@nalimilan nalimilan added the docs This change adds or pertains to documentation label Jun 5, 2016
@bicycle1885
Copy link
Member

In Bio.jl, we use name!(obj, newname) rather than setname!(obj, newname) (https://github.com/BioJulia/Contributing/blob/master/CONTRIBUTING.md). I don't remember why, but it seems to be accepted by members in Bio.jl.

@JeffreySarnoff
Copy link
Contributor

As I recall, using fieldname(x) for the getter and fieldname!(x, newfieldvalue) for the setter were preferred for consistent simplicity once the decision to use change! to mean change-in-place was made.

@nalimilan
Copy link
Member Author

That's fine with me, that's also the convention used in DataArrays and DataFrames. Though I just noticed NamedArrays follow the set* convention.

@davidavdav Would you be OK with switching to the fieldname! rule?

@toivoh
Copy link
Contributor

toivoh commented Jun 10, 2016 via email

@nalimilan
Copy link
Member Author

Though I agree name conflicts are often annoying, I'm afraid the getters naming convention is too established to be changed at this point.

@davidavdav
Copy link
Contributor

So then we have fieldnames(object) that shows the member variables in a type, and fieldname(object) to show names of internal structures of the type? Is that not a little bit confusing? (Especially for native speakers of languages that don't have plural forms)

As an unrelated topic, is this a moment to consider lobbying for a syntax like df$field meaning something like df[:field] again? $ for XOR is IMHO a waste of ascii operator space #1974.

@nalimilan
Copy link
Member Author

So then we have fieldnames(object) that shows the member variables in a type, and fieldname(object) to show names of internal structures of the type? Is that not a little bit confusing? (Especially for native speakers of languages that don't have plural forms)

No, the plural is not the question here. The debate is setX!() vs. X!(), with X the name of the field (which may not even exist in the underlying type). fieldnames was just an example of a name.

As an unrelated topic, is this a moment to consider lobbying for a syntax like df$field meaning something like df[:field] again? $ for XOR is IMHO a waste of ascii operator space #1974.

As you say, that's quite unrelated, so I'd better keep this debate separate. I have no idea when is the best time for this, though.

@davidavdav
Copy link
Contributor

Oh, all right then. The question is then to change setnames!(::NamedArray ,...) into names!(::NamedArray, ...) . I don't have a problem with that, specifically because we can just define both for a while.

In the particular case of fieldname== names, where should we import names from? It is currently a deprecated method from Base, and names! does not exist there. Well, this is another unrelated subject I suppose.

@nalimilan
Copy link
Member Author

OK, cool. Only some methods for names are deprecated, so it's OK to extend it. names! doesn't exist in Base unfortunately, which means it's doomed to create conflicts between packages until we find a home for it.

@davidavdav
Copy link
Contributor

yes, it would be nice if there is a general solution for these cases. Similar problems occur with ambiguities, when two packages extend a method from base in different ways.

@simonbyrne
Copy link
Contributor

simonbyrne commented Sep 30, 2016

Personally I prefer the setXXX! convention, as it describes exactly what the function does. Also it makes it easier to find the correct setter via tab-completion.

e.g. for the linked issue, it was completely unclear to me what ordered! would do: my initial guess was that it was something like sort!.

@simonbyrne
Copy link
Contributor

simonbyrne commented Sep 30, 2016

Another argument is that the unofficial convention in Base seems to be if both XXX and XXX! exist, they both do pretty much the same thing except the latter can mutate its arguments, or accept a pre-allocated object e.g. sort/sort!, quantile/quantile!, cholfact/cholfact!. This would not be the case with getter/setters, as they have different fundamental purposes.

@davidavdav
Copy link
Contributor

I am happy enough with the set...!() version for setters. Indeed it shows a clear difference from the sort() sort!() pairing.

@JeffreySarnoff
Copy link
Contributor

While the earlier patterns of use were as I mentioned

get: ( )
set: !( )
ask: ?( ( ) )

or

get_( )
set_( )
ask_( , )

When paired wirh the getless form of field retrieval, there is some pragmatic utility to the setfull form of field reassignment:
It is visually too easy to gloss over the !()
and misread the in place set as the get.

All considered, use of get___ with set___ for individual fields of a type (and for implicit/computed/composite entities that inform or enhance a type) is more sanity-preserving and less inviting of incorrectness.

If set___ stays, I am using get___ as this keeps coding with Julia nearer the immediacy of expressing design.

@bicycle1885
Copy link
Member

Honestly speaking, I think setter and getter interfaces should be achieved by overloading object.attribute. The discussion happens here: #1974. If this really happens before the next Julia release (i.e. Julia 0.6 or Julia 1.0), I prefer it to any options discussed above.

@nalimilan
Copy link
Member Author

Please keep this issue focused on the naming convention. It's kind of orthogonal whether we recommend using field overloading later: right now there are many setters and getters out in the wild, and we need a convention for them.

@StefanKarpinski
Copy link
Member

Indeed, having a consistent convention will make it much easier to transition to field overloading later.

@nalimilan
Copy link
Member Author

@StefanKarpinski I think you'll have to make the decision if you want this thread to give a result before 1.0. ;-)

@diegozea
Copy link
Contributor

diegozea commented Nov 9, 2016

Sometimes, using get... and set... helps to avoid name collisions:

julia> using DataFrames

julia> data = DataFrame(A=[1,2], B=[3,4])
2×2 DataFrames.DataFrame
│ Row │ A │ B │
├─────┼───┼───┤
│ 113 │
│ 224 │

julia> names(data)
2-element Array{Symbol,1}:
 :A
 :B

julia> names = ["Julia", "R", "Python"]
WARNING: imported binding for names overwritten in module Main
3-element Array{String,1}:
 "Julia" 
 "R"     
 "Python"

julia> names(data)
ERROR: MethodError: objects of type Array{String,1} are not callable
Use square brackets [] for indexing an Array.

@davidavdav
Copy link
Contributor

Yes, sometimes I wish I would be possible to have variables and functions under the same name, but this can't happen because the value of a variable can be a function. (And it might be confusing to read the code). With the specific names() example I am struggling in NamedArrays constructors where I tend to want to call the variable containing the names names, but I sometimes also need acces to the function names()---although this could happen via NamedArrays.names() I suppose.

@bicycle1885
Copy link
Member

Name collision between local variables and functions can be avoided by prefixing the module name of function; you can always use Base.names to mean the names function exported from Base unless you assign Base to something else.

@bicycle1885
Copy link
Member

I'm inclined to think setxxx! is a better convention than xxx! as @simonbyrne pointed above (#16770 (comment)). But eventually we will need some standard getter/setter system in the language because these names easily conflict with names from other packages.

@martinholters
Copy link
Member

There is a point to be made for function names being (based on) verbs, which would indicate to prefer setxxx!. Function names being (based on) a noun usually imply a "get" or "compute", e.g. mean, so it is less of an argument in favor of getxxx instead of just xxx.

Also, without the set, there is some semantic ambiguity: Does xxx!(y,z) set the xxx property of y to z or does it store the xxx property of z in y?

@diegozea
Copy link
Contributor

object.fieldname is the nicer than fieldname(object) or get_fieldname(object) . Maybe have object.fieldname (or object$fieldname) being a call to getpublicfield and object..fieldname being the actual getfield could be a good option. In that way, types should define getpublicfield instead of getters...

@KristofferC
Copy link
Member

That discussion is likely better to have in #1974.

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
Projects
None yet
Development

No branches or pull requests

10 participants