Bad code implementation
-
Too complexed classes, methods. Too many nested IF/Else statements and etc. The plugin is messy.
-
Hi @skarabeq,
Thanks for your comment.
We are continuously looking into new ways of improving the code of our plugins as you can read in this post. In v14.0 we introduced the indexables feature that aims at improving site speed significantly. We also have a developers portal where we provide detailed information on implementation decisions and coding choices. Of course, new coding and performance improvements will come in the future.
That said, if you come across any piece of code in the plugin that you think it could be better implemented, we’d really appreciate it if you could point it out to our developers by creating a new issue here: https://github.com/Yoast/featurerequests/issues
Thanks!
Hi, @monbauza
Thank you for your response!
I can give you an example of a too complicated code. For example when I download the plugin, in this file –wordpress-seo/src/generated/container.php
I saw nested short IF statements. For example, the return statement of methodgetPaginationHelperService
is on one row, but it has a big IF statement which is too nested:protected function getPaginationHelperService() { return $this->services['Yoast\\WP\\SEO\\Helpers\\Pagination_Helper'] = new \Yoast\WP\SEO\Helpers\Pagination_Helper(${($_ = isset($this->services['Yoast\\WP\\SEO\\Wrappers\\WP_Rewrite_Wrapper']) ? $this->services['Yoast\\WP\\SEO\\Wrappers\\WP_Rewrite_Wrapper'] : ($this->services['Yoast\\WP\\SEO\\Wrappers\\WP_Rewrite_Wrapper'] = new \Yoast\WP\SEO\Wrappers\WP_Rewrite_Wrapper())) && false ?: '_'}, ${($_ = isset($this->services['Yoast\\WP\\SEO\\Wrappers\\WP_Query_Wrapper']) ? $this->services['Yoast\\WP\\SEO\\Wrappers\\WP_Query_Wrapper'] : ($this->services['Yoast\\WP\\SEO\\Wrappers\\WP_Query_Wrapper'] = new \Yoast\WP\SEO\Wrappers\WP_Query_Wrapper())) && false ?: '_'}); }
Another example in this file
wordpress-seo/src/generators/schema/organization.php
:The original code of the method
fetch_social_profiles
is:private function fetch_social_profiles() { $profiles = []; $social_profiles = [ 'facebook_site', 'instagram_url', 'linkedin_url', 'myspace_url', 'youtube_url', 'pinterest_url', 'wikipedia_url', ]; foreach ( $social_profiles as $profile ) { $social_profile = $this->helpers->options->get( $profile, '' ); if ( $social_profile !== '' ) { $profiles[] = urldecode( $social_profile ); } } $twitter = $this->helpers->options->get( 'twitter_site', '' ); if ( $twitter !== '' ) { $profiles[] = 'https://twitter.com/' . $twitter; } /** * Filter: 'wpseo_schema_organization_social_profiles' - Allows filtering social profiles for the * represented organization. * * @api string[] $profiles */ $profiles = \apply_filters( 'wpseo_schema_organization_social_profiles', $profiles ); return $profiles; }
But it can be refactored to this one:
private function fetch_social_profiles() { $profiles = []; $social_profiles = [ 'facebook_site', 'instagram_url', 'linkedin_url', 'myspace_url', 'youtube_url', 'pinterest_url', 'wikipedia_url', 'twitter_site' ]; foreach ( $social_profiles as $profile ) { $social_profile = $this->helpers->options->get( $profile, '' ); if ( $social_profile !== '' ) { $prefix = ($profile === 'twitter_site') ? 'https://twitter.com/' : ''; $profiles[] = urldecode( $prefix . $social_profile ); } } /** * Filter: 'wpseo_schema_organization_social_profiles' - Allows filtering social profiles for the * represented organization. * * @api string[] $profiles */ $profiles = \apply_filters( 'wpseo_schema_organization_social_profiles', $profiles ); return $profiles; }
I can give you other examples, but I think this is enough to understand what I mean.
Cheers,
- This reply was modified 4 years, 5 months ago by skarabeq.
Hi @skarabeq,
Thanks for taking the time to provide examples. I’ll pass this information along to our developers so that they can review it.
- The topic ‘Bad code implementation’ is closed to new replies.