Talk:Getter and setter policy

Revision as of 02:15, 16 July 2010

Don't tell me everyone agrees where is the fun in that?

I just have to completely agree with the first paragraph. I also try to always use this->FunctionName() to make the difference between internal and external function calls clear. Python, for instance, enforces such a syntax ( self.FunctionName() ). --TobiW

Very nice little style guide there, I would back that 100% --AlexGee 02:09, 28 July 2009 (UTC)

If the object has to call its getters and settings to change its own variables then changes to the variables internally would be subjected to the same constraints as calls through the public interface. This also would mean that internal calls would always have the same constraints as public calls. Would you want this? I mean might there be exceptions to this rule? --BenMcDonald 06:44, 28 July 2009 (UTC)

Sure there would be exceptions for example dealing with a collection. Thus I used 'ideally' --AlexGee 09:45, 28 July 2009 (UTC)

I know it would not be very nice design, but there is nothing stopping a developer from adding additional functionality to a getter/setter. If we want to stop that from happening, possibly this should also be included in the policy. Related to Command query separation. --Matthew Harward 21:04, 28 July 2009 (UTC)

After some research, my current opinion is: -"Objects should store all their data in private variables": Yes, this maintains encapsulation. -"Getter and setter methods should be produced for any variable you might have been tempted to declare public": HELL NO. This is wrong. Getter and setter = BAD. This breaks the concept of Tell, Don't Ask, the Law of Demeter, encapsulation and possibly (as Matthew has indicated) Command query separation in certain situations. Read this document: [1] to gain a better understanding of why it is wrong to use getter and setter methods. -"Classes should ideally call their own getter and setters when accessing their internal data": Yes, but only when you can't avoid using the Satanic getters and setters in the first place. Also, the get/set methods should comply with Command query separation. -- Douglas

Ok, having read the article, it does make a good point. However, please consider, the suggestions made by you and in the article may break the Behavioral completeness maxim. This may be an acceptable compromise. --Matthew Harward 00:26, 29 July 2009 (UTC)

I am not sure that it breaks the Behavioural completeness maxim, from my understanding it would actually support it. To use the truck suspension example, the wheel object can pass the force data to the axle object by saying: "here are some forces, deal with them", which places the responsibility for the behaviour task with the axle, which in real life (when massively simplified) is what happens. If I have misunderstood this principle please correct me. --Douglas 01:55, 29 July 2009 (UTC)

I think you misunderstand the idea proposed we are not suggesting that objects should store data just for other people to use. Writing a getter or setter does not mean you can not have a method that preforms actions upon the internal data, 'Tell, don't ask' as you say. Ideally in a perfect world everything would be done in this manner and there would be no public members. In reality though other classes will often at some point want to know our state or change our state themselves. I am not advocating writing getters and setters for everything. I am saying do away with public members use private members and have getters and setters for them. This provides reliability and robustness while accommodating the realities of programming. --AlexGee 03:18, 29 July 2009 (UTC)

Maybe getters/setters should be treated with a DBC-like perspective. By this, I mean that getters and setters are allowed as long as you make it explicit what they are there for. E.g. if you hava a getName() method, this should have accompanying comments to say that this will get a String in format X that should only be got by classes in particular packages, or should be avoided or something. --Matthew Harward 00:30, 29 July 2009 (UTC)

Agreed all getters and setters must have a well documented contract and as Douglas says have CQS. Though I thought that last point was somewhat self explanatory --AlexGee 03:18, 29 July 2009 (UTC)

I should probably justify my position a bit further, for those that don't read Allen Holub's article [2] and so others have more avenues to hurl abuse at me. Granted, there are likely to be situations where get/set methods are unavoidable, or where redesigning the class to remove them will introduce excessive complexity that is not easily justified, indeed that would go against Keep it simple. Treating them in a design by contract fashion seems like a reasonable way to limit the negative effects, provided all other developers adhere to their limitations, but we all know how likely that is going to be when the pressure is on. Ideally, (there is that word again) they should be avoided wherever possible. The get and set methods expose the implementation of the class to other classes. This goes against the Law of Demeter (minimum knowledge) and encapsulation principles, though I will concede that it depends on the Encapsulation boundary to some extent. We should all by now understand the maintenance/modification problems that come with exposing internal specifications of objects, remember those countless hours spent chasing bugs around a 325 project just because you wanted a string instead of an int. At the basic level get and set methods merely make a private field public, it is the same as locking your front door then entirely removing the back door and installing a large neon sign pointing to the rear of the house saying "entrance here". Certainly you could include validation in your setter method to control modification, in fact to be safe you probably should as well as including a boolean return and/or try-catch. However, that starts (in my opinnon) to blur the setter method into something that is closer to completely dealing with whatever task it is connected to anyway, so then why wouldn't you just have that task being managed in that class? Hence, Tell, Don't Ask. 'Get' could be argued as being reasonably safe, since it is not modifying the data, but it is still exposing the implementation. The favoured course of action seems to be to use an Interface object to raise the level of abstraction away from the class, allowing any modification you like. Presumably this approach also has drawbacks. I think the concept of each object knowing how to do its job and only its job, rather than having other objects do the work for it, fits well with OO. Mind you, last week I thought get and set were reasonably valid too. Perhaps I am just easily swayed. Holy crap this is a long statement. --Douglas 02:13, 29 July 2009 (UTC)

