Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap the order of the fields width/height #7

Open
ndmitchell opened this issue Nov 7, 2014 · 3 comments
Open

Swap the order of the fields width/height #7

ndmitchell opened this issue Nov 7, 2014 · 3 comments

Comments

@ndmitchell
Copy link
Contributor

In almost all API's the fields width/height are in that order, with width first. Some examples:

It's very surprising to see the fields the other way round.

@ndmitchell
Copy link
Contributor Author

BTW, I realise this would break the API, and potentially break existing clients without realising it. One solution to make the breakage more explicit would be to rename Window to Size at the same time, since the data type arguably is representing Size not Window. The other alternative of course is to not make this change at all, which I can certainly see the attraction of. As it stands, I would never rely on the order of the fields if they go height/width (since it's going to trip me up if I look back later), so am currently sticking to the selectors.

@supki
Copy link
Member

supki commented Nov 7, 2014

The current order mimics original POSIX API. I don't have much of an opinion on this but it doesn't look unreasonable not to export the constructor at all, so the users are forced to use height and width explicitly.

@ndmitchell
Copy link
Contributor Author

Hmm, that's the only API I've ever seen that way round, but I can see it's reasonable to follow it. Not exporting the constructor might be wise, or maybe just a comment saying why the fields are that way round, and calling out that it's different to what you might expect. Even if you don't export the constructor, the Foldable instance still leaks the order of the fields.

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

No branches or pull requests

2 participants