On Mon, Apr 8, 2019 at 6:05 AM David Rodrigues <david.pro...@gmail.com>
wrote:
> Current solution:
>
> $itemsPerPage = Input::get('itemsPerPage') ?: null;
> $itemsPerPage = $itemsPerPage !== null ? (int) $itemsPerPage : null;
>
> $paginator->setItemsPerPage($itemsPerPage); // OK
>
> With this new feature: just...
>
> $paginator->setItemsPerPage((?int) Input::get('itemsPerPage')); // OK
>
> Question: why just not check if $itemsPerPage is set like:
>
> $itemsPerPage = Input::get('itemsPerPage');
>
> if ($itemsPerPage) {
> $paginator->setItemsPerPage((int) $itemsPerPage); // OK
> }
>
> Answer: because in this example we could do that (although the new solution
> is much more practical). In another case, I have a static factory method
> that depends of an ?int to be created, and I can't just skip it with an
> if() if user input is empty.
>
Hi!
I'm not a fan of the approach for a few reasons:
- First and foremost, we would introduce a cast where you can't be sure of
what you'll get back. I understand the use-case for when you want to pass
something to a nullable parameter, but if you think about this cast in
isolation, it hardly makes sense.
- Second, maybe this is particular to your example, but your are forcing
yourself to process something that the user didn't set. If your paginator
already has a sane default(15) in a property, just don't call its setter
when the user didn't ask you to. If you insist that you really want to save
LoC, just assume that 0 means "use the default" and you can use the cast in
all cases.
Regards,
Pedro
