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:
1 | <?php |
||
21 | class BasicFunctionalTest extends OrmFunctionalTestCase |
||
22 | { |
||
23 | protected function setUp() |
||
24 | { |
||
25 | $this->useModelSet('cms'); |
||
26 | parent::setUp(); |
||
27 | } |
||
28 | |||
29 | public function testBasicUnitsOfWorkWithOneToManyAssociation() |
||
30 | { |
||
31 | // Create |
||
32 | $user = new CmsUser; |
||
33 | $user->name = 'Roman'; |
||
34 | $user->username = 'romanb'; |
||
35 | $user->status = 'developer'; |
||
36 | $this->em->persist($user); |
||
37 | |||
38 | $this->em->flush(); |
||
39 | |||
40 | self::assertInternalType('numeric', $user->id); |
||
41 | self::assertTrue($this->em->contains($user)); |
||
42 | |||
43 | // Read |
||
44 | $user2 = $this->em->find(CmsUser::class, $user->id); |
||
45 | self::assertSame($user, $user2); |
||
46 | |||
47 | // Add a phonenumber |
||
48 | $ph = new CmsPhonenumber; |
||
49 | $ph->phonenumber = "12345"; |
||
50 | $user->addPhonenumber($ph); |
||
51 | $this->em->flush(); |
||
52 | self::assertTrue($this->em->contains($ph)); |
||
53 | self::assertTrue($this->em->contains($user)); |
||
54 | |||
55 | // Update name |
||
56 | $user->name = 'guilherme'; |
||
57 | $this->em->flush(); |
||
58 | self::assertEquals('guilherme', $user->name); |
||
59 | |||
60 | // Add another phonenumber |
||
61 | $ph2 = new CmsPhonenumber; |
||
62 | $ph2->phonenumber = "6789"; |
||
63 | $user->addPhonenumber($ph2); |
||
64 | $this->em->flush(); |
||
65 | self::assertTrue($this->em->contains($ph2)); |
||
66 | |||
67 | // Delete |
||
68 | $this->em->remove($user); |
||
69 | self::assertTrue($this->em->getUnitOfWork()->isScheduledForDelete($user)); |
||
70 | self::assertTrue($this->em->getUnitOfWork()->isScheduledForDelete($ph)); |
||
71 | self::assertTrue($this->em->getUnitOfWork()->isScheduledForDelete($ph2)); |
||
72 | $this->em->flush(); |
||
73 | self::assertFalse($this->em->getUnitOfWork()->isScheduledForDelete($user)); |
||
74 | self::assertFalse($this->em->getUnitOfWork()->isScheduledForDelete($ph)); |
||
75 | self::assertFalse($this->em->getUnitOfWork()->isScheduledForDelete($ph2)); |
||
76 | self::assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->em->getUnitOfWork()->getEntityState($user)); |
||
77 | self::assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->em->getUnitOfWork()->getEntityState($ph)); |
||
78 | self::assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->em->getUnitOfWork()->getEntityState($ph2)); |
||
79 | } |
||
80 | |||
81 | public function testOneToManyAssociationModification() |
||
82 | { |
||
83 | $user = new CmsUser; |
||
84 | $user->name = 'Roman'; |
||
85 | $user->username = 'romanb'; |
||
86 | $user->status = 'developer'; |
||
87 | |||
88 | $ph1 = new CmsPhonenumber; |
||
89 | $ph1->phonenumber = "0301234"; |
||
90 | $ph2 = new CmsPhonenumber; |
||
91 | $ph2->phonenumber = "987654321"; |
||
92 | |||
93 | $user->addPhonenumber($ph1); |
||
94 | $user->addPhonenumber($ph2); |
||
95 | |||
96 | $this->em->persist($user); |
||
97 | $this->em->flush(); |
||
98 | |||
99 | // Remove the first element from the collection |
||
100 | unset($user->phonenumbers[0]); |
||
101 | $ph1->user = null; // owning side! |
||
102 | |||
103 | $this->em->flush(); |
||
104 | |||
105 | self::assertCount(1, $user->phonenumbers); |
||
106 | self::assertNull($ph1->user); |
||
107 | } |
||
108 | |||
109 | public function testBasicOneToOne() |
||
110 | { |
||
111 | //$this->em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); |
||
|
|||
112 | $user = new CmsUser; |
||
113 | $user->name = 'Roman'; |
||
114 | $user->username = 'romanb'; |
||
115 | $user->status = 'developer'; |
||
116 | |||
117 | $address = new CmsAddress; |
||
118 | $address->country = 'Germany'; |
||
119 | $address->city = 'Berlin'; |
||
120 | $address->zip = '12345'; |
||
121 | |||
122 | $user->address = $address; // inverse side |
||
123 | $address->user = $user; // owning side! |
||
124 | |||
125 | $this->em->persist($user); |
||
126 | $this->em->flush(); |
||
127 | |||
128 | // Check that the foreign key has been set |
||
129 | $userId = $this->em->getConnection()->executeQuery( |
||
130 | "SELECT user_id FROM cms_addresses WHERE id=?", [$address->id] |
||
131 | )->fetchColumn(); |
||
132 | self::assertInternalType('numeric', $userId); |
||
133 | |||
134 | $this->em->clear(); |
||
135 | |||
136 | $user2 = $this->em->createQuery('select u from \Doctrine\Tests\Models\CMS\CmsUser u where u.id=?1') |
||
137 | ->setParameter(1, $userId) |
||
138 | ->getSingleResult(); |
||
139 | |||
140 | // Address has been eager-loaded because it cant be lazy |
||
141 | self::assertInstanceOf(CmsAddress::class, $user2->address); |
||
142 | self::assertNotInstanceOf(GhostObjectInterface::class, $user2->address); |
||
143 | } |
||
144 | |||
145 | /** |
||
146 | * @group DDC-1230 |
||
147 | */ |
||
148 | public function testRemove() |
||
149 | { |
||
150 | $user = new CmsUser; |
||
151 | $user->name = 'Guilherme'; |
||
152 | $user->username = 'gblanco'; |
||
153 | $user->status = 'developer'; |
||
154 | |||
155 | self::assertEquals(UnitOfWork::STATE_NEW, $this->em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_NEW"); |
||
156 | |||
157 | $this->em->persist($user); |
||
158 | |||
159 | self::assertEquals(UnitOfWork::STATE_MANAGED, $this->em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_MANAGED"); |
||
160 | |||
161 | $this->em->remove($user); |
||
162 | |||
163 | self::assertEquals(UnitOfWork::STATE_NEW, $this->em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_NEW"); |
||
164 | |||
165 | $this->em->persist($user); |
||
166 | $this->em->flush(); |
||
167 | $id = $user->getId(); |
||
168 | |||
169 | $this->em->remove($user); |
||
170 | |||
171 | self::assertEquals(UnitOfWork::STATE_REMOVED, $this->em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_REMOVED"); |
||
172 | $this->em->flush(); |
||
173 | |||
174 | self::assertEquals(UnitOfWork::STATE_NEW, $this->em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_NEW"); |
||
175 | |||
176 | self::assertNull($this->em->find(CmsUser::class, $id)); |
||
177 | } |
||
178 | |||
179 | public function testOneToManyOrphanRemoval() |
||
180 | { |
||
181 | $user = new CmsUser; |
||
182 | $user->name = 'Guilherme'; |
||
183 | $user->username = 'gblanco'; |
||
184 | $user->status = 'developer'; |
||
185 | |||
186 | View Code Duplication | for ($i=0; $i<3; ++$i) { |
|
187 | $phone = new CmsPhonenumber; |
||
188 | $phone->phonenumber = 100 + $i; |
||
189 | $user->addPhonenumber($phone); |
||
190 | } |
||
191 | |||
192 | $this->em->persist($user); |
||
193 | |||
194 | $this->em->flush(); |
||
195 | |||
196 | $user->getPhonenumbers()->remove(0); |
||
197 | self::assertCount(2, $user->getPhonenumbers()); |
||
198 | |||
199 | $this->em->flush(); |
||
200 | |||
201 | // Check that there are just 2 phonenumbers left |
||
202 | $count = $this->em->getConnection()->fetchColumn("SELECT COUNT(*) FROM cms_phonenumbers"); |
||
203 | self::assertEquals(2, $count); // only 2 remaining |
||
204 | |||
205 | // check that clear() removes the others via orphan removal |
||
206 | $user->getPhonenumbers()->clear(); |
||
207 | $this->em->flush(); |
||
208 | self::assertEquals(0, $this->em->getConnection()->fetchColumn("select count(*) from cms_phonenumbers")); |
||
209 | } |
||
210 | |||
211 | public function testBasicQuery() |
||
212 | { |
||
213 | $user = new CmsUser; |
||
214 | $user->name = 'Guilherme'; |
||
215 | $user->username = 'gblanco'; |
||
216 | $user->status = 'developer'; |
||
217 | $this->em->persist($user); |
||
218 | $this->em->flush(); |
||
219 | |||
220 | $query = $this->em->createQuery("select u from Doctrine\Tests\Models\CMS\CmsUser u"); |
||
221 | |||
222 | $users = $query->getResult(); |
||
223 | |||
224 | self::assertCount(1, $users); |
||
225 | self::assertEquals('Guilherme', $users[0]->name); |
||
226 | self::assertEquals('gblanco', $users[0]->username); |
||
227 | self::assertEquals('developer', $users[0]->status); |
||
228 | //self::assertNull($users[0]->phonenumbers); |
||
229 | //self::assertNull($users[0]->articles); |
||
230 | |||
231 | $usersArray = $query->getArrayResult(); |
||
232 | |||
233 | self::assertInternalType('array', $usersArray); |
||
234 | self::assertCount(1, $usersArray); |
||
235 | self::assertEquals('Guilherme', $usersArray[0]['name']); |
||
236 | self::assertEquals('gblanco', $usersArray[0]['username']); |
||
237 | self::assertEquals('developer', $usersArray[0]['status']); |
||
238 | |||
239 | $usersScalar = $query->getScalarResult(); |
||
240 | |||
241 | self::assertInternalType('array', $usersScalar); |
||
242 | self::assertCount(1, $usersScalar); |
||
243 | self::assertEquals('Guilherme', $usersScalar[0]['u_name']); |
||
244 | self::assertEquals('gblanco', $usersScalar[0]['u_username']); |
||
245 | self::assertEquals('developer', $usersScalar[0]['u_status']); |
||
246 | } |
||
247 | |||
248 | View Code Duplication | public function testBasicOneToManyInnerJoin() |
|
249 | { |
||
250 | $user = new CmsUser; |
||
251 | $user->name = 'Guilherme'; |
||
252 | $user->username = 'gblanco'; |
||
253 | $user->status = 'developer'; |
||
254 | $this->em->persist($user); |
||
255 | $this->em->flush(); |
||
256 | |||
257 | $query = $this->em->createQuery("select u from Doctrine\Tests\Models\CMS\CmsUser u join u.phonenumbers p"); |
||
258 | |||
259 | $users = $query->getResult(); |
||
260 | |||
261 | self::assertCount(0, $users); |
||
262 | } |
||
263 | |||
264 | public function testBasicOneToManyLeftJoin() |
||
265 | { |
||
266 | $user = new CmsUser; |
||
267 | $user->name = 'Guilherme'; |
||
268 | $user->username = 'gblanco'; |
||
269 | $user->status = 'developer'; |
||
270 | $this->em->persist($user); |
||
271 | $this->em->flush(); |
||
272 | |||
273 | $query = $this->em->createQuery("select u,p from Doctrine\Tests\Models\CMS\CmsUser u left join u.phonenumbers p"); |
||
274 | |||
275 | $users = $query->getResult(); |
||
276 | |||
277 | self::assertCount(1, $users); |
||
278 | self::assertEquals('Guilherme', $users[0]->name); |
||
279 | self::assertEquals('gblanco', $users[0]->username); |
||
280 | self::assertEquals('developer', $users[0]->status); |
||
281 | self::assertInstanceOf(PersistentCollection::class, $users[0]->phonenumbers); |
||
282 | self::assertTrue($users[0]->phonenumbers->isInitialized()); |
||
283 | self::assertEquals(0, $users[0]->phonenumbers->count()); |
||
284 | } |
||
285 | |||
286 | public function testBasicRefresh() |
||
302 | |||
303 | /** |
||
304 | * @group DDC-833 |
||
305 | */ |
||
306 | View Code Duplication | public function testRefreshResetsCollection() |
|
307 | { |
||
308 | $user = new CmsUser; |
||
309 | $user->name = 'Guilherme'; |
||
310 | $user->username = 'gblanco'; |
||
311 | $user->status = 'developer'; |
||
312 | |||
313 | // Add a phonenumber |
||
314 | $ph1 = new CmsPhonenumber; |
||
315 | $ph1->phonenumber = "12345"; |
||
316 | $user->addPhonenumber($ph1); |
||
317 | |||
318 | // Add a phonenumber |
||
319 | $ph2 = new CmsPhonenumber; |
||
320 | $ph2->phonenumber = "54321"; |
||
321 | |||
322 | $this->em->persist($user); |
||
323 | $this->em->persist($ph1); |
||
324 | $this->em->persist($ph2); |
||
325 | $this->em->flush(); |
||
326 | |||
327 | $user->addPhonenumber($ph2); |
||
328 | |||
329 | self::assertCount(2, $user->phonenumbers); |
||
330 | $this->em->refresh($user); |
||
331 | |||
332 | self::assertCount(1, $user->phonenumbers); |
||
333 | } |
||
334 | |||
335 | /** |
||
336 | * @group DDC-833 |
||
337 | */ |
||
338 | public function testDqlRefreshResetsCollection() |
||
370 | |||
371 | /** |
||
372 | * @group DDC-833 |
||
373 | */ |
||
374 | public function testCreateEntityOfProxy() |
||
406 | |||
407 | public function testAddToCollectionDoesNotInitialize() |
||
408 | { |
||
409 | $user = new CmsUser; |
||
410 | $user->name = 'Guilherme'; |
||
411 | $user->username = 'gblanco'; |
||
412 | $user->status = 'developer'; |
||
413 | |||
414 | View Code Duplication | for ($i=0; $i<3; ++$i) { |
|
446 | |||
447 | public function testInitializeCollectionWithNewObjectsRetainsNewObjects() |
||
487 | |||
488 | public function testOneToManyCascadeRemove() |
||
521 | |||
522 | public function testTextColumnSaveAndRetrieve() |
||
558 | |||
559 | public function testFlushDoesNotIssueUnnecessaryUpdates() |
||
602 | |||
603 | public function testRemoveEntityByReference() |
||
625 | |||
626 | public function testQueryEntityByReference() |
||
627 | { |
||
628 | $user = new CmsUser; |
||
629 | $user->name = 'Guilherme'; |
||
630 | $user->username = 'gblanco'; |
||
631 | $user->status = 'developer'; |
||
632 | |||
633 | $address = new CmsAddress; |
||
634 | $address->country = 'Germany'; |
||
635 | $address->city = 'Berlin'; |
||
636 | $address->zip = '12345'; |
||
637 | |||
638 | $user->setAddress($address); |
||
639 | |||
640 | $this->em->transactional(function($em) use($user) { |
||
641 | $em->persist($user); |
||
642 | }); |
||
643 | $this->em->clear(); |
||
644 | |||
645 | //$this->em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); |
||
646 | |||
647 | $userRef = $this->em->getReference(CmsUser::class, $user->getId()); |
||
648 | $address2 = $this->em->createQuery('select a from Doctrine\Tests\Models\CMS\CmsAddress a where a.user = :user') |
||
649 | ->setParameter('user', $userRef) |
||
650 | ->getSingleResult(); |
||
651 | |||
652 | /* @var $fetchedUser CmsUser|GhostObjectInterface */ |
||
653 | $fetchedUser = $address2->getUser(); |
||
654 | |||
655 | self::assertInstanceOf(GhostObjectInterface::class, $fetchedUser); |
||
656 | self::assertSame($userRef, $fetchedUser); |
||
657 | self::assertFalse($userRef->isProxyInitialized()); |
||
658 | self::assertEquals('Germany', $address2->country); |
||
659 | self::assertEquals('Berlin', $address2->city); |
||
660 | self::assertEquals('12345', $address2->zip); |
||
661 | } |
||
662 | |||
663 | public function testOneToOneNullUpdate() |
||
688 | |||
689 | /** |
||
690 | * @group DDC-600 |
||
691 | * @group DDC-455 |
||
692 | */ |
||
693 | public function testNewAssociatedEntityDuringFlushThrowsException() |
||
714 | |||
715 | /** |
||
716 | * @group DDC-600 |
||
717 | * @group DDC-455 |
||
718 | */ |
||
719 | public function testNewAssociatedEntityDuringFlushThrowsException2() |
||
748 | |||
749 | /** |
||
750 | * @group DDC-600 |
||
751 | * @group DDC-455 |
||
752 | */ |
||
753 | public function testNewAssociatedEntityDuringFlushThrowsException3() |
||
771 | |||
772 | public function testOneToOneOrphanRemoval() |
||
822 | |||
823 | public function testGetPartialReferenceToUpdateObjectWithoutLoadingIt() |
||
845 | |||
846 | /** |
||
847 | * @group DDC-952 |
||
848 | */ |
||
849 | public function testManyToOneFetchModeQuery() |
||
882 | |||
883 | /** |
||
884 | * @group DDC-1278 |
||
885 | */ |
||
886 | public function testClear() |
||
927 | |||
928 | /** |
||
929 | * @group DDC-1585 |
||
930 | */ |
||
931 | View Code Duplication | public function testWrongAssociationInstance() |
|
948 | } |
||
949 |
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.