Правилно ли е написан този клас?

vik96 · 1 Декември 2010

Код:

<?

class MassMail 
{
public $text;
public $zaglavie;
private $mysql = false;
private $table;
private $username;
private $emailpole;
private $header = 'From: Google.bg';
private $vreme = 3;


public function SetInfo($table,$username,$emailpole,$baza)
		{
		$this -> mysql = $baza;
		if ( $this->mysql == null ) { return $this->Error(); }
		$this->table = $table;
		$this->username = $username;
		$this->emailpole = $emailpole;
		}
		
public function Email($text,$zaglavie,$from)
		{
		$this->header = ( isset($from) ) ? "From: $from" : $this->header;
		$this->zaglavie = $zaglavie;
		$this->text = $text;
		return $this->DoQuery();
		}
		
public function Interval($vreme)
		{
		if ( is_numeric($vreme) )
			{
			$this->vreme = $vreme;
			}
		}

private function DoQuery()
		{
		if ( $this->mysql == false ) { return $this->Error(); }
		$send = $this->mysql->query("SELECT `$this->username`,`$this->emailpole` FROM `$this->table`");
	
		while ( $send_mail = $send -> fetch_assoc() )
			{
			 mail($send_mail[$this->emailpole],$this->zaglavie,$this->text,$this->header);
			sleep($this->vreme);
			}
		}
		
public function Error()
		{
		echo "Няма Връзка с БАЗА ДАННИ!";
		}
}


$mysqli = new mysqli("localhost", "users", "parola", "saita");
$mal = new MassMail;
$mal -> SetInfo("tablica","POLE_POTREBITEL","Email",$mysqli);
$mal -> Interval(5);
$mal -> Email("Kvo stava","Lqlqlqlqlql","Google.bg");
?>

Правилно ли е написах този клас? И ако не е може ли да ми кажете къде са ми грешките да не ги допускам в бъдеще

Karcho · 2 Декември 2010

Аз мисля, че е правилен, но нека да се изкаже някой по-опитен

shterev · 2 Декември 2010

за php 4 или php 5 ?

vik96 · 2 Декември 2010

shterev каза:
за php 4 или php 5 ?

5

SveXteZ · 2 Декември 2010

Аз си мисля че ще е по-добре да си направиш една функция validate и една errors. При validate ще са ти всичките проверки вместо да са пръснати по други функции и ако errors не върне нищо значи няма грешки и може да се изпрати. В errors да се записват грешките от validate. Setinfo да се замени с __construct и validate да се изпълнява в email.
Предполагам че го написах доста объркано де, но в момента нямам много време. Ако някой друг не ти е отговорил като се върна ще ти го направя в писмен вид.

bgspace · 2 Декември 2010

Класът не е написан правилно, защото не отговяря на изискванията. Не се притеснявай но виждам, че не си се знимавал с по-строго обекно ориентирани езици и за това е така. Първо конекцията към базата ??? защо я задаваш като пропърти на MassMail не трябва. Инстанцираш си конецията към базата и след това когато използваш връзка към базата в самия метод правиш така:

Код:

private function DoQuery()
      {
        global $mysqli;
      if ( !$mysql ) return false;
      $send = $mysql->query("SELECT `$this->username`,`$this->emailpole` FROM `$this->table`");
   
      while ( $send_mail = $send -> fetch_assoc() )
         {
          mail($send_mail[$this->emailpole],$this->zaglavie,$this->text,$this->header);
         sleep($this->vreme);/* тази нишка за какво ти е тука сложена безмислено слага се само ако има паралелен процес.
Как на линееен алгоритам правиш тази глупост.
*/
         }
      }

Втората ми забележка е защо не работиш с масиви, а за всяко нещо дефинираш порпърти ????

Код:

public $text;
public $zaglavie;
private $header = 'From: Google.bg';
// трябва да е така
private $mailbody = array();
//и в конструктора правиш така
$this->mailbody = array(
"header"=>"From:Google.bg"
, "zaglavie" =>""
, "text" => ""
);
//друга безмилена грешка ????
$this->header = ( isset($from) ) ? "From: $from" : $this->header; 
/*това е безмислено провереваш дали е сетнат задължителен параметър. Той винаги е сетнат трябва да го проверяваш за празно empty()
Просто както си си дефинирал фунцията с три параметъра ако не подадеш
един от тях и ще ти даде грешка и пак са безмислени защото на публично пропърти го претираш като частно ??? 
Най- нарая цялат тази бумащина трябва да се изпълни в конструктора ако се опира на ООП
*/

