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 |
||
6 | class Ban extends DataObject |
||
7 | { |
||
8 | private $type; |
||
9 | private $target; |
||
10 | private $user; |
||
11 | private $reason; |
||
12 | private $date; |
||
13 | private $duration; |
||
14 | private $active; |
||
15 | |||
16 | /** |
||
17 | * Gets all bans, expired and active filtered by the optional target. |
||
18 | * @param $target string The email, IP, or name of the target of the ban |
||
19 | * @param PdoDatabase $database gGetDb() |
||
20 | * @return Ban[] |
||
21 | */ |
||
22 | public static function getAllBans($target = null, PdoDatabase $database = null) |
||
23 | { |
||
24 | if ($database == null) { |
||
25 | $database = gGetDb(); |
||
26 | } |
||
27 | |||
28 | if ($target != null) { |
||
29 | $query = "SELECT * FROM ban WHERE target = :target;"; |
||
30 | $statement = $database->prepare($query); |
||
31 | $statement->bindValue(":target", $target); |
||
32 | } |
||
33 | else { |
||
34 | $query = "SELECT * FROM ban;"; |
||
35 | $statement = $database->prepare($query); |
||
36 | } |
||
37 | |||
38 | $statement->execute(); |
||
39 | |||
40 | $result = array(); |
||
41 | /** @var Ban $v */ |
||
42 | foreach ($statement->fetchAll(PDO::FETCH_CLASS, get_called_class()) as $v) { |
||
43 | $v->isNew = false; |
||
44 | $v->setDatabase($database); |
||
45 | $result[] = $v; |
||
46 | } |
||
47 | |||
48 | return $result; |
||
49 | } |
||
50 | |||
51 | /** |
||
52 | * Gets all active bans, filtered by the optional target. |
||
53 | * @param $target |
||
54 | * @param PdoDatabase $database |
||
55 | * @return Ban[] |
||
56 | */ |
||
57 | public static function getActiveBans($target = null, PdoDatabase $database = null) |
||
58 | { |
||
59 | if ($database == null) { |
||
60 | $database = gGetDb(); |
||
61 | } |
||
62 | |||
63 | if ($target != null) { |
||
64 | $query = "SELECT * FROM ban WHERE target = :target AND (duration > UNIX_TIMESTAMP() OR duration = -1) AND active = 1;"; |
||
65 | $statement = $database->prepare($query); |
||
66 | $statement->bindValue(":target", $target); |
||
67 | } |
||
68 | else { |
||
69 | $query = "SELECT * FROM ban WHERE (duration > UNIX_TIMESTAMP() OR duration = -1) AND active = 1;"; |
||
70 | $statement = $database->prepare($query); |
||
71 | } |
||
72 | |||
73 | $statement->execute(); |
||
74 | |||
75 | $result = array(); |
||
76 | |||
77 | /** @var Ban $v */ |
||
78 | foreach ($statement->fetchAll(PDO::FETCH_CLASS, get_called_class()) as $v) { |
||
79 | $v->isNew = false; |
||
80 | $v->setDatabase($database); |
||
81 | $result[] = $v; |
||
82 | } |
||
83 | |||
84 | return $result; |
||
85 | } |
||
86 | |||
87 | /** |
||
88 | * Gets a ban by it's ID if it's currently active. |
||
89 | * @param $id |
||
90 | * @param PdoDatabase $database |
||
91 | * @return Ban |
||
92 | */ |
||
93 | public static function getActiveId($id, PdoDatabase $database = null) |
||
94 | { |
||
95 | if ($database == null) { |
||
96 | $database = gGetDb(); |
||
97 | } |
||
98 | |||
99 | $statement = $database->prepare(<<<SQL |
||
100 | SELECT * |
||
101 | FROM ban |
||
102 | WHERE id = :id AND (duration > UNIX_TIMESTAMP() OR duration = -1) AND active = 1; |
||
103 | SQL |
||
104 | ); |
||
105 | $statement->bindValue(":id", $id); |
||
106 | |||
107 | $statement->execute(); |
||
108 | |||
109 | $resultObject = $statement->fetchObject(get_called_class()); |
||
110 | |||
111 | if ($resultObject != false) { |
||
112 | $resultObject->isNew = false; |
||
113 | $resultObject->setDatabase($database); |
||
114 | } |
||
115 | |||
116 | return $resultObject; |
||
117 | } |
||
118 | |||
119 | /** |
||
120 | * Get all active bans for a target and type. |
||
121 | * @param string $target |
||
122 | * @param string $type |
||
123 | * @param PdoDatabase $database |
||
124 | * @return Ban |
||
125 | */ |
||
126 | public static function getBanByTarget($target, $type, PdoDatabase $database = null) |
||
127 | { |
||
128 | if ($database == null) { |
||
129 | $database = gGetDb(); |
||
130 | } |
||
131 | |||
132 | $query = "SELECT * FROM ban WHERE type = :type AND target = :target AND (duration > UNIX_TIMESTAMP() OR duration = -1) AND active = 1;"; |
||
133 | $statement = $database->prepare($query); |
||
134 | $statement->bindValue(":target", $target); |
||
135 | $statement->bindValue(":type", $type); |
||
136 | |||
137 | $statement->execute(); |
||
138 | |||
139 | $resultObject = $statement->fetchObject(get_called_class()); |
||
140 | |||
141 | if ($resultObject != false) { |
||
142 | $resultObject->isNew = false; |
||
143 | $resultObject->setDatabase($database); |
||
144 | } |
||
145 | |||
146 | return $resultObject; |
||
147 | } |
||
148 | |||
149 | public function save() |
||
150 | { |
||
151 | if ($this->isNew) { |
||
152 | // insert |
||
153 | $statement = $this->dbObject->prepare("INSERT INTO `ban` (type, target, user, reason, date, duration, active) VALUES (:type, :target, :user, :reason, CURRENT_TIMESTAMP(), :duration, :active);"); |
||
154 | $statement->bindValue(":type", $this->type); |
||
155 | $statement->bindValue(":target", $this->target); |
||
156 | $statement->bindValue(":user", $this->user); |
||
157 | $statement->bindValue(":reason", $this->reason); |
||
158 | $statement->bindValue(":duration", $this->duration); |
||
159 | $statement->bindValue(":active", $this->active); |
||
160 | if ($statement->execute()) { |
||
161 | $this->isNew = false; |
||
162 | $this->id = $this->dbObject->lastInsertId(); |
||
|
|||
163 | } |
||
164 | else { |
||
165 | throw new Exception($statement->errorInfo()); |
||
166 | } |
||
167 | } |
||
168 | else { |
||
169 | // update |
||
170 | $statement = $this->dbObject->prepare("UPDATE `ban` SET duration = :duration, active = :active, user = :user WHERE id = :id;"); |
||
171 | $statement->bindValue(":id", $this->id); |
||
172 | $statement->bindValue(":duration", $this->duration); |
||
173 | $statement->bindValue(":active", $this->active); |
||
174 | $statement->bindValue(":user", $this->user); |
||
175 | |||
176 | if (!$statement->execute()) { |
||
177 | throw new Exception($statement->errorInfo()); |
||
178 | } |
||
179 | } |
||
180 | } |
||
181 | |||
182 | public function getType() |
||
183 | { |
||
184 | return $this->type; |
||
185 | } |
||
186 | |||
187 | public function setType($type) |
||
188 | { |
||
189 | $this->type = $type; |
||
190 | } |
||
191 | |||
192 | public function getTarget() |
||
193 | { |
||
194 | return $this->target; |
||
195 | } |
||
196 | |||
197 | public function setTarget($target) |
||
198 | { |
||
199 | $this->target = $target; |
||
200 | } |
||
201 | |||
202 | public function getUser() |
||
203 | { |
||
204 | $user = User::getById($this->user, gGetDb()); |
||
205 | |||
206 | return $user; |
||
207 | } |
||
208 | |||
209 | public function setUser($user) |
||
210 | { |
||
211 | if (User::getById($user, gGetDb()) == false) { |
||
212 | $u = User::getByUsername($user, gGetDb()); |
||
213 | if ($u == false) { |
||
214 | throw new Exception("Unknown user trying to create ban!"); |
||
215 | } |
||
216 | |||
217 | $this->user = $u->getId(); |
||
218 | } |
||
219 | else { |
||
220 | $this->user = $user; |
||
221 | } |
||
222 | } |
||
223 | |||
224 | public function getReason() |
||
225 | { |
||
226 | return $this->reason; |
||
227 | } |
||
228 | |||
229 | public function setReason($reason) |
||
232 | } |
||
233 | |||
234 | public function getDate() |
||
237 | } |
||
238 | |||
239 | public function getDuration() |
||
240 | { |
||
241 | return $this->duration; |
||
242 | } |
||
243 | |||
244 | public function setDuration($duration) |
||
245 | { |
||
246 | $this->duration = $duration; |
||
247 | } |
||
248 | |||
249 | public function getActive() |
||
250 | { |
||
251 | return $this->active; |
||
252 | } |
||
253 | |||
254 | public function setActive($active) |
||
255 | { |
||
256 | $this->active = $active; |
||
257 | } |
||
258 | |||
259 | public function getObjectDescription() |
||
262 | } |
||
263 | } |
||
264 |
This check looks for assignments to scalar types that may be of the wrong type.
To ensure the code behaves as expected, it may be a good idea to add an explicit type cast.