adamjakab /
SuiteCRM
| Conditions | 25 |
| Paths | 2514 |
| Total Lines | 139 |
| Code Lines | 82 |
| Lines | 0 |
| Ratio | 0 % |
| Tests | 0 |
| CRAP Score | 650 |
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 |
||
| 89 | function change_component_order($magnitude, $direction, $parent_id=""){ |
||
| 90 | |||
| 91 | if(!empty($this->type) && $this->type=="Save"){ |
||
| 92 | |||
| 93 | //safety check |
||
| 94 | $wall_test = $this->check_wall($magnitude, $direction, $parent_id); |
||
| 95 | |||
| 96 | if($wall_test==false){ |
||
|
|
|||
| 97 | return; |
||
| 98 | } |
||
| 99 | |||
| 100 | |||
| 101 | if($direction=="Left"){ |
||
| 102 | if($this->focus->controller_def['list_x']=="Y"){ |
||
| 103 | $new_x = $this->focus->list_order_x -1; |
||
| 104 | $affected_x = $this->focus->list_order_x; |
||
| 105 | } else { |
||
| 106 | $new_x = ""; |
||
| 107 | $affected_x = ""; |
||
| 108 | } |
||
| 109 | if($this->focus->controller_def['list_y']=="Y"){ |
||
| 110 | |||
| 111 | $new_y = $this->focus->list_order_y; |
||
| 112 | $affected_y = $this->focus->list_order_y; |
||
| 113 | } else { |
||
| 114 | $new_y = ""; |
||
| 115 | $affected_y = ""; |
||
| 116 | } |
||
| 117 | |||
| 118 | $affected_id = $this->get_affected_id($parent_id, $new_x, $new_y); |
||
| 119 | //end if direction Left |
||
| 120 | } |
||
| 121 | if($direction=="Right"){ |
||
| 122 | |||
| 123 | if($this->focus->controller_def['list_x']=="Y"){ |
||
| 124 | $new_x = $this->focus->list_order_x + 1; |
||
| 125 | $affected_x = $this->focus->list_order_x; |
||
| 126 | } else { |
||
| 127 | $new_x = ""; |
||
| 128 | $affected_x = ""; |
||
| 129 | } |
||
| 130 | if($this->focus->controller_def['list_y']=="Y"){ |
||
| 131 | $new_y = $this->focus->list_order_y; |
||
| 132 | $affected_y = $this->focus->list_order_y; |
||
| 133 | } else { |
||
| 134 | $new_y = ""; |
||
| 135 | $affected_y = ""; |
||
| 136 | } |
||
| 137 | $affected_id = $this->get_affected_id($parent_id, $new_x, $new_y); |
||
| 138 | //end if direction Right |
||
| 139 | } |
||
| 140 | |||
| 141 | if($direction=="Up"){ |
||
| 142 | if($this->focus->controller_def['list_x']=="Y"){ |
||
| 143 | $new_x = $this->focus->list_order_x; |
||
| 144 | $affected_x = $this->focus->list_order_x; |
||
| 145 | } else { |
||
| 146 | $new_x = ""; |
||
| 147 | $affected_x = ""; |
||
| 148 | } |
||
| 149 | if($this->focus->controller_def['list_y']=="Y"){ |
||
| 150 | |||
| 151 | $new_y = $this->focus->list_order_y - 1; |
||
| 152 | $affected_y = $this->focus->list_order_y; |
||
| 153 | } else { |
||
| 154 | $new_y = ""; |
||
| 155 | $affected_y = ""; |
||
| 156 | } |
||
| 157 | |||
| 158 | $affected_id = $this->get_affected_id($parent_id, $new_x, $new_y); |
||
| 159 | |||
| 160 | //end if direction Up |
||
| 161 | } |
||
| 162 | if($direction=="Down"){ |
||
| 163 | |||
| 164 | if($this->focus->controller_def['list_x']=="Y"){ |
||
| 165 | $new_x = $this->focus->list_order_x; |
||
| 166 | $affected_x = $this->focus->list_order_x; |
||
| 167 | } else { |
||
| 168 | $new_x = ""; |
||
| 169 | $affected_x = ""; |
||
| 170 | } |
||
| 171 | if($this->focus->controller_def['list_y']=="Y"){ |
||
| 172 | |||
| 173 | $new_y = $this->focus->list_order_y + 1; |
||
| 174 | $affected_y = $this->focus->list_order_y; |
||
| 175 | } else { |
||
| 176 | $new_y = ""; |
||
| 177 | $affected_y = ""; |
||
| 178 | } |
||
| 179 | $affected_id = $this->get_affected_id($parent_id, $new_x, $new_y); |
||
| 180 | //end if direction Down |
||
| 181 | } |
||
| 182 | |||
| 183 | //This takes care of the component being pushed out of its position |
||
| 184 | $this->update_affected_order($affected_id, $affected_x, $affected_y); |
||
| 185 | |||
| 186 | //This takes care of the new positions for itself |
||
| 187 | if($this->focus->controller_def['list_x']=="Y"){ |
||
| 188 | $this->focus->list_order_x = $new_x; |
||
| 189 | } |
||
| 190 | |||
| 191 | if($this->focus->controller_def['list_y']=="Y"){ |
||
| 192 | $this->focus->list_order_y = $new_y; |
||
| 193 | } |
||
| 194 | |||
| 195 | } else { |
||
| 196 | //this is a new component, set the x or y value to the max + 1 |
||
| 197 | |||
| 198 | $query = "SELECT MAX(".$this->focus->controller_def['start_var'].") max_start from ".$this->focus->table_name." |
||
| 199 | WHERE ".$this->focus->controller_def['parent_var']."='$parent_id' |
||
| 200 | AND ".$this->focus->table_name.".deleted='0' |
||
| 201 | "; |
||
| 202 | $result = $this->db->query($query,true," Error capturing max start order: "); |
||
| 203 | $row = $this->db->fetchByAssoc($result); |
||
| 204 | |||
| 205 | if(!is_null($row['max_start'])){ |
||
| 206 | |||
| 207 | if($this->focus->controller_def['start_axis']=="x") { |
||
| 208 | $this->focus->list_order_x = $row['max_start'] + 1; |
||
| 209 | if($this->focus->controller_def['list_y']=="Y") $this->focus->list_order_y = 0; |
||
| 210 | } |
||
| 211 | |||
| 212 | if($this->focus->controller_def['start_axis']=="y") { |
||
| 213 | $this->focus->list_order_y = $row['max_start'] + 1; |
||
| 214 | if($this->focus->controller_def['list_x']=="Y") $this->focus->list_order_x = 0; |
||
| 215 | } |
||
| 216 | |||
| 217 | } else { |
||
| 218 | //first row |
||
| 219 | if($this->focus->controller_def['list_x']=="Y") $this->focus->list_order_x = 0; |
||
| 220 | if($this->focus->controller_def['list_y']=="Y") $this->focus->list_order_y = 0; |
||
| 221 | |||
| 222 | //end if else to check if this is first entry |
||
| 223 | } |
||
| 224 | //end if else on whether this is a new entry |
||
| 225 | } |
||
| 226 | //end function change_component_order |
||
| 227 | } |
||
| 228 | |||
| 356 | ?> |
When comparing two booleans, it is generally considered safer to use the strict comparison operator.