Малко съм критичен в упреците си но ако искаш да се научеш някой трябва да ти ги каже. Но още ти е рано за ООП ти правиш елементрани грешки на основно ниво. Не е достатъчно да направиш нещо като клас а да има смисъл от него. Ето ти тук аз съм писъл една дашбоард библиотечка: библиотека Може да си я свалиш има и подробна документация към нея. Описал сам я на PHP 5 и енджийна е метод фабрика. Ако имаш някакви запитвания може да ми пиеш ЛС.

SveXteZ · 2 Декември 2010

Не претендирам, че е много правилен класа, но смятам че така е по-добре.
Всичко, което колегата каза преди мен е хубаво да го прочетеш, но избягвай да ползваш global, просто забрави че го има !
Накрая можеш и сам да си го допишеш

Преди да почнеш да си пишеш сам класовете разгледай чужди класове и начинът им на работа. Има си сайтове за php класове и се намират полезни, пробвай и codeigniter например, за да можеш да ползваш ActiveRecods ( по-удобно нещо от това не съм срещал до сега ).
Просто прочети малко повече преди да почнеш сам да пишеш, защото ще се получи така че ще пишеш обектно базирани неща и ще свикнеш с тях и после трудно ще отвикнеш.
ПП: Класа не съм пробвал дали работи, така че не се нахвърляйте веднага ...
ПП2: не ползвай echo за излизане от функция или като цяло във функция. Когато почнеш да работиш с някой template ( препоръчвам ти smarty ) ще е доста кофти.

Код:

<?php

class MassMail {
    private $maildata;
    private  $errors;

    function __construct($table,$username,$emailpole, $from, $text, $zaglavie, $vreme){
        
        $this->errors = array();
        $this->maildata = array();
        $this->maildata['table'] = $table;
        $this->maildata['username'] = $username;
        $this->maildata['emailpole'] = $emailpole;
        $this->maildata['header'] = $from;
        $this->maildata['text'] = $text;
        $this->maildata['zaglavie'] = $zaglavie;
        $this->maildata['vreme'] = $vreme;
    }

    private function addError($errorMsg){
        $this->errors = $errorMsg;
    }

    function showErrors(){
        return $this->errors;
    }

    private function validate(){
      if(empty($this->maildata['header'])){
          $this->maildata['header'] = 'From: Google.bg';// тук можеш и с константа
      }

      global $mysqli;
      if(!$mysqli){
          $this->addError('няма връзка с ДБ');
      }

      if(!is_numeric($this->maildata['vreme'])){
          $this->addError('Времето не е в числов формат');
      }
    }

    function sendMails(){
        if(!empty($this->errors)){
            return false;
            //След като направиш инстанция ако ти върне false изпълняваш showErrors и виждаш кои са грешките
        }
        // и тук си правиш за пращането на email-a, взимането от дб и т.н.
    }
}
?>

vik96 · 2 Декември 2010

Благодаря много на отзовалите се

Направих някой промени и сега как е?

Код:

<?

class MassMail
{
private $maildata = array("header" => "From: YourWebSite.com");


function __construct($table,$username,$emailpole)
   {
    $this->maildata['table'] = $table;
    $this->maildata['username'] = $username;
    $this->maildata['emailpole'] = $emailpole;
    }

public function Email($text,$zaglavie,$from)
   {
   if ( empty($from) || empty($text) || empty($zaglavie) ) { return $this->Error(); }
   $this->maildata['text'] = $text;
    $this->maildata['zaglavie'] = $zaglavie;
   $this->maildata['header'] = "From: $from";
   return $this->DoQuery();
   }
   
private function DoQuery()
   {
   global $mysql;
      $send = $mysql->query("SELECT `".$this->maildata['username']."`,`".$this->maildata['emailpole']."` FROM `".$this->maildata['table']."`");
      while ( $send_mail = $send->fetch_assoc() )
         {
       mail($send_mail[$this->maildata['emailpole']],$this->maildata['zaglavie'],$this->maildata['text'],$this->maildata['header']);
        print_r($send_mail);
         }
   }
   
public function Error()
      {
      echo "Error..";
      }
}


