Constant use of static of code. Why not loop so that it can be dynamic?

Quote

Post by Qphoria » Tue Aug 19, 2008 11:07 pm

I've been noticing that some files have this constant reuse of the exact same code with static variable specified in each. Is there a reason for that?

A good example is the admin/controller/setting.php file

It has a lot of this:

Code: Select all

        foreach ($results as $result) {
             $setting_info[$result['type']][$result['key']] = $result['value'];
        }                
                
        if ($request->has('global_config_store', 'post')) {
			$view->set('global_config_store', $request->get('global_config_store', 'post'));
		} else {
			$view->set('global_config_store', @$setting_info['global']['config_store']);
		}

		if ($request->has('global_config_owner', 'post')) {
			$view->set('global_config_owner', $request->get('global_config_owner', 'post'));
		} else {
			$view->set('global_config_owner', @$setting_info['global']['config_owner']);
		}

		if ($request->has('global_config_address', 'post')) {
			$view->set('global_config_address', $request->get('global_config_address', 'post'));
		} else {
			$view->set('global_config_address', @$setting_info['global']['config_address']);
		}

		if ($request->has('global_config_telephone', 'post')) {
			$view->set('global_config_telephone', $request->get('global_config_telephone', 'post'));
		} else {
			$view->set('global_config_telephone', @$setting_info['global']['config_telephone']);
		}

		if ($request->has('global_config_fax', 'post')) {
			$view->set('global_config_fax', $request->get('global_config_fax', 'post'));
		} else {
			$view->set('global_config_fax', @$setting_info['global']['config_fax']);
		}

		if ($request->has('catalog_config_url_alias', 'post')) {
			$view->set('catalog_config_url_alias', $request->get('catalog_config_url_alias', 'post'));
		} else {
			$view->set('catalog_config_url_alias', @$setting_info['catalog']['config_url_alias']);
		}
It's the exact same code over and over with static calls to variables. Now if you add a new setting, you have to add a new static call, as well as add the template setting and the template call.

I've replaced all of that by making it dynamic inside the existing for loop at the top:

Code: Select all

foreach ($results as $result) {
			$setting_info[$result['type']][$result['key']] = $result['value'];
            $combo = $result['type'] . "_" . $result['key'];
            if ($request->has($combo, 'post')) {
                $view->set($combo, $request->get($combo, 'post'));
            } else {
                $view->set($combo, @$setting_info[$result['type']][$result['key']]);
            }
		}
That's like 5 pages of static code done in one loop. I can't see any reason why they were all specified manually. Can anyone clue me in on this?
Last edited by Qphoria on Tue Aug 19, 2008 11:56 pm, edited 1 time in total.

Image

User avatar
Qphoria
Administrator
Posts
20059
Joined
Tue Jul 22, 2008 3:02 am
Top

Re: Constant use of Static of code. Why not loop so that it can be dynamic?

Quote

Post by Qphoria » Tue Aug 19, 2008 11:52 pm

Same goes for the delete from and re-insert into the db

From this (static):

Code: Select all

