Skip to content
This repository has been archived by the owner on Jan 2, 2023. It is now read-only.

Parameters' getLimit returns string #71

Closed
byCedric opened this issue Dec 1, 2015 · 4 comments
Closed

Parameters' getLimit returns string #71

byCedric opened this issue Dec 1, 2015 · 4 comments

Comments

@byCedric
Copy link

byCedric commented Dec 1, 2015

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
Copy link
Owner

Agreed, PR welcome.

@byCedric
Copy link
Author

byCedric commented Dec 1, 2015

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
Copy link
Owner

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
Copy link
Author

byCedric commented Dec 2, 2015

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants