Класс на обсуждение - учусь

Redjik

Джедай-мастер
PHP:
<?php

/**
 * Обертка для создания 1го запроса ко всем полям.
 * Ограничение в 63 Join никто не отменял
 * => максимум 63 тв поля
 * @author Иван aka Redjik
 *
 */
class DB_wrapper {
	
	private $modx;
	private $db;
	protected static $instance;
	public $criteria;
	
	/**
	 * @param DocumentParser $modx
	 */
	private function __construct(DocumentParser $modx){
		
		$this->modx = $modx;
		$this->db = $modx->db;
	}
	
	public static function init(DocumentParser $modx){
		
		if ( is_null(self::$instance) ) {
			self::$instance = new DB_wrapper($modx);
		}
		self::$instance->criteria = null;
		return self::$instance;
	}
	
	/**
	 * Returns all records accroding to $where criteria.
	 * @param int $template_id
	 * @param array $where
	 * @return array
	 * NB! excludes all inappropriate chars in TV names
	 */
	public function getAll($template_id){
		
		$modx = $this->modx;
		
		$tvs = $this->getTVsFromTemplate($template_id);
		
		$tvSQL = $this->prepareTvsForSQL($tvs);
		
		$whereSQL = $this->prepareCriteria();
		
		$sql = 'SELECT main.*'.$tvSQL['selectString'].' FROM '.$modx->getFullTableName('site_content').'AS main';
		$sql .= $tvSQL['joinString'];
		$sql .= $whereSQL;
		
		$result = $modx->db->query($sql);
		
		self::$instance->criteria = null;
		return $modx->db->makeArray($result);
		
	}
	
	/**
	 * Get all tvs for selected template
	 * @param int|string $template_id
	 * @return array
	 */
	public function getTVsFromTemplate($template_id){
		
		
		$modx = $this->modx;
		$sql = 'SELECT * FROM '.$modx->getFullTableName('site_tmplvars').' 
				LEFT JOIN '.$modx->getFullTableName('site_tmplvar_templates').'ON id = tmplvarid
				WHERE templateid = '.(int)$template_id;
		
		$result = $modx->db->query($sql);
		
		return $modx->db->makeArray($result);
		
	}
	
	
	/**
	 * Prepares tvs for SQL
	 * @param array $tvs
	 * @return multiline:string
	 * Returns two string fields with 'selectString' and 'joinString' keys...
	 * Strings are passed in MySQL syntax style. 
	 * NB! excludes all inappropriate chars in TV names
	 */
	private function prepareTvsForSQL(array $tvs){
		$modx = $this->modx;
		
		$selectString = '';
		$joinString = '';
		if (count($tvs)){
			foreach ($tvs as $tv){
				
			    $tv['name'] = preg_replace('#[^A-Za-z0-9]#U', '', $tv['name']);
				$selectString .= ', tv'.$tv['id'].'.value AS '.$tv['name'];
				$joinString .= ' LEFT JOIN '.$modx->getFullTableName('site_tmplvar_contentvalues').' AS tv'.$tv['id'].' ON
					(tv'.$tv['id'].'.tmplvarid = '.$tv['id'].' AND main.id = tv'.$tv['id'].'.contentid) ';
				
				
			}
		}
		return array('selectString'=>$selectString, 'joinString'=>$joinString);
		
		
	}
	
	/**
	 * Prepares criteria for SQL
	 */
	private function prepareCriteria(){
		
		$where = ' WHERE ';
		
		$criteria = $this->criteria;
		if (count($criteria)){
			
			foreach ($criteria as $key => $groupArrays){
				
				if ($key != 0){
					
					$where.= '(';
					
				}
										
				foreach ($groupArrays as $secondKey => $groupArray){
					
					if ($key == 0 && $secondKey == 0){
						
						$where.= ' '.$groupArray[0];
						
					} else {
						
						$where.= ' '.$groupArray[1].' '.$groupArray[0];
					}
					
				}
				
				if ($key != 0){
						
					$where.= ')';
						
				}
				
			}
			
		}else{
			
			return $where.' 1';
			
		}
		
		return $where;
		
	}
	
	/**
	 * Adds where condition for SELECT
	 * @param string $string Condition to seacrh
	 * @param string $conjunction  AND|OR
	 * @param int $group  For grouping
	 * @return DB_wrapper
	 */
	public function addCondition($string,$conjunction = 'AND',$group = 0){
		
		
			$this->criteria[$group][]=array($string,$conjunction);
		
		
		return self::$instance;
	}
	
	
	
	
	
}
Сразу оговорюсь, что у меня на criteria пробил творческий кризис, принцип работы я переделаю =)
Есть несколько вопросов...

