[theme-reviewers] THEME: Suffusion - 3.5.4

Simon Prosser pross at pross.org.uk
Sun Jul 4 11:51:41 UTC 2010


On 04/07/2010 04:17, Sayontan Sinha wrote:
> In any case, I did resubmit version 3.5.5 after removing the deprecated
> API calls.
> 
> Sayontan.
> 
> On Sat, Jul 3, 2010 at 6:52 PM, Sayontan Sinha <sayontan at gmail.com
> <mailto:sayontan at gmail.com>> wrote:
> 
>     Simon,
>     This is exactly what I am trying to explain to you - the big picture.
> 
>     Take the options loop for example:
> 
>     foreach ($options as $value) {
>            if (!isset($suffusion_options[$
> 
>         value['id']])) {
>                $$value['id'] = $value['std'];
>            }
>            else {
>                        $$value['id'] = $suffusion_options[$value['id']];
>            }
>         }
> 
> 
>     $options is a very big array that comes from theme-options.php,
>     where *each* of the 400 odd elements with an id has an "std" value.
>     The entire options framework is predicated upon this foundation (for
>     the last 10 months). You will never get the so-called error. Having
>     your extra condition will cause it no difference - your example is
>     more lines of code that will give you no advantage. You approach
>     this from the angle that if $value['std'] is not set (which will
>     never happen in that array), $$value['id'] will not get set, while
>     the code in theme-options.php ensures that $value['std'] will be
>     present for anything with an id. If it isn't, my theme will have
>     bigger problems to handle than a debug message.
> 
>     You are trying to split hairs between isset and is_null. I am simply
>     making use of the fact that if something is not set, its value is
>     still null (though that will give you a notice, but only if you have
>     debug turned on - never otherwise). It doesn't damage the code in
>     any way and it surely doesn't affect how the theme runs. All these
>     arrays that you are asking me to write conditions around, these are
>     arrays that I control, not something that a user defines. Hence I
>     know what is in them - checks such as this will only increase cycle
>     times in so many cases without adding value. The next thing you
>     know, you will ask me to check for isset around different global
>     variables, because all my global variables are initiated through the
>     above code and if I have an "isset" around $$value['id'], there is a
>     high likelihood that a global variable will not be initialized
>     according to you - a condition, which according to me will never
>     occur in the theme.
> 
>     I don't know how else to explain this - it is such a simple concept
>     that should raise no eyebrows, yet you seem to be bent upon trying
>     to find flaws in it.
> 
>     Sayontan.
> 
> 
>     On Sat, Jul 3, 2010 at 6:12 PM, Simon Prosser <pross at pross.org.uk
>     <mailto:pross at pross.org.uk>> wrote:
> 
>         On 04/07/2010 01:09, Sayontan Sinha wrote:
>         > Pick the very first one for example, which says that there is
>         no index
>         > called "#horizontal-outer-widgets-1 a:hover/color" in
>         > theme-definitions.php. That is incorrect information and you
>         really need
>         > to see the code to see how it works. This particular index has
>         been
>         > defined multiple times, and there is an inheritance array
>         defined, so if
>         > the variable is not defined in one place it picks it from the
>         parent etc.
>         first few examples:
>         if ($style_settings[$mapped_style] != null) {
>         $style_settings[$mapped_style] is not defined so you get the Notice.
> 
>         if ( isset($style_settings[$mapped_style]) ) {
>         does the same thing with no errors.
> 
> 
>         else {
>                $parent = $style_settings["parent"];
>         $style_settings["parent"] is empty and causing the Notice, so
>         again we
>         check:
> 
> 
>         else {
> 
>                if ( isset( $style_settings["parent"] ) ) { $parent =
>         $style_settings["parent"]; }
> 
>         Most of the admin section errors come from this section:
> 
>         foreach ($options as $value) {
>                if (!isset($suffusion_options[$value['id']])) {
>                $$value['id'] = $value['std'];
>            }
>            else {
>                        $$value['id'] = $suffusion_options[$value['id']];
>            }
>         }
> 
>         which can be written as:
> 
>         foreach ($options as $value) {
>         if ( isset( $value['id'] ) ) {
>                if (!isset($suffusion_options[$value['id']])) {
>                if ( isset( $value['std'] ) ) {
>                $$value['id'] = $value['std'];
>                }
>            }
>            else {
>                        $$value['id'] = $suffusion_options[$value['id']];
>            }
>                }
>         }
> 
>         The snippet above is used in numerous files to get the theme
>         options and
>         is responsible for 99% of the notices displayed.
> 
>         --
>         MyBlog : http://www.pross.org.uk/
>         Plugins : http://www.pross.org.uk/plugins/
>         Themes: http://wordpress.org/extend/themes/profile/pross
> 
> 
> 
> 
>     -- 
>     Sayontan Sinha
>     http://mynethome.net | http://mynethome.net/blog
>     --
>     Lake Chargoggagoggmanchauggagoggchaubunagungamaugg - 45-letter
>     Native American name for Lake Webster, Webster, Massachusetts,
>     meaning "You fish on your side; I fish on my side; nobody fish in
>     the middle."
> 
> 
> 
> 
> -- 
> Sayontan Sinha
> http://mynethome.net | http://mynethome.net/blog
> --
> Lake Chargoggagoggmanchauggagoggchaubunagungamaugg - 45-letter Native
> American name for Lake Webster, Webster, Massachusetts, meaning "You
> fish on your side; I fish on my side; nobody fish in the middle."

Thanks for removing the deprecated functions. I'll make sure Cais or
Joseph review this new version as soon as possible.

-- 
MyBlog : http://www.pross.org.uk/
Plugins : http://www.pross.org.uk/plugins/
Themes: http://wordpress.org/extend/themes/profile/pross


More information about the theme-reviewers mailing list