• Jetzt anmelden. Es dauert nur 2 Minuten und ist kostenlos!

Code nicht wiederholen

D

Darkman

Neues Mitglied
Hallo,

ich habe eine Verständnisfrage bzw. würde gerne wissen, ob es eine "bessere" Lösung für mein Problem gibt. Ich habe eine Klasse, in der sich zwei Funktionen befinden, die jeweils E-Mails mit Swiftmailer versenden. Einzige Unterscheidung ist jeweils das einzulesende Template, die E-Mailadresse, an die die Mail geht und die Parameter, die ersetzt werden sollen:

PHP: 
class A
{
    public function aAction()
    {
        $msg = file_get_contents('file.html');
        $msg = str_replace(
            '${name}',
        ), array(
            $lastname,
        ), $msg);

        // Send message
        $transport = \Swift_MailTransport::newInstance();
        $message   = \Swift_Message::newInstance();

        $message
            ->setSubject('Titel bzw. Betreff')
            ->setFrom(array(EMAIL_RETURN_PATH))
            ->setReturnPath(EMAIL_RETURN_PATH)
            ->setTo(array($email))
            ->setBody($msg, 'text/html');

        // Send email
        $mailer = \Swift_Mailer::newInstance($transport);
        $mailer->send($message);
    }

    public function bAction()
    {
        $msg = file_get_contents('file2.html');
        $msg = str_replace(
            '${street}',
        ), array(
            $street,
        ), $msg);

        // Send message
        $transport = \Swift_MailTransport::newInstance();
        $message   = \Swift_Message::newInstance();

        $message
            ->setSubject('Titel bzw. Betreff')
            ->setFrom(array(EMAIL_RETURN_PATH))
            ->setReturnPath(EMAIL_RETURN_PATH)
            ->setTo(array($email))
            ->setBody($msg, 'text/html');

        // Send email
        $mailer = \Swift_Mailer::newInstance($transport);
        $mailer->send($message);
    }
}

Ich hatte vor, eine Wrapper-Klasse zu schreiben, um den wiederholenden Code noch weiter zu kapseln, weil sich die Zeilen an weiteren Stellen wieder und wieder gleichen würden. Meine Frage lautet einfach: ist mein Weg der richtige oder macht man das anders? Was habe ich für (bessere / schönere) Alternativen? Natürlich könnte ich da so lassen, aber ändere ich etwas an einer Stelle, muss ich alle anderen auch anpassen. Dem möchte ich bereits jetzt vorbeugen.

Ich hoffe der Code ist verständlich genug. Ich kann nicht alles hier reinkopieren.

Vielen Dank.
 
Wundert mich, dass hier noch niemand etwas geschrieben hat.

Es ist relativ einfach. Du hast in jeder Funktion nur 3 unterschiedliche Texte stehen. (Fest definierte Variablen in Klassen sind immer sehr verpöhnt.)

'file.html' und 'file2.html'
'${name}' und '${street}'
$lastname und $street

Wenn du jetzt definitiv nur eine Variable in deinen Templates hast und auch immer nur eine Template Datei, dann würde ich diese beiden sachen einfach so ersetzen.

PHP: 
class A{
  public function sendMail ($sFile, $sVar, $mVal) {
    $msg=file_get_contents($sFile);
    $msg=str_replace('${' . $sVar . '}', $mVal, $msg);

    // und hier alles was unter dem str_replace in deiner Funktion steht.
  }

  // wenn du mehrere verschiedene Mails senden willst, lohnt es sich auch noch, Das erstellen des Mail objektes in den construktor auszulagern
}

Ich werde dir nicht den ganzen Code hier hin schreiben da du die ganzen Variablen die in deinem Code nicht richtig benannt sind selber reparieren sollst. :) Ist nicht so schwer.

Wenn du denkst das es nicht zu viel für dich ist, habe ich hier auch noch eine Template Klasse welche man auch wunderbar für E-Mail Templates und alle anderen arten von Templates verwenden kann.
http://www.html.de/threads/php-code-verkuertzen.51074/#post-351758

lg
 
Hallo,

vielen Dank für Deine Antwort.

