dziękuje za wszelkie porady, uwagi. zabieram się do poprawek.

proszę również o dalsze komentarze.

Odłożyłem na chwile stronicowanie, potrzeba chwili wymogła na mnie zrobienie newslettera. Nie chce zakładać kolejnego tematu więc pokaże go tu. Jest to najprostsze wysyłanie maila, tymczasowo bez nagłówków. Powiedzcie mi czy idę w dobrą stronę.

newsletter-class.php

[PHP] pobierz, plaintext 
<?php
class Newsletter
{
    public function GetMailsFromFile($file, $separator)
    {
        $string = file_get_contents($file);
        $mails_file = explode($separator, $string);
        return $mails_file;
    }
    public function GetMailsFromDb($sql)
    {
        $sql_query = mysql_query($sql);
        while ($rows = mysql_fetch_row($sql_query))
        {
            $mails_db[] = $rows[0];
        }
        return $mails_db;
    }
    public function Send($where, $subject, $message)
    {
        foreach($where as $mails)
        {
            mail($mails, $subject, $message);
        }
    }
}
?>
[PHP] pobierz, plaintext 

index.php

[PHP] pobierz, plaintext 
<?php
require_once ('newsletter-class.php');
require_once ('mysql.php');
 
$topic = 'Test';
$msg = 'Wiadomość testowa';
 
$sql = 'SELECT mail FROM emails';
 
$news = new Newsletter();
$file = $news -> GetMailsFromFile('mail.txt', ';');
$db = $news -> GetMailsFromDb($sql);
// z pliku
$news -> Send($file, $topic, $msg);
//lub lub bazy
$news -> Send($db, $topic, $msg);
?>
[PHP] pobierz, plaintext 

Ogólnie idziesz w dobrą stronę ale poczytaj o tym, co to jest polimorfizm i jak można go wykorzystać. Konkretnie chodzi mi o metody:
- GetMailsFromFile
- GetMailsFromDb
Generalnie nie powinno się tak robić (tworzyć osobnych metod do pobierania danych z różnych źródeł). Zdecydowanie lepiej utworzyć obiekt typu Loader, który by pobierał dane w speclajny sposób, coś na wzór:

[PHP] pobierz, plaintext 
<?php
abstract class Loader {
    abstract public function load();
}
 
class Loader_File extends Loader {
    public function load() {
        // wczytaj adresy z pliku
    }
}
 
class Loader_Database extends Loader {
    public function load() {
        // wczytaj adresy z bazy danych
    }
}
?>
[PHP] pobierz, plaintext 

Klasa Newsletter mogłaby wyglądać mniej-więcej tak:

[PHP] pobierz, plaintext 
<?php
class Newsletter {
    private $loader;
    
    public function setLoader(Loader $loader) {
        $this->loader = $loader;
    }
    
    public function getMails() {
        return $this->loader->load();
    }
}
?>
[PHP] pobierz, plaintext 

oraz wykorzystanie:

[PHP] pobierz, plaintext 
<?php
$obj = new Newsletter();
$obj->setLoader(new Loader_File());
print_r($obj->getMails());
?>
[PHP] pobierz, plaintext 

Hmn... obsługa wyjątków, szczególnie przy pracy na pliku? Zawsze wydawało mi się, że nazewnictwo plików powinno wyglądać tak: class.Nazwaklasy.php, ale może to tylko moja konwencja

~phpion dzięki za przykład, uczę się dopiero i nie bardzo mi to wychodzi. zrobiłem coś takiego:

[PHP] pobierz, plaintext 
<?php
abstract class Loader 
{
    abstract public function load();
}
class Loader_File extends Loader 
{
    public function load() 
    {
        $string = file_get_contents($file);
        $mails_file = explode($separator, $string);
        print_r($mails_file);
    }
}
class Loader_Database extends Loader 
{
    public function load() 
    {
        $sql_query = mysql_query($sql);
        while ($rows = mysql_fetch_row($sql_query))
        {
            $mails_db[] = $rows[0];
        }
        print_r($mails_db);
    }
}
class Newsletter
{
    private $loader;
 
    public function setLoader(Loader $loader) 
    {
        $this->loader = $loader;
    }
 
    public function getMails() 
    {
        return $this->loader->load();
    }
}
?>
[PHP] pobierz, plaintext 

