ContactModel::doSendEmail() - Code Metrics - Inspection of "Connect with Scrutinizer. Fix minor issues. Improv..." - mermshaus/kaloa-renderer - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( cb46a0...974133 )

by Marc

created 2016-03-22 15:34 UTC

ContactModel::doSendEmail() B

↳ Parent: ContactModel

Complexity

Conditions	6
Paths	4

Size

Total Lines	57
Code Lines	32

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
c	1
b	0
f	0
dl	0
loc	57
rs	8.7433
cc	6
eloc	32
nc	4
nop	1

How to fix Long Method

<?php

namespace org\example\Contact;

use PHPMailer;

/**
 * Application logic for contact form
 */
class ContactModel
{
    /**
     * @var array Holds all titles (salutations) that may be chosen from
     */
    protected $titles = array(
       #0 => intentionally left blank ("not chosen")

        1 => 'Mr',
        2 => 'Ms',
        3 => 'Company'
    );

    /**
     * @var string Path to PHPMailer directory in local file system
     */
    protected $pathToMailer = './library/phpmailer/PHPMailer_v5.1';

    /**
     * @var string All e-mails will be sent to this address
     */
    protected $emailRecipient = '[email protected]';

    /**
     * @var int Number of seconds a session has to wait before a second e-mail
     *          will be accepted (protection from spam and accidental double
     *          posts)
     * @todo Probably not the most clever way to do this
     */
    protected $emailInterval = 60;

    /**
     * Returns all titles (salutations) a user may chose from
     *
     * @return array Titles to chose from
     */
    public function getTitles()
    {
        return $this->titles;
    }

    /**
     * Sanitizes and validates input, calls the method to mail the supplied
     * contact form data and returns an array of errors
     *
     * @return array Errors (an empty array indicates there are no errors)
     */
    public function sendEmailFromPOST()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $errors = array();

        // Do not overwrite original data in $_POST
        $data = $_POST;

        // Make sure every field is set
        $data['email'] = (isset($data['email']))
                       ? trim((string) $data['email']) : '';
        $data['title'] = (isset($data['title']))
                       ? (int) $data['title'] : 0;
        $data['name']  = (isset($data['name']))
                       ? trim((string) $data['name']) : '';
        $data['phone'] = (isset($data['phone']))
                       ? trim((string) $data['phone']) : '';
        $data['callback'] = (isset($data['callback']))
                          ? (bool) $data['callback'] : false;

        // Validate input
        $data['email'] = preg_match('/^[^@\s]+@[^@\s]+\.[^@\s.]+$/',
                                    $data['email'])
                       ? $data['email'] : '';
        $data['title'] = (isset($this->titles[$data['title']]))
                       ? $data['title'] : 0;

        // Generate error messages for fields that have to be filled
        if ($data['email'] === '') {
            $errors[] = 'Please provide a valid e-mail address.';
        }
        if ($data['title'] === 0) {
            $errors[] = 'Please choose a title from the list.';
        }
        if ($data['name'] === '') {
            $errors[] = 'Please enter your name.';
        }

        // In case of error, return
        if (!empty($errors)) {
            return $errors;
        }

        // Otherwise, we're clear to mail the data
        $errors = $this->doSendEmail($data);

        return $errors;
    }

    /**
     * Mails data from contact form
     *
     * @param array $data Sanitized and validated form input
     * @return array Errors (an empty array indicates there are no errors)
     */
    protected function doSendEmail($data)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        // Application relies on session for spam protection
        if (!isset($_SESSION['contact']['lastEmailSent'])) {
            return array('You have to accept the session cookie.');
        }

        // See if enough time has passed to send another e-mail
        if (isset($_SESSION['contact']['lastEmailSent'])
                && ($_SESSION['contact']['lastEmailSent']
                        + $this->emailInterval > time())
        ) {
            return array(sprintf(
                    'You have to wait a total of %d seconds before you can '
                    . 'send another e-mail.', $this->emailInterval));
        }

        require_once $this->pathToMailer . '/class.phpmailer.php';

        // Generate body
        $body = sprintf("E-Mail:   %s\n", $data['email'])
              . sprintf("Title:    %s\n", $this->titles[$data['title']])
              . sprintf("Name:     %s\n", $data['name'])
              . sprintf("Phone:    %s\n", $data['phone'])
              . sprintf("Callback: %s", ($data['callback']) ? 'yes' : 'no');

        $mail = new PHPMailer();

        // Setup mailing method
        /** @todo Configuration settings should be moved to a config file */
        $mail->IsSMTP(true);
        $mail->Host     = 'ssl://smtp.example.org:465';
        $mail->SMTPAuth = true;
        $mail->Username = '';
        $mail->Password = '';

        // Set e-mail headers to more or less fitting values
        $mail->Subject = 'example.org -- Contact inquiry';
        $mail->Body    = $body;
        $mail->CharSet = 'UTF-8';

        $mail->AddReplyTo($data['email'], $data['name']);
        $mail->SetFrom('[email protected]');
        
        $mail->AddAddress($this->emailRecipient);        

        // Try to send mail, return errors on failure
        if (!$mail->Send()) {
            return array('Mailer Error: ' . $mail->ErrorInfo);
        }

        // E-mail sent, as far as we can tell. No errors
        $_SESSION['contact']['lastEmailSent'] = time();
        $_POST = array();

        return array();
    }
}

