jashkenas / coffeescript

Unfancy JavaScript
https://coffeescript.org/
MIT License
16.49k stars 1.98k forks source link

Remove implicit returns? #2477

Closed devongovett closed 12 years ago

devongovett commented 12 years ago

After writing CoffeeScript for quite a long a while, I've come to the realization that implicit returns are actually more annoying than useful. Oftentimes I'll have to go back to a function later and add an explicit return statement to prevent CoffeeScript from adding implicit returns. They are the source of many bugs, and reduce the readability and predictability of CoffeeScript code.

  1. Having to go back an add explicit returns happens especially often when a loop is the last thing in the function, where very rarely do I want the function to return an array (if I did, I could easily add an explicit return). Generating and returning that array when I very rarely want it can also have a potentially large performance impact on the function as well, so I always have to remember to add a return statement at the end of many of my functions to prevent this. Just looking at a function with a loop at the end doesn't tell me that it returns an array so when it turns out it does it can be kinda strange. Usually you end up finding out when you discover another bug related to it or you look at the compiled JavaScript.
  2. It can also be inconsistent when combined with conditional logic. For example:
test = ->
        if someCondition
            doSomething()

This function returns the result of doSomething() only if someCondition is truth-y, otherwise it returns undefined. The question is why I would ever want a return value sometimes and not others. Without an explicit return statement, this seems strange. It isn't clear that this function returns anything just from looking at it.

  1. It can be annoying for functions that do something based on another function's return value. An example of such a function is jQuery's each function, which uses the false sentinel return value as a way to break out of the loop. Using the function in CoffeeScript when the last operation in that function sets a variable to false can be a source of bugs since CoffeeScript implicitly returns false and this stops the loop. For example:
arr = ['a', 'b', 'c']
map = {}
$.each arr, ->
        map[this] = false

Just reading this example doesn't make it clear that the loop will only run once rather than the expected three times. The variable map would equal { a: false } after running this loop. I would have expected to have gotten { a: false, b: false, c: false }. Adding an explicit return as the last line of the each function fixes the problem, but you wouldn't know to do that without reading the compiled JavaScript.

I could probably come up with many more examples to show that implicit returns are usually not a good idea. I think CoffeeScript should actually remove implicit returns from the language. Typing out return really isn't so hard and will lead to more readable and predictable code and keep the language closer to the semantics of JavaScript (a good thing when working with libraries written in JavaScript like jQuery from CoffeeScript code). It would probably be less typing in the end since you will no longer have to add return statements to prevent CoffeeScript from implicitly returning the wrong thing. I know this would be a large change but I think it would be for the better since it is more of a cause of bugs than a helpful language feature.

Removing features from a language is hard, but sometimes the best thing to do. What do you think?

devongovett commented 12 years ago

Well it wasn't really about jQuery.each in particular and I don't dispute the fact that I would probably use a comprehension in CoffeeScript. It is, however, an example of the type of function where you only return something in unusual circumstances (like breaking out of the loop), and unintentionally returning something might break the program. There are other examples of this type of function, but jQuery.each was the first one that popped into my head at the time. I'm just worried about unintentionally returning something I didn't mean to and causing something else, which leads to putting returns at the end of every function. But maybe I'm crazy.

epidemian commented 12 years ago

I usually put a return at the end of all of my functions that shouldn't return anything to avoid these problems, and simply wondered if the language could help me avoid doing that.

I think this might be the crux of the problem here. If you're adding boilerplate code to many (or even most) of your functions to "fix" the behaviour of the language, maybe you're fighting against the language.

Someone could open an issue asking for static type declarations in CoffeeScript because they're tired of writing throw new TypeError unless someParam instanceof SomeType in all their functions, because, well, if they didn't write these checks, someone could call the function the wrong type of parameters and it would behave wrongly maybe without even breaking. There's nothing wrong with not liking duck typing. But trying to force static typing in a language that encourages duck typing is fighting against it.

So, assuming some function is like a procedure: it does something, so it's only important because its side effects. Something like this:

# Closes the dialog and triggers a "close" event.
MyDialog::close = ->
  @trigger 'close'
  @$el.hide()

The close function is returning the $el jQuery element, but users of the close function should not be counting on that, the same way that they shouldn't be accessing "private" things of other object like someObj._something (and there are lots of issues requesting private visibility implemented in CoffeeScript too =D).