Wundert mich, dass hier noch niemand etwas geschrieben hat.
Zum Vergrößern anklicken....
Das ist nicht weiter schlimm. Besser wenige, interessante Antworten auf lange Sicht als viele unnötige in sehr kurzer Zeit.

Wenn du jetzt definitiv nur eine Variable in deinen Templates hast und auch immer nur eine Template Datei, dann würde ich diese beiden sachen einfach so ersetzen.
Zum Vergrößern anklicken....
Da habe ich mich zu meinem Bedauern falsch ausgedrückt. Derzeit werden die E-Mails in nur einer Klasse versendet. Später sollen auch E-Mails aus anderen Klassen heraus versendet werden, auch mit unterschiedlichen und mehreren Parametern. Und meine Intention das zu lösen war eine Klasse zu schreiben, die diesen Code kapselt. Ungefähr so:

PHP: 
class Mail
{
    private $templateFile;
    private $vars;

    public function setTemplate($templateFile)
    {
        $this->templateFile = $templateFile;
    }

    // Array in folgender Form:
    // array(
    //    '{placeholder}' => 'value'
    //)
    public function setVars($vars)
    {
        $this->vars = $vars;
    }

    public function send()
    {
        $msg = file_get_contents($this->templateFile);
        $msg = str_replace(array_keys($this->vars)), $this->vars, $msg);
      
        // Send message
        $transport = \Swift_MailTransport::newInstance();
        $message   = \Swift_Message::newInstance();
      
        $message
            ->setSubject('Titel bzw. Betreff')
            ->setFrom(array(EMAIL_RETURN_PATH))
            ->setReturnPath(EMAIL_RETURN_PATH)
            ->setTo(array($email))
            ->setBody($msg, 'text/html');
      
        // Send email
        $mailer = \Swift_Mailer::newInstance($transport);
        $mailer->send($message);
    }
}

Die Nutzung der Klasse würde dann ungefähr so aussehen:

PHP: 
class A
{
    // $app wird injiziert
    public function aAction(Application $app)
    {
        $mail = $app['Mail']; // Instanz der Klasse erstellen, gleicht $msg = new ...
        $mail->setTemplate('file.html');
        $mail->setVars(array(
            '{firstname}' => 'Vorname',
            '{lastname}'  => 'Nachname',
            [...]
        ));
        $mail->send(); // Entsprechender Rückgabetype, usw.
    }
}

Ich werde dir nicht den ganzen Code hier hin schreiben da du die ganzen Variablen die in deinem Code nicht richtig benannt sind selber reparieren sollst. :) Ist nicht so schwer.
Zum Vergrößern anklicken....
Nein, das brauchst Du nicht, das schaffe ich selbst. Kannst Du mir erläutern, was DU damit meinst, dass die Variablen nicht richtig benannt sind? Meinst Du, dass sie nicht zum Typ passen, was rein geschrieben wird?

Wenn du denkst das es nicht zu viel für dich ist, habe ich hier auch noch eine Template Klasse welche man auch wunderbar für E-Mail Templates und alle anderen arten von Templates verwenden kann.
Zum Vergrößern anklicken....
Danke, das werde ich mit Beizeiten ansehen.

Macht meine Idee Sinn? Ich möchte meine Kenntnisse unbedingt weiter professionalisieren und es "best of practice" umsetzen und denke, dass jemand mit bedeutend mehr Erfahrung sagen kann, ob das der richtige Weg ist. Der Code ist nur so angerissen, kann sein, dass dort Fehler enthalten sind, weil das kein Code ist, der so irgendwo existiert.

Du hast mir auf jeden Fall schon mal weiter geholfen. Vielleicht kannst Du das nochmal tun oder gerne auch weitere Meinungen.

Gruß
 
Darkman schrieb:
Nein, das brauchst Du nicht, das schaffe ich selbst. Kannst Du mir erläutern, was DU damit meinst, dass die Variablen nicht richtig benannt sind? Meinst Du, dass sie nicht zum Typ passen, was rein geschrieben wird?
Zum Vergrößern anklicken....

Damit meine ich nur das in der Klasse "A" die du gepostet hast auf Variablen zugegriffen wurde die nicht vorhanden waren, und das der Code somit nicht funktionieren kann. So sieht es schon viel besser aus.

