r112171 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r112170‎ | r112171 | r112172 >
Date:00:41, 23 February 2012
Author:pgehres
Status:ok (Comments)
Tags:
Comment:
Updating wmfMessageExists to actully do what we want. Adding wfLangSpecificFallback
Modified paths:
  • /trunk/extensions/DonationInterface/gateway_common/DataValidator.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DonationInterface/gateway_common/DataValidator.php
@@ -238,10 +238,59 @@
239239 * @return boolean - true if message exists, otherwise false.
240240 */
241241 public static function wmfMessageExists( $msg_key, $language ){
242 - return wfMessage( $msg_key )->inLanguage( $language )->exists();
 242+ if ( wfMessage( $msg_key )->inLanguage( $language )->exists() ){
 243+ # if we are looking for English, we already know the answer
 244+ if ( $language == 'en' ){
 245+ return true;
 246+ }
 247+
 248+ # get the english version of the message
 249+ $msg_en = wfMessage( $msg_key )->inLanguage( 'en' )->text();
 250+ # attempt to get the message in the specified language
 251+ $msg_lang = wfMessage( $msg_key )->inLanguage( $language )->text();
 252+
 253+ # if the messages are the same, the message fellback to English, return false
 254+ return strcmp( $msg_en, $msg_lang ) != 0;
 255+ }
 256+ return false;
243257 }
244258
 259+ /**
 260+ * wfLangSpecificFallback - returns the text of the first existant message
 261+ * in the requested language. If no messages are found in that language, the
 262+ * function returns the first existant fallback message.
 263+ *
 264+ * @param $language the requested language
 265+ * @return String the text of the first existant message
 266+ * @throws MWException if no message keys are specified
 267+ */
 268+ public static function wfLangSpecificFallback( $language /*...*/ ){
 269+ $msg_keys = func_get_args();
 270+ array_shift( $msg_keys );
245271
 272+ if ( count( $msg_keys ) < 1 ){
 273+ throw new MWException( __FUNCTION__ . " BAD PROGRAMMER. No message keys given." );
 274+ }
 275+
 276+ # look for the first message that exists
 277+ foreach ( $msg_keys as $m ){
 278+ if ( self::wmfMessageExists( $m, $language) ){
 279+ return wfMessage( $m )->inLanguage( $language )->text();
 280+ }
 281+ }
 282+
 283+ # we found nothing in the requested language, return the first fallback message that exists
 284+ foreach ( $msg_keys as $m ){
 285+ if ( wfMessage( $m )->inLanguage( $language )->exists() ){
 286+ return wfMessage( $m )->inLanguage( $language )->text();
 287+ }
 288+ }
 289+
 290+ # somehow we still don't have a message, return a default error message
 291+ return wfMessage( $msg_keys[0] )->text();
 292+ }
 293+
 294+
246295 /**
247296 * validate
248297 * Run all the validation rules we have defined against a (hopefully

Follow-up revisions

RevisionCommit summaryAuthorDate
r112175A few little changes for khorn. FU r112171pgehres01:11, 23 February 2012
r112177Updating RapidHTML after r112175 and r112171. Partial revert of r111992pgehres01:22, 23 February 2012
r112245New DataValidator class, and donationinterface.php Only: MFT r107299, r10755...khorn21:42, 23 February 2012

Comments

#Comment by Khorn (WMF) (talk | contribs)   00:58, 23 February 2012

Mostly, this looks great and it probably works as-is. However, there are a couple things we can do to it to make it just a little more water-tight.

$language == 'en' 

It's just a little cleaner to wrap that $language in a strtolower(). This is particularly important in a class full of static functions like this one, that basically exists to assist anything in DI might want to call it.

$msg_keys = func_get_args();

I realize that everything I am about to say is an opinion. I personally hate func_get_args() with a fiery passion. If you pass in an explicit array of fallback keys, and default the parameter to array(), the next dev to come along and try to use this function will be able to tell at a glance what you're up to. func_get_args() is a total speed bump for the next guy, and typically you don't (as a team, or otherwise) gain anything by using it.

#Comment by Khorn (WMF) (talk | contribs)   01:00, 23 February 2012

Though, I should add that I have, on occasion, used func_get_args... but ONLY in cases where there was some weird inheritance going on, and php_strict was whining at me for not having the same structure in inherited functions.
...that I remember. If you find any where this is not the case, please feel free to smack me.

Status & tagging log