The only case where i see that implicit returns do not make sense is in asynchronous callback-based code. Callback functions do not really behave as functions in those cases, they are more like continuations of code. But really, i think that style of programming is so terrible that it doesn't matter how well a programming language suits it, it will still suck.

devongovett commented 12 years ago

Closing this now, since the discussion appears to have ended. Thanks for your input, everyone!

silviopaganini commented 11 years ago

Hi there

Sorry to be stating a new discussing about this.

I have seen that you guys been discussing a lot about the implicit returns on CoffeeScript. I have to say that when benchmarking a website using the developer tools you can see the browser's GC struggling to keep up with the loads of variables being allocated unnecessarily when you don't write a more structured code.

In a big project is really complicated to keep up with different people and different code styles. You can't really tell if everyone is doing the right thing and when you see the GC struggling you start to get scared, and when you see that one of the main reasons of it was the unnecessary return in the end of each method you go mad. To quick fix that problem we went though all 100000 CS files and put a null return. BOOM it works like a charm.

Obviously this is not a good practise, so I'm wondering if there's any chance of including a parameter to the compile so it only returns the last statement of a method if the developer decides too.

Thanks

jashkenas commented 11 years ago

@silviopaganini

To quick fix that problem we went though all 100000 CS files and put a null return. BOOM it works like a charm.

It's very interesting that you found a visible effect there -- but much of the time, the return shouldn't have a noticeable hit on overall performance. The exception to the rule is when your function ends with a loop, and you're returning a comprehension unnecessarily.

It would be great if you could profile your app a bit, and discover if the change you observed is because of the removal of a couple of comprehensions in your hot code path ... or if there's something else afoot.

silviopaganini commented 11 years ago

hi there.. thanks for the quick reply

once we release the site, I'll try to create a branch without all return nulls and profile it, I can also try to get you an URL so you can see it too.

ghost commented 10 years ago

Maybe this was covered in the discussion above (though I didn't see it), but why can't this simply be a compiler option? Allow each development shop to decide CS' default behavior for themselves... It would probably be a minor change to allow, and it would give people more freedom/options.

michaelficarra commented 10 years ago

Are any other CoffeeScript semantics determined by a compiler flag? If not, I wouldn't want to start.

jashkenas commented 10 years ago

but why can't this simply be a compiler option?

It can be, in your fork ;) Call it ReturnOfCoffeeScript, and I bet you'll get some folks who appreciate the change.

johannesjo commented 10 years ago

Just to for the record: This incredibly stubborn and (sorry) dumb decision killed coffeescript for me unfortunately. But why should I learn a new syntax, if it solves some problems at the cost of many new ones (scoping, compilation, slowing down my ide and being depended on other devs being able to write cs)?

xixixao commented 10 years ago

@johannesjo Stick with assembler. It is very predictable.

Now seriously: yes, you have to watch out for this behavior. With great power comes great responsibility. It is far from stubborn and dumb decision.

johannesjo commented 10 years ago

@xixixao : First of all, sorry for my harsh tone. I wouldn't be so upset, if I wouldn't care for cs.

The thing is that coffeescript is 'just' a wrapper for JavaScript. The output should be intuitive for someone coming from JS to CS wherever possible imho. Sass and less are doing a great job here while cs is trying to force a specific design-paradigm, which could be good, but should be an option for those who like this style not a default for everyone. With the implicit return cs is trying to be more than just a wrapper for JS. This leads to problems as many examples above show.

That there are lots of unnecessary, even possibly problematic unintended returns in the cs source itself clearly illustrates to me it not only ain't intuitive for many devs, but that the luxury of writing a single word less (and I agree with @andrewrk that this is not even the case) doesn't outweigh the cost of possible bugs, which in some cases might be very hard to spot.

xixixao commented 10 years ago

Implicit last expression is longer because it occurs 1 / 5 of the time

I certainly don't have every 4 out of 5 functions ending in return. I agree with your argument, given CS was just a wrapper for JS - I personally disagree with this or find it slightly misleading statement. Of course, depends very much on how you define wrapper. I, personally, think that CS is a different, better, language that uses JS runtime (this is academic discussion anyway). Would I want CS with two different function symbols or simply without implicit returns? I have written thousands of lines of CS and in my experience the benefits far outweigh the negatives. CS nudges you towards functional and that I find a positive, not a negative.