If I found someone was writing a setter that simply copied the argument into the object I would fire them as they obviously do not understand the concept of a setter. Encapsulation is all fine and dandy and something that I completely agree with. Getters and setters provide encapsulation. A getter and a setter can be considered a lightweight interface in many respects. They are there to provide an abstraction so that you can change the internal working of the class and maintain the same external behavior. Defining an interface just to deal with these simple operations is pointless. Only define an interface if multiple classes are going to use it. --AlexGee 03:18, 29 July 2009 (UTC)

I agree with Douglas about the statement "Getter and setter methods should be produced for any variable you might have been tempted to declare public." It could encourage the type of overuse warned against. Another alternative to public variables would be say refactoring so that data is processed internally, see Tell, Don't Ask. The reason that public variables should not be used is not only because getters and setters exist to use instead. --BenMcDonald

"Getter and setter methods should be produced for any variable you might have been tempted to declare public." I totally disagree with this. I think that needing to access the privates of another object should indicate that you need to carefully review your design. As mentioned before the supporting evidence of this is Tell, Don't Ask and considerations of the principle of Encapsulation. We need to remove this line from the policy.

On a different note... I don't believe that we can necessarily escape getters and setters either. For example, COSC122 just recently worked with a design for a Phone Book system. One class was named DirectoryEntry and was responsible for storing the names and numbers of people. It didn't do anything but it clearly exists in the domain. In my mind we could call this a Data object for this reason.

In this case a DirectoryEntry requires getters and setters to allow other classes to access its data. So in this case it seems one of two things are the case

1. It is wrong to make data objects.
2. It is acceptable in this case.

I personally believe 2. is the better answer but feel uncomfortable holding that view. --BenjaminTaylor 01:32, 30 July 2009 (UTC)

We seem to be converging on something here. I believe that getters and setters should be used only when absolutely necessary. Often they can certainly suggest that the design is poor. However, it is often not clear where behaviour should go. I'm not raising anything new, only trying to deepen what's already been discussed. To raise an archetypal example, consider two classes; Bread and Toaster. Where should the toastBread() method go? In either case, the method must access some information from the other object. I think the question boils down to this: Is it possible to have data which logically belongs in one class being useful to behaviour that logically belongs to another? I think the answer is obviously yes. This is the type of situation that no design can work around, and in these situations getters and setters are useful to provide a thin interface. --Aidan Bebbington 04:25, 30 July 2009 (UTC)

I quite like the term thin interfaces it has a nice ring to it. --AlexGee 05:36, 30 July 2009 (UTC)

I agree with the general sentiment that getters and setters are unavoidable in some circumstances, especially when you are separating concerns that may require the same data (i.e. the frog exporting itself to XML). However, I have a general problem with getters that exposed mutable fields directly to their clients. By that, I mean that if a class has a reference type as a field and exposes that directly through a getter, clients can change the internal state of the object without having to go through getters. This is obviously bad because it's hard to trace where the change came from. This is inline with Riel's heuristic "Do not change the state of an object without going through its public interface" and Don't expose mutable attributes. So I would propose that while getters and setters may be ok in some cases, getters that expose mutable fields directly are never ok. --JaninaVoigt

Yes, I agree. Any changes to the state of an object should occur within that object. Ideally this would mean that an object is told to change, but not exactly how to change. This is just Tell, Don't Ask again. If this is not possible, at the very least we can still mediate changes made to an object at a single point. --Aidan Bebbington 22:16, 30 July 2009 (UTC)

Just wanted to point out one place where Allen Holub's article [3] seems to miss something. It states: "Getter and setter methods (also known as accessors) are dangerous for the same reason that public fields are dangerous: They provide external access to implementation details." However, this is exactly what Alex and others have been saying that getters and setters can be used to avoid. They can provide a thin interface between the class and its clients for converting between interface and implementation, and for some checking and other misc things. --Aidan Bebbington 04:34, 30 July 2009 (UTC)

I think Holub doesn't mean this separation of interface and implementation in code, but that you have to make some details of your implementation (mainly the type of your attributes) available to the public. And I have to agree with him: having to change a lot of code because I changed the type of a return value has happened to me many times. So I guess I'll try to avoid getters and setters as much as possible, although they are still necessary in my opinion (especially for data objects). --TobiW

The problem of having to make major changes because of changing the type of a single attribute is certainly a real issue. I'm saying that getters and setters can actually avoid this situation in many cases, because the internal type of the attribute can often be changed without affecting the type returned/received by the getter/setter. The getter/setter then takes up the responsibility of converting between the internal type and the external type. --Aidan Bebbington 05:19, 30 July 2009 (UTC)

I think that Aidan put it quite well. But I will elaborate with an example. Say that we have a class that deals with time in our code and makes system api time calls. This class can have a method called getTime() which returns a unix timestamp of type unsigned int. Let us say that this is writen for a unix system. The getter then simply calls time() and passes back the value. However we later move the program to compile on a windows system and we have use the GetSystemTime() call. The getter can now impliment the math required to convert a windows timestamp into a unix timestamp thus still returning an unsigned int. This allows other classes to remain unaltered. Hopefully this helps clarify the point made by Aidan and I. --AlexGee 05:36, 30 July 2009 (UTC)

I would like to point to C#'s properties, and note that while it can be confusing to mask a method call under field access syntax I think this is a good approach. Not only does this approach allow slightly more fine-grained access control (you can essentially make a field public to read, private to write), but I think field access syntax is in many ways much more readable than getters/setters, especially as such an operation is instantly identifiable as accessing properties rather than performing some more complicated operation. --Lukas Korsika 02:15, 16 July 2010 (UTC)

I was going to add an edit to encourage object encapsulation but then realised that people hadn't agreed on that yet, see Equals vs the Encapsulation Boundary. -BenMcDonald 23:41, 30 July 2009 (UTC)