| Conditions | 13 |
| Paths | 10 |
| Total Lines | 163 |
| Lines | 0 |
| Ratio | 0 % |
| 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 |
||
| 217 | function random_int($min, $max) |
||
| 218 | { |
||
| 219 | /** |
||
| 220 | * Type and input logic checks |
||
| 221 | * |
||
| 222 | * If you pass it a float in the range (~PHP_INT_MAX, PHP_INT_MAX) |
||
| 223 | * (non-inclusive), it will sanely cast it to an int. If you it's equal to |
||
| 224 | * ~PHP_INT_MAX or PHP_INT_MAX, we let it fail as not an integer. Floats |
||
| 225 | * lose precision, so the <= and => operators might accidentally let a float |
||
| 226 | * through. |
||
| 227 | */ |
||
| 228 | |||
| 229 | try { |
||
| 230 | /** @var int $min */ |
||
| 231 | $min = RandomCompat_intval($min); |
||
| 232 | } catch (TypeError $ex) { |
||
| 233 | throw new TypeError( |
||
| 234 | 'random_int(): $min must be an integer' |
||
| 235 | ); |
||
| 236 | } |
||
| 237 | |||
| 238 | try { |
||
| 239 | /** @var int $max */ |
||
| 240 | $max = RandomCompat_intval($max); |
||
| 241 | } catch (TypeError $ex) { |
||
| 242 | throw new TypeError( |
||
| 243 | 'random_int(): $max must be an integer' |
||
| 244 | ); |
||
| 245 | } |
||
| 246 | |||
| 247 | /** |
||
| 248 | * Now that we've verified our weak typing system has given us an integer, |
||
| 249 | * let's validate the logic then we can move forward with generating random |
||
| 250 | * integers along a given range. |
||
| 251 | */ |
||
| 252 | if ($min > $max) { |
||
| 253 | throw new Error( |
||
| 254 | 'Minimum value must be less than or equal to the maximum value' |
||
| 255 | ); |
||
| 256 | } |
||
| 257 | |||
| 258 | if ($max === $min) { |
||
| 259 | return (int) $min; |
||
| 260 | } |
||
| 261 | |||
| 262 | /** |
||
| 263 | * Initialize variables to 0 |
||
| 264 | * |
||
| 265 | * We want to store: |
||
| 266 | * $bytes => the number of random bytes we need |
||
| 267 | * $mask => an integer bitmask (for use with the &) operator |
||
| 268 | * so we can minimize the number of discards |
||
| 269 | */ |
||
| 270 | $attempts = $bits = $bytes = $mask = $valueShift = 0; |
||
| 271 | /** @var int $attempts */ |
||
| 272 | /** @var int $bits */ |
||
| 273 | /** @var int $bytes */ |
||
| 274 | /** @var int $mask */ |
||
| 275 | /** @var int $valueShift */ |
||
| 276 | |||
| 277 | /** |
||
| 278 | * At this point, $range is a positive number greater than 0. It might |
||
| 279 | * overflow, however, if $max - $min > PHP_INT_MAX. PHP will cast it to |
||
| 280 | * a float and we will lose some precision. |
||
| 281 | * |
||
| 282 | * @var int|float $range |
||
| 283 | */ |
||
| 284 | $range = $max - $min; |
||
| 285 | |||
| 286 | /** |
||
| 287 | * Test for integer overflow: |
||
| 288 | */ |
||
| 289 | if (!is_int($range)) { |
||
| 290 | |||
| 291 | /** |
||
| 292 | * Still safely calculate wider ranges. |
||
| 293 | * Provided by @CodesInChaos, @oittaa |
||
| 294 | * |
||
| 295 | * @ref https://gist.github.com/CodesInChaos/03f9ea0b58e8b2b8d435 |
||
| 296 | * |
||
| 297 | * We use ~0 as a mask in this case because it generates all 1s |
||
| 298 | * |
||
| 299 | * @ref https://eval.in/400356 (32-bit) |
||
| 300 | * @ref http://3v4l.org/XX9r5 (64-bit) |
||
| 301 | */ |
||
| 302 | $bytes = PHP_INT_SIZE; |
||
| 303 | /** @var int $mask */ |
||
| 304 | $mask = ~0; |
||
| 305 | |||
| 306 | } else { |
||
| 307 | |||
| 308 | /** |
||
| 309 | * $bits is effectively ceil(log($range, 2)) without dealing with |
||
| 310 | * type juggling |
||
| 311 | */ |
||
| 312 | while ($range > 0) { |
||
| 313 | if ($bits % 8 === 0) { |
||
| 314 | ++$bytes; |
||
| 315 | } |
||
| 316 | ++$bits; |
||
| 317 | $range >>= 1; |
||
| 318 | /** @var int $mask */ |
||
| 319 | $mask = $mask << 1 | 1; |
||
| 320 | } |
||
| 321 | $valueShift = $min; |
||
| 322 | } |
||
| 323 | |||
| 324 | /** @var int $val */ |
||
| 325 | $val = 0; |
||
| 326 | /** |
||
| 327 | * Now that we have our parameters set up, let's begin generating |
||
| 328 | * random integers until one falls between $min and $max |
||
| 329 | */ |
||
| 330 | /** @psalm-suppress RedundantCondition */ |
||
| 331 | do { |
||
| 332 | /** |
||
| 333 | * The rejection probability is at most 0.5, so this corresponds |
||
| 334 | * to a failure probability of 2^-128 for a working RNG |
||
| 335 | */ |
||
| 336 | if ($attempts > 128) { |
||
| 337 | throw new Exception( |
||
| 338 | 'random_int: RNG is broken - too many rejections' |
||
| 339 | ); |
||
| 340 | } |
||
| 341 | |||
| 342 | /** |
||
| 343 | * Let's grab the necessary number of random bytes |
||
| 344 | */ |
||
| 345 | $randomByteString = random_bytes($bytes); |
||
| 346 | |||
| 347 | /** |
||
| 348 | * Let's turn $randomByteString into an integer |
||
| 349 | * |
||
| 350 | * This uses bitwise operators (<< and |) to build an integer |
||
| 351 | * out of the values extracted from ord() |
||
| 352 | * |
||
| 353 | * Example: [9F] | [6D] | [32] | [0C] => |
||
| 354 | * 159 + 27904 + 3276800 + 201326592 => |
||
| 355 | * 204631455 |
||
| 356 | */ |
||
| 357 | $val &= 0; |
||
| 358 | for ($i = 0; $i < $bytes; ++$i) { |
||
| 359 | $val |= ord($randomByteString[$i]) << ($i * 8); |
||
| 360 | } |
||
| 361 | /** @var int $val */ |
||
| 362 | |||
| 363 | /** |
||
| 364 | * Apply mask |
||
| 365 | */ |
||
| 366 | $val &= $mask; |
||
| 367 | $val += $valueShift; |
||
| 368 | |||
| 369 | ++$attempts; |
||
| 370 | /** |
||
| 371 | * If $val overflows to a floating point number, |
||
| 372 | * ... or is larger than $max, |
||
| 373 | * ... or smaller than $min, |
||
| 374 | * then try again. |
||
| 375 | */ |
||
| 376 | } while (!is_int($val) || $val > $max || $val < $min); |
||
| 377 | |||
| 378 | return (int) $val; |
||
| 379 | } |
||
| 380 | } |
||
| 383 | ?> |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: