What problem is solved by chaining? All I see here is cost for no benefit. On Nov 16, 2011 5:02 PM, "Brion Vibber" <[email protected]> wrote:
> On Wed, Nov 16, 2011 at 3:15 AM, Owen Davis <[email protected]> wrote: > > > John Du Hart <compwhizii <at> gmail.com> writes: > > > > > It's just another style I've encountered on other projects and I > > > personally like. > > > > The syntax itself is fine, but at Wikia we have found (after a recent > post > > mortem) that out of 23 "Fatal Error" code defects found in production, > > 7 of them were due to method calls on null object references. If any > > method in that chain returns null then the request fails. It most cases > > core MediaWiki objects do return a valid stub object of some kind, but > > not all of them do (and in some cases they return null. intermittently > > so the code works "most of the time", which is in many ways the worse > > scenario). Introducing a pattern like this in a code base this large is > > therefore problematic. The "clean looking code" benefit is perhaps > > outweighed by the fact that you need to add extra "if" conditions or > > try/catch blocks everywhere to handle local null object exceptions. > > In our case, we are trying to ensure during code reviews that we just > > check for null objects in all "if" conditions, which does also lead to > more > > highly nested and less readable code. > > > > Any individual interface should generally have one of three patterns: > > A) cannot fail, ever -- always returns a live object > B) always returns a live object -OR- throws an exception > C) returns a live object -OR- null/false as error > > The most annoying problems seem to come from treating C code as if it's A: > everything works fine until you come across some invalid input, and then > somewhere else in your code you die, utterly surprised. > > The thing I like about B) is that failing to handle the error case gives > more immediate feedback: the error output & backtrace tell you where the > *thing that didn't find its data* happened, instead of where some other > code tried to use that previously-fetched data. > > My ideal unicorn-magic world tends to fall mostly in the A-B spectrum > (though exceptions do not always make sense in every case.). > > > Throwing chaining onto things like OutputPage could be done but seem > unlikely to be worthwhile; the operations are generally independent of each > other. > > Chaining is good for things that work conceptually like a filter chain. For > instance in jQuery code we'll often do something like: > > $(element).find('a') > .click(magicClickHandler) > .attr('title', 'Click for magic!'); > > The real magic actually came from the progress from $(element) to > .find('a') and then to performing operations on the resulting collection. > > There's a much smaller difference when there's a single variable as a > common prefix: > > $foo > .click(magicClickHandler) > .attr('title', 'Click for magic!'); > > vs > > $foo.click(magicClickHandler) > $foo.attr('title', 'Click for magic!'); > > is only slightly "cleaner". > > > > Everything is a tradeoff, but IMHO you do need to check for null in > chains > > like that, which means you can't really get away with it (at least not > for > > long). > > > > jQuery's chaining API is explicitly built to make sure that you don't -- > it's style A in the layout above. Each step in the chain returns a > collection, even if it's an empty collection... acting on an empty > collection may not do anything, but it doesn't cause a fatal error! > > Any chaining APIs we do add should similarly be built explicitly this way, > such as the Message class which we have both PHP and JavaScript versions > of. > > -- brion > _______________________________________________ > Wikitech-l mailing list > [email protected] > https://lists.wikimedia.org/mailman/listinfo/wikitech-l > _______________________________________________ Wikitech-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikitech-l
