Timeline for Do ALL your variables need to be declared private?
Current License: CC BY-SA 2.5
25 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Jan 4, 2015 at 14:57 | comment | added | back2dos | @supercat: For somebody who has a point to make, you are writing an awful lot. You haven't yet said anything that is relevant to my criticism to this answer. Or to the question. This question was asked in a C# context, where setters are syntactically transparent. Your semantic comparison is a false dichotomy. You are confounding collections and tuples. You open a side topic on atomicity, which you get for free with immutability, which really only undermines my stance. And so on and so forth. Please stick to this answer and my remarks to it. Because this is going nowhere. | |
| Jan 3, 2015 at 17:52 | comment | added | supercat | @back2dos: An aggregate should be designed such that atomically reading out the visible state of an instance whose identity-hash value has never been read, copying that state to a new instance, and replacing all references to the original with references to the new instance, would not alter the state of the universe in any observable fashion. There's nothing wrong with aggregates offering "utility" methods, but such methods should simply provide convenient way of doing things which could be done just as well without them. Any abstractly-defined "behavior" should be in client code. | |
| Jan 3, 2015 at 17:37 | comment | added | supercat | @back2dos: The issue isn't just one of performance. Semantically, I think "update a minmax-aggregate to take additional data into account" is cleaner than "assign to this minmax-tuple variable a tuple which reflects a pre-existing tuple along with some additional data". Further, using accessors creates ambiguity about what mm.setMin(12.0); mm.setMax(10.0); will do. By contrast, the effect of mm.min=12.0; mm.max=10.0; is unambiguous. | |
| Jan 2, 2015 at 11:08 | comment | added | back2dos | @supercat: Blanket statements about efficiency are weak arguments. If there is a hard performance requirement, that can demonstrably be met only by adding mutability, then ok. FYI, a modern JIT will be able to stack allocate tuples where it matters. Immutability only makes this kind of optimization easier. Still, my original point is that the example code is very poor to start with. You are not contradicting that. You are also not giving any reason why the mutable records you speak of shouldn't have accessors rather than public fields. Most importantly: we're going quite a bit off topic ;) | |
| Jan 1, 2015 at 19:05 | comment | added | supercat | Note that array types in Java and .NET are effectively mutable aggregates which may be assigned arbitrary fixed sizes at instantiation. IMHO Java and .NET should also have included immutable array types, but I see no reason to consider a mutable aggregate with a hardcoded size and named members to be any "worse" than arrays, which seem to be non-controversial. | |
| Jan 1, 2015 at 19:02 | comment | added | supercat | ...passing it to a method which updates it according to array contents may be cleaner and more efficient than having every min/max query operation generate a new tuple for the minimum and maximum, and having to either give each method call the tuple from the previous one, or examine the min/max objects afterward to find the global minimum and maximum. The key point would be that the MutableMinMaxHolder would be defined as a mutable container which holds two numbers; any "behaviors" related to those numbers would be entirely the product of client code. | |
| Jan 1, 2015 at 18:58 | comment | added | supercat | @back2dos: Mutable aggregates are less common, and have different usage patterns from immutable tuples, but they can be advantageous in some situations. For example, a MutableMinMaxHolder with exposed fields Minimum and Maximum may be usefully employed with a method which, if passed a MutableMinMaxHolder along with an array, starting index, count, will update its Minimum and Maximum fields according to the indicated array elements. If code wishes to find the global minimum and maximum values for a number of distinct array segments, creating one MutableMinMaxHolder and... | |
| Dec 31, 2014 at 10:09 | comment | added | back2dos | @supercat: Tuples (which I think is a more common name for what you speak of) are immutable, which makes guarantees about the integrity of a tuple invariant, thus actually giving the code that creates the compound value the means to statically enforce the responsibility you speak of. The same cannot be said for an object with a bunch of public fields. Notice how even .NET tuples are actually defined with readonly properties for just that reason. So: the use case you are describing has nothing to do with this answer and it actually drives home the point contended in the question. | |
| Dec 31, 2014 at 8:59 | comment | added | stijn | @supercat good point. Haven't looked at this question for a while, but now I just realize that what you describe, and what the class in my answer does, i.e. just provide some kind of group of public data fields, is basically already builtin in lots of languages: tuple (or even the specialized case for 2 values, pair), or in case of Python even namedtuple. Especially the latter is semantically and functionally pretty much exactly the same as the class in the answer. This just again shows there's definitely a need for such types. | |
| Dec 31, 2014 at 6:01 | comment | added | supercat | ...shouldn't try to behave like an "object", but rather have fields, each of which represents nothing more nor less than "the last thing stored in this field". All responsibility for the content of the fields should lie with the code which returns the object. | |
| Dec 31, 2014 at 6:00 | comment | added | supercat | @back2dos: Not everything should behave as an object. A method to find both the minimum and maximum values within some portion of a double[] may take only slightly longer to execute than a method to compute either one of those things, so if code will need both values having a method to compute both in one pass may be helpful. Such a method should, semantically, return an aggregate rather than an object, but Java doesn't have aggregate types; the closest it can come is an object which behaves like an aggregate. IMHO, types which are coded to satisfy code's need for an aggregate... | |
| Apr 11, 2012 at 13:45 | comment | added | java_mouse | I would add a constructor to set the fields instead of setters to solve the issue. We should always avoid the tumble weed pattern which does not enforce the client to construct a valid full object. | |
| Feb 8, 2012 at 16:31 | comment | added | back2dos | @stijn: This question is all about OOP. Asking this question outside the context of OOP is pointless. Your answer comes down to "No, not all fields must have accessors. Look at how this piece of bad code becomes even worse when I add them." Your example is extremely bad to start with in a question tagged "best-practices". | |
| Feb 8, 2012 at 15:50 | comment | added | stijn | @back2dos the question is not about Tell Dont Ask nor about 'Good' OOP. It's about whether all variables should always be private and the core of the answer is no. No the example isn't chosen particularly well but that's obviously not the interest of the OP. | |
| Feb 8, 2012 at 15:24 | comment | added | back2dos | This example is a great illustration of how to not do OOP. It completely kicks Tell Don't Ask in the teeth. | |
| Feb 8, 2012 at 14:54 | comment | added | David Thornley | Don't be so hard on the professors. There are valid educational reasons for enforcing rules that are almost always valid. I'd consider "never make variables public, except if the class/struct is a bundle of related data with no behavior" a good rule. Once the student is experienced enough, he or she will know why the rules are there and when to break them. | |
| Feb 8, 2012 at 14:27 | comment | added | Michael K | Another reason for getter/setter pairs is for unit tests to inject dependencies. Granted, that does not mean that you blindly autogenerate them for every property. | |
| Mar 14, 2011 at 7:11 | comment | added | Sal Rahman | I agree with @delnan. But, the thing is, I just want quickly write one little variable, then quickly test my program. If everything works fine, then I will refactor the code to have it's respective getters and setters. | |
| Mar 13, 2011 at 11:32 | comment | added | stijn | well I admit you're right, should have made it more clear.. I also realize the example is not very well chosen. | |
| Mar 13, 2011 at 10:20 | comment | added | user7043 | @stjin: But if you're condemning X, you should at least mention that there are legimitmate use cases (assmuning there are, of course). Your answer as-is, on its own, is barely better than "you must always write getter/setters", as it implies getters/setter have no reason to exist. Also, the validity of a struct-like class is questionable from a pure OOP perspective. | |
| Mar 13, 2011 at 10:11 | comment | added | stijn | @delnan indeed, but KeesDijk's answer already pointed to that information so I saw no need to repeat it @Thorbjørn MyConfig is just a simple sample of a class of which all members have as requirement they need to be read from and written to within the application, hence needing both getters and setters. Or, if no validation/whatever required, are suitable candidates for making them public and omitting the getters/setters. | |
| Mar 13, 2011 at 9:13 | comment | added | user7043 | You fail to mention the main reason sane persons write getters/setters: For validation or other more complex logic on getting/setting, and for future-proofness in case you need to add some of this. | |
| Mar 13, 2011 at 9:01 | vote | accept | Sal Rahman | ||
| Mar 13, 2011 at 9:00 | comment | added | Sal Rahman | I found your answer to be refreshing. It just took a whole load of weight off my chest. Thanks! | |
| Mar 13, 2011 at 8:54 | history | answered | stijn | CC BY-SA 2.5 |