ale jest źle. dlaczego? ponieważ nie wiem gdzie mam podać nazwę pliku/query do bazy.

~Cypherq
wiem o nazwach narazie testuje wszytko, później będę wyjątki łapał. narazie chce to uruchomić.

#EDIT

jak podam na sztywno nazwy pliku/query działa.

#EDIT2
zrobiłem tak, powiedzcie co o tym sądzicie:

newsletter-class.php

[PHP] pobierz, plaintext 
<?php
abstract class Loader 
{
    abstract public function load();
}
class Loader_File extends Loader 
{
    public function __construct($file, $separator)
    {
        $this->file = $file;
        $this->sep = $separator;
    }
    public function load() 
    {
        $string = file_get_contents($this->file);
        $mails_file = explode($this->sep, $string);
        return $mails_file;
    }
}
class Loader_Database extends Loader 
{
    public function __construct($sql)
    {
        $this->sql = $sql;
    }
    public function load() 
    {
        $sql_query = mysql_query($this->sql);
        while ($rows = mysql_fetch_row($sql_query))
        {
            $mails_db[] = $rows[0];
        }
        return $mails_db;
    }
}
class Newsletter
{
    private $loader;
 
    public function setLoader(Loader $loader) 
    {
        $this->loader = $loader;
    }
 
    public function getMails() 
    {
        return $this->loader->load();
    }
    public function Send($where, $subject, $message)
   {
       foreach($where as $mails)
       {
           mail($mails, $subject, $message);
       }
   }
}
?>
[PHP] pobierz, plaintext 

index.php

[PHP] pobierz, plaintext 
<?php
require_once ('newsletter-class.php');
require_once ('mysql.php');
 
$topic = 'Test';
$msg = 'Wiadomość testowa';
 
$sql = 'SELECT mail FROM emails';
$f = 'mail.txt';
 
$news = new Newsletter();
$file = new Loader_File($f, ';');
$db = new Loader_Database($sql);
 
$news -> setLoader($file);
 
print_r($news -> getMails());
 
$news -> Send($news->getMails(), $topic, $msg);
?>
[PHP] pobierz, plaintext 

Wydzieliłem z http://forum.php.pl/index.php?showtopic=118120

Dziękować.

hmm..
1. jak będę posiadał 60k maili w bazie czy pliku ... To myślisz że wszystko gładko wyśle za jednym zamachem ?
2. Może warto byłoby zaopatrzyć maila w nagłówki i autoryzację smtp. przy obecnym stanie większość serwerów oznaczy to flagą SPAM

@piotrooo89 a dlaczego wymyślasz koło od nowa jest klasa phpMailer ma wszystko czego tobie potrzeba.

Może dlatego, że się chłopak uczy?

tak tak już o tym pomyślałem. dodam odpowiednie funkcje.



nagłóweczki już są choć narazie chce zrobić ogólny zarys klasy i zapytać innych o zdanie.



tak jak napisał ~phpion:



i chyba bardziej cieszy coś własnego

a wracając do tematu jak oceniacie? co zmienić, dodać?

aktualnie wygląda to tak:

newsletter.Class.php

[PHP] pobierz, plaintext 
<?php
abstract class Loader 
{
    abstract public function load();
    public $errors = 
        array(
        'no_file' => 'Plik nie istnieje',
        'wrong_separator' => 'Nie podano separatora',
        'wrong_sql' => 'Brak zapytania SQL',
        'wrong_sql_col' => 'Podaj kolumne z adresami e-mail',
        'wrong_sql_tab' => 'Podaj tabele SQL'
        );
}
class Loader_File extends Loader 
{
    public function __construct($file, $separator)
    {
        if (empty($separator))
            throw new Exception($this->errors['wrong_separator']);
 
        $this->file = $file;
        $this->sep = $separator;
    }
    public function load()    
    {
        if (!file_exists($this->file))
            throw new Exception($this->errors['no_file']);
            
            $string = file_get_contents($this->file);
            $mails_file = explode($this->sep, $string);
            return $mails_file;
    }
}
class Loader_Database extends Loader 
{
    public function __construct($col, $tab)
    {
        if (empty($col) && empty($tab))
            throw new Exception($this->errors['wrong_sql']);
        elseif (empty($col))
            throw new Exception($this->errors['wrong_sql_col']);
        elseif (empty($tab))
            throw new Exception($this->errors['wrong_sql_tab']);
            
        $this->col = $col;
        $this->tab = $tab;
    }
    public function load() 
    {
        $sql_query = mysql_query("SELECT `".$this->col."` FROM `".$this->tab."`");
        while ($rows = mysql_fetch_row($sql_query))
        {
            $mails_db[] = $rows[0];
        }
        return $mails_db;
    }
}
class Newsletter
{
    private $loader;
 