if (($request->isPost()) && ($this->validate())) {
          $database->query("delete from setting where `group` = 'config'");
			

          $database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_store', `value` = '?'", $request->get('global_config_store', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_owner', `value` = '?'", $request->get('global_config_owner', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_address', `value` = '?'", $request->get('global_config_address', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_telephone', `value` = '?'", $request->get('global_config_telephone', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_fax', `value` = '?'", $request->get('global_config_fax', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_template', `value` = '?'", $request->get('catalog_config_template', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_max_rows', `value` = '?'", $request->get('catalog_config_max_rows', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_url_alias', `value` = '?'", $request->get('catalog_config_url_alias', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_parse_time', `value` = '?'", $request->get('catalog_config_parse_time', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_ssl', `value` = '?'", $request->get('catalog_config_ssl', 'post')));
			$database->query($database->parse("insert into setting set type = 'admin', `group` = 'config', `key` = 'config_template', `value` = '?'", $request->get('admin_config_template', 'post')));
			$database->query($database->parse("insert into setting set type = 'admin', `group` = 'config', `key` = 'config_max_rows', `value` = '?'", $request->get('admin_config_max_rows', 'post')));
			$database->query($database->parse("insert into setting set type = 'admin', `group` = 'config', `key` = 'config_parse_time', `value` = '?'", $request->get('admin_config_parse_time', 'post')));
			$database->query($database->parse("insert into setting set type = 'admin', `group` = 'config', `key` = 'config_ssl', `value` = '?'", $request->get('admin_config_ssl', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_country_id', `value` = '?'", $request->get('global_config_country_id', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_zone_id', `value` = '?'", $request->get('global_config_zone_id', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_language', `value` = '?'", $request->get('global_config_language', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_currency', `value` = '?'", $request->get('global_config_currency', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_weight_class_id', `value` = '?'", $request->get('global_config_weight_class_id', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_tax', `value` = '?'", $request->get('global_config_tax', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_order_status_id', `value` = '?'", $request->get('global_config_order_status_id', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_stock_check', `value` = '?'", $request->get('catalog_config_stock_check', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_stock_checkout', `value` = '?'", $request->get('catalog_config_stock_checkout', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_stock_subtract', `value` = '?'", $request->get('catalog_config_stock_subtract', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_vat', `value` = '?'", $request->get('catalog_config_vat', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_account_id', `value` = '?'", $request->get('catalog_config_account_id', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_checkout_id', `value` = '?'", $request->get('catalog_config_checkout_id', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_email', `value` = '?'", $request->get('global_config_email', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_email_send', `value` = '?'", $request->get('global_config_email_send', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_cache_query', `value` = '?'", $request->get('global_config_cache_query', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_compress_output', `value` = '?'", $request->get('global_config_compress_output', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_compress_level', `value` = '?'", $request->get('global_config_compress_level', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_image_resize', `value` = '?'", $request->get('global_config_image_resize', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_image_width', `value` = '?'", $request->get('global_config_image_width', 'post')));
			$database->query($database->parse("insert into setting set type = 'global', `group` = 'config', `key` = 'config_image_height', `value` = '?'", $request->get('global_config_image_height', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_download', `value` = '?'", $request->get('catalog_config_download', 'post')));
			$database->query($database->parse("insert into setting set type = 'catalog', `group` = 'config', `key` = 'config_download_status', `value` = '?'", $request->get('catalog_config_download_status', 'post')));

			$session->set('message', $language->get('text_message'));

			$response->redirect($url->ssl('setting'));
		}
to this (dynamic):

Code: Select all

        if (($request->isPost()) && ($this->validate())) {
			$results = $database->getRows("select * from setting where `group` = 'config'");
          $database->query("delete from setting where `group` = 'config'");
            
			foreach ($results as $result) {
                $combo = $result['type'] . "_" . $result['key'];
                $database->query($database->parse("insert into setting set type = '" . $result['type'] . "', `group` = 'config', `key` = '" . $result['key'] . "', `value` = '?'", $request->get($combo, 'post')));   
          }
          
          $session->set('message', $language->get('text_message'));

			$response->redirect($url->ssl('setting'));
		}
Now you can actually add new settings to the setting menu with a simple sql insert. A tpl change is still required (at this time) but that can actually be added to the db too. (i.e. fieldtype = "option", "text", "combo", etc) and that can be generated.

At least for now you don't have to muck with the settings controller file.

Image

User avatar
Qphoria
Administrator
Posts
20059
Joined
Tue Jul 22, 2008 3:02 am
Top

Re: Constant use of static of code. Why not loop so that it can be dynamic?

Quote

Post by ogun » Wed Aug 20, 2008 3:44 am

i guess it's the same as a lot of things. the long, inflexible method can sometimes be the fastest way to get a thing out there - especially when you're working on you own. categories, the cart, menu systems, reports for the admin panel, payment gateways,.. they're all big jobs in their own right and a bit of copypasta can be the difference between having something you can use today or waiting indefinitely.
Daniel wrote: OK the way I have set the next version up is very simple coding. It uses design patterns and MVC. This means its going to be easier code with and to add new features.

After v0.8 is released let other community members into the development so we can build up the feature list.
with 0.8, people in the community should be able to start contributing to opencart, giving Daniel breathing room to concentrate on refactoring and improvements to specific elements.

wandering off-topic, new ways to improve opencart are appearing on these forums all the time. off the top of my head, there's:
  • refactoring the code
  • making the admin menu db driven
  • implementing an override system so that the core can remain untouched
  • support for popular payment gateways like authorize.net and worldpay
  • moving the SQL out to its own file(s) or tidying it up with some other method
  • making sure that the 'core' can be interacted with in a variety of ways (e.g. a soap service for resellers).
the new few point releases/major mods and contributions are going to be good to see :)

in the meantime, you could submit your changes to the bug tracker for 0.7.8 as an improvement (..if google-code works like that, i'll have to check).
ogun
Active Member
Posts
189
Joined
Tue Aug 14, 2007 6:04 am
Top

Re: Constant use of static of code. Why not loop so that it can be dynamic?

Quote

Post by Qphoria » Wed Aug 20, 2008 4:24 am

ogun wrote:off the top of my head, there's:
  • refactoring the code
  • making the admin menu db driven
  • implementing an override system so that the core can remain untouched
  • support for popular payment gateways like authorize.net and worldpay
  • moving the SQL out to its own file(s) or tidying it up with some other method
  • making sure that the 'core' can be interacted with in a variety of ways (e.g. a soap service for resellers).
Well I like those for sure ;D ;D ;D

Image

User avatar
Qphoria
Administrator
Posts
20059
Joined
Tue Jul 22, 2008 3:02 am
Top
Locked
Return to “Development”
Who is online

Users browsing this forum: No registered users and 44 guests