Update ActionDispatch::Request#xml_http_request? documentation to match its behavior regarding return values

[Empact]

The docs say this returns true on success, which it doesn't.

As noted in the debate about this method (see #5329), plenty of core library interrogative methods return values other then true and false, but from my checks they also consistently and correctly document what is returned.

E.g.:
http://ruby-doc.org/core-1.9.3/FileTest.html#method-i-world_writable-3F
http://ruby-doc.org/core-1.9.3/FileTest.html#method-i-world_readable-3F
http://ruby-doc.org/core-1.9.3/File.html#method-c-size-3F
http://ruby-doc.org/core-1.9.3/Encoding.html#method-c-compatible-3F
http://ruby-doc.org/core-1.9.3/Kernel.html#method-i-autoload-3F

Incidentally, in these other methods the returned values also all have additional informational content beyond "I am a truthy stand-in for true". But that is a separate discussion.

[fxn]

Thanks Ben.

As already explained, there is no need to change this or any other predicate to return singletons (you know that). And the documentation does not need (and we never do) to leak the implementation. The contract is a true or false value according to Ruby semantics.

The methods in the standard library document the return value because at the core level you commit to an implementation/contract that preserves that value and you believe the value may be useful to the caller.

For example

if offset = (string =~ regexp)
  # there's a match, and I am going to use offset
end

But in our predicates the exact values do not belong to the contract, we only tell the user that the predicate behaves like a predicate.

Thanks anyway.

[Empact]

Xavier, true has a distinct and exact meaning in ruby. It means true. Saying the method "Returns true" is simply false.

[fxn]

@Empact No, no. When you refer to the singleton you use fixed-width font. That's a standard practice followed in particular by our documentation, and that is part of our documentation guidelines.

The docs sometimes use the formula "Says whether the request is Ajax" and sometimes "Returns true if the request is Ajax". We do not use "trueish" because we should do that in the entire documentation and it reads ugly. And using regular font is a common and enough convention.

The type of the returned value is not documented, the contract that you have a predicate that acts as a predicate.

In particular, there is no point in saying that xhr? returns 0, it's of no use to the user, and it is a detail we do not need to commit to.

Use the predicate as a predicate.

[Empact]

You say "In particular, there is no point in saying that xhr? returns 0, it's of no use to the user, and it is a detail we do not need to commit to."

This is demonstrably false. See the thread for #5329, where at least one person reports it causing bugs in their app.

I listed the core library documents to make a point: when the core library departs from true/false, it documents what it returns (and thus implicitly why it diverged from the default).

Finally, I'll bet most readers of the doc won't read "true" and "true" as having distinct meanings.

[fxn]

Returning zero is not causing any bug in that application, as it has been explained. The bug is caused by not using a predicate as a predicate.

I'd be open to revise the guidelines so that we say "returns a true value", in regular font. That may emphasize we are not talking abot the singleton. That would need to be done at the project level, though. A single method edit is pointless.

[fxn]

That's the convention in Flanagan & Matz by the way, see section 4.6.8 for example.

[kenmazaika]

+1 to having accurate docs.

edit: removed suggestion, this had been discussed on related issue

[fxn]

@kenmazaika saying that some particular predicate returns true (or a true value) in such and such case is as accurate as it can be.

Knowing that =~ returns the offset when true may be of use to the user because the value may be helpful by itself. I cannot see a use case where the user of xhr? can find the offset value helpful. So we can afford not committing to an implementation. The predicate has a standard predicate contract, it is enough.

[fxn]

On the other hand, our convention is also the convention in the Pickaxe:

"In this book, when we want to talk about a general true or false value, we use regular Roman type: true and false. When we want to refer to the actual constants, we write true and false.

The fact that nil is considered to be false is convenient. For example, IO#gets, which returns the next line from a file, returns nil at the end of file, enabling you to write loops such as this:

while line = gets
  # process line
end

However, C, C++, and Perl programmers sometimes fall into a trap. The number zero is not interpreted as a false value. Neither is a zero-length string. This can be a tough habit to break."

I wrote the API documentation guidelines some four years ago, and that has been our convention ever since.

[steveklabnik]

? methods should return something truthy. We're Ruby programmers, not C programmers. Something that returns 0 is returning true.

[richo]

Hey, is this a familiar argument to anyone?

Regular users should understand that there's magic under the hood and not make logical expectations. Be quiet.

Fast forward 12 hours and GitHub gets haxed as a result of making logical expectations

Whoopsie.. merge

[banister]

Here's a point from purely practical convenience -- checking a predicate method predicate? in a REPL can flood your screen (if the 'truthy' object is large). When ruby is used non-interactively this isn't an issue of course, however I think Rubyists are fairly heavy REPL users and so this aspect should be taken into consideration.

[crystalneth]

I agree with most of the arguments for returning true, not truthy, unless the truthy value is documented and useful. For instance, =~ returns a useful value and can also be used to get the position of the match. In the case of xhr?, the return value is just noise and breaks encapsulation, revealing implementation detail unnecessarily. It is also a performance issue to rely on returning truthy values, since useless truthy values in many case are more expensive to serialize or pass by value.

Rails core preference for trivialy simplifying the rails code at the expense of providing the clearest public interface possible is disappointing, and frankly contrary to the way most of Rails is written. Insistence that those who find the return value confusing "learn Ruby" goes against what many consider to be a mission of Rails - to make web development more accessible by reducing cognitive clutter of old frameworks. By admission of both those for and against, this definitely increases necessary knowledge of Ruby without any benefit.

My philosophy on boolean APIs is that they should return true or false unless there is a documented reason to diverge from this pattern to double purpose the API, eg =~.

I'm confident once egos are checked, that this is the correct course. In the meantime, while this is a trivial issue, it goes to the core of what Rails is and how decisions are made. This decision strikes me as dictatorial, unpopular, ego laden, and likely wrong. It makes me a little bit sad.