[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