Completed
Push — master ( 987505...ce3c0b )
by Hamish
8s
created

TestMailer::sendHTML()   B

Complexity

Conditions 1
Paths 1

Size

Total Lines 24
Code Lines 19

Duplication

Lines 16
Ratio 66.67 %

Importance

Changes 2
Bugs 0 Features 0
Metric Value
cc 1
eloc 19
c 2
b 0
f 0
nc 1
nop 8
dl 16
loc 24
rs 8.9713

How to fix   Many Parameters   

Many Parameters

Methods with many parameters are not only hard to understand, but their parameters also often become inconsistent when you need more, or different data.

There are several approaches to avoid long parameter lists:

1
<?php
2
3
namespace SilverStripe\BehatExtension\Utility;
4
5
/**
6
 * Same principle as core TestMailer class,
7
 * but saves emails in {@link TestSessionEnvironment}
8
 * to share the state between PHP calls (CLI vs. browser).
9
 */
10
class TestMailer extends \Mailer
11
{
12
13
    /**
14
     * @var TestSessionEnvironment
15
     */
16
    protected $testSessionEnvironment;
17
18
    public function __construct()
19
    {
20
        $this->testSessionEnvironment = \Injector::inst()->get('TestSessionEnvironment');
21
    }
22
23
    /**
24
     * Send a plain-text email.
25
     * TestMailer will merely record that the email was asked to be sent, without sending anything.
26
     */
27 View Code Duplication
    public function sendPlain($to, $from, $subject, $plainContent, $attachedFiles = false, $customHeaders = false)
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in 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...
28
    {
29
        $this->saveEmail(array(
30
            'Type' => 'plain',
31
            'To' => $to,
32
            'From' => $from,
33
            'Subject' => $subject,
34
            'Content' => $plainContent,
35
            'PlainContent' => $plainContent,
36
            'AttachedFiles' => $attachedFiles,
37
            'CustomHeaders' => $customHeaders,
38
        ));
39
40
        return true;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return true; (boolean) is incompatible with the return type of the parent method Mailer::sendPlain of type string[]|false.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
41
    }
42
    
43
    /**
44
     * Send a multi-part HTML email
45
     * TestMailer will merely record that the email was asked to be sent, without sending anything.
46
     */
47 View Code Duplication
    public function sendHTML(
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in 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...
48
        $to,
49
        $from,
50
        $subject,
51
        $htmlContent,
52
        $attachedFiles = false,
53
        $customHeaders = false,
54
        $plainContent = false,
55
        $inlineImages = false
0 ignored issues
show
Unused Code introduced by
The parameter $inlineImages is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
56
    ) {
57
    
58
        $this->saveEmail(array(
59
            'Type' => 'html',
60
            'To' => $to,
61
            'From' => $from,
62
            'Subject' => $subject,
63
            'Content' => $htmlContent,
64
            'PlainContent' => $plainContent,
65
            'AttachedFiles' => $attachedFiles,
66
            'CustomHeaders' => $customHeaders,
67
        ));
68
69
        return true;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return true; (boolean) is incompatible with the return type of the parent method Mailer::sendHTML of type string[]|false.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
70
    }
71
    
72
    /**
73
     * Clear the log of emails sent
74
     */
75
    public function clearEmails()
76
    {
77
        $state = $this->testSessionEnvironment->getState();
78
        if (isset($state->emails)) {
79
            unset($state->emails);
80
        }
81
        $this->testSessionEnvironment->applyState($state);
82
    }
83
84
    /**
85
     * Search for an email that was sent.
86
     * All of the parameters can either be a string, or, if they start with "/", a PREG-compatible regular expression.
87
     *
88
     * @param $to
89
     * @param $from
90
     * @param $subject
91
     * @param $content
92
     * @return array Contains the keys: 'type', 'to', 'from', 'subject', 'content', 'plainContent', 'attachedFiles',
93
     *               'customHeaders', 'htmlContent', 'inlineImages'
94
     */
95
    public function findEmail($to = null, $from = null, $subject = null, $content = null)
96
    {
97
        $matches = $this->findEmails($to, $from, $subject, $content);
98
                //got the count of matches emails
99
                $emailCount = count($matches);
100
                //get the last(latest) one
101
        return $matches ? $matches[$emailCount-1] : null;
102
    }
103
104
    /**
105
     * Search for all emails.
106
     * All of the parameters can either be a string, or, if they start with "/", a PREG-compatible regular expression.
107
     *
108
     * @param $to
109
     * @param $from
110
     * @param $subject
111
     * @param $content
112
     * @return array Contains the keys: 'type', 'to', 'from', 'subject', 'content', 'plainContent', 'attachedFiles',
113
     *               'customHeaders', 'htmlContent', 'inlineImages'
114
     */
115
    public function findEmails($to = null, $from = null, $subject = null, $content = null)
0 ignored issues
show
Unused Code introduced by
The parameter $to is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
Unused Code introduced by
The parameter $from is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
Unused Code introduced by
The parameter $subject is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
Unused Code introduced by
The parameter $content is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
116
    {
117
        $matches = array();
118
        $args = func_get_args();
119
        $state = $this->testSessionEnvironment->getState();
120
        $emails = isset($state->emails) ? $state->emails : array();
121
        foreach ($emails as $email) {
122
            $matched = true;
123
124
            foreach (array('To', 'From', 'Subject', 'Content') as $i => $field) {
125
                if (!isset($email->$field)) {
126
                    continue;
127
                }
128
                $value = (isset($args[$i])) ? $args[$i] : null;
129
                if ($value) {
130
                    if ($value[0] == '/') {
131
                        $matched = preg_match($value, $email->$field);
132
                    } else {
133
                        $matched = ($value == $email->$field);
134
                    }
135
                    if (!$matched) {
136
                        break;
137
                    }
138
                }
139
            }
140
            if ($matched) {
141
                $matches[] = $email;
142
            }
143
        }
144
145
        return $matches;
146
    }
147
148
    protected function saveEmail($data)
149
    {
150
        $state = $this->testSessionEnvironment->getState();
151
        if (!isset($state->emails)) {
152
            $state->emails = array();
153
        }
154
        $state->emails[] = array_filter($data);
155
        $this->testSessionEnvironment->applyState($state);
156
    }
157
}
158