$mysql = new mysqli("localhost", "viktor", "", "");
$mal = new MassMail("таблица","усер","мейл");

$mal -> Email("Kvo staa","yep","Google.bg");
$mal -> Email("Nice a?","yep","Google.bg");
?>

bgspace · 2 Декември 2010

Ако трябва да е близко до ООП трябва да е подобно на това:

Код:

<?php
define("MSG_E", "MSG_ERROR");
define("MSG_W", "MSG_WARNING");
define("MSG_I", "MSG_INFO");
define("MSG_DB", "MSG_DB_ERROR");
$mysql = new mysqli("localhost", "viktor", "", "");

class clsMailBase {
	
	private $mailList = array();
	private $err = array();
	private $maildata = array();
	
	public function __construct() {
		$this->defineMailData();
		$this->getMailList();
		$this->sendMail();
	}//__construct()
	
	public function defineMailData() {} //abstract
	
	protected function defineParam($Name, $Desc, $Type, $params = array()) {
		$this->maildata[$Name] = array(
			"name" => $Name
			, "description" => $Desc
			, "type" => $Type
			, "min" => isset($params["min"])?$params["min"]:NULL
			, "max" => isset($params["max"])?$params["max"]:NULL
			, "default" => isset($params["default"])?$params["default"]:NULL
			, "recommended" => isset($params["recommended"])?$params["recommended"]:false
			, "required" => isset($params["required"])?$params["required"]:false
			, "custom" => isset($params["custom"])?$params["custom"]:NULL
		);
	}//defineParam()
	
	public function paramSelected($Name) {
		return (isset($this->maildata[$Name]["name"])? $this->maildata[$Name]["name"]: false);
	}//paramSelected()
	
	public function getParamValue($Name) {
		return (isset($this->maildata[$Name])? $this->maildata[$Name]["default"]: false);
	}//getParamValue()
	
	private function getMailList() {
		global $mysql;
		
		$send = $mysql->query("SELECT `idXX`,`".$this->paramSelected('username')."`,`".$this->paramSelected('emailpole')."` FROM `".$this->paramSelected('table')."`");
		while ($send_mail = $send->fetch_assoc()) {
			$this->mailList[$send_mail["idXX"]] = $send_mail;
		}
		
	}//getMailList()
	
	private function sendMail() {
		foreach ($this->mailList as $key => $param) {
			mail($param[$this->paramSelected('emailpole')],$this->getParamValue('zaglavie'),$this->getParamValue('text'),$this->getParamValue('header'));
		}
	}//sendMail()
	
	public function msg_err($msg, $type = "MSG_E") {
		$this->err[$type][] = $msg;
	}//msg_err
} //clsMailBase

class clsMassMail extends clsMailBase {
	public function __construct() {
		parent::__construct();
	}//__construct()
	
	public function defineMailData() {
		$this->defineParam("username", "username ala bala", "string",array("max" => "60", "required" => true));
		$this->defineParam("emailpole", "emailpole ala bala", "string",array("max" => "120", "required" => true));
		$this->defineParam("table", "table ala bala", "string",array("max" => "60", "required" => true));
		$this->defineParam("zaglavie", "zaglavie ala bala", "string",array("default" => "ala bala", "recommended" => true));
		$this->defineParam("header", "header ala bala", "string",array("max" => "160", "default" => "From: YourWebSite.com"));
		$this->defineParam("text", "text ala bala", "string", array("default" => "ala bala"));
	}
}


$mail = &new clsMassMail();

StormBreaker · 2 Декември 2010

Само ми е интересно защо никой не спомена exceptions.

Правилно ли е написан този клас?

vik96

Registered

Karcho

Registered

shterev

Registered

vik96

Registered

SveXteZ

Registered

bgspace

Registered

SveXteZ

Registered

vik96

Registered

bgspace

Registered

StormBreaker

Registered