Migration_Setup_Notices::up() - Code Metrics - Inspection of "notice system + ability to hide notices | closes #..." - DakuTree/manga-tracker - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 056f30...ecabd3 )

by Angus

created 2017-02-25 23:29 UTC

Migration_Setup_Notices::up() A

↳ Parent: Migration_Setup_Notices

Complexity

Conditions	1
Paths	1

Size

Total Lines	52
Code Lines	27

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	1
eloc	27
nc	1
nop	0
dl	0
loc	52
rs	9.4929
c	0
b	0
f	0

How to fix Long Method

<?php defined('BASEPATH') OR exit('No direct script access allowed');

class Migration_Setup_Notices extends CI_Migration {
	public function __construct() {
		parent::__construct();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
		$this->load->dbforge();
	}

	public function up() {
		//Notice table
		$this->dbforge->add_field(array(
			'id' => array(
				'type'           => 'MEDIUMINT',
				'constraint'     => '8',
				'unsigned'       => TRUE,
				'auto_increment' => TRUE
			),
			'notice' => array(
				'type'           => 'VARCHAR',
				'constraint'     => '255'
				//FOREIGN KEY
			),
			'created_at' => array(
				//Despite not actually creating the field here (it's instead done below), we still need this here so a key can be created properly.
			//	'type'    => 'TIMESTAMP',
			//	'null'    => FALSE,
			//	'on_update' => FALSE
			)
		));
		$this->dbforge->add_field('created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP'); //CI is annoying and auto-appends ON UPDATE which we don't want.
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		$this->dbforge->add_key('id', TRUE);
		$this->dbforge->add_key('created_at');
		$this->dbforge->create_table('tracker_notices');

		//User notice hidden table
		$this->dbforge->add_field(array(
			'user_id' => array(
				'type'           => 'MEDIUMINT',
				'constraint'     => '8',
				'unsigned'       => TRUE,
				//FOREIGN KEY
			),
			'hidden_notice_id' => array(
				'type'           => 'MEDIUMINT',
				'constraint'     => '8',
				'unsigned'       => TRUE
				//FOREIGN KEY
			)
		));
		$this->dbforge->add_key('user_id', TRUE);
		$this->dbforge->create_table('tracker_user_notices');

		/*** Unique/Foreign Keys ***/
		//For whatever reason, dbforge lacks a unique/foreign key function.
		$this->db->query('
			ALTER TABLE `tracker_user_notices`
				ADD CONSTRAINT `FK_tracker_user_notices_auth_users` FOREIGN KEY (`user_id`) REFERENCES `auth_users`(`id`) ON UPDATE NO ACTION ON DELETE NO ACTION,
				ADD CONSTRAINT `FK_tracker_user_notices_tracker_notices` FOREIGN KEY (`hidden_notice_id`) REFERENCES `tracker_notices` (`id`) ON UPDATE NO ACTION ON DELETE NO ACTION;'
		);
	}

	public function down() {
		$this->dbforge->drop_table('tracker_notices', TRUE);
		$this->dbforge->drop_table('tracker_user_notices', TRUE);
	}
}


1			<?php defined('BASEPATH') OR exit('No direct script access allowed');
2
3			class Migration_Setup_Notices extends CI_Migration {
4			public function __construct() {
5			parent::__construct();
			0 ignored issues – show Bug introduced 2017-02-25 23:32 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `CI_Migration` as the method `__construct()` does only exist in the following sub-classes of `CI_Migration`: `Migration_Auth_add_apikey`, `Migration_Favourites_Rework`, `Migration_Favourites_Rework_Revert`, `Migration_Install_ion_auth`, `Migration_Setup_Favourites`, `Migration_Setup_Notices`, `Migration_Setup_Rate_Limit`, `Migration_Setup_Sessions`, `Migration_Setup_Signup_Verification`, `Migration_Setup_Title_History`, `Migration_Setup_Tracker`, `Migration_Setup_User_History`, `Migration_Setup_User_Options`, `Migration_Tracker_Add_Active`, `Migration_Tracker_Add_Site_Custom`, `Migration_Tracker_Add_Site_Status`, `Migration_Tracker_Add_Title_Status`, `Migration_Tracker_add_category`, `Migration_Tracker_add_complete`, `Migration_Tracker_add_last_checked`, `Migration_Tracker_add_tags`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
6			$this->load->dbforge();
7			}
8
9			public function up() {
10			//Notice table
11			$this->dbforge->add_field(array(
12			'id' => array(
13			'type' => 'MEDIUMINT',
14			'constraint' => '8',
15			'unsigned' => TRUE,
16			'auto_increment' => TRUE
17			),
18			'notice' => array(
19			'type' => 'VARCHAR',
20			'constraint' => '255'
21			//FOREIGN KEY
22			),
23			'created_at' => array(
24			//Despite not actually creating the field here (it's instead done below), we still need this here so a key can be created properly.
25			// 'type' => 'TIMESTAMP',
26			// 'null' => FALSE,
27			// 'on_update' => FALSE
28			)
29			));
30			$this->dbforge->add_field('created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP'); //CI is annoying and auto-appends ON UPDATE which we don't want.
			0 ignored issues – show Documentation introduced 2017-02-25 23:32 UTC by Report Bug Copy Issue Report `'created_at TIMESTAMP NO...AULT CURRENT_TIMESTAMP'` is of type `string`, but the function expects a `array`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
31			$this->dbforge->add_key('id', TRUE);
32			$this->dbforge->add_key('created_at');
33			$this->dbforge->create_table('tracker_notices');
34
35			//User notice hidden table
36			$this->dbforge->add_field(array(
37			'user_id' => array(
38			'type' => 'MEDIUMINT',
39			'constraint' => '8',
40			'unsigned' => TRUE,
41			//FOREIGN KEY
42			),
43			'hidden_notice_id' => array(
44			'type' => 'MEDIUMINT',
45			'constraint' => '8',
46			'unsigned' => TRUE
47			//FOREIGN KEY
48			)
49			));
50			$this->dbforge->add_key('user_id', TRUE);
51			$this->dbforge->create_table('tracker_user_notices');
52
53			/* Unique/Foreign Keys */
54			//For whatever reason, dbforge lacks a unique/foreign key function.
55			$this->db->query('
56			ALTER TABLE `tracker_user_notices`
57			ADD CONSTRAINT `FK_tracker_user_notices_auth_users` FOREIGN KEY (`user_id`) REFERENCES `auth_users`(`id`) ON UPDATE NO ACTION ON DELETE NO ACTION,
58			ADD CONSTRAINT `FK_tracker_user_notices_tracker_notices` FOREIGN KEY (`hidden_notice_id`) REFERENCES `tracker_notices` (`id`) ON UPDATE NO ACTION ON DELETE NO ACTION;'
59			);
60			}
61
62			public function down() {
63			$this->dbforge->drop_table('tracker_notices', TRUE);
64			$this->dbforge->drop_table('tracker_user_notices', TRUE);
65			}
66			}
67

DakuTree / manga-tracker

Push — master ( 056f30...ecabd3 )

Migration_Setup_Notices::up() A

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Available Fixes

Duplication Side-by-Side

Filter issues like