idk about you all but I always have way too long looking lines in my GUI code due to all these getters. on top of that I often have to convert bounds to float before getting started.
const auto bounds = getLocalBounds().toFloat();
const auto width = bounds.getWidth();
const auto height = bounds.getHeight();
this is already way too long imo, especially for what little it does and for how often i have to write this. that’s why I suggest:
const auto bounds = getLocalBounds().toFloat();
const auto width = bounds.width;
const auto height = bounds.height;
looks much better, more to the point and it’s shorter: happier programmers! same for juce::Line btw. just look at juce::Point or juce::ColourGradient, where it’s already working just fine like that!
CONS:
future updates might require those getters to do more than just getting the member out of there.
people might set a member variable by doing that but the setter had some additional functionality so the object doesn’t work correctly anymore then.
getters and setters let the online API show additional info about the methods.
using getters to get width is “much clearer”. (ask ben if you want to know why)
more stable / maintainable API (just in case a rectangle’s width-getter ever changes)
having to write intermediat variables to get short lines of code is useful for debugging as well.
PROS:
what kinda future update is that supposed to be where setting or getting width from a rectangle does more than setting or getting the width from the rectangle? it’s likely that an asteroid kills us all before that happens.
even if getting a member out of there will require more work at some point it could be public until then.
even if some people accidently abuse it to set a value and run into errors, it’s not that they wouldn’t notice and learn from that. once they knew what to do and what not to do they’d just enjoy the shorter lines.
it doesn’t break existing code bases because the getters can just stay there as well.
whenever an object actually has getters it gives a wrong impression about the function. one would think it’s there because additional stuff is needed and make people avoid those methods if possible, at least until they ctrl+click on them and find out they are indeed just getting the members.
a lot of JUCE forum posts seem to deal with people being unhappy about stuff being not public, so it’s definitely worth it to consider rewriting all that.
rewriting it would be easy, as it’s literally just 3 steps: checking what the getter does, checking what the setter does, and if they don’t do anything fancy, put the “private” keyword slightly somewhere else
in rare cases it might even be beneficial to skip the other stuff a getter does, because the functionality is really not needed. like when trying to use audioBuffer’s getArrayOfWritePointers() but without any code that uses its clear-state. (offtopic here, because no GUI code and more complex.)
there is no additional info needed for something as simple as a rectangle’s width.
some juce code already works like this and works perfectly fine, so apparently it’s not a problem (like juce::Point)
The example code you posted are not long lines at all.
Getters are often a much better option than public data members, because it makes the public API much clearer, easier to document, and more stable.
Implementing this change would be a huge time sink, refactoring much of the JUCE codebase for stylistic reasons that would result in a less maintainable API.
they are long considering how often they have to be written. most of the time i don’t even write out “width” and “height” anymore but just “w” and “h” because it’s so long (with the rest of the line).
about the API: yeah i get that. and since the getters can just stay there the API will also stay just as good as it already is. but for simple things like rectangles there is really no reason to assume that a getter is remotely needed. it could theoretically be entirely undocumented and still be understood.
Implementing this would be fast, as mentioned before. 3 steps per class and especially useful in these smaller and often used classes like rectangle and line.
the reasons are not just “stylistic”. it’s more for the mental sanity. when i’m writing the same stuff over and over again and keep having to add words that don’t do anything for me that just feels super unsatisfying
// before
const auto width = bounds.getWidth();
const auto height = bounds.getHeight();
// after
const auto width = bounds.width;
const auto height = bounds.height;
Both lines are 5 characters shorter. If the before versions are too long for you, I don’t see how the after versions are any better.
they are better by 5 characters, which is huge considering that there can be more complex lines as well where the difference would be even more drastical.
const auto x = static_cast<int>(bounds0.getX() + widgetEnvelope * (bounds1.getX() - bounds0.getX()));
const auto y = static_cast<int>(bounds0.getY() + widgetEnvelope * (bounds1.getY() - bounds0.getY()));
const auto w = static_cast<int>(bounds0.getWidth() + widgetEnvelope * (bounds1.getWidth() - bounds0.getWidth()));
const auto h = static_cast<int>(bounds0.getHeight() + widgetEnvelope * (bounds1.getHeight() - bounds0.getHeight()));
like this. one could argue ok… just outsource that difference into extra lines and call it xRange, yRange etc to make all lines shorter. but i say that this would disrupt the flow of reading as well because it would make it less obvious that these are just 4 lerps to interpolate 2 bounds. it makes sense wanting to have them in exactly 4 lines, but the way it looks right now is super ugly and convoluted and took long to write, even considering some stuff can be copy-pasted sometimes, but you know, that requires taking one hand off the keyboard and grabbing the mouse or alternatively doing fancy stuff with shift-key and that’s not really a cool workflow for something that should just be simple to type out.
yeah, and i often do that. but that’s the thing. we always have to write extra lines to save variables into new variables just because using the varibales directly is too long. and all that just because c++ programmers feel like getters and setters make code look more professional or something. can’t we just get over it?
but i’m not with you when it comes to the maintainability of the framework. especially when we talk about such simple stuff as a rectangle’s x, y, width, height values. there is just no way, not in a million years, that these getters will ever do something more fancy than getting the members. so maintainability is not really threatened at all by such a change.
if we were talking about my audiobuffer example… ok. that’s probably a more involved topic. but rectangles and lines? no.
Besides, if they ever do need to add additional logic in there, then the pubic facing API wouldn’t need to change at all because of the usage of getters and setters. As a programming practice, it’s almost always the best option to choose the most future-proof solution and write APIs that are hard to use incorrectly – getters/setters are both.
people always claim that they try to write code that still works in a year or so, but in the short time that i was a juce programmer now there have been so many major changes that made a lot of people having to relearn certain parts of the framework, like the async file chooser thing, or even the need to write juce’ namespace before everything were things that broke everyone’s code super hard. but was it a problem? no. people just learned what they had to change to make it work again and it was fine then.
this isn’t even as much of a difference as the examples i just mentioned from things that already happened
There are setters especially in Rectangle, which change the members differently, like setWidth(), setRight() etc.
All those setters communicate perfectly what they do and that is the most important for maintainability.
Thanks to optimizers and inlining there is no performance loss choosing one over the other.
Having the setters allow adding jasserts when the user adds stupid values, like a degraded Range (where max < min) etc.
Using public members is only for const members an option e.g. like the AudioFormatReader (even there I am not perfectly happy with the choice made).
There is no way to make something private as it would break existing code. Exposing internals is an one way street.
the things you mention are exactly why the JUCE team makes such an effort to future-proof the API as much as possible. Exposing internal members of a class is inviting future breaking changes, and IMHO would be a pretty bad design choice and a poor usage of the JUCE team’s time.
i’m not addressing performance issues in this post, just to make that clear.
yeah that’s true. but one could also say "if you choose to ignore the getters and setters it’s your own fault if it breaks, but at least it’s possible for the ones who already know which asserts are in the getters and setters.
the things i mentioned are choices that made a lot of people having to rewrite their code, just like when something that was public is set to private suddenly. but that’s just how it is. and if the juce team ever makes something private and people don’t like it they can just come here and tell the juce team why they would like that to be public and then it can be discussed again. that’s just how the framework evolves to keep improving its workflow
The asserts are not for documentation, as if you didn’t know how to use the class. It is to give a heads-up as early as possible. After all the assertt guards against runtime values that were computed from somewhere and accidently result in a value that is impossible to cope with.
on top of that some things don’t even need a jassert. like… when you make a rectangle with negative width and it crashes your app or just looks weird and wrong… it’s not that you need a jassert to notice that. you run the app and see it with your eyes. if we accepted the jassert argument that would be like “yeah why don’t we just write way too much code constantly, just so people don’t have to open their eyes when compiling?”
