Yeah I did very nearly write const auto height = height (bounds); before realising the issue there…
Tbh it’s short enought that I’d probably just use height (bounds) in-place where I needed it, rather than using a local var. But you could also put the functions in a namespace to avoid that issue.
Imagine that in the future, we add a Size<> type to JUCE that stores a width and a height, but nothing else. Then, we might want to go back and change the implementation of Rectangle to use our fancy new Size<> type internally - but if users are depending on there being a width member then there’s no way of doing that. There are many other implementation strategies for rectangles, too. You might store “top, left, bottom, right”, or “left, top, width, height”, or “centreX, centreY, width, height”… and it might be nice to group some of those members too. The current implementation uses a Point for the top left position, for example. Exposing the data members locks you into a single implementation for all time.
Given that the implementation must store either a plain width and height, or a Point<> for the bottom left (or some other representation), it becomes difficult to implement a consistent getter-free API. If the user wants to get the bottom-left Point, but the type stores a width and a height, then we’ll need a getter to construct the point for us. If we store the Point directly instead, then we’ll need getters for the width and height. Using getters everywhere allows us to provide a more consistent interface, and consistency is important for usability and readability.
Finally, I’ll add that there are plenty of places in JUCE where we do expose plain data members, and a few of them have caused difficulties when trying to update their implementations (AudioProcessorParameterWithID). I’d much prefer that getters and setters were used from the outset in those cases.
For these reasons, I’d lean in favour of explicit getters/setters in most situtations, at least in the public API.
that’s very unlikely to happen, because if you template width or height rectangle types are not easily comparable with each other anymore. or am i missing something?
that is actually something i did not consider yet and it perfectly explains why x and y can’t be directly accessible members at the moment. and i can also see how it might be annoying trying to change this since a bunch of layout-related rectangle functions probably depend on this being a point with point’s functions.
(and so on)
yeah i get that. i wouldn’t want all the getters from rectangle to be members. that would be ridiculous because a lot of these getters calculate their values rather than just reading them. i was just talking about the main variables x, y, width, height
consistency is actually one of the reasons why i made this post, even though i didn’t use that word yet. but if you think of it, from an outsider pov seeing that Point has members but a lot of other stuff just has getters, that looks inconsistent.
that’s why i said it’s a different story for the more complex types, like AudioBuffer, yes. if we keep on discussing this we should ignore the harder cases and only focus on the easy things, like rectangles and lines tbh.
aight! i see you have a lot of good reasons to keep it like this. even if it’s not perfect, it does the job and it’s not too bad anyway
I agree that in this case it makes sense to hide the implementation, as Reuk has explained very clearly. I’ve thought about it a lot, as I also find Rectangle very cumbersome to work with. There’s no need to be condescending about this though -it doesn’t have anything to do with typing less. It’s just that the code tends to look so verbose that sometimes you have to read it like literature to know what it’s doing. I much prefer to write code whose behaviour can be broadly understood upon first glance, because it looks structural already. Of course this is not always possible, or at least it’s not the only measure when designing a public API.
Those differences in rectangle implementations are not just academic. Apple uses various rectangles composed of a CGPoint origin and a CGSize size, whereas a Windows RECT is defined by left, top, right, bottom.
sry about that to everyone btw. i gotta say i don’t find it cool when people just make jokes about my ideas here and that can make me a little aggressive, but not an excuse to be mean. i can be a little demanding sometimes, but that’s just because i’m searching for the reasons why things are the way they are, especially when they superficially look like they could be better.
When the Rectangle uses Size there wouldn‘t be a problem. If Rectangle becomes a template itself you could of course only compare Rectangles with the same type.
When you use a constant ref everything should be inlined by the compiler. And using constant ref wouldn‘t create a copy anyway.
someone once claimed const refs create a reference, which is the size of pointer or double but i guess that’s not the case when stuff is inlined. from all the keywords inline is one that i have least of an intuition for yet though. i mean everytime i put it in front of some method things just worked exactly the same it seemed. there has never been a noticable difference in performance or functionality. and then people say that the compiler inlines stuff automatically anyways which makes it even more weird. i know that’s a completely different topic now, but just saying. i just don’t know how complex a function must be to not be inlinable anymore and i also don’t wanna test every one of my functions for that or so. i try to write my code in a way that it makes no difference if things are inlined, if… that makes sense^^
Inline doesn’t mean inline anymore, compilers decide what to inline on their own criteria. The keyword now really means, as cppreference puts it, “multiple definitions are permitted” -it allows to place things in a header that otherwise would need to be in a cpp. There are non-standard ways of forcing inlining, but it’s best to be spare with them, and a forceinline that’s good today may be bad a couple of compiler versions later…
For data structures with a small set of simple members, like Rectangle, you should prefer passing by value. This imposes far fewer restrictions on the optimiser and will, in non-trivial cases, provide more efficient code. Marking something as a const reference will make no difference to the optimiser (all consts get removed very early on in the compiler passes), but introducing a reference to some memory outside of the function most certainly can slow things down.
Your summary is correct: don’t use inline to attempt to guess what would be best for the compiler unless you really know what you are asking.
This video will probably be enlightening. If you have not seen it before I would recommend watching.
Then the users would just need to maintain their code to use size.width instead width. Not a big deal with static type checking, which C++ has except for in templates.
users maintaining code is not automatically a sign of bad framework development. it’s only annoying if it happens too often and doesn’t feel like a gain to most users. for example the juce:: thing had us having to rewrite almost all lines of code with juce objects in them. but it was for a good cause, to prevent ambiguity. the only real question is: how good of a cause is reducing the length of gui code lines compared to potentially having to revert those changes causing people to have to rewrite their code? this is a subjective question and it’s ok to discuss it, but it’s not that one solution has the clear upper hand over the other one, as seen in the variety of arguments and ideas proposed in this post’s replies
This has come up a couple of times, but I don’t think it’s accurate. There’s an option in the Projucer to re-enable the using namespace juce - or you can just write this yourself. The same code should build before or after the change, as long as the namespace option is enabled in the Projucer. Of course, I don’t recommend using this option, but we put it in specifically because we didn’t want to force users to make hundreds of changes to their projects.
“Not forcing users to rewrite working code” is a major consideration when maintaining the framework, and we do our best not to add source-compatibility breakage unless there’s a really good reason. Admittedly, we don’t always succeed (the framework is quite large and it’s sometimes difficult to foresee all the side-effects of a particular change), but we do try!
Something underlying all this is that UI layout in an imperative language is just awkward. You can try your best to make it as lean as possible but it’s always ugly in some way or another. For example, I had a bunch of layout code written in terms of removeFrom*, which I ended up rewriting with setBounds and moving positions. Conceptually it made more sense before, but it was so long-winded that I had to search for the dimensions among the sea of API calls. I can’t blame the API for this -the facilities are there, they work, they’re alright, it’s just not a nice thing to do with C++.
You could even specify user-defined structured binding support to juce::Rectangle – we did something similar for juce::dsp::ProcessorChain before the JUCE team added built-in structured binding support for that class.