| Conditions | 7 |
| Paths | 203 |
| Total Lines | 84 |
| Code Lines | 54 |
| Lines | 0 |
| Ratio | 0 % |
| Changes | 6 | ||
| Bugs | 0 | Features | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
| 1 | <?php |
||
| 41 | public function execute(InputInterface $input, OutputInterface $output) |
||
| 42 | { |
||
| 43 | /** @var QuestionHelper $questionHelper */ |
||
| 44 | $questionHelper = $this->getHelper('question'); |
||
| 45 | |||
| 46 | $output->writeln([ |
||
| 47 | '<error>WARNING: you are about to migrate institution configurations' |
||
| 48 | . ' to work with identifiers based on insitutions with normalized casing.</error>', |
||
| 49 | '<error>This command is intended to be run only once.</error>', |
||
| 50 | '' |
||
| 51 | ]); |
||
| 52 | |||
| 53 | $confirmationQuestion = new ConfirmationQuestion( |
||
| 54 | 'Are you sure you want to run the institution configuration migrations? [y/N]', |
||
| 55 | false |
||
| 56 | ); |
||
| 57 | $confirmed = $questionHelper->ask($input, $output, $confirmationQuestion); |
||
| 58 | |||
| 59 | if (!$confirmed) { |
||
| 60 | $output->writeln('<info>Exiting without running migrations.</info>'); |
||
| 61 | return; |
||
| 62 | } |
||
| 63 | |||
| 64 | /** @var Container $container */ |
||
| 65 | $container = $this->getApplication()->getKernel()->getContainer(); |
||
|
|
|||
| 66 | |||
| 67 | $tokenStorage = $container->get('security.token_storage'); |
||
| 68 | $connectionHelper = $container->get('surfnet_stepup_middleware_middleware.dbal_connection_helper'); |
||
| 69 | $provider = $container->get('surfnet_stepup_middleware_middleware.institution_configuration_provider'); |
||
| 70 | $pipeline = $container->get('pipeline'); |
||
| 71 | $entityManager = $container->get('doctrine.orm.middleware_entity_manager'); |
||
| 72 | |||
| 73 | // The InstitutionConfiguration commands require ROLE_MANAGEMENT, AddRaLocation requires ROLE_RA |
||
| 74 | // Note that the new events will not have any actor metadata associated with them |
||
| 75 | $tokenStorage->setToken( |
||
| 76 | new AnonymousToken('cli.institution_configuration_migration', 'cli', ['ROLE_MANAGEMENT', 'ROLE_RA']) |
||
| 77 | ); |
||
| 78 | |||
| 79 | $output->writeln('<info>Starting institution configuration migrations</info>'); |
||
| 80 | $connectionHelper->beginTransaction(); |
||
| 81 | |||
| 82 | try { |
||
| 83 | $output->writeln('<info>Loading existing institution configuration data</info>'); |
||
| 84 | $state = $provider->loadData(); |
||
| 85 | |||
| 86 | $output->writeln('<info>Removing existing institution configurations</info>'); |
||
| 87 | foreach ($state->inferRemovalCommands() as $removalCommand) { |
||
| 88 | $pipeline->process($removalCommand); |
||
| 89 | } |
||
| 90 | $entityManager->flush(); |
||
| 91 | $entityManager->clear(); |
||
| 92 | |||
| 93 | $output->writeln('<info>Creating new institution configurations</info>'); |
||
| 94 | foreach ($state->inferCreateCommands() as $createCommand) { |
||
| 95 | $pipeline->process($createCommand); |
||
| 96 | } |
||
| 97 | $entityManager->flush(); |
||
| 98 | $entityManager->clear(); |
||
| 99 | |||
| 100 | $output->writeln('<info>Reconfiguring institution configurations</info>'); |
||
| 101 | foreach ($state->inferReconfigureCommands() as $reconfigureCommand) { |
||
| 102 | $pipeline->process($reconfigureCommand); |
||
| 103 | } |
||
| 104 | $entityManager->flush(); |
||
| 105 | $entityManager->clear(); |
||
| 106 | |||
| 107 | $output->writeln('<info>Adding RA locations</info>'); |
||
| 108 | foreach ($state->inferAddRaLocationCommands() as $addRaLocationCommand) { |
||
| 109 | $pipeline->process($addRaLocationCommand); |
||
| 110 | } |
||
| 111 | $entityManager->flush(); |
||
| 112 | $entityManager->clear(); |
||
| 113 | |||
| 114 | $connectionHelper->commit(); |
||
| 115 | $output->writeln('<info>Successfully migrated institution configurations.</info>'); |
||
| 116 | } catch (Exception $exception) { |
||
| 117 | $output->writeln( |
||
| 118 | sprintf('<error>Could not migrate institution configurations: %s</error>', $exception->getMessage()) |
||
| 119 | ); |
||
| 120 | $connectionHelper->rollBack(); |
||
| 121 | |||
| 122 | throw $exception; |
||
| 123 | } |
||
| 124 | } |
||
| 125 | } |
||
| 126 |
Let’s take a look at an example:
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:
Add an additional type-check:
Add the method to the parent class: