Was haltet ihr davon und was könnte man anders Lösen

[hmmNaGut]

Die Idee was ich habe mit dem Model, stammt natürlich nicht von mir,
aber wenn ich soetwas schreiben wolltet wie würdet ihr das Lösen,

Es soll auch kein Framework oder sowas darstellen,
sonder mehr oder weniger soll es nach dem ich immer
mit Frameworks arbeite mal wieder plain PHP sein, jedoch will ich die Ansätze trotzdem nutzen und da ich eingefleischt CAKEPHP fan bin.. naja man sieht es was dabei rauskommt...



############

Wie gefällt euch der Code und was würdet ihr anders machen..:

Das ding kann derzeit nicht viel, speichern und nach einem Datensatz suchen,
bin noch beim Ausbau

Gehört zu einem kleinen Kalenderscript das ich gerne schreiben würde...

Bitte eventuell auf unsichere teile
aufmerksam machen nicht escaped ...

Lg. Patrick

<?php
	Class AppointmentModel{
		var $table = 'appointments';
		var $mysql = null;
				
		var $querys = array(
			'save'=>'INSERT INTO %table% (%fields%) VALUES (%values%)',
			'select'=>'SELECT %fields% FROM %table% WHERE %conditions%  1=1 %group% %having% %orderby%  %% %limit%',
			
		);

		function __construct(&$mysql){
			$this->mysql = $mysql;
		}

		// speichern für einzeleinträge
		public function save($data, $save_fields=array()){
			if(empty($data)){
				return array();
			}
			
			$fields = array();
			$values = array();
						
			if(isset($data[$this->table])){
				foreach($data[$this->table] as $field=>$value){					
					$value = '\''. $this->mysql->real_escape_string($value) .'\'';
					if(!empty($save_fields) && array_search($field, $save_fields)){
						continue;
					}
					
					$fields[] = $field;
					$values[] = $value;
				}	
			}
			
			$queryOptions = array(
				'fields'=>$fields,
				'values'=>$values
			);
			
			$this->makeQuery('save', $queryOptions);
			
		}
		
		public function generateResultArray($result, $multiply = true){
			$data = array('_num_rows'=>$result->num_rows, '_field_count'=>$result->field_count);
			if(!$multiply){
				$data = array_merge($data, array($this->table => $result->fetch_assoc() ));
			}
			return $data;
		}
		
		public function findById($id, $options=array()){
			$command = 'select';
			
			if(!isset($options['fields']) && empty($options['fields'])){
				$options['fields'][] = '*';
			}
			
			$options['conditions']= array('id'=>$id);
			return $this->generateResultArray($this->makeQuery($command, $options), false);
		}

		public function findFirst($options=array()){
			$command = 'select';
			if(!isset($options['fields']) && empty($options['fields'])){
				$options['fields'][] = '*';

			}		
			$options['limit'] = 1;
			return $this->generateResultArray($this->makeQuery($command, $options), false);
		}		
		
		public function makeQuery($command, $options){
			$values="";
			$fields ="";
			$conditions = "";
			$extras ="";
			$limit = "";
			
			if(isset($options['fields'])){
				$fields = implode(',', $options['fields']);
			}
			
			if(isset($options['values'])){
				$values = implode(',', $options['values']);
			}
			
			if(isset($options['conditions'])){
				$conditions = $this->_parseConditions($options['conditions']);
			}
			
			
			if(isset($options['limit'])){
				if(is_array($options['limit'])){
					$limit = 'LIMIT '.implode(',', $options['limit']);
				}
				else{
					$limit = 'LIMIT ' . $options['limit'];
				}
			}
			
			$command = $this->querys[$command];
			$command= str_replace('%table%', $this->table, $command);
			$command= str_replace('%fields%', $fields, $command);
			$command= str_replace('%values%', $values, $command);	
			$command= str_replace('%conditions%', $conditions, $command);	
			$command= str_replace('%extras%', $extras, $command);
			$command= str_replace('%limit%', $limit, $command);	
			debug($command);		
			return  $this->mysql->query($command);
		}
		
		protected function _parseConditions($conditions, $type='AND'){
			$string = '';			
			foreach($conditions as $field => $condition){
				if($field == 'OR' || $field == 'AND'){
					if(is_array($condition)){
						$string .= $this->_parseConditions($condition, $field);
					}	
				}
				else{
						if(is_array($condition)){
							$string .= $field . ' IN (' . implode(',', $condition) .')' . ' ' . $type .' ';
						}
						else{
							$string .=$field . '=' .  '\''  . $this->mysql->real_escape_string($condition) .'\''. ' ' . $type .' ';
						}
				}
			}
			
			return $string;
		}
		
		protected function validate($data){
			return true;
		}
	}

 

[Yaslaw]

item: Ich würde vor allem den Code dokumentieren bevor ich ihn jemandem zum lesen gebe. Ansonsten raten wir hier ein wenig herum, was da warum überhaupt passieren soll...

item: Die Function validate() ist so überflüssig. Zuerst dachte ich, du hast ev. ein Interface oder eine Abstrakte Klasse die diese Funktion vorschreibt. Dem ist aber nicht so. Ergo weg damit.

item: In der Funktion make_query() solltest du beim $command='save' überprüfen ob die Anzahl fields mit der Anzahl values übereinstimmt.

item: Nochmals in der Funktion make_query(). Ich würde die veiel str_replace durch einen sauberen sprintf() ersetzen.

item: Für $conditions und $options erwartest du klar definerte Arrays. Mindestens dokumentieren solltest du diese, damit man beim Anwenden der Klasse nicht jedesmal den Code anschauen muss um herauszufinden was für Einträge da notwendig sind. Ansonsten kann an daraus auch einfache Klassen mit klar defienrten Properties machen.

Soviel mal auf die Schnelle. Das alles sind nur Ideen. Kein Muss und keine Wertung deines Codes.

 

[hmmNaGut]

Danke yaslaw,
Natürlich hast du recht mit dem kommentieren,
und das validate war eingeplant habe ich aber jetzt schon gekickt..

Danke für den Tipp mit dem command save, an das hatte ich nicht gedacht.

sprintf wollte ich nicht verwenden da ich mir nicht sicher war wegen der Reihenfolge.


Danke dir schon mal für die ersten Schläge

Mir ist klar die Klasse macht nicht sehr viel Sinn.. da es solche schon tausende gibt,
allerdings wollte ich es einfach mal machen

Lg Patrick

 

[Yaslaw]

Mir ist klar die Klasse macht nicht sehr viel Sinn.. da es solche schon tausende gibt,
allerdings wollte ich es einfach mal machen

Dann macht es sehr viel Sinn. Wenn man immer nur bestehende Klassen übernimmt, lernt man weniger als wenn man sich hinsetzt und sowas selber entwickelt. Ich habe sehr viele Klassen, Funktionen etc. entwickelt und trotzdem ist meine Webseite mit Drupal umgesetzt.
Aber gelernt habe ich viel dabei.