johannesjo commented 10 years ago

@xixixao Thank you for explaining what cs means to you and what the difference in our way of thinking of it is.

I just think, that if cs aims to be a better other language, it is doomed to 'fail' in the long run to be as successful as it should be. There have been so many attempts to replace JavaScript and they all disappeared, not because they're bad, but because JavaScript is the smallest common denominator and because people tend to get annoyed if there is no full compatibility to all the nice things already out there, if there are problems created instead of solved. I'm pretty sure that also dart will fail in ever replacing JavaScript. It's not a tragedy, because all those attempts influenced JS to the better in one way or the other, but I personally would like being able to use a more modern approach right now.

If cs really would want to become big, the conversion from cs to JavaScript but also the other way round should be made as painless and flawless as possible.

Anyhow this probably getting too far off topic...

andrewrk commented 10 years ago

I say this as someone who is also against implicit returns: you guys are wasting your breath in this thread.

johannesjo commented 10 years ago

@andrewrk You're right...

davidchambers commented 10 years ago
(x) -> x ** 2

A world in which relationships between values can be expressed without function or return is a beautiful world indeed.

johannesjo commented 10 years ago

@davidchambers I think nobody said anything against those one-liners. It's very neat I agree. But for more complex functions its neither more expressive nor more beautiful.

davidchambers commented 10 years ago

It works nicely for multi-line functions, too:

product = (nums) ->
  if nums.length is 0
    1
  else
    nums[0] * product nums[1..]

Python ended up with both functions and lambdas. I'd hate to see the same thing happen to CoffeeScript.

johannesjo commented 10 years ago

@davidchambers maybe you should spend a little thought on why python ended up with both of them...

davidchambers commented 10 years ago

From a BDFL post, The fate of reduce() in Python 3000:

Why drop lambda? Most Python users are unfamiliar with Lisp or Scheme, so the name is confusing; also, there is a widespread misunderstanding that lambda can do things that a nested function can't -- I still recall Laura Creighton's Aha!-erlebnis after I showed her there was no difference! Even with a better name, I think having the two choices side-by-side just requires programmers to think about making a choice that's irrelevant for their program; not having the choice streamlines the thought process.

The considerations are somewhat different in the case of CoffeeScript, but in both cases one must decide whether the advantages of having a secondary, slightly different function form offset the costs: extra decisions and potential for confusion.

xixixao commented 10 years ago

@andrewrk Certainly in terms of getting this change through. I am always glad to discuss why someone doesn't like/switched away from CoffeeScript, it is instructive.

xixixao commented 10 years ago

You guys might also be interested in https://github.com/clutchski/coffeelint/pull/30 (as jmreidy was suggesting)

johannesjo commented 10 years ago

@xixixao very nice. Thank you! Maybe I'll give it a try in one of my own projects some time.

KATT commented 10 years ago

I know this discussion is dead and the decision is made, but I still have hope of the discussion getting revived anyway.

I've had a few WTF-moments with it, here is one example. Let's gather up all the results and have a massive memory heap party!

The other obvious is when using it in combination with event listeners or forEach/$.each and you have an assertion that happens to be false at the end of the callback.

It just creates so many hard-to-detect bugs. The trade-off of being able to create sexy Ruby-esque oneliners is just not worth it.

demisx commented 10 years ago

I love CS, but having to put in all these empty returns at the end of each function, switch cases is a pain in the rear. Without it, CS would shine on me much brighter.

crissdev commented 10 years ago

@devongovett I ended up here most probably due to the same reason. Implicit returns are bad, and they are the source of a lot of noise, bugs, performance issues (unresolved / unexpected promises => not cool!), in the source files, since we have to manually add the empty return statements. After working a lot promises, jQuery etc. in CoffeeScript, I began to feel desperate with each empty return statement being added, or bug to fix due to these implicit returns. I thought to a solution for quite some time...and always the final thought is to go back to plain JavaScript. ES6 is coming anyway. So good bye CoffeeScript.

Now let's convert all the coffee's in the project into a really readable code. :shit:

swayf commented 10 years ago

npm uninstall -g coffee-script npm install -g blackcoffee :)

rlidwka commented 10 years ago

