А не много ли кода для такого модуля?

[dimitrius]

Пишу модуль для преста шоп, пытаюсь использовать их МВЦ модель. Модуль должен добавить две колонки к таблице и потом вставлять в них данные. Первую часть по вставке написал, но мне кажется, что очень много кода или это нормально? Прошу тех кто работает в продакшене взглянуть и сказать, так как я любитель, но хочу научится делать как надо.

<?php
if (!defined('_PS_VERSION_'))
  exit;
class AttributesFeatures extends Module {
	public function __construct(){
		$this->version = 0.1;
		$this->name = 'AttributesFeatures';
		$this->displayName = $this->l('Attributes Features');
		$this->description = $this->l('Add features for attributes like: visible, excludable etc.');
		$this->author = 'myOwn';
		$this->tab = 'administration';
		$this->need_instance = 0;
		
		parent::__construct();
	}
	public function install(){
		if(parent::install())
			return true;
		return false;
	}
	public function uninstall(){
		if(parent::uninstall())
			return true;
		return false;
	}
	public function getContent(){
		$columnExist = Db::getInstance()->getRow('SELECT SQL_NO_CACHE * FROM '._DB_PREFIX_.'attribute_group',false);
		if(!array_key_exists('hide_on_product', $columnExist) || !array_key_exists('subtract_options', $columnExist)){
			if((int)(Tools::getValue('submitAlterTable'))===1){
				$result = 1;
				if(!array_key_exists('hide_on_product', $columnExist))
					$result = Db::getInstance()->execute('ALTER TABLE `'._DB_PREFIX_.'attribute_group` 
							ADD `hide_on_product` TINYINT(1) DEFAULT 0 NOT NULL');
				if(!$result)
					return $this->erroreMsg(1);
				if(!array_key_exists('subtract_options', $columnExist))
					$result = Db::getInstance()->execute('ALTER TABLE `'._DB_PREFIX_.'attribute_group`
							ADD `subtract_options` TINYINT(1) DEFAULT 0 NOT NULL');
				if(!$result)
					return $this->erroreMsg(2);
				return $this->succsesMsg(1);
			}
			else
				return $this->displayIntro();
		}
	}
	private function formAction(){
		$adminTabs = new AdminModules();
		global $currentIndex;
		return $href = $currentIndex.'&configure='.urlencode($this->name).'&token='
				.$adminTabs->token.'&module_name='.urlencode($this->name).'&tab_module='.$this->tab;
	}
	private function displayIntro(){
		global $smarty;
		$smarty->assign('action', $this->formAction());
		$smarty->assign('moduleNmae', $this->displayName);
		return $this->display(__FILE__, 'intro.tpl');
	}
	private function erroreMsg($id = 0){
		switch ($id){
			case 1:
				$msg = 'Couldn\'t create column `hide_on_product`';
				break;
			case 2:
				$msg = 'Couldn\'t create column `subtract_options`';
				break;
			default:
				$msg = 'Unexpected error';
		}
		global $smarty;
		$smarty->assign('action', $this->formAction());
		$smarty->assign('msg', $msg);
		return $this->display(__FILE__, 'inform.tpl');
	}
	private function succsesMsg($id = 0){
		switch ($id){
			case 1:
				$msg = 'Additional columns are created';
				break;
			default:
				return $this->erroreMsg();
		}
		global $smarty;
		$smarty->assign('action', $this->formAction());
		$smarty->assign('msg', $msg);
		return $this->display(__FILE__, 'inform.tpl');
	}
}

П.С. И очень ли плохо выходить за рамки MVC системы CMS?

 

[hell0w0rd]

Добротный говнокод

    public function install(){
        if(parent::install())
            return true;
        return false;
    }
    public function uninstall(){
        if(parent::uninstall())
            return true;
        return false;
    }

А можете пояснить какой смысл в этих функциях?

 

[dimitrius]

А можете пояснить какой смысл в этих функциях?

extends Module
модуль должен устанавливаться - тоесть записывать инфу о себе в БД и для установки и отображения результата установки используются эти функции

 

[Вурдалак]

Ещё один безнадёжный.

 

[dimitrius]

Ещё один безнадёжный.

?? - но ведь все когда-то начинали.

 

[Redjik]

А можете пояснить какой смысл в этих функциях?

made my day =)

это к индусам надо репостнуть

dimitrius
молодец - учись

 

[hell0w0rd]

extends Module
модуль должен устанавливаться - тоесть записывать инфу о себе в БД и для установки и отображения результата установки используются эти функции

вы допустили две нелепых ошибки:

if ($var) {
    return true;
}
return false;

Очевидно что это можно преобразовать в

return $var;

Ну и внутри переопределяемой функции вы ничего не делаете, кроме того что вызываете родительскую, если убрать вышеупомянутый бред.
Таким образом вы можете смело убрать эти две функции
и вообще, если админка заставляет вас использовать global - валите с этой админки, это хуже чем весь ваш плагин

 

[dimitrius]

вы допустили две нелепых ошибки:
Очевидно что это можно преобразовать в

return $var;

Спасибо.

и вообще, если админка заставляет вас использовать global - валите с этой админки, это хуже чем весь ваш плагин

?? почему? - это не критичные переменные, чем то напоминают куки. Это глобальные в скрипте, а не регистр глобал.

 

[hell0w0rd]

dimitrius
потому что в любой части скрипта вы можете случайно переопределить одну из таких переменных, или какой-то плагин ее переопределит, а вы этого не заметите, пока дебагом не пройдетесь по всему скрипту. Это антипаттерн, а их нужно избегать, тут даже обсуждать нечего