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

More guidelines on method names #36

Open
Tokazama opened this issue Oct 6, 2019 · 6 comments
Open

More guidelines on method names #36

Tokazama opened this issue Oct 6, 2019 · 6 comments

Comments

@Tokazama
Copy link

Tokazama commented Oct 6, 2019

Thanks for this wonderful guide! I wanted to ask if there could be slightly more clarification/modification concerning method names. Sep

It would be nice to have consistency with base when defining methods like isimmutable where readability isn't as much of an issue. This would be especially nice for cases where a package may have code that eventually moves to base or the standard library (however rare that may be).

Potential solutions:

  • Identify specific patterns that violate the underscore rule but are acceptable.
  • Some sort of word combination limit (like 2 letter words don't need to be followed with an underscore)
@oxinabox
Copy link
Member

oxinabox commented Oct 6, 2019

Base actually has a style-guidance (that may or may not be written down)
that if at all readable, exported functionnames are to be lowercase, no spaces.

This is acceptable for Base, because Base only does things that are easy to describe,
and thus rarely endup with names that are more than one word.
Here is the full list of names in Base that break that rule:

catch_backtrace
code_lowered
code_typed
count_ones
count_zeros
current_task
disable_sigint
escape_string
get_zero_subnormals
include_dependency
include_string
leading_ones
leading_zeros
pointer_from_objref
process_exited
process_running
promote_rule
promote_shape
promote_type
redirect_stderr
redirect_stdin
redirect_stdout
reenable_sigint
set_zero_subnormals
task_local_storage
time_ns
to_indices
trailing_ones
trailing_zeros
unescape_string
unsafe_copyto!
unsafe_load
unsafe_pointer_to_objref
unsafe_read
unsafe_store!
unsafe_string
unsafe_trunc
unsafe_wrap
unsafe_write

There are 40 of them.
Out of the 682 function names that Base exports.
vs all the (examples chosen by what i could tab complete)

hasfield    haskey       hasmethod    hasproperty
getfield             getkey
gethostname          getpid
getindex             getproperty
setenv               setprecision
setdiff             setfield!            setproperty!
setdiff!            setindex!            setrounding
splitdir    splitdrive  splitext    splitpath

Bases guidelines of this as I said are acceptable for a core library that only does obvious tasks, most of which are given by 1 word.
Though I think it could have benifited from actually following the always seperate with _,
rule,
look at issetequal istaskstarted isdispatchtuple issetgid istextmime.
Something to keep in mind for 2.0
(or for if someone ever wants to do a big round of adding a ton of silent deprecations)

For normal packages, should actually just alwaus follow the guideline of _s and do is_immutable.

This would be especially nice for cases where a package may have code that eventually moves to base or the standard library (however rare that may be).

This is so rare that it doesn't bare mention in the style-guide,
and falls under the thing at the start which is about "use sense, exceptions apply".

Adding rules that will apply to like <0.1% of all packages is just too much noise to signal for my liking.

@nickrobinson251
Copy link
Contributor

The recommendation here won't change, but is it worth adding another example to this section?

e.g.

# Yes:
is_green(g::Goblin) = ...

# No:
isgreen(g::Goblin) = ...
isGreen(g::Goblin) = ...

@Tokazama
Copy link
Author

Tokazama commented Oct 6, 2019

The integration of a feature into base is just an example (perhaps a poor one). I think it's also important that a user doesn't need to know if a get, set, etc function is from base or another package. However, if what you're saying is that this is a sort of push for this type of syntax to transition into base then it makes sense to start out with that inconsistency.

To be clear I'm not arguing that everything here be 100% consistent with base. I just think that user facing syntax that will strongly influence people who aren't developing packages needs to be consistent.

@oxinabox
Copy link
Member

The recommendation here won't change, but is it worth adding another example to this section?

Yes, lets add another example.

@oxinabox
Copy link
Member

oxinabox commented Nov 1, 2019

Appeal to authority falicy aside:
Stefan says:

I strongly encourage 3rd party packages to use snake case; germanic case is a self-imposed restriction for Base itself

image

@ScottPJones
Copy link

Wow!!! I do totally agree with Stefan here! 🐍 case rules!

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

4 participants