coffee-script should really have macros/plugins instead of forks...

boundvariable commented 10 years ago

I'm against explicit returns...

That said, one option might be a little bit of rust inspiration and have expressions ending in a semi-colon evaluate to undefined.

a = -> 
  if true
    4
  else
    2

a()
# 4

aSemiColon = ->
  if true
    4
  else 
    2
  ;

aSemiColon()
# undefined

I know it just looks like a shorthand return but it could have more sophisticated uses.

Of course, semi-colons are currently valid syntax in CS but I hardly ever see them so this might not be such a big deal.

Just a thought.

davidchambers commented 10 years ago

I prefer the explicitness of return to a special character such as ;.

$ python -c 'import this' | grep 'only one'
There should be one-- and preferably only one --obvious way to do it.
boundvariable commented 10 years ago

In the scenario mentioned, semicolon means "evaluates to undefined" would apply to all expressions. Not just functions.

demisx commented 10 years ago

Using ; still requires an explicit line added similar to the explicit return. I think the approach taken by BlackCoffee is a cleaner one. Just define function with ! if you want it to return nothing:

fn = !->
  some_calll()
  another_call()

Just looks and feels cleaner this way IMHO.

KATT commented 10 years ago

Maybe it would make sense to have it similar to ES6 arrow functions?

statements or expression Multiple statements need to be enclosed in braces, {}. A single expression, however, requires no braces. The expression is also the implicit return value of that function.

I.e. (pardon the bad examples)

square = (x) -> x*x

to

// (unchanged to current implementation)
var square;

square = function(x) {
  return x * x;
};

and

cube = (x) ->
  _square = x*x
  _cube = _square*x

to

// (no implicit return, would return nothing)
var cube;

cube = function(x) {
  var _cube, _square;
  _square = x * x;
  _cube = _square * x;
};
remoteportal commented 10 years ago

I hate implicit returns so much I feel like forking my own CoffeeScript just to remove that "feature."

crissdev commented 10 years ago

@KATT That would definetely make me want to go back to coffeescript. That's how it should work - considering how other languages implement arrow functions/lambda.

@remoteportal I know the feeling of that...I currently try to get rid of all the coffeescript from my project and use plain javascript. The only place I use it though, is to generate JSON files (from CSON files) 'cause it saves a lot of double quotes and curly braces :-)

rlidwka commented 10 years ago

The only place I use it though, is to generate JSON files (from CSON files)

Why not use YAML instead of those JSON's?

vendethiel commented 10 years ago

(YAML is pretty terrible, and there's a definite lack of tools to parse it, because it's so complex)

rlidwka commented 10 years ago

there's a definite lack of tools to parse it

js-yaml ?

crissdev commented 10 years ago

@rlidwka Because I have better support for CSON in gulp tasks and YAML seems to have a pretty sophisticated syntax for my use case.

xyrius commented 10 years ago

'return if true ' is already a return statement. What follows is not clear. The error seems to be correct.

On Sat, Sep 13, 2014 at 8:05 PM, yetanotherusernamebecausegithubisbugged < notifications@github.com> wrote:

I must admit that implicit return with loops in my humble opinion is a bug factory.

http://programmaticallyspeaking.com/why-i-hate-implicit-return-in-coffeescript.html

http://awardwinningfjords.com/2012/05/08/beware-coffeescript-comprehensions.html

It's not so hard to write return statement when it's really required.

a = -> return if true 4 else 2

instead of

a = -> if true 4 else 2

would be great. Though it not compiles right now. error: unexpected indentation

Also, no need to fork. Probably some --strict-mode key to CLI? I think it would be needed sooner or later to softly eliminate bad practices because coffeescript is still young programming language.

— Reply to this email directly or view it on GitHub https://github.com/jashkenas/coffeescript/issues/2477#issuecomment-55501717 .

xyrius commented 10 years ago

I did a proposal (#3583) to have an initial return value without predjudice of any later return statement like so:

a= true -> return false if somePredicate

Then you can also default return a the context like so:

a: @ ( val ) -> someOperations( val )

Or automatically return undefined when no initial is given.

I hope it makes sense to someone:)

On Sat, Sep 13, 2014 at 8:05 PM, yetanotherusernamebecausegithubisbugged < notifications@github.com> wrote:

