Coffeescript ternary if-statement wrong logic

Question

I have a pretty much simple logic in a return function, but it doesn't work as expected. Of course I can make the code slightly longer and solve the issue, but I want it to be as small as possible.

Here is my code:

#Return title if exists or false otherwise
getPageTitleFromMainContent = (mainContent) ->
  mainContent.find('#pageTitle') ?.length ?=  false

if y = (getPageTitleFromMainContent $("#mainContent"))
  y.css color:red

As you see, if it finds the #pageTitle in #mainContent, it should make it red. But the function doesn't return the #pageTitle if found, it returns .length.

From js2coffee.org I see that the code is compiled into:

var getPageTitleFromMainContent, y;

getPageTitleFromMainContent = function(mainContent) {
  var _ref, _ref1;
  return (_ref = mainContent.find('#pageTitle')) != null ? (_ref1 = _ref.length) != null ? **_ref1 : _ref.length = false : void 0;**
};

if (y = getPageTitleFromMainContent($("#mainContent"))) {
  y.css({
    color: red
  });
}

And it should be _ref : _ref.length = false : void 0;, not _ref**1** : _ref.length = false : void 0; .

http://jsfiddle.net/X8VjJ/1/

Thank you!

Ian Henry · Accepted Answer · 2013-02-21 15:24:48Z

3

if it finds the #pageTitle in #mainContent, it should make it red

You can accomplish this with the much simpler:

 $('#mainContent #pageTitle').css(color: 'red')

Since, if it doesn't find #pageTitle in #mainContent, it will try to change the css of an empty set of elements -- a no-op.

The code as you've presented it doesn't really make sense. ?. is unnecessary, as the jQuery selector will not return null or undefined if it doesn't match; it will return an empty set of elements. So it will always be returning length, which will always be a number, so the assignment will never execute, since it depends on length returning null or undefined. Which is good, since you probably don't want to set the length of the elements to false.

Finally, this isn't the ternary if statement. CoffeeScript's ternary if statement looks like this: if foo then bar else baz.

answered Feb 21, 2013 at 15:24

Ian Henry

22.4k4 gold badges53 silver badges61 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

ioan Over a year ago

The actual code is not as simple as changing the color - that's just an example. Your logic about sizzle return's value is right. But isn't there a way of accomplishing what @Marius Kjeldahl 's code does but without using any variables like el ?

Ian Henry Over a year ago

I mean, you can do the search twice. if mainContent.find('#pageTitle').length > 0 then mainContent.find('#pageTitle') else false, or something horrible like mainContent.find('#pageTitle').length && mainContent.find('#pageTitle'). That's worse. Write the most readable code you can. There's no benefit from compressing it into a single expression, and it'll make the code harder to understand for you or others in the future.

ioan Over a year ago

Right. I think I tried all possibilities of solving this little issue. _ref get's the #mainContent, but when coffeescript sees .length ?, it can't return via a operator that _ref variable, it returns _ref1 which is the length. I will let this question unanswered for a bit; just hopping.

Marius Kjeldahl Over a year ago

As Ian points out, easily read and understood code trumphs smartypants expressions and operators every time, and becomes even more important as the time between "every" becomes larger.

Marius Kjeldahl · Accepted Answer · 2013-02-21 19:25:56Z

2

Not sure that code makes sense. You're effectively trying to assign TO the length property, unless length is defined. If it is defined, it simply returns the length property. Looks like the code and behaviour is correct, but your understanding of the existential operator and return values is wrong. If you want to return the found element you probably need to disconnect it from the length check.

Maybe something like:

getPageTitleFromMainContent = (mainContent) ->
  arr = mainContent.find('#pageTitle')
  if arr.length then arr else false

As Ian explained in his more elegant answer, you do not need to use the existential operator on arr (assuming jquery), since it will always be an array of elements (with zero length if not found).

edited Feb 21, 2013 at 19:25

answered Feb 21, 2013 at 15:18

Marius Kjeldahl

6,8243 gold badges37 silver badges37 bronze badges

2 Comments

ioan Over a year ago

If the element doesn't exists, the jquery will return an empty array, and I don't want that; I want it to return false. Also, as I said, would be nice to make it without assign values. The coffeescript is very powerful and I would like to make use of it.

Marius Kjeldahl Over a year ago

Updated my answer with suggested solution.

Collectives™ on Stack Overflow

Coffeescript ternary if-statement wrong logic

2 Answers 2

4 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related