-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[RFC] endless method definition #9080
Comments
I don't see the benefit, compared to: def inc(x); x + 1 end
def square(x); x * x end Saving 1/2 lines for another, rare syntax to learn, which is more limited than the exisiing one? Don't worth it, and even harmful. That's another example of Ruby "syntax multiplication", both syntax will end up being mixed to do the same thing: def inc(x) =
x + 1
def inc(x)
x + 1
end |
However it is clearer it returns a value. |
In general, I don't think this is worth it, mostly because of what @j8r pointed out: trends of "python-style" method definitions and managing consistency. Mixing the two doesn't seem nice. But, I actually wouldn't mind this for delegations (without using def foo(x) = @inner.foo(x)
def bar(y) = @inner.bar(y) I can't see myself using it outside of this though. Perhaps it would be nice for mathematical related code that often uses small methods to give names to expressions. |
I think it's nice for things like: def default_value = nil
def default_count = 0 stuff like that. That said, it's just a new style and then there will be comments in PRs and reviews saying that some stuff should be in one line, not with |
I like the aesthetics. Still no word whether it will be adopted by Ruby The percentage of one liners methods is probably higher in Ruby, |
This format was adopted in Scala over a decade ago, and I think Ruby's PR was inspired by it. https://docs.scala-lang.org/style/declarations.html def add(x: Int, y: Int): Int = x + y It is often the case that the logic itself only needs one line, such as specifying a default value or defining a simple function. But now it's four lines due to a boilerplate called For examplehttps://github.com/crystal-lang/crystal/blob/master/src/array.cr#L2066-L2072 private def check_needs_resize
double_capacity if @size == @capacity
end
private def double_capacity
resize_to_capacity(@capacity == 0 ? 3 : (@capacity * 2))
end
RFC private def check_needs_resize = double_capacity if @size == @capacity
private def double_capacity = resize_to_capacity(@capacity == 0 ? 3 : (@capacity * 2)) I agree with the introduction because I feel that having this format increases the amount of information on one screen and makes it more readable. Furthermore, I hope that this format will be a catalyst for the further evolution of future Crystal, such as the First class and Higher order Functions, Currying and Partial Functions. |
@maiha I disagree on your examples being an improvement. To me this seems much less readable when you have lots of things packed into a single line. I can see the appeal for really short method bodies consisting only of a simple expression. But as soon as conditionals come into play, I don't see no benefit. |
Just as an example, I need to scroll to the right to see the full definition under "RFC", but in the previous code snippet everything is clear at a glance. |
I'm ambivalent. I probably wouldn't use this myself, I find the four lines (including spacing) which a small method takes up perfectly fine. I'm worried about this introducing another "style" variable though. Now you have two ways of writing trivial methods. And style problems can get quite contentions and cause unnecessary friction, as we've seen recently. I don't want to shoot this down, but we don't need to introduce every ruby feature either. |
I feel like this would fall slightly in to the category of an "alias" since it doesn't change any sort of functionality, and Crystal tries to avoid having aliases. One thing that's nice in crystal is by having 1 way to do things, all code starts to look similar. As a teacher that teaches brand new people how to code, trying to explain multiple different syntaxes to do the exact same thing to students is really difficult. Having multiple developers working on the same code base over the course of years could lead to code mismatched, and even having the formatter to support would just mean that you couldn't auto fix to one way or the other. I also like what @RX14
It's great that we're "inspired" by Ruby, but we don't need to just be another Ruby. |
Yes, I wrote bad example. def any?
!empty?
end
def any? = !empty? |
I think it's more of a syntax sugar category than an alias category. For example, we should avoid having write one way def name
@name
end
syntax sugar getter name Should we remove the |
Ok, I can see that. That makes sense. What about setter methods though? class Person
@name : String?
def name = @name
def name=(@name)
end Where the first one is the getter, and the second one is the setter... These two are super close and could lead to beginning programmers getting lost and confused. |
The latter would correctly look like this. def name=(@name)
end
# or RFC style (This case gets complicated in reverse.)
def name=(v) = @name = v It can't be helped that the endless and the equal method are similar. |
I ran this Ruby script under Crystal repo and result is commented: files = Dir["src/**/*.cr"].map { |path| File.read(path) }
simple_def_count = files
.map { |content| content.scan(/^(\s*)def.*\n.*\n\1end$/).size }
.sum
p simple_def_count # => 4287
all_def_count = files
.map { |content| content.scan(/^(\s*)def.*\n(.|\n)*?\n\1end$/).size }
.sum
p all_def_count # => 9618 4287/9618 = 0.45. In short, most half (45%) methods are potentially simplified by this RFC. |
That's pretty neat to see those stats! That's also a testament to the language and all the hard work everyone has put in to see that we can write half the language in such a simple way. In Lucky, we also wanted a way to clean up some of the small methods, so we have a macro to handle doing that. Maybe we could add that macro to the language instead? That would solve the issue of shrinking up the codebase, give us syntactic sugar for small definitions, and not impact the compiler parsing at all. |
Heresy! (just kidding) Thank you for the stats! I don't think the stats are correct. For example here are some results that that script considers "one liners": def total
utime + stime + cutime + cstime
end
def to_s(io : IO) : Nil
io.printf " %.6f %.6f %.6f ( %.6f)", utime, stime, total, real
end
def human_mean
mean.humanize(precision: 2, significant: false, prefixes: Number::SI_PREFIXES_PADDED).rjust(7)
end
def step(*, to = nil, by = 1)
StepIterator.new(self + (by - by), to, by)
end
def sign
self < 0 ? -1 : (self == 0 ? 0 : 1)
end
def clone
Range.new(@begin.clone, @end.clone, @exclusive)
end
def number?
ascii? ? ascii_number? : Unicode.number?(self)
end
def initialize(@target : String, tolerance : Int? = nil)
@tolerance = tolerance || (target.size / 5.0).ceil.to_i
end
def read_abbreviations(io)
@abbreviations = Abbrev.read(io, debug_abbrev_offset)
end
def inspect(io : IO) : Nil
io << "#{self.class.name}(type=#{type}, name=#{name.inspect}, sect=#{sect}, desc=#{desc}, value=#{value})"
end
def values_at(*columns : Int)
columns.map { |column| row_internal[column] }
end
def self.each_row(string_or_io : String | IO, separator : Char = DEFAULT_SEPARATOR, quote_char : Char = DEFAULT_QUOTE_CHAR)
Parser.new(string_or_io, separator, quote_char).each_row
end Now written with the endless method syntax: def total = utime + stime + cutime + cstime
def to_s(io : IO) : Nil = io.printf " %.6f %.6f %.6f ( %.6f)", utime, stime, total, real
def human_mean = mean.humanize(precision: 2, significant: false, prefixes: Number::SI_PREFIXES_PADDED).rjust(7)
def step(*, to = nil, by = 1) = StepIterator.new(self + (by - by), to, by)
def sign = self < 0 ? -1 : (self == 0 ? 0 : 1)
def clone = Range.new(@begin.clone, @end.clone, @exclusive)
def number? = ascii? ? ascii_number? : Unicode.number?(self)
def initialize(@target : String, tolerance : Int? = nil) = @tolerance = tolerance || (target.size / 5.0).ceil.to_i
def read_abbreviations(io) = @abbreviations = Abbrev.read(io, debug_abbrev_offset)
def inspect(io : IO) : Nil = io << "#{self.class.name}(type=#{type}, name=#{name.inspect}, sect=#{sect}, desc=#{desc}, value=#{value})"
def values_at(*columns : Int) = columns.map { |column| row_internal[column] }
def self.each_row(string_or_io : String | IO, separator : Char = DEFAULT_SEPARATOR, quote_char : Char = DEFAULT_QUOTE_CHAR) = Parser.new(string_or_io, separator, quote_char).each_row Now, I'm not saying adding endless method is a bad idea, it's just that the places where it will actually be applied are not that many. I can imagine for integer literals, |
You can append newline after |
I ran this imperfect command: |
I find personally the syntax less readable, imagine: def [x, y]= = @hash[x] = y An option is to use a a block: def [x, y]= { @hash[x] = y } It is closer with the |
You are right. To tell you the truth, I've been applying this to some of the existing codebase and it does look less noisy, specially if you can put the expression on a separate line. In the end, less code and less lines to read, if it not makes things more cryptic (and this is not one of those cases if used well) is good. So 👍 from me on this feature. |
This is pretty subjective, but I like how it looks when it's a simple method like: def inc(x) = x + 1 But I don't like theses cases: def inc(x) =
x + 1
def foo=(x) = @foo = x
def foo(x) = @foo = x
def foo(x) = puts("Hi!"); x + 1 In other words, if this syntax is adopted I think I would restrict it to:
|
We have a conflict of opinions 😆 |
True :-) I was just looking at # Returns `true` if this char is a number according to unicode.
#
# ```
# '1'.number? # => true
# 'a'.number? # => false
# ```
def number?
ascii? ? ascii_number? : Unicode.number?(self)
end
# Returns `true` if this char is a lowercase ASCII letter.
#
# ```
# 'c'.ascii_lowercase? # => true
# 'ç'.lowercase? # => true
# 'G'.ascii_lowercase? # => false
# '.'.ascii_lowercase? # => false
# ```
def ascii_lowercase?
'a' <= self <= 'z'
end
# Returns `true` if this char is a lowercase letter.
#
# ```
# 'c'.lowercase? # => true
# 'ç'.lowercase? # => true
# 'G'.lowercase? # => false
# '.'.lowercase? # => false
# ```
def lowercase?
ascii? ? ascii_lowercase? : Unicode.lowercase?(self)
end With this change, not allowing newlines it could be changed to: # Returns `true` if this char is a number according to unicode.
#
# ```
# '1'.number? # => true
# 'a'.number? # => false
# ```
def number? = ascii? ? ascii_number? : Unicode.number?(self)
# Returns `true` if this char is a lowercase ASCII letter.
#
# ```
# 'c'.ascii_lowercase? # => true
# 'ç'.lowercase? # => true
# 'G'.ascii_lowercase? # => false
# '.'.ascii_lowercase? # => false
# ```
def ascii_lowercase? ='a' <= self <= 'z'
# Returns `true` if this char is a lowercase letter.
#
# ```
# 'c'.lowercase? # => true
# 'ç'.lowercase? # => true
# 'G'.lowercase? # => false
# '.'.lowercase? # => false
# ```
def lowercase? = ascii? ? ascii_lowercase? : Unicode.lowercase?(self) but I feel the lines become too long. The method bodies are pretty simple so if we write them like this: # Returns `true` if this char is a number according to unicode.
#
# ```
# '1'.number? # => true
# 'a'.number? # => false
# ```
def number? =
ascii? ? ascii_number? : Unicode.number?(self)
# Returns `true` if this char is a lowercase ASCII letter.
#
# ```
# 'c'.ascii_lowercase? # => true
# 'ç'.lowercase? # => true
# 'G'.ascii_lowercase? # => false
# '.'.ascii_lowercase? # => false
# ```
def ascii_lowercase? =
'a' <= self <= 'z'
# Returns `true` if this char is a lowercase letter.
#
# ```
# 'c'.lowercase? # => true
# 'ç'.lowercase? # => true
# 'G'.lowercase? # => false
# '.'.lowercase? # => false
# ```
def lowercase? =
ascii? ? ascii_lowercase? : Unicode.lowercase?(self) it looks a bit better, and because there's no |
The relation between this syntax and plain-old |
@asterite maybe we should have inspired on Python syntax then 🤣 |
The problem with Python is that you can't write a formatter for it because you are forced to keep the indentation. Another thought that came to my mind: I guess this can't be applied to macros. |
If we're going to put restrictions on what the right hand side expression can be, isn't that just admitting this is too easy to abuse? If that's the case, I'd rather not have it. Nobody has complained about the current syntax until now! If we're not sure it's a good idea, better to not have it. |
I feel this is going to be an endless discussion... |
@RX14 well, nobody complained about Crystal not existing, and here we are... 🙂 This is just syntax sugar that other languages already have, and maybe we'd like to adopt something similar. |
I count number of one-line body methods and such methods having arguments in Crystal repo. files = Dir["src/**/*.cr"].map { |path| File.read(path) }
simple_def_count = files
.map { |content| content.scan(/^(\s*)def.*\n.*\n\1end$/).size }
.sum
p simple_def_count # => 4301
simple_arg_def_count = files
.map { |content| content.scan(/^(\s*)def.*\(.*\).*\n.*\n\1end$/).size }
.sum
p simple_arg_def_count # => 2502 2502/4301 = 0.58. 58% one-line body methods have arguments. For good examples of such methods:
# Returns `true`: `Nil` has only one singleton value: `nil`.
def ==(other : Nil)
true
end
# Returns `true`: `Nil` has only one singleton value: `nil`.
def same?(other : Nil)
true
end
# Returns `false`.
def same?(other : Reference)
false
end
# See `Object#hash(hasher)`
def hash(hasher)
hasher.nil
end We can rewrite the above by using this RFC: # Returns `true`: `Nil` has only one singleton value: `nil`.
def ==(other : Nil) = true
# Returns `true`: `Nil` has only one singleton value: `nil`.
def same?(other : Nil) = true
# Returns `false`.
def same?(other : Reference) = false
# See `Object#hash(hasher)`
def hash(hasher) = hasher.nil |
@wontruefree This means being more C-like, nothing will stop to do: def foo(x) {
x + 100
} Got this code: class Object
macro cdef(name, *args, &block)
def {{name.id}}({{args.splat.id}})
{{block.body}}
end
end
end
cdef(hey) { "Hey" }
cdef mul, x : Int32, y : Int32 { x * y }
puts hey #=> Hey
puts mul(2, 3) #=> 6 With annotations, API docs arguments types are properly present. I am not that happy with this implementation, since it supports C-like definitions. |
I collect my thoughts and previous discussions for now as RFC again. If you think this is too long, please read the below "Conclusion" section at least. [RFC] endless method definitionAbstractThis proposal introduces a new syntax named "endless method definition". def inc(x)
x + 1
end Then, this proposal allows to rewrite it to def inc(x) = x + 1 SpecificationAll method definition options (scope specifier, arguments, return type restriction and annotation) are also allowed by endless method definition. For example, this is full example of endless method definition: @[AlwaysInline]
protected def self.ascii_char?(c : Char) : Bool =
0 <= c.ord <= 0xFE Remaining discussionWhy not macroWe can write a macro for short definition easily. When use this RFCIndeed 45% of Crystal methods body are one-line. For example, we shouldn't apply this syntax to the following example: class Nil
def to_s(io)
io << "nil"
end
end Because this method is operative. On the other hand, we should apply this syntax to the following example: class Nil
def same?(x : Nil)
true
end
end Because it returns a value and, this body expression is one-line and deadly simple. class Nil
def same?(x : Nil) = true
end Good! However, we can think it is ambiguous that there are two ways for doing one thing. IMHO, the last example is clearer than before, and this syntax is more descriptive that it returns a value. Which delimiter this syntax usesWe now consider uses # Hash syntax analogy
def inc(x) => x + 1
def inc(x): x + 1
# keyword version
def inc(x) return x + 1 I think Confliction with setter method nameThis syntax conflicts setter method naming in our mind. (The parser can distinguish them of course.) # `=` is appeared twice
def foo=(x) = x
# more bad example
def foo= = x I think such examples are too arbitrary, and special rule makes us confusing. What expression is allowed as right-hand sideI don't explain what expression is allowed as right-hand side of endless method, and I think this is largest discussion point of this RFC. IMHO, we should not allow suffix keywords in this expression. # This
def first? = self[0] unless self.empty?
# ... is consfusing which is parsed as:
(def first? = self[0]) unless self.empty?
# or as:
def first? = (self[0] unless self.empty?) Moreover I think parsing from ternary expression level is better (it is a bit technical, sorry). This has some benefits:
For example, the following examples are INVALID if the above is accepted: # Error: unexpected keyword 'unless'
def first? = self[0] unless self.empty?
# Error: unexpected token '='
# (We should show more detailed error message like "please wrap (...)"?)
def foo=(x) = @foo = x On the other hand, the following examples are VALID: def first? = (self[0] unless self.empty?)
def foo=(x) = (@foo = x) Note that ConclusionWe needs to get agreements with others that:
We needs to decide how do about:
When the above TODO lists are all satisfied, I will create a new Pull Request. (cc @asterite) Thank you! |
@makenowjust Fantastic and very clear writeup:
|
def test = (
a = 0
a += 1
a += 2
) Should be forbidden, it defeats the purpose of having small methods. |
@j8r I see no problem with that. To reply to @makenowjust points: @makenowjust Fantastic and very clear writeup:
|
@asterite However argument default value expression does not accept trailing So, I'd suggest another idea: parsing right-hand side expression of endless method is same as right-hand side expression of assignment (and it is same as argument default value expression.) I think sharing same rule on parsing right-hand side between |
One good thing about being as close to another language as Crystal is to Ruby, is that in some cases it may be perfectly fine to let them implement stuff, and then we can look at adoption and see if a: the feature is used So I'd propose to wait and see the fallout in Ruby as I don't feel the problem it solves is big enough to rush it. |
In fact Ruby's one was alternated yesterday. It is good to wait until Ruby's specification is fixed. |
I like those ruby changes minus the one about requiring parens even if there are no args. Looks weird and very unruby like. Thanks for linking |
Yeah. I think that |
Then again, we don't have to do what Ruby does. |
Yes so let's not 😂 Let's not make |
could we use another keyword like |
I think "don't have to do what Ruby does" does not mean "discard this RFC". |
One reason this makes more sense to me than ruby is that we have type overloads in crystal. A an overloaded constructor often looks like this: class Foo
def initialize(@s : String)
end
def initialize(s : Int32)
new(s.to_s)
end
# or, with RFC
def initialize(s : Int32) = new(s.to_s)
end That makes a lot of overoladed initializers easier to write |
@robacarp As a general note, constructor overloads should better be defined on And I have some doubts regarding the practical usefulness of your argument. Considering realistic use cases, you'll often find: more than a single argument, expressive argument names, longer type paths (maybe also union types), default values and longer conversion methods. Thus method signatures in the real world are usually considerably longer than your artificial example. |
Overloading needs a bit complex method header (signature) in fact. However this syntax works for a simple method body (not for header), and I think overloading method body is simple in many case, so the actual benefit is nice too. |
was there any resolution on whether this is happening? i kind of agree with @robacarp but for exactly what @straight-shoota mentions - e.g. class CustomThing
getter str : String
getter num : Int32
getter char : Char
# this has always looked weird to me, but is usually the right answer
def initialize(@str, @num, @char)
end
end where i would greatly prefer class CustomThing
getter str : String
getter num : Int32
getter char : Char
# no `=` necessary.
def initialize(@str, @num, @char)
end this use-case does seem like it's easily solved with a macro, unlike some of the other examples in this thread, but that feels even weirder than a zero line method definition. ...plus it just seems like something a curious reader would have to look up only to be disappointed when they peek under the hood 🤣 |
@makenowjust can you elaborate on why we wouldn't use the endless method syntax for the following? class Nil
# original method:
def to_s(io)
io << "nil"
end
# endless method:
def to_s(io) = io << "nil"
end
As in it performs an operation instead of returning a value? Why should that be excluded from this syntax? |
I have seen an interesting syntax from Elixir, which also takes inspirations from Ruby: defmodule Math do
def sum(a, b) do
a + b
end
end This syntax, def sum(a, b) { a + b }
# Maybe even
class Error < Exception { } We can say curly braces look ugly, but they are already used for block syntax. The good point is it won't really be an entirely new syntax. Here is another example: class Klass
# Already valid, memoization
getter name : String { "klass" }
def mul : Int32 { @num * 2 }
def initialize(@num : Int32) { }
end Finally, compared to the other main proposal Note: I think this whole proposal – single line methods – is more a quality of life than a necessity, we can take time to discuss. This shouldn't of course impact Crystal 1.0. We have lived and can live fine without 😄 |
My issue was closed as duplicate: #11165 Moving here. Crystal, like Ruby, has too many ends. I am paranoid that end ends Crystal. Let's make Crystal endless. def hello(name):
puts("Hello, #{ name }")
class Object
def has_instance_var?(name): Bool
{{ @type.instance_vars.map &.name.stringify }}.includes? name |
No love for endless Crystal. :( |
If your idea is to make Crystal indentation based, it it already answered in faq: https://github.com/crystal-lang/crystal/wiki/FAQ#why-isnt-the-language-indentation-based endless methods is quite another topic - allow to define single-line methods while keeping |
Recently, Ruby introduces a new method definition syntax called endless method definition (ticket and pull request):
It seems good to me. So, I'd propose to add this feature to Crystal.
Thank you.
The text was updated successfully, but these errors were encountered: