TestMailer - Code Metrics - Inspection of "FIX: Refactor TestMailer to better be base class" - sminnee/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — testmailer-refactor ( eeb7fa...5a124f )

by Sam

created 2017-01-11 21:03 UTC

TestMailer A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	129
Duplicated Lines	13.18 %

Coupling/Cohesion

Components	1
Dependencies	1

Importance

Changes

Metric	Value
dl	17
loc	129
rs	10
c	0
b	0
f	0
wmc	11
lcom	1
cbo	1

5 Methods

Rating	Name	Duplication	Size	Complexity
B	sendHTML()	0	28	1
A	sendPlain()	17	17	1
A	saveEmail()	0	4	1
A	clearEmails()	0	4	1
C	findEmail()	0	23	7

How to fix Duplicated Code

<?php

namespace SilverStripe\Dev;

use SilverStripe\Control\Email\Mailer;

class TestMailer extends Mailer
{
    protected $emailsSent = array();

    /**
     * Send a plain-text email.
     * TestMailer will merely record that the email was asked to be sent, without sending anything.
     *
     * @param string $to
     * @param string $from
     * @param string $subject
     * @param string $plainContent
     * @param bool $attachedFiles
     * @param bool $customHeaders
     * @return bool|mixed
     */
    public function sendPlain($to, $from, $subject, $plainContent, $attachedFiles = false, $customHeaders = false)

    {
        $this->saveEmail([
            'Type' => 'plain',
            'To' => $to,
            'From' => $from,
            'Subject' => $subject,

            'Content' => $plainContent,
            'PlainContent' => $plainContent,

            'AttachedFiles' => $attachedFiles,
            'CustomHeaders' => $customHeaders,
        ]);

        return true;
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());
}
    }

    /**
     * Send a multi-part HTML email
     * TestMailer will merely record that the email was asked to be sent, without sending anything.
     *
     * @param string $to
     * @param string $from
     * @param string $subject
     * @param string $htmlContent
     * @param bool $attachedFiles
     * @param bool $customHeaders
     * @param bool $plainContent
     * @param bool $inlineImages
     * @return bool|mixed
     */
    public function sendHTML(
        $to,
        $from,
        $subject,
        $htmlContent,
        $attachedFiles = false,
        $customHeaders = false,
        $plainContent = false,
        $inlineImages = false
    ) {

        $this->saveEmail([
            'Type' => 'html',
            'To' => $to,
            'From' => $from,
            'Subject' => $subject,

            'Content' => $htmlContent,
            'PlainContent' => $plainContent,
            'HtmlContent' => $htmlContent,

            'AttachedFiles' => $attachedFiles,
            'CustomHeaders' => $customHeaders,
            'InlineImages' => $inlineImages,
        ]);

        return true;
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());
}
    }

    /**
     * Save a single email to the log
     * @param $data A map of information about the email
     */
    protected function saveEmail($data)
    {
        $this->emailsSent[] = $data;
    }

    /**
     * Clear the log of emails sent
     */
    public function clearEmails()
    {
        $this->emailsSent = array();
    }

    /**
     * Search for an email that was sent.
     * All of the parameters can either be a string, or, if they start with "/", a PREG-compatible regular expression.
     *
     * @param string $to
     * @param string $from
     * @param string $subject
     * @param string $content
     * @return array Contains the keys: 'type', 'to', 'from', 'subject', 'content', 'plainContent', 'attachedFiles',
     *               'customHeaders', 'htmlContent', 'inlineImages'
     */
    public function findEmail($to, $from = null, $subject = null, $content = null)
    {
        foreach ($this->emailsSent as $email) {
            $matched = true;

            foreach (array('To','From','Subject','Content') as $field) {
                if ($value = $$field) {
                    if ($value[0] == '/') {
                        $matched = preg_match($value, $email[$field]);
                    } else {
                        $matched = ($value == $email[$field]);
                    }
                    if (!$matched) {
                        break;
                    }
                }
            }

            if ($matched) {
                return $email;
            }
        }
    }
}


1		<?php
2
3		namespace SilverStripe\Dev;
4
5		use SilverStripe\Control\Email\Mailer;
6
7		class TestMailer extends Mailer
8		{
9		protected $emailsSent = array();
10
11		/**
12		* Send a plain-text email.
13		* TestMailer will merely record that the email was asked to be sent, without sending anything.
14		*
15		* @param string $to
16		* @param string $from
17		* @param string $subject
18		* @param string $plainContent
19		* @param bool $attachedFiles
20		* @param bool $customHeaders
21		* @return bool\|mixed
22		*/
23	View Code Duplication	public function sendPlain($to, $from, $subject, $plainContent, $attachedFiles = false, $customHeaders = false)
		0 ignored issues – show Duplication introduced 2017-01-11 21:11 UTC by Report Bug Copy Issue Report 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...
24		{
25		$this->saveEmail([
26		'Type' => 'plain',
27		'To' => $to,
28		'From' => $from,
29		'Subject' => $subject,
30
31		'Content' => $plainContent,
32		'PlainContent' => $plainContent,
33
34		'AttachedFiles' => $attachedFiles,
35		'CustomHeaders' => $customHeaders,
36		]);
37
38		return true;
		0 ignored issues – show Bug Best Practice introduced 2016-09-09 23:25 UTC by Report Bug Copy Issue Report The return type of `return true;` (`boolean`) is incompatible with the return type of the parent method `SilverStripe\Control\Email\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...
39		}
40
41		/**
42		* Send a multi-part HTML email
43		* TestMailer will merely record that the email was asked to be sent, without sending anything.
44		*
45		* @param string $to
46		* @param string $from
47		* @param string $subject
48		* @param string $htmlContent
49		* @param bool $attachedFiles
50		* @param bool $customHeaders
51		* @param bool $plainContent
52		* @param bool $inlineImages
53		* @return bool\|mixed
54		*/
55		public function sendHTML(
56		$to,
57		$from,
58		$subject,
59		$htmlContent,
60		$attachedFiles = false,
61		$customHeaders = false,
62		$plainContent = false,
63		$inlineImages = false
64		) {
65
66		$this->saveEmail([
67		'Type' => 'html',
68		'To' => $to,
69		'From' => $from,
70		'Subject' => $subject,
71
72		'Content' => $htmlContent,
73		'PlainContent' => $plainContent,
74		'HtmlContent' => $htmlContent,
75
76		'AttachedFiles' => $attachedFiles,
77		'CustomHeaders' => $customHeaders,
78		'InlineImages' => $inlineImages,
79		]);
80
81		return true;
		0 ignored issues – show Bug Best Practice introduced 2016-09-09 23:25 UTC by Report Bug Copy Issue Report The return type of `return true;` (`boolean`) is incompatible with the return type of the parent method `SilverStripe\Control\Email\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...
82		}
83
84		/**
85		* Save a single email to the log
86		* @param $data A map of information about the email
87		*/
88		protected function saveEmail($data)
89		{
90		$this->emailsSent[] = $data;
91		}
92
93		/**
94		* Clear the log of emails sent
95		*/
96		public function clearEmails()
97		{
98		$this->emailsSent = array();
99		}
100
101		/**
102		* Search for an email that was sent.
103		* All of the parameters can either be a string, or, if they start with "/", a PREG-compatible regular expression.
104		*
105		* @param string $to
106		* @param string $from
107		* @param string $subject
108		* @param string $content
109		* @return array Contains the keys: 'type', 'to', 'from', 'subject', 'content', 'plainContent', 'attachedFiles',
110		* 'customHeaders', 'htmlContent', 'inlineImages'
111		*/
112		public function findEmail($to, $from = null, $subject = null, $content = null)
113		{
114		foreach ($this->emailsSent as $email) {
115		$matched = true;
116
117		foreach (array('To','From','Subject','Content') as $field) {
118		if ($value = $$field) {
119		if ($value[0] == '/') {
120		$matched = preg_match($value, $email[$field]);
121		} else {
122		$matched = ($value == $email[$field]);
123		}
124		if (!$matched) {
125		break;
126		}
127		}
128		}
129
130		if ($matched) {
131		return $email;
132		}
133		}
134		}
135		}
136

sminnee / silverstripe-framework

Push — testmailer-refactor ( eeb7fa...5a124f )

TestMailer A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

5 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like