Wenn du deinen Code so schreiben willst, dass der möglichst wiederverwendet werden kann, würde ich die Templates noch mit einer (meiner Meinung nach natürlich mit meiner) Template Klasse erstellen. Die Nachrichten würden dann ungefähr so aussehen.

Nachricht (/root/templates/mail/pay.txt.php)
PHP: 
<?= $this->anrede ?>,

Vielen Dank für ihren Einkauf oder sonst irgendeine Nachricht.

Bitte überweisen Sie bis zum <?= $this->due ?> den Betrag von <?= $this->amount ?> an folgendes Konto.

... usw

Und der Code würde dann so aussehen (ich verwende die von mir erstellte Klasse als beispiel)
PHP: 
$oTplPay = new Template('/root/templates/mail/pay.txt.php');
$oTplPay->anrede = 'Sehr geehrter Herr Mustermann';
$oTplPay->due    = '01.03.2014';
$oTplPay->amount = '123,45 €';
$sMessage = $oTplPay->fetch(); // nachricht parsen

Nun musst du $sMessage nur noch an deine Mail Klasse übergeben.

Wenn du mit der Klasse übrigens mehrere verschiedenne Mails versenden willst würde ich noch folgendes Hinzufügen

PHP: 
protected $oMailer = null;

public function __construct () {
  $this->oMailer = \Swift_Mailer::newInstance(
    \Swift_MailTransport::newInstance()
  );
}

Und Wenn du mehrere gleiche Emails an mehrere Kunden schicken willst würde ich nach einer möglichkeit suchen die Empfänger als BCC zu setzen, dann musst du nur eine einzige Mail verschicken, keiner der Empfänger wird wissen das du die E-Mail an mehrere Leute gesendet hast und das verteilen übernimmt dein Mailserver. :)

lg
 
Damit meine ich nur das in der Klasse "A" die du gepostet hast auf Variablen zugegriffen wurde die nicht vorhanden waren, und das der Code somit nicht funktionieren kann. So sieht es schon viel besser aus.
Zum Vergrößern anklicken....
Okay, das war mir bewusst. Ich hatte nur angedeuteten Code gepostet, der so in keiner Form irgendwo bei mir existierte. Es handelte sich nicht um Code, den ich aus meinem Projekt kopiert hatte. Aber dennoch danke, dass Du auf sowas achtest.

Wenn du deinen Code so schreiben willst, dass der möglichst wiederverwendet werden kann, würde ich die Templates noch mit einer (meiner Meinung nach natürlich mit meiner) Template Klasse erstellen. Die Nachrichten würden dann ungefähr so aussehen.
Zum Vergrößern anklicken....
Das habe ich bereits gemacht. Als Template-Engine nutze ich allerdings Twig. Daher orientieren sich auch die Platzhalter an jener Syntax, da zu jeder E-Mail auch ein Direktlink existiert (Alternative, falls der Nutzer die E-Mail aufgrund von Einstellungen nur als Text empfängt oder die E-Mail nicht korrekt dargestellt werden kann). Ich verstehe natürlich, dass Du Deine Klassen weiter empfehlen möchtest :)

Und Wenn du mehrere gleiche Emails an mehrere Kunden schicken willst würde ich nach einer möglichkeit suchen die Empfänger als BCC zu setzen, dann musst du nur eine einzige Mail verschicken, keiner der Empfänger wird wissen das du die E-Mail an mehrere Leute gesendet hast und das verteilen übernimmt dein Mailserver.
Zum Vergrößern anklicken....
Danke für die Hinweise. Ich werde mir das vormerken, falls es zu solch Situationen kommen sollte. Derzeit werden nur einzelne Mails zur Aktivierung von Kundenkonten oder Ähnlichem versendet.

Im Ganzen würde ich sagen, ist das Thema erledigt. Ich weiß wie ich die Sache anpacken werde und wurde in meinem Vorhaben bestätigt bzw. verbessert. Danke für die Mühen die Du Dir gemacht hast.
 
Du musst dich einloggen oder registrieren, um hier zu antworten.

Neueste Beiträge

Zurück
Oben