search

tobyzerner / json-api-php

JSON-API (http://jsonapi.org) responses in PHP.
MIT License
436 stars 79 forks source link

Parameters' getLimit returns string #71

Closed byCedric closed 8 years ago

byCedric commented 8 years ago

I was implementing the package at yet another project, until I noticed that the Parameters own getLimit returns a string. Yes, it does state in the docblock that it should return a string but I find this quite confusing. Mostly because within the LinksTrait addPaginationLinks the limit is used as an integer...

My main point is should the getLimit method within Parameters not return integer, and 0 by default instead of ''.

tobyzerner commented 8 years ago

Agreed, PR welcome.

byCedric commented 8 years ago

Sure, just one more question. If no $max is provided, and no query was set, it should return 0 right? But what if a $max is, but no query was set. Should it still return 0, or the provided $max? The last one is more useful I think since you do not depend on user input, by default. Will write a PR in the morning!

tobyzerner commented 8 years ago

No, it should return 0 in both cases. This is because an app may want to have different default limit to its max limit, so it needs to be able to tell when no query was set. e.g. https://github.com/flarum/core/blob/master/src/Api/Controller/AbstractSerializeController.php#L179

In fact, would it be more correct to return null instead of 0 if no query is set?

byCedric commented 8 years ago

I would go for null, since it is the more literal description of what is happening with the limit value. It also is a falsy value, so it has no effect when using it in expressions like if ($param->getLimit()) .... Even if you still want an int value, you can cast it to 0. Will write a PR now, thanks for your input!