    public function setLoader(Loader $loader) 
    {
        $this->loader = $loader;
    }
    public function getMails() 
    {
        return $this->loader->load();
    }
    public function Send($where, $subject, $message, $head)
    {
        foreach($where as $mails)
        {
            mail($mails, $subject, $message, $head);
        }
    }
}
?>
[PHP] pobierz, plaintext 

index.php

[PHP] pobierz, plaintext 
<?php
require_once ('newsletter.Class.php');
require_once ('mysql.php');
 
$topic = 'Test';
$msg = 'Wiadomość testowa';
$headers .= "From: Olaszewski <piotroo89@gmail.com>r\n"; 
$headers .= "Reply-To: Olaszewski <piotroo89@gmail.com>r\n";
$headers .= "Return-Path: Olaszewski <piotroo89@gmail.com>r\n";
$headers .= 'X-Mailer: PHP/' . phpversion() ."r\n";
$headers .= 'MIME-Version: 1.0'. "\n"; 
$headers .= 'Content-type: text/html; charset=utf-8' . "r\n"; 
$headers .= "Content-transfer-encoding: utf-8r\n"; 
 
$column_sql = 'mail';
$table_sql = 'emails';
 
$f = 'mail.txt';
 
try
{
    $news = new Newsletter();
    $file = new Loader_File($f, ';');
    $db = new Loader_Database($column_sql, $table_sql);
    $news -> setLoader($db);
 
    print_r($news->getMails());
 
    $news -> Send($news->getMails(), $topic, $msg, $headers);
}
catch(Exception $e)
{
    $error = ($e->getMessage());
    echo $error;
}
?>
[PHP] pobierz, plaintext 

Moje 2 uwagi:

1. Loader::$errors - wg mnie komunikaty błędów powinny znajdować się w wyspecjalizowanych klasach, ponieważ teraz każda klasa pochodna będzie miała wszystkie komunikaty (po co np. w Loader_File komunikat wrong_sql?). Ponadto rozważyłbym zapis komunikatów w postaci stałych wewnątrz klasy (const).

2. Nie podoba mi się sposób dodawania nagłówków. Zrobiłbym to raczej w ten sposób:

[PHP] pobierz, plaintext 
<?php
class Newsletter {
    private $headers = array();
 
    //...
 
    public function addHeader($name, $value) {
        $this->headers[$name] = $value;
    }
}
?>
[PHP] pobierz, plaintext 

i potem przy wysyłaniu maila wystarczyłoby przekształcić tą tablicę na stringa (tak, jak to robisz w przykładzie). Dzięki temu rozwiązaniu możesz już w konstruktorze machnąć kilka domyślnych nagłówków (np. ustawić kodowanie oraz typ wysyłanego maila).

dzięki za kolejne rady. obecnie klasa wygląda tak:

newsletter.Class.php

[PHP] pobierz, plaintext 
<?php
abstract class Loader 
{
    abstract public function load();
}
class Loader_File extends Loader 
{
    const WRONG_FILE = 'Plik nie istnieje';
    const WRONG_SEPARATOR = 'Nie podano separatora';
    
    public function __construct($file, $separator)
    {
        if (empty($separator))
            throw new Exception(Loader_File::WRONG_SEPARATOR);
 
        $this->file = $file;
        $this->sep = $separator;
    }
    public function load()    
    {
        if (!file_exists($this->file))
            throw new Exception(Loader_File::WRONG_FILE);
            
            $string = file_get_contents($this->file);
            $mails_file = explode($this->sep, $string);
            return $mails_file;
    }
}
class Loader_Database extends Loader 
{
    const WRONG_SQL = 'Brak zapytania SQL';
    const WRONG_SQL_COL = 'Podaj kolumne z adresami e-mail';
    const WRONF_SQL_TAB = 'Podaj tabele SQL';
    
