"Chad" innocentkiller@gmail.com wrote in message news:j2h5924f50a1004041133v522f88e9vf1349e6447667253@mail.gmail.com...
On Sun, Apr 4, 2010 at 1:43 PM, Happy-melon happy-melon@live.com wrote:
As Roan says, a Message object essentially deprecates the (IMO pig-ugly) Status class, which is used erratically throughout the codebase as essentially a way to bundle up a message key, some parameters, and a success flag without converting to String too soon. Status has always struck me as an unpleasant implementation: processes should throw exceptions on fatal errors, return messages on non-catastrophic problems, and bool true on success.
As opposed to returning a single class which can handle errors, warnings and OK statuses just fine? I really don't know why people don't like the Status class, I'm trying to use it *more* in the new-installer, rather than returning weird mixtures of booleans, strings and exceptions.
-Chad
It's pretty rare for 'error', 'warning', and 'ok' to all have independent meanings in MW. I can see the installer being an exception; but in general, we don't implement an "I did your action but this problem meant it's not entirely as you would expect" result; we fail and give the user an option to try again; that's not really any different to an error.
The real distinction is between errors that *preclude* a repeat attempt, and those that just indicate a failure this time around or with that particular set of data. Errors like read-only-wiki or not-authorised should be handled with exceptions which are caught high up in the call stack, probably at the level of Wiki.php; the message you get if you try to edit a page on a read-only wiki needn't be much different to trying to change userrights on a read-only wiki. Ditto for permissions errors, and anything else that leads to a 'dead end' error screen. All we're achieving with our if(wfReadOnly()){global $wgOut; $wgOut->readOnlyPage(); return false; } is to terminate call stacks early; that's exactly what exceptions should be used for. We just need a wfAssertWritable() function which throws a MWReadOnlyException on a locked wiki.
Any error which should be handled by redisplaying the form unquestionably needs to be accompanied by an informative error message, and conversely the "Your foo has been barred" success message is either going to be state-independent ("we successfully made the requested changes to your Foo") or will have done object instantiation ("based on your input, we have created a shiny new Foo"), which you'd keep in a member variable anyway. As such, every success needs no data returned from it, every transient error needs a message associated with it, and every permanent error should be handled by exceptions. Why complicate things with a wrapper class to convey information you can work out from context anyway?
--HM