Mail::sendMail()   C
last analyzed

Complexity

Conditions 14
Paths 48

Size

Total Lines 55

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 0
CRAP Score 210

Importance

Changes 0
Metric Value
dl 0
loc 55
ccs 0
cts 46
cp 0
rs 6.2666
c 0
b 0
f 0
cc 14
nc 48
nop 3
crap 210

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
(!cfip()) ? header('HTTP/1.1 401 Unauthorized') : 0;
3
4
class Mail extends Base {
5
  /**
6
  * Mail form contact site admin
7
  * @param senderName string senderName
8
  * @param senderEmail string senderEmail
9
  * @param senderSubject string senderSubject
10
  * @param senderMessage string senderMessage
11
  * @param email string config Email address
12
  * @param subject string header subject
13
  * @return bool
14
  **/
15
  public function contactform($senderName, $senderEmail, $senderSubject, $senderMessage) {
16
    $this->debug->append("STA " . __METHOD__, 4);
17 View Code Duplication
    if (empty($senderEmail) || !filter_var($senderEmail, FILTER_VALIDATE_EMAIL)) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
18
      $this->setErrorMessage($this->getErrorMsg('E0023'));
19
      return false;
20
    }
21
    if (strlen(strip_tags($senderMessage)) < strlen($senderMessage)) {
22
      $this->setErrorMessage($this->getErrorMsg('E0024'));
23
      return false;
24
    }
25
    $aData['senderName'] = $senderName;
0 ignored issues
show
Coding Style Comprehensibility introduced by
$aData was never initialized. Although not strictly required by PHP, it is generally a good practice to add $aData = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
26
    $aData['senderEmail'] = $senderEmail;
27
    $aData['senderSubject'] = $senderSubject;
28
    $aData['senderMessage'] = $senderMessage;
29
    $aData['email'] = $this->setting->getValue('website_email', '[email protected]');
0 ignored issues
show
Bug introduced by
The property setting does not exist. Did you maybe forget to declare it?

In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:

class MyClass { }

$x = new MyClass();
$x->foo = true;

Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion:

class MyClass {
    public $foo;
}

$x = new MyClass();
$x->foo = true;
Loading history...
30
    $aData['subject'] = 'Contact Form';
31
      if ($this->sendMail('contactform/body', $aData)) {
32
        return true;
33
     } else {
34
       $this->setErrorMessage( 'Unable to send email' );
35
       return false;
36
     }
37
    return false;
0 ignored issues
show
Unused Code introduced by
return false; does not seem to be reachable.

This check looks for unreachable code. It uses sophisticated control flow analysis techniques to find statements which will never be executed.

Unreachable code is most often the result of return, die or exit statements that have been added for debug purposes.

function fx() {
    try {
        doSomething();
        return true;
    }
    catch (\Exception $e) {
        return false;
    }

    return false;
}

In the above example, the last return false will never be executed, because a return statement has already been met in every possible execution path.

Loading history...
38
  }
39
40
  /**
41
   * Send a mail with templating via Smarty and Siftmailer
42
   * @param template string Template name within the mail folder, no extension
43
   * @param aData array Data array with some required fields
44
   *     subject : Mail Subject
45
   *     email   : Destination address
46
   **/
47
  public function sendMail($template, $aData, $throttle=false) {
48
    // Prepare SMTP transport and mailer
49
    $transport_type = $this->config['swiftmailer']['type'];
0 ignored issues
show
Bug introduced by
The property config does not exist. Did you maybe forget to declare it?

In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:

class MyClass { }

$x = new MyClass();
$x->foo = true;

Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion:

class MyClass {
    public $foo;
}

$x = new MyClass();
$x->foo = true;
Loading history...
50
    if ($transport_type == 'sendmail') {
51
      $transport = Swift_SendmailTransport::newInstance($this->config['swiftmailer'][$transport_type]['path'] . ' ' . $this->config['swiftmailer'][$transport_type]['options']);
52
    } else if ($this->config['swiftmailer']['type'] == 'smtp') {
53
      $transport = Swift_SmtpTransport::newInstance($this->config['swiftmailer']['smtp']['host'], $this->config['swiftmailer']['smtp']['port'], $this->config['swiftmailer']['smtp']['encryption']);
54
      if (!empty($this->config['swiftmailer']['smtp']['username']) && !empty($this->config['swiftmailer']['smtp']['password'])) {
55
        $transport->setUsername($this->config['swiftmailer']['smtp']['username']);
56
        $transport->setPassword($this->config['swiftmailer']['smtp']['password']);
57
      }
58
    }
59
    $mailer = Swift_Mailer::newInstance($transport);
0 ignored issues
show
Bug introduced by
The variable $transport does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
60
61
    // Throttle mails to x per minute, used for newsletter for example
62
    if ($this->config['swiftmailer']['type'] == 'smtp' && $throttle) {
63
      $mailer->registerPlugin(new Swift_Plugins_ThrottlerPlugin(
64
        $this->config['swiftmailer']['smtp']['throttle'], Swift_Plugins_ThrottlerPlugin::MESSAGES_PER_MINUTE
65
      ));
66
    }
67
68
    // Prepare the smarty templates used
69
    $this->smarty->clearCache(TEMPLATE_DIR . '/mail/' . $template . '.tpl');
70
    $this->smarty->clearCache(TEMPLATE_DIR . '/mail/subject.tpl');
71
    $this->smarty->assign('WEBSITENAME', $this->setting->getValue('website_name'));
72
    $this->smarty->assign('SUBJECT', $aData['subject']);
73
    $this->smarty->assign('DATA', $aData);
74
75
    // Create new message for Swiftmailer
76
    $senderEmail = $this->setting->getValue('website_email', '[email protected]');
77
    $senderName = $this->setting->getValue('website_name', '[email protected]');
78
    $message = Swift_Message::newInstance()
79
      ->setSubject($this->smarty->fetch(TEMPLATE_DIR . '/mail/subject.tpl'))
80
      ->setFrom(array( $senderEmail => $senderName))
81
      ->setTo($aData['email'])
82
      ->setSender($senderEmail)
83
      ->setReturnPath($senderEmail)
84
      ->setBody($this->smarty->fetch(TEMPLATE_DIR . '/mail/' . $template . '.tpl'), 'text/html');
85
    if (isset($aData['senderName']) &&
86
        isset($aData['senderEmail']) &&
87
        strlen($aData['senderName']) > 0 &&
88
        strlen($aData['senderEmail']) > 0 &&
89
        filter_var($aData['senderEmail'], FILTER_VALIDATE_EMAIL))
90
      $message->setReplyTo(array($aData['senderEmail'] => $aData['senderName']));
91
92
    // Send message out with configured transport
93
    try {
94
      if ($mailer->send($message)) return true;
95
    } catch (Exception $e) {
96
      $this->setErrorMessage($e->getMessage());
97
      return false;
98
    }
99
    $this->setErrorMessage($this->sqlError('E0031'));
100
    return false;
101
  }
102
}
103
104
// Make our class available automatically
105
$mail = new Mail ();
106
$mail->setDebug($debug);
107
$mail->setMysql($mysqli);
108
$mail->setSmarty($smarty);
109
$mail->setConfig($config);
110
$mail->setSetting($setting);
111
$mail->setErrorCodes($aErrorCodes);
112