    public function __construct($col, $tab)
    {
        if (empty($col) && empty($tab))
            throw new Exception(Loader_Database::WRONG_SQL);
        elseif (empty($col))
            throw new Exception(Loader_Database::WRONG_SQL_COL);
        elseif (empty($tab))
            throw new Exception(Loader_Database::WRONF_SQL_TAB);
            
        $this->col = $col;
        $this->tab = $tab;
    }
    public function load() 
    {
        $sql_query = mysql_query("SELECT `".$this->col."` FROM `".$this->tab."`");
        while ($rows = mysql_fetch_row($sql_query))
        {
            $mails_db[] = $rows[0];
        }
        return $mails_db;
    }
}
class Newsletter
{
    private $loader;
    private $headers = array();
 
    public function setLoader(Loader $loader) 
    {
        $this->loader = $loader;
    }
    public function getMails() 
    {
        return $this->loader->load();
    }
    public function addHeaders($value)
    {
        $this->headers[] = $value;
    }
    public function getHeaders()
    {
        foreach($this->headers as $heads)
        {
            $show_heads .= $heads;
        }
        return $show_heads;
    }
    public function Send($where, $subject, $message, $head)
    {
        foreach($where as $mails)
        {
            mail($mails, $subject, $message, $head);
        }
    }
}
?>
[PHP] pobierz, plaintext 

index.php

[PHP] pobierz, plaintext 
<?php
require_once ('newsletter.Class.php');
require_once ('mysql.php');
 
$topic = 'Test';
$msg = 'Wiadomość testowa';
/*************
$headers .= "From: Olaszewski <piotroo89@gmail.com>r\n"; 
$headers .= "Reply-To: Olaszewski <piotroo89@gmail.com>r\n";
$headers .= "Return-Path: Olaszewski <piotroo89@gmail.com>r\n";
$headers .= 'X-Mailer: PHP/' . phpversion() ."r\n";
$headers .= 'MIME-Version: 1.0'. "\n"; 
$headers .= 'Content-type: text/html; charset=utf-8' . "r\n"; 
$headers .= "Content-transfer-encoding: utf-8r\n"; 
*************/
 
$column_sql = 'mail';
$table_sql = 'emails';
 
$f = 'mail.txt';
 
try
{
    $news = new Newsletter();
    $file = new Loader_File($f, ';');
    $db = new Loader_Database($column_sql, $table_sql);
    
    $news->setLoader($file);
    
    $news->addHeaders("From: Olaszewski <piotroo89@gmail.com>r\n");
    $news->addHeaders("Reply-To: Olaszewski <piotroo89@gmail.com>r\n");
    $news->addHeaders("Content-type: text/html; charset=utf-8r\n");
 
    $news -> Send($news->getMails(), $topic, $msg, $news->getHeaders());
}
catch(Exception $e)
{
    $error = ($e->getMessage());
    echo $error;
}
?>
[PHP] pobierz, plaintext 

czekam na dalsze rady (optymalizacja, poprawienie działania etc.).

Może

[PHP] pobierz, plaintext 
<?php
$news->addHeaders('Content-type', 'text/html; charset=utf-8');
?>
[PHP] pobierz, plaintext 

a \r\n dodawać automatycznie ?

Według mnie mógłbyś dać więcej metod typu

Albo jeszcze sprawdzać czy wiadomość nie jest pusta.

[PHP] pobierz, plaintext 
<?php
public function addHeaders($value)
   {
       $this->headers[] = $value;
   }
public function Send($where, $subject, $message)
   {
       foreach($where as $mails)
       {
           mail($mails, $subject, $message, $his->headers);
       }
   }
?>
[PHP] pobierz, plaintext 

Po co wstawiasz do metody kolejny parametr jak możesz je wyciągać wewnątrz klasy.
Tak samo jak message i where i subject i miałbyś

[PHP] pobierz, plaintext 
<?php
$news->send();
?>
[PHP] pobierz, plaintext 

Te komunikaty o błędach, zawsze myślałem, że wygodniej trzymać w osobnym pliku. Wtedy zmieniając język komunikatów nie grzebiemy w klasie, a chyba o to chodzi?