Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like QueryTest often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use QueryTest, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 24 | class QueryTest extends OrmFunctionalTestCase |
||
| 25 | { |
||
| 26 | protected function setUp() |
||
| 27 | { |
||
| 28 | $this->useModelSet('cms'); |
||
| 29 | |||
| 30 | parent::setUp(); |
||
| 31 | } |
||
| 32 | |||
| 33 | public function testSimpleQueries() |
||
| 34 | { |
||
| 35 | $user = new CmsUser; |
||
| 36 | $user->name = 'Guilherme'; |
||
| 37 | $user->username = 'gblanco'; |
||
| 38 | $user->status = 'developer'; |
||
| 39 | $this->_em->persist($user); |
||
| 40 | $this->_em->flush(); |
||
| 41 | $this->_em->clear(); |
||
| 42 | |||
| 43 | $query = $this->_em->createQuery("select u, upper(u.name) from Doctrine\Tests\Models\CMS\CmsUser u where u.username = 'gblanco'"); |
||
| 44 | |||
| 45 | $result = $query->getResult(); |
||
| 46 | |||
| 47 | $this->assertEquals(1, count($result)); |
||
| 48 | $this->assertInstanceOf(CmsUser::class, $result[0][0]); |
||
| 49 | $this->assertEquals('Guilherme', $result[0][0]->name); |
||
| 50 | $this->assertEquals('gblanco', $result[0][0]->username); |
||
| 51 | $this->assertEquals('developer', $result[0][0]->status); |
||
| 52 | $this->assertEquals('GUILHERME', $result[0][1]); |
||
| 53 | |||
| 54 | $resultArray = $query->getArrayResult(); |
||
| 55 | $this->assertEquals(1, count($resultArray)); |
||
| 56 | $this->assertTrue(is_array($resultArray[0][0])); |
||
| 57 | $this->assertEquals('Guilherme', $resultArray[0][0]['name']); |
||
| 58 | $this->assertEquals('gblanco', $resultArray[0][0]['username']); |
||
| 59 | $this->assertEquals('developer', $resultArray[0][0]['status']); |
||
| 60 | $this->assertEquals('GUILHERME', $resultArray[0][1]); |
||
| 61 | |||
| 62 | $scalarResult = $query->getScalarResult(); |
||
| 63 | $this->assertEquals(1, count($scalarResult)); |
||
| 64 | $this->assertEquals('Guilherme', $scalarResult[0]['u_name']); |
||
| 65 | $this->assertEquals('gblanco', $scalarResult[0]['u_username']); |
||
| 66 | $this->assertEquals('developer', $scalarResult[0]['u_status']); |
||
| 67 | $this->assertEquals('GUILHERME', $scalarResult[0][1]); |
||
| 68 | |||
| 69 | $query = $this->_em->createQuery("select upper(u.name) from Doctrine\Tests\Models\CMS\CmsUser u where u.username = 'gblanco'"); |
||
| 70 | $this->assertEquals('GUILHERME', $query->getSingleScalarResult()); |
||
| 71 | } |
||
| 72 | |||
| 73 | public function testJoinQueries() |
||
| 74 | { |
||
| 75 | $user = new CmsUser; |
||
| 76 | $user->name = 'Guilherme'; |
||
| 77 | $user->username = 'gblanco'; |
||
| 78 | $user->status = 'developer'; |
||
| 79 | |||
| 80 | $article1 = new CmsArticle; |
||
| 81 | $article1->topic = "Doctrine 2"; |
||
| 82 | $article1->text = "This is an introduction to Doctrine 2."; |
||
| 83 | $user->addArticle($article1); |
||
| 84 | |||
| 85 | $article2 = new CmsArticle; |
||
| 86 | $article2->topic = "Symfony 2"; |
||
| 87 | $article2->text = "This is an introduction to Symfony 2."; |
||
| 88 | $user->addArticle($article2); |
||
| 89 | |||
| 90 | $this->_em->persist($user); |
||
| 91 | $this->_em->persist($article1); |
||
| 92 | $this->_em->persist($article2); |
||
| 93 | |||
| 94 | $this->_em->flush(); |
||
| 95 | $this->_em->clear(); |
||
| 96 | |||
| 97 | $query = $this->_em->createQuery('select u, a from ' . CmsUser::class . ' u join u.articles a ORDER BY a.topic'); |
||
| 98 | $users = $query->getResult(); |
||
| 99 | $this->assertEquals(1, count($users)); |
||
| 100 | $this->assertInstanceOf(CmsUser::class, $users[0]); |
||
| 101 | $this->assertEquals(2, count($users[0]->articles)); |
||
| 102 | $this->assertEquals('Doctrine 2', $users[0]->articles[0]->topic); |
||
| 103 | $this->assertEquals('Symfony 2', $users[0]->articles[1]->topic); |
||
| 104 | } |
||
| 105 | |||
| 106 | public function testUsingZeroBasedQueryParameterShouldWork() |
||
| 107 | { |
||
| 108 | $user = new CmsUser; |
||
| 109 | $user->name = 'Jonathan'; |
||
| 110 | $user->username = 'jwage'; |
||
| 111 | $user->status = 'developer'; |
||
| 112 | $this->_em->persist($user); |
||
| 113 | $this->_em->flush(); |
||
| 114 | $this->_em->clear(); |
||
| 115 | |||
| 116 | $q = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u WHERE u.username = ?0'); |
||
| 117 | $q->setParameter(0, 'jwage'); |
||
| 118 | $user = $q->getSingleResult(); |
||
| 119 | |||
| 120 | $this->assertNotNull($user); |
||
| 121 | } |
||
| 122 | |||
| 123 | View Code Duplication | public function testUsingUnknownQueryParameterShouldThrowException() |
|
| 124 | { |
||
| 125 | $this->expectException(QueryException::class); |
||
| 126 | $this->expectExceptionMessage('Invalid parameter: token 2 is not defined in the query.'); |
||
| 127 | |||
| 128 | $q = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u WHERE u.name = ?1'); |
||
| 129 | $q->setParameter(2, 'jwage'); |
||
| 130 | $user = $q->getSingleResult(); |
||
|
|
|||
| 131 | } |
||
| 132 | |||
| 133 | View Code Duplication | public function testTooManyParametersShouldThrowException() |
|
| 134 | { |
||
| 135 | $this->expectException(QueryException::class); |
||
| 136 | $this->expectExceptionMessage('Too many parameters: the query defines 1 parameters and you bound 2'); |
||
| 137 | |||
| 138 | $q = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u WHERE u.name = ?1'); |
||
| 139 | $q->setParameter(1, 'jwage'); |
||
| 140 | $q->setParameter(2, 'jwage'); |
||
| 141 | |||
| 142 | $user = $q->getSingleResult(); |
||
| 143 | } |
||
| 144 | |||
| 145 | public function testTooFewParametersShouldThrowException() |
||
| 146 | { |
||
| 147 | $this->expectException(QueryException::class); |
||
| 148 | $this->expectExceptionMessage('Too few parameters: the query defines 1 parameters but you only bound 0'); |
||
| 149 | |||
| 150 | $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u WHERE u.name = ?1') |
||
| 151 | ->getSingleResult(); |
||
| 152 | } |
||
| 153 | |||
| 154 | public function testInvalidInputParameterThrowsException() |
||
| 162 | |||
| 163 | public function testSetParameters() |
||
| 164 | { |
||
| 165 | $parameters = new ArrayCollection(); |
||
| 166 | $parameters->add(new Parameter(1, 'jwage')); |
||
| 167 | $parameters->add(new Parameter(2, 'active')); |
||
| 182 | |||
| 183 | public function testSetParametersBackwardsCompatible() |
||
| 196 | |||
| 197 | /** |
||
| 198 | * @group DDC-1070 |
||
| 199 | */ |
||
| 200 | public function testIterateResultAsArrayAndParams() |
||
| 241 | |||
| 242 | public function testIterateResult_IterativelyBuildUpUnitOfWork() |
||
| 281 | |||
| 282 | public function testIterateResultClearEveryCycle() |
||
| 317 | |||
| 318 | /** |
||
| 319 | * @expectedException \Doctrine\ORM\Query\QueryException |
||
| 320 | */ |
||
| 321 | public function testIterateResult_FetchJoinedCollection_ThrowsException() |
||
| 326 | |||
| 327 | /** |
||
| 328 | * @expectedException Doctrine\ORM\NoResultException |
||
| 329 | */ |
||
| 330 | public function testGetSingleResultThrowsExceptionOnNoResult() |
||
| 335 | |||
| 336 | /** |
||
| 337 | * @expectedException Doctrine\ORM\NoResultException |
||
| 338 | */ |
||
| 339 | public function testGetSingleScalarResultThrowsExceptionOnNoResult() |
||
| 344 | |||
| 345 | /** |
||
| 346 | * @expectedException Doctrine\ORM\NonUniqueResultException |
||
| 347 | */ |
||
| 348 | public function testGetSingleScalarResultThrowsExceptionOnNonUniqueResult() |
||
| 375 | |||
| 376 | public function testModifiedLimitQuery() |
||
| 377 | { |
||
| 378 | View Code Duplication | for ($i = 0; $i < 5; $i++) { |
|
| 379 | $user = new CmsUser; |
||
| 380 | $user->name = 'Guilherme' . $i; |
||
| 381 | $user->username = 'gblanco' . $i; |
||
| 382 | $user->status = 'developer'; |
||
| 383 | $this->_em->persist($user); |
||
| 384 | } |
||
| 385 | |||
| 386 | $this->_em->flush(); |
||
| 387 | $this->_em->clear(); |
||
| 388 | |||
| 389 | $data = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u') |
||
| 390 | ->setFirstResult(1) |
||
| 391 | ->setMaxResults(2) |
||
| 392 | ->getResult(); |
||
| 393 | |||
| 394 | $this->assertEquals(2, count($data)); |
||
| 395 | $this->assertEquals('gblanco1', $data[0]->username); |
||
| 396 | $this->assertEquals('gblanco2', $data[1]->username); |
||
| 397 | |||
| 398 | $data = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u') |
||
| 399 | ->setFirstResult(3) |
||
| 400 | ->setMaxResults(2) |
||
| 401 | ->getResult(); |
||
| 402 | |||
| 403 | $this->assertEquals(2, count($data)); |
||
| 404 | $this->assertEquals('gblanco3', $data[0]->username); |
||
| 405 | $this->assertEquals('gblanco4', $data[1]->username); |
||
| 406 | |||
| 407 | $data = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u') |
||
| 408 | ->setFirstResult(3) |
||
| 409 | ->setMaxResults(2) |
||
| 410 | ->getScalarResult(); |
||
| 411 | } |
||
| 412 | |||
| 413 | public function testSupportsQueriesWithEntityNamespaces() |
||
| 427 | |||
| 428 | /** |
||
| 429 | * @group DDC-604 |
||
| 430 | */ |
||
| 431 | public function testEntityParameters() |
||
| 457 | |||
| 458 | /** |
||
| 459 | * @group DDC-952 |
||
| 460 | */ |
||
| 461 | public function testEnableFetchEagerMode() |
||
| 487 | |||
| 488 | /** |
||
| 489 | * @group DDC-991 |
||
| 490 | */ |
||
| 491 | public function testgetOneOrNullResult() |
||
| 511 | |||
| 512 | /** |
||
| 513 | * @group DDC-991 |
||
| 514 | */ |
||
| 515 | View Code Duplication | public function testgetOneOrNullResultSeveralRows() |
|
| 516 | { |
||
| 517 | $user = new CmsUser; |
||
| 518 | $user->name = 'Guilherme'; |
||
| 519 | $user->username = 'gblanco'; |
||
| 520 | $user->status = 'developer'; |
||
| 521 | $this->_em->persist($user); |
||
| 522 | $user = new CmsUser; |
||
| 523 | $user->name = 'Roman'; |
||
| 524 | $user->username = 'romanb'; |
||
| 525 | $user->status = 'developer'; |
||
| 526 | $this->_em->persist($user); |
||
| 527 | $this->_em->flush(); |
||
| 528 | $this->_em->clear(); |
||
| 529 | |||
| 530 | $query = $this->_em->createQuery("select u from Doctrine\Tests\Models\CMS\CmsUser u"); |
||
| 531 | |||
| 532 | $this->expectException(NonUniqueResultException::class); |
||
| 533 | |||
| 534 | $fetchedUser = $query->getOneOrNullResult(); |
||
| 535 | } |
||
| 536 | |||
| 537 | /** |
||
| 538 | * @group DDC-991 |
||
| 539 | */ |
||
| 540 | public function testgetOneOrNullResultNoRows() |
||
| 548 | |||
| 549 | /** |
||
| 550 | * @group DBAL-171 |
||
| 551 | */ |
||
| 552 | public function testParameterOrder() |
||
| 586 | |||
| 587 | public function testDqlWithAutoInferOfParameters() |
||
| 617 | |||
| 618 | public function testQueryBuilderWithStringWhereClauseContainingOrAndConditionalPrimary() |
||
| 631 | |||
| 632 | public function testQueryWithArrayOfEntitiesAsParameter() |
||
| 663 | |||
| 664 | public function testQueryWithHiddenAsSelectExpression() |
||
| 693 | |||
| 694 | /** |
||
| 695 | * @group DDC-1651 |
||
| 696 | */ |
||
| 697 | public function testSetParameterBindingSingleIdentifierObject() |
||
| 716 | |||
| 717 | /** |
||
| 718 | * @group DDC-2319 |
||
| 719 | */ |
||
| 720 | public function testSetCollectionParameterBindingSingleIdentifierObject() |
||
| 766 | |||
| 767 | /** |
||
| 768 | * @group DDC-1822 |
||
| 769 | */ |
||
| 770 | public function testUnexpectedResultException() |
||
| 802 | |||
| 803 | public function testMultipleJoinComponentsUsingInnerJoin() |
||
| 835 | |||
| 836 | public function testMultipleJoinComponentsUsingLeftJoin() |
||
| 870 | } |
||
| 871 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVarassignment in line 1 and the$higherassignment in line 2 are dead. The first because$myVaris never used and the second because$higheris always overwritten for every possible time line.