It looks to me like this code is a trope: if ( $repo->isVirtualUrl( $path ) ) { $path = $repo->resolveVirtualUrl( $path ); }
Or in this form: if ( self::isVirtualUrl( $file ) ) { // This is a virtual url, resolve it $path = $this->resolveVirtualUrl( $file ); } else { // This is a full file name $path = $file; }
Can we replace it by a resolveVirtualUrl which simply returns what it was given if there is no mwrepo:// at its beginning?
And ... there are only two calls to isVirtualUrl which are not part of this trope, and one of them is labelled with FIXME. :) The other one is used as a front-end to splitVirtualUrl, which does its own check to see if the url is virtual, and it's only called in one place.
Simplify, simplify!
Anybody object to me refactoring this code?
On 5/14/11 4:55 AM, Russell N. Nelson - rnnelson wrote:
It looks to me like this code is a trope: if ( $repo->isVirtualUrl( $path ) ) { $path = $repo->resolveVirtualUrl( $path ); }
Or in this form: if ( self::isVirtualUrl( $file ) ) { // This is a virtual url, resolve it $path = $this->resolveVirtualUrl( $file ); } else { // This is a full file name $path = $file; }
Can we replace it by a resolveVirtualUrl which simply returns what it was given if there is no mwrepo:// at its beginning?
How about an FSRepo::resolve() that does that?
"resolveVirtualUrl" suggests that it can only deal with virtual urls.
wikitech-l@lists.wikimedia.org