1) Правильно ли я оформляю комментарии.
2) В статичных методах можно обращаться только к статичным свойствам?
3) Если ответ на второй вопрос да ... то вот это
PHP:
self::$instance->criteria = null;
единственный возможный вариант обнулить свйоство, через инициализацию одиночки?
 

~WR~

Новичок
Подумалось...
Когда мне присылают кусочки кода перед собеседованиями, первым делом нажимаю Ctrl + F и ищу слово 'escape'. Если не нашел - не смотрю дальше.

Все, что динамически подставляется в запрос, должно фильтроваться на уровне драйвера БД. Даже если пришло из супер-пупер "доверенного" источника. Я искренне не понимаю, почему 2\3 людей об этом напрочь забывает, если учесть, что в любой книге по PHP это написано едва ли не капсом и по десять раз.

2) В статичных методах можно обращаться только к статичным свойствам?
К нестатичным оно и не даст. Скажет, что $this там нет.
 

tz-lom

Продвинутый новичок
~WR~
т.е. если присылают код без работы с базой или c работой через ORM вы его тоже не смотрите? :)
 

Redjik

Джедай-мастер
Сразу оговорюсь, что у меня на criteria пробил творческий кризис, принцип работы я переделаю =)
Какбе там будет escape =) Спасибо - учту =)
А в целом можно какую-нибудь оценку, правильным путем я иду?
 

Alien85

I like my cat
Иван Redjik Матвеев
Для чего этот класс нужен? По-моему, если использовать его для выборок, то мягко говоря, он кажется тяжеловатым и не читаемым.

Подумалось...
Когда мне присылают кусочки кода перед собеседованиями, первым делом нажимаю Ctrl + F и ищу слово 'escape'. Если не нашел - не смотрю дальше.
Наверное, если есть escap-ы, сразу отказываете? (ORM)
 

c0dex

web.dev 2002-...
Команда форума
Партнер клуба
~WR~
А если там плейсхолдеры?
 

~WR~

Новичок
Да отстаньте, я образно написал :p
Смотрю, фильтруются ли переменные хоть как-то. У половины $_GET идет сразу в SQL-запрос.

Один раз взяли человека, который так делал. Подумали, что научится, если нормально объяснить.
Хрен там. Вместо этого прямо на входе в контроллерах на каждую переменную вызывался pg_escape_string. Потом еще иногда в моделях несколько раз. Типа, чтобы я отстал со своим экранированием. А потом еще базу чистили от многократно задваивающихся кавычек.

Я считаю, лучше никого не взять вовсе, чем потом вычищать всё это месиво.
 

Redjik

Джедай-мастер
Иван Redjik Матвеев
Для чего этот класс нужен? По-моему, если использовать его для выборок, то мягко говоря, он кажется тяжеловатым и не читаемым.
Для сложной выборки... Попробую в двух словах обьяснить... Движок modx при выгрузке все информации по одной странице делает два запроса... В таблицу site_content и в таблицу с доп полями site_template_var.

То же самое он делает для групп ресурсов... например мне нужно 10 статей и у каждой есть тв поле - он делает 11 запросов... 1 для контента и потом циколм дополнительные поля...

Я классом свел все к 2м запросам... Даже не класс, а просто оберточка, для сложного запроса...
 

AmdY

Пью пиво
Команда форума
1. DB_wrapper - DB_Wrapper
2. поставь тип для переменных, а то не будет работать автокомплит
/**
* @var ClassName
*/
private $modx;
3. В Одиночке следует закрывать ещё метод __clone
4. а Одиночка и прочая статика здесь точно нужна?

p.s. Я в первую очередь рассматриваю аккуратность, если написан бред, но аккуратно и последовательно, то из человека может что-то получиться.
p.p.s. Иван Redjik Матвеев я видел. теперь в ModX для tv можно делать хелперы, типа выбора картинки из галлереи, а можно ли теперь эти хелперы делать самому или только предустановленные?
 

iceman

говнокодер
Иван Redjik Матвеев
правила оформления кода прочти в доке по Zend Framework

~WR~
а как же Magic Quotes? я их в .htaccess файле включал, иначе бы система не работала.
 

Redjik

Джедай-мастер
AmdY
4) Нет, не особо нужно, просто решил потренироваться =) сейчас как раз разорился на Гамма Хелм Джонсон Влассидес, вот и пробую ... =)

По тв: можно и раньше было, с напильником - теперь без напильника через @FILE вроде (в ветке EVO, в Revo сразу можно было)
 
Сверху