I must admit that implicit return with loops in my humble opinion is a bug factory.

http://programmaticallyspeaking.com/why-i-hate-implicit-return-in-coffeescript.html

http://awardwinningfjords.com/2012/05/08/beware-coffeescript-comprehensions.html

It's not so hard to write return statement when it's really required.

a = -> return if true 4 else 2

instead of

a = -> if true 4 else 2

would be great. Though it not compiles right now. error: unexpected indentation

Also, no need to fork. Probably some --strict-mode key to CLI? I think it would be needed sooner or later to softly eliminate bad practices because coffeescript is still young programming language.

— Reply to this email directly or view it on GitHub https://github.com/jashkenas/coffeescript/issues/2477#issuecomment-55501717 .

yvbeek commented 9 years ago

The implicit returns are reallllly annoying.

When I'm writing in CoffeeScript for my Angular projects I really have to make sure that I add "return" to some of my functions, otherwise I'll spend the rest of the day going on a frustrating bughunt.

I would suggest:

Or make <- an alias for return statements and drop implicit returns.

Regardless it would be great to have a compiler option to disable implicit returns. I'd rather disable it and write "return" when I have to, instead of having all of these useless return statements in my code.

carlsmith commented 9 years ago

If you're putting return statements at the end of lots of functions, you're doing something unusual. If you don't want the return value, don't assign it to anything. It only matters if the last value evaluated is exceptionally large and you don't want to keep it.

Implicit returns also flow naturally with conditionals as expressions and so on. Removing one, would make other things less consistent.

gt = (x, y) -> if x > y then true else false

JavaScript already has implicit returns too. It just returns undefined instead. The suggested syntax for setting a default return value is just sugar for something that's sweet enough already.

crissdev commented 9 years ago

@carlsmith Some libraries really care about the return value. I would say the most trivial example is by using the [jQuery.each](We can break the $.each%28%29 loop at a particular iteration by making the callback function return false) function. For more advanced bugs :laughing: implicitly return a promise when you don't want to. CoffeScript remains a great choise if you can't develop with ES6.

carlsmith commented 9 years ago

@crissdev - Yeah, you're right; there's more than just very large return values to worry about. I've not used promises, so that didn't occur to me. Still, the idea that...

f = true (x) -> return false if x

...is better than...

f = (x) -> if x then false else true

...isn't much of a sales pitch.

Not having a way to make a function implicitly return undefined seems to be a small, but significant and growing annoyance. Having a syntax for setting the default value would solve that problem, but is a whole new feature to fix an issue with an existing one. IMO unless there's a strong case for the new feature, we should just fix the existing one, which isn't actually broken at all, just awkward at times.

yvbeek commented 9 years ago

@carlsmith when working with Angular nearly everything is wrapped in functions. In many cases the implicit returns don't make sense and even cause Angular to break.

I think that CoffeeScript is great and it simplifies / reduces a lot of javascript. But this is really bugging me. Perhaps it is time to check out Traceur :). ES6 seems to fix a lot of javascript frustrations (I guess my no 1 is having to type "function" all the time and the weird for-each loops)

michaelficarra commented 9 years ago

I'm going to lock this issue, as there's no reason to have a discussion about this -- it's not going to change. If you want to write procedural code, don't choose to use a language where (nearly) everything is an expression. In CoffeeScript code, you are meant to be working with values much more frequently than you work with side effects. If that's not the case, sorry, it's going to be a burden. Use the right tool for the job.

jashkenas commented 9 years ago

I agree that this is never going to change — except in your very own fork of CoffeeScript ... but let's not lock issues on here. Folks should be able to keep on talking about stuff as long as they care to. Otherwise, we're just going to end up with a whole bunch more duplicated issues being created and locked and scattered to the wind.

epidemian commented 9 years ago

Thanks for keeping the discussion open, @jashkenas.

I'd like to ask some open-ended questions to (hopefully) move the discussion into other directions:

Why is this such a problem with CoffeeScript?

Why is this not a problem in Ruby, where method/block invocations also evaluate to their last expression?

Is this mostly a case of people coming from different programming languages (e.g. JavaScript, Python) having different expectations? Or is the assumption that most functions should evaluate to something meaningful ill-suited for the environment where CoffeeScript is used (i.e. browsers, node)? If the last is true, can we do something to remedy that?