1			<?php
2
3			namespace org\example\Contact;
4
5			use PHPMailer;
6
7			/**
8			* Application logic for contact form
9			*/
10			class ContactModel
11			{
12			/**
13			* @var array Holds all titles (salutations) that may be chosen from
14			*/
15			protected $titles = array(
16			#0 => intentionally left blank ("not chosen")
			0 ignored issues – show Unused Code Comprehensibility introduced 2016-03-22 15:39 UTC by Report Bug Copy Issue Report `39%` of this comment could be valid code. Did you maybe forget this after debugging? Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
17			1 => 'Mr',
18			2 => 'Ms',
19			3 => 'Company'
20			);
21
22			/**
23			* @var string Path to PHPMailer directory in local file system
24			*/
25			protected $pathToMailer = './library/phpmailer/PHPMailer_v5.1';
26
27			/**
28			* @var string All e-mails will be sent to this address
29			*/
30			protected $emailRecipient = '[email protected]';
31
32			/**
33			* @var int Number of seconds a session has to wait before a second e-mail
34			* will be accepted (protection from spam and accidental double
35			* posts)
36			* @todo Probably not the most clever way to do this
37			*/
38			protected $emailInterval = 60;
39
40			/**
41			* Returns all titles (salutations) a user may chose from
42			*
43			* @return array Titles to chose from
44			*/
45			public function getTitles()
46			{
47			return $this->titles;
48			}
49
50			/**
51			* Sanitizes and validates input, calls the method to mail the supplied
52			* contact form data and returns an array of errors
53			*
54			* @return array Errors (an empty array indicates there are no errors)
55			*/
56			public function sendEmailFromPOST()
			0 ignored issues – show Coding Style introduced 2016-03-22 15:39 UTC by Report Bug Copy Issue Report `sendEmailFromPOST` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
57			{
58			$errors = array();
59
60			// Do not overwrite original data in $_POST
61			$data = $_POST;
62
63			// Make sure every field is set
64			$data['email'] = (isset($data['email']))
65			? trim((string) $data['email']) : '';
66			$data['title'] = (isset($data['title']))
67			? (int) $data['title'] : 0;
68			$data['name'] = (isset($data['name']))
69			? trim((string) $data['name']) : '';
70			$data['phone'] = (isset($data['phone']))
71			? trim((string) $data['phone']) : '';
72			$data['callback'] = (isset($data['callback']))
73			? (bool) $data['callback'] : false;
74
75			// Validate input
76			$data['email'] = preg_match('/^[^@\s]+@[^@\s]+\.[^@\s.]+$/',
77			$data['email'])
78			? $data['email'] : '';
79			$data['title'] = (isset($this->titles[$data['title']]))
80			? $data['title'] : 0;
81
82			// Generate error messages for fields that have to be filled
83			if ($data['email'] === '') {
84			$errors[] = 'Please provide a valid e-mail address.';
85			}
86			if ($data['title'] === 0) {
87			$errors[] = 'Please choose a title from the list.';
88			}
89			if ($data['name'] === '') {
90			$errors[] = 'Please enter your name.';
91			}
92
93			// In case of error, return
94			if (!empty($errors)) {
95			return $errors;
96			}
97
98			// Otherwise, we're clear to mail the data
99			$errors = $this->doSendEmail($data);
100
101			return $errors;
102			}
103
104			/**
105			* Mails data from contact form
106			*
107			* @param array $data Sanitized and validated form input
108			* @return array Errors (an empty array indicates there are no errors)
109			*/
110			protected function doSendEmail($data)
			0 ignored issues – show Coding Style introduced 2016-03-22 15:39 UTC by Report Bug Copy Issue Report `doSendEmail` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2016-03-22 15:39 UTC by Report Bug Copy Issue Report `doSendEmail` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
111			{
112			// Application relies on session for spam protection
113			if (!isset($_SESSION['contact']['lastEmailSent'])) {
114			return array('You have to accept the session cookie.');
115			}
116
117			// See if enough time has passed to send another e-mail
118			if (isset($_SESSION['contact']['lastEmailSent'])
119			&& ($_SESSION['contact']['lastEmailSent']
120			+ $this->emailInterval > time())
121			) {
122			return array(sprintf(
123			'You have to wait a total of %d seconds before you can '
124			. 'send another e-mail.', $this->emailInterval));
125			}
126
127			require_once $this->pathToMailer . '/class.phpmailer.php';
128
129			// Generate body
130			$body = sprintf("E-Mail: %s\n", $data['email'])
131			. sprintf("Title: %s\n", $this->titles[$data['title']])
132			. sprintf("Name: %s\n", $data['name'])
133			. sprintf("Phone: %s\n", $data['phone'])
134			. sprintf("Callback: %s", ($data['callback']) ? 'yes' : 'no');
135
136			$mail = new PHPMailer();
137
138			// Setup mailing method
139			/** @todo Configuration settings should be moved to a config file */
140			$mail->IsSMTP(true);
141			$mail->Host = 'ssl://smtp.example.org:465';
142			$mail->SMTPAuth = true;
143			$mail->Username = '';
144			$mail->Password = '';
145
146			// Set e-mail headers to more or less fitting values
147			$mail->Subject = 'example.org -- Contact inquiry';
148			$mail->Body = $body;
149			$mail->CharSet = 'UTF-8';
150
151			$mail->AddReplyTo($data['email'], $data['name']);
152			$mail->SetFrom('[email protected]');
153
154			$mail->AddAddress($this->emailRecipient);
155
156			// Try to send mail, return errors on failure
157			if (!$mail->Send()) {
158			return array('Mailer Error: ' . $mail->ErrorInfo);
159			}
160
161			// E-mail sent, as far as we can tell. No errors
162			$_SESSION['contact']['lastEmailSent'] = time();
163			$_POST = array();
164
165			return array();
166			}
167			}

mermshaus / kaloa-renderer

Push — master ( cb46a0...974133 )

ContactModel::doSendEmail() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like