Complex classes like COMMENTREPORT 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 COMMENTREPORT, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 23 | class COMMENTREPORT { |
||
|
|
|||
| 24 | |||
| 25 | public $report_id = ''; |
||
| 26 | public $comment_id = ''; |
||
| 27 | public $firstname = ''; |
||
| 28 | public $lastname = ''; |
||
| 29 | public $body = ''; |
||
| 30 | public $reported = NULL; // datetime |
||
| 31 | public $resolved = NULL; // datetime |
||
| 32 | public $resolvedby = ''; // user_id |
||
| 33 | public $locked = NULL; // datetime |
||
| 34 | public $lockedby = ''; // user_id |
||
| 35 | public $upheld = ''; // boolean |
||
| 36 | |||
| 37 | // If the user was logged in, this will be set: |
||
| 38 | public $user_id = ''; |
||
| 39 | // If the user wasn't logged in, this will be set: |
||
| 40 | public $email = ''; |
||
| 41 | |||
| 42 | |||
| 43 | public function __construct($report_id='') { |
||
| 44 | // Pass it a report id and it gets and sets this report's data. |
||
| 45 | |||
| 46 | $this->db = new ParlDB; |
||
| 47 | |||
| 48 | if (is_numeric($report_id)) { |
||
| 49 | |||
| 50 | $q = $this->db->query("SELECT commentreports.comment_id, |
||
| 51 | commentreports.user_id, |
||
| 52 | commentreports.body, |
||
| 53 | DATE_FORMAT(commentreports.reported, '" . SHORTDATEFORMAT_SQL . ' ' . TIMEFORMAT_SQL . "') AS reported, |
||
| 54 | DATE_FORMAT(commentreports.resolved, '" . SHORTDATEFORMAT_SQL . ' ' . TIMEFORMAT_SQL . "') AS resolved, |
||
| 55 | commentreports.resolvedby, |
||
| 56 | commentreports.locked, |
||
| 57 | commentreports.lockedby, |
||
| 58 | commentreports.upheld, |
||
| 59 | commentreports.firstname, |
||
| 60 | commentreports.lastname, |
||
| 61 | commentreports.email, |
||
| 62 | users.firstname AS u_firstname, |
||
| 63 | users.lastname AS u_lastname |
||
| 64 | FROM commentreports, |
||
| 65 | users |
||
| 66 | WHERE commentreports.report_id = :report_id |
||
| 67 | AND commentreports.user_id = users.user_id |
||
| 68 | ", array( |
||
| 69 | ':report_id' => $report_id |
||
| 70 | )); |
||
| 71 | |||
| 72 | if ($q->rows() > 0) { |
||
| 73 | $this->report_id = $report_id; |
||
| 74 | $this->comment_id = $q->field(0, 'comment_id'); |
||
| 75 | $this->body = $q->field(0, 'body'); |
||
| 76 | $this->reported = $q->field(0, 'reported'); |
||
| 77 | $this->resolved = $q->field(0, 'resolved'); |
||
| 78 | $this->resolvedby = $q->field(0, 'resolvedby'); |
||
| 79 | $this->locked = $q->field(0, 'locked'); |
||
| 80 | $this->lockedby = $q->field(0, 'lockedby'); |
||
| 81 | $this->upheld = $q->field(0, 'upheld'); |
||
| 82 | |||
| 83 | if ($q->field(0, 'user_id') == 0) { |
||
| 84 | // The report was made by a non-logged-in user. |
||
| 85 | $this->firstname = $q->field(0, 'firstname'); |
||
| 86 | $this->lastname = $q->field(0, 'lastname'); |
||
| 87 | $this->email = $q->field(0, 'email'); |
||
| 88 | } else { |
||
| 89 | // The report was made by a logged-in user. |
||
| 90 | $this->firstname = $q->field(0, 'u_firstname'); |
||
| 91 | $this->lastname = $q->field(0, 'u_lastname'); |
||
| 92 | $this->user_id = $q->field(0, 'user_id'); |
||
| 93 | } |
||
| 94 | } else { |
||
| 95 | $q = $this->db->query("SELECT commentreports.comment_id, |
||
| 96 | commentreports.user_id, |
||
| 97 | commentreports.body, |
||
| 98 | DATE_FORMAT(commentreports.reported, '" . SHORTDATEFORMAT_SQL . ' ' . TIMEFORMAT_SQL . "') AS reported, |
||
| 99 | DATE_FORMAT(commentreports.resolved, '" . SHORTDATEFORMAT_SQL . ' ' . TIMEFORMAT_SQL . "') AS resolved, |
||
| 100 | commentreports.resolvedby, |
||
| 101 | commentreports.locked, |
||
| 102 | commentreports.lockedby, |
||
| 103 | commentreports.upheld, |
||
| 104 | commentreports.firstname, |
||
| 105 | commentreports.lastname, |
||
| 106 | commentreports.email |
||
| 107 | FROM commentreports |
||
| 108 | WHERE commentreports.report_id = :report_id", array( |
||
| 109 | ':report_id' => $report_id |
||
| 110 | )); |
||
| 111 | |||
| 112 | if ($q->rows() > 0) { |
||
| 113 | $this->report_id = $report_id; |
||
| 114 | $this->comment_id = $q->field(0, 'comment_id'); |
||
| 115 | $this->body = $q->field(0, 'body'); |
||
| 116 | $this->reported = $q->field(0, 'reported'); |
||
| 117 | $this->resolved = $q->field(0, 'resolved'); |
||
| 118 | $this->resolvedby = $q->field(0, 'resolvedby'); |
||
| 119 | $this->locked = $q->field(0, 'locked'); |
||
| 120 | $this->lockedby = $q->field(0, 'lockedby'); |
||
| 121 | $this->upheld = $q->field(0, 'upheld'); |
||
| 122 | $this->firstname = $q->field(0, 'firstname'); |
||
| 123 | $this->lastname = $q->field(0, 'lastname'); |
||
| 124 | $this->email = $q->field(0, 'email'); |
||
| 125 | } |
||
| 126 | } |
||
| 127 | } |
||
| 128 | } |
||
| 129 | |||
| 130 | |||
| 131 | public function report_id() { return $this->report_id; } |
||
| 132 | public function comment_id() { return $this->comment_id; } |
||
| 133 | public function user_id() { return $this->user_id; } |
||
| 134 | public function user_name() { return $this->firstname . ' ' . $this->lastname; } |
||
| 135 | public function firstname() { return $this->firstname; } |
||
| 136 | public function lastname() { return $this->lastname; } |
||
| 137 | public function email() { return $this->email; } |
||
| 138 | public function body() { return $this->body; } |
||
| 139 | public function reported() { return $this->reported; } |
||
| 140 | public function resolved() { return $this->resolved; } |
||
| 141 | public function resolvedby() { return $this->resolvedby; } |
||
| 142 | public function locked() { return $this->locked; } |
||
| 143 | public function lockedby() { return $this->lockedby; } |
||
| 144 | public function upheld() { return $this->upheld; } |
||
| 145 | |||
| 146 | public function create($COMMENT, $reportdata) { |
||
| 147 | // For when a user posts a report on a comment. |
||
| 148 | // $reportdata is an array like: |
||
| 149 | // array ( |
||
| 150 | // 'body' => 'some text', |
||
| 151 | // 'firstname' => 'Billy', |
||
| 152 | // 'lastname' => 'Nomates', |
||
| 153 | // 'email' => '[email protected]' |
||
| 154 | // ) |
||
| 155 | // But if the report was made by a logged-in user, only the |
||
| 156 | // 'body' element should really contain anything, because |
||
| 157 | // we use $THEUSER's id to get the rest. |
||
| 158 | |||
| 159 | // $COMMENT is an existing COMMENT object, needed for setting |
||
| 160 | // its modflag and comment_id. |
||
| 161 | |||
| 162 | global $THEUSER, $PAGE; |
||
| 163 | |||
| 164 | if (!$THEUSER->is_able_to('reportcomment')) { |
||
| 165 | $PAGE->error_message ("Sorry, you are not allowed to post reports."); |
||
| 166 | return false; |
||
| 167 | } |
||
| 168 | |||
| 169 | if (is_numeric($THEUSER->user_id()) && $THEUSER->user_id() > 0) { |
||
| 170 | // Flood check - make sure the user hasn't just posted a report recently. |
||
| 171 | // To help prevent accidental duplicates, among other nasty things. |
||
| 172 | // (Non-logged in users are all id == 0.) |
||
| 173 | |||
| 174 | $flood_time_limit = 20; // How many seconds until a user can post again? |
||
| 175 | |||
| 176 | $q = $this->db->query("SELECT report_id |
||
| 177 | FROM commentreports |
||
| 178 | WHERE user_id = '" . $THEUSER->user_id() . "' |
||
| 179 | AND reported + 0 > NOW() - $flood_time_limit"); |
||
| 180 | |||
| 181 | if ($q->rows() > 0) { |
||
| 182 | $PAGE->error_message("Sorry, we limit people to posting one report per $flood_time_limit seconds to help prevent duplicate reports. Please go back and try again, thanks."); |
||
| 183 | return false; |
||
| 184 | } |
||
| 185 | } |
||
| 186 | |||
| 187 | |||
| 188 | // Tidy up body. |
||
| 189 | $body = filter_user_input($reportdata['body'], 'comment'); // In utility.php |
||
| 190 | |||
| 191 | $time = gmdate("Y-m-d H:i:s"); |
||
| 192 | |||
| 193 | if ($THEUSER->isloggedin()) { |
||
| 194 | $sql = "INSERT INTO commentreports |
||
| 195 | (comment_id, body, reported, user_id) |
||
| 196 | VALUES (:comment_id, |
||
| 197 | :body, |
||
| 198 | :time, |
||
| 199 | :user_id |
||
| 200 | ) |
||
| 201 | "; |
||
| 202 | $params = array( |
||
| 203 | ':comment_id' => $COMMENT->comment_id(), |
||
| 204 | ':body' => $body, |
||
| 205 | ':time' => $time, |
||
| 206 | ':user_id' => $THEUSER->user_id() |
||
| 207 | ); |
||
| 208 | } else { |
||
| 209 | $sql = "INSERT INTO commentreports |
||
| 210 | (comment_id, body, reported, firstname, lastname, email) |
||
| 211 | VALUES (:comment_id, |
||
| 212 | :body, |
||
| 213 | :time, |
||
| 214 | :firstname, |
||
| 215 | :lastname, |
||
| 216 | |||
| 217 | ) |
||
| 218 | "; |
||
| 219 | $params = array( |
||
| 220 | ':comment_id' => $COMMENT->comment_id(), |
||
| 221 | ':body' => $body, |
||
| 222 | ':time' => $time, |
||
| 223 | ':firstname' => $reportdata['firstname'], |
||
| 224 | ':lastname' => $reportdata['lastname'], |
||
| 225 | ':email' => $reportdata['email'], |
||
| 226 | ); |
||
| 227 | } |
||
| 228 | |||
| 229 | $q = $this->db->query($sql, $params); |
||
| 230 | |||
| 231 | if ($q->success()) { |
||
| 232 | // Inserted OK, so set up this object's variables. |
||
| 233 | $this->report_id = $q->insert_id(); |
||
| 234 | $this->comment_id = $COMMENT->comment_id(); |
||
| 235 | $this->body = $body; |
||
| 236 | $this->reported = $time; |
||
| 237 | |||
| 238 | if ($THEUSER->isloggedin()) { |
||
| 239 | $this->user_id = $THEUSER->user_id(); |
||
| 240 | $this->firstname = $THEUSER->firstname(); |
||
| 241 | $this->lastname = $THEUSER->lastname(); |
||
| 242 | } else { |
||
| 243 | $this->email = $reportdata['email']; |
||
| 244 | $this->firstname = $reportdata['firstname']; |
||
| 245 | $this->lastname = $reportdata['lastname']; |
||
| 246 | } |
||
| 247 | |||
| 248 | |||
| 249 | // Set the comment's modflag to on. |
||
| 250 | $COMMENT->set_modflag('on'); |
||
| 251 | |||
| 252 | |||
| 253 | // Notify those who need to know that there's a new report. |
||
| 254 | |||
| 255 | $URL = new \MySociety\TheyWorkForYou\Url('admin_commentreport'); |
||
| 256 | $URL->insert(array( |
||
| 257 | 'rid'=>$this->report_id, |
||
| 258 | 'cid'=>$this->comment_id |
||
| 259 | )); |
||
| 260 | |||
| 261 | $emailbody = "A new comment report has been filed by " . $this->user_name() . ".\n\n"; |
||
| 262 | $emailbody .= "COMMENT:\n" . $COMMENT->body() . "\n\n"; |
||
| 263 | $emailbody .= "REPORT:\n" . $this->body . "\n\n"; |
||
| 264 | $emailbody .= "To manage this report follow this link: https://" . DOMAIN . $URL->generate('none') . "\n"; |
||
|
1 ignored issue
–
show
|
|||
| 265 | |||
| 266 | send_email(REPORTLIST, 'New comment report', $emailbody); |
||
|
1 ignored issue
–
show
|
|||
| 267 | |||
| 268 | |||
| 269 | // Send an email to the user to thank them. |
||
| 270 | |||
| 271 | if ($THEUSER->isloggedin()) { |
||
| 272 | $email = $THEUSER->email(); |
||
| 273 | } else { |
||
| 274 | $email = $this->email(); |
||
| 275 | } |
||
| 276 | |||
| 277 | $data = array ( |
||
| 278 | 'to' => $email, |
||
| 279 | 'template' => 'report_acknowledge' |
||
| 280 | ); |
||
| 281 | $merge = array ( |
||
| 282 | 'FIRSTNAME' => $this->firstname(), |
||
| 283 | 'LASTNAME' => $this->lastname(), |
||
| 284 | 'COMMENTURL' => "https://" . DOMAIN . $COMMENT->url(), |
||
| 285 | 'REPORTBODY' => strip_tags($this->body()) |
||
| 286 | ); |
||
| 287 | |||
| 288 | |||
| 289 | // send_template_email in utility.php. |
||
| 290 | send_template_email($data, $merge); |
||
| 291 | |||
| 292 | return true; |
||
| 293 | } else { |
||
| 294 | return false; |
||
| 295 | } |
||
| 296 | |||
| 297 | } |
||
| 298 | |||
| 299 | |||
| 300 | public function display() { |
||
| 301 | |||
| 302 | $data = array(); |
||
| 303 | |||
| 304 | if (is_numeric($this->report_id)) { |
||
| 305 | $data = array ( |
||
| 306 | 'report_id' => $this->report_id(), |
||
| 307 | 'comment_id' => $this->comment_id(), |
||
| 308 | 'user_id' => $this->user_id(), |
||
| 309 | 'user_name' => $this->user_name(), |
||
| 310 | 'body' => $this->body(), |
||
| 311 | 'reported' => $this->reported(), |
||
| 312 | 'resolved' => $this->resolved(), |
||
| 313 | 'resolvedby' => $this->resolvedby(), |
||
| 314 | 'locked' => $this->locked(), |
||
| 315 | 'lockedby' => $this->lockedby(), |
||
| 316 | 'upheld' => $this->upheld() |
||
| 317 | ); |
||
| 318 | } |
||
| 319 | |||
| 320 | $this->render($data); |
||
| 321 | } |
||
| 322 | |||
| 323 | |||
| 324 | |||
| 325 | public function render($data) { |
||
| 326 | global $PAGE; |
||
| 327 | |||
| 328 | $PAGE->display_commentreport($data); |
||
| 329 | |||
| 330 | } |
||
| 331 | |||
| 332 | |||
| 333 | public function lock() { |
||
| 334 | // Called when an admin user goes to examine a report, so that |
||
| 335 | // only one person can edit at once. |
||
| 336 | |||
| 337 | global $THEUSER, $PAGE; |
||
| 338 | |||
| 339 | if ($THEUSER->is_able_to('deletecomment')) { |
||
| 340 | $time = gmdate("Y-m-d H:i:s"); |
||
| 341 | |||
| 342 | $q = $this->db->query ("UPDATE commentreports |
||
| 343 | SET locked = '$time', |
||
| 344 | lockedby = '" . $THEUSER->user_id() . "' |
||
| 345 | WHERE report_id = '" . $this->report_id . "' |
||
| 346 | "); |
||
| 347 | |||
| 348 | if ($q->success()) { |
||
| 349 | $this->locked = $time; |
||
| 350 | $this->lockedby = $THEUSER->user_id(); |
||
| 351 | return true; |
||
| 352 | } else { |
||
| 353 | $PAGE->error_message ("Sorry, we were unable to lock this report."); |
||
| 354 | return false; |
||
| 355 | } |
||
| 356 | } else { |
||
| 357 | $PAGE->error_message ("You are not authorised to delete annotations."); |
||
| 358 | return false; |
||
| 359 | } |
||
| 360 | } |
||
| 361 | |||
| 362 | |||
| 363 | public function unlock() { |
||
| 364 | // Unlock a comment so it can be examined by someone else. |
||
| 365 | |||
| 366 | $q = $this->db->query ("UPDATE commentreports |
||
| 367 | SET locked = NULL, |
||
| 368 | lockedby = NULL |
||
| 369 | WHERE report_id = '" . $this->report_id . "' |
||
| 370 | "); |
||
| 371 | |||
| 372 | if ($q->success()) { |
||
| 373 | $this->locked = NULL; |
||
| 374 | $this->lockedby = NULL; |
||
| 375 | return true; |
||
| 376 | } else { |
||
| 377 | return false; |
||
| 378 | } |
||
| 379 | } |
||
| 380 | |||
| 381 | |||
| 382 | |||
| 383 | public function resolve($upheld, $COMMENT) { |
||
| 451 | } |
||
| 452 | } |
||
| 453 | |||
| 454 | |||
| 455 | |||
| 456 | } |
||
| 457 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.