[wp-trac] [WordPress Trac] #22587: Cast image sizes to array before looping

[WordPress Trac]

#22587: Cast image sizes to array before looping
------------------------------+------------------------------
 Reporter:  griffinjt         |       Owner:
     Type:  defect (bug)      |      Status:  new
 Priority:  normal            |   Milestone:  Awaiting Review
Component:  Warnings/Notices  |     Version:  trunk
 Severity:  normal            |  Resolution:
 Keywords:  has-patch         |
------------------------------+------------------------------

Comment (by nacin):

 MikeSchinkel: scribu has a point. If there's no good reason, there's no
 good reason. Justin was fine to suggest that there may be a parity
 argument, and scribu was fine to (in his usually colorful way) clarify
 that parity doesn't really work for such a general case. I imagine both of
 them would have continued to go back and forth until there was some kind
 of agreement, consensus, or at least understanding.

 Instead, you stepped in to try to make a big deal about something/nothing.
 Since your comment, Justin returned and said he didn't have a problem with
 scribu's comment, the reporter returned and made a point, and scribu
 return with two succinct points that I explain in much greater depth
 below. Three helpful contributions.

 ----

 To the ticket. image_size_names_choose gets called in a few places — 3
 now, I think, so that patch needs some more love.

 There are two schools of thought for return values of filters:
  * Cast. It is better to make sure that a plugin doing it wrong does not
 mess up core execution.
  * Don't cast. A developer seeing an E_WARNING as a warning to fix their
 code is better.

 These are not mutually exclusive. I think it depends on the situation for
 which makes the most sense. I am sure some casts in core should have been
 omitted, and indeed *some* could probably be removed (such as in cases
 where the variable is *always* an array). Most casts are probably in older
 code that just did it out of habit. Casting inside a foreach statement is
 one of two things, typically: a deliberate choice, or generic bad
 practice. In the end, it doesn't particularly matter. As long as we make
 common sense decisions for the two schools: when is a developer error is
 fine, and when does core really need to not run into an issue here?

 The problem with casting false to an array is it does not end up with
 `array()`. '''<== This is important.''' You end up with `array( 0 => false
 )`. The only thing casting to an array helps with, is if the filter
 accidentally does not return anything, as in null, as casting null to an
 array does result in an empty array. Generally, casting makes more sense
 for an indexed array (just values) than it would for an associative array,
 as the key is likely important, while casting doesn't give you a key.

 In this case, the issue isn't devs doing it wrong (very little evidence of
 that here). And given the previous few paragraphs, casting in this case
 doesn't make a lot of sense.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/22587#comment:8>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software