| Conditions | 8 | 
| Paths | 10 | 
| Total Lines | 75 | 
| Code Lines | 40 | 
| Lines | 36 | 
| Ratio | 48 % | 
| Changes | 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 | ||
| 55 | public function run($request) | ||
| 56 |     { | ||
| 57 | // get all users from LDAP, but only get the attributes we need. | ||
| 58 | // this is useful to avoid holding onto too much data in memory | ||
| 59 | // especially in the case where getUser() would return a lot of users | ||
| 60 | $users = $this->ldapService->getUsers(array_merge( | ||
| 61 | ['objectguid', 'samaccountname', 'useraccountcontrol', 'memberof'], | ||
| 62 |             array_keys(Config::inst()->get('SilverStripe\\Security\\Member', 'ldap_field_mappings')) | ||
| 63 | )); | ||
| 64 | |||
| 65 | $start = time(); | ||
| 66 | |||
| 67 | $count = 0; | ||
| 68 | |||
| 69 | View Code Duplication |         foreach ($users as $data) { | |
| 70 |             $member = Member::get()->filter('GUID', $data['objectguid'])->limit(1)->first(); | ||
| 71 | |||
| 72 |             if (!($member && $member->exists())) { | ||
| 73 | // create the initial Member with some internal fields | ||
| 74 | $member = new Member(); | ||
| 75 | $member->GUID = $data['objectguid']; | ||
| 76 | |||
| 77 | $this->log(sprintf( | ||
| 78 | 'Creating new Member (GUID: %s, sAMAccountName: %s)', | ||
| 79 | $data['objectguid'], | ||
| 80 | $data['samaccountname'] | ||
| 81 | )); | ||
| 82 |             } else { | ||
| 83 | $this->log(sprintf( | ||
| 84 | 'Updating existing Member "%s" (ID: %s, GUID: %s, sAMAccountName: %s)', | ||
| 85 | $member->getName(), | ||
| 86 | $member->ID, | ||
| 87 | $data['objectguid'], | ||
| 88 | $data['samaccountname'] | ||
| 89 | )); | ||
| 90 | } | ||
| 91 | |||
| 92 | // Sync attributes from LDAP to the Member record. This will also write the Member record. | ||
| 93 | // this is also responsible for putting the user into mapped groups | ||
| 94 |             try { | ||
| 95 | $this->ldapService->updateMemberFromLDAP($member, $data); | ||
| 96 |             } catch (Exception $e) { | ||
| 97 | $this->log($e->getMessage()); | ||
| 98 | } | ||
| 99 | |||
| 100 | // cleanup object from memory | ||
| 101 | $member->destroy(); | ||
| 102 | |||
| 103 | $count++; | ||
| 104 | } | ||
| 105 | |||
| 106 | // remove Member records that were previously imported, but no longer exist in the directory | ||
| 107 | // NOTE: DB::query() here is used for performance and so we don't run out of memory | ||
| 108 |         if ($this->config()->destructive) { | ||
| 109 |             foreach (DB::query('SELECT "ID", "GUID" FROM "Member" WHERE "GUID" IS NOT NULL') as $record) { | ||
| 110 |                 if (!isset($users[$record['GUID']])) { | ||
| 111 | $member = Member::get()->byId($record['ID']); | ||
| 112 | $member->delete(); | ||
| 113 | |||
| 114 | $this->log(sprintf( | ||
| 115 | 'Removing Member "%s" (GUID: %s) that no longer exists in LDAP.', | ||
| 116 | $member->getName(), | ||
| 117 | $member->GUID | ||
| 118 | )); | ||
| 119 | |||
| 120 | // cleanup object from memory | ||
| 121 | $member->destroy(); | ||
| 122 | } | ||
| 123 | } | ||
| 124 | } | ||
| 125 | |||
| 126 | $end = time() - $start; | ||
| 127 | |||
| 128 |         $this->log(sprintf('Done. Processed %s records. Duration: %s seconds', $count, round($end, 0))); | ||
| 129 | } | ||
| 130 | |||
| 142 |