Archive for setter

I suffer from OCD: Odorless Code Dreamer

Posted in Design Issues with tags , , , , on February 22, 2009 by moffdub

It sucks. It sucks, I say.

exportTo:exporter
    ”exports the internal state of the object”

    exporter setRoomData:roomDS.
    equipmentList do:
    [:equipment|
        equipment exportTo:exporter.
    ].

Continue reading

Getters and Setters: a people problem

Posted in Design Issues with tags , , , , , , , on December 10, 2008 by moffdub

Greetings ladies and gentleman, it is I, El MoffDo, here to escort you on this excursion into Programming Excellence for the next 1300 words on this, the EIP web-ring.

For no particular reason, I was perusing a Smalltalk tutorial and duly noted this:

Class Date defines an instance method named “year” that answers the value of the instance variable named “year” in the instance of Date to which the message “year” is sent.

So this was Smalltalk, the revered and noble language that really made me “get” what objects were all about, tacitly sanctioning a getter. Narrowing my Google search down, I was able to find an entire page dedicated to the topic.

Continue reading

I get it – pun intended

Posted in Design Issues with tags , , , , , , , , , , , , on July 8, 2008 by moffdub

Going back to The Getter Setter Debate, I knew that, like most things you learn in software, you don’t really get it until you do it. And seeing examples that are convenient, simple, and complex enough to illustrate a situation is nice for reference, but, for me, it doesn’t stick until I have to use the new knowledge in my own situation.

You might remember such gems as these:

account.appendTransactionTo(statement);
statement.appendTransactionsFrom(account);
Money total = …;
Money amount = …;
total.increaseBy( amount );

What they all have in common is that they read like English sentences: “Hey account, append transaction to statement”. They surely get the point across to the reader, but like writing a compiler, until you do it yourself, you don’t get it.

The epic Martin Fowler book, Refactoring, was the second required reading in the course that also introduced me to Evans’ Domain-Driven Design and changed the way I think about software. While not as impactful on the impressionable me of a mere year ago, the Fowler book is brimming with code examples.

One code smell, among many, to which I have since become cognizant, is the Message Chain smell:

You see message chains when a client asks one object for another object, which the client then asks for yet another object, which the client then asks for yet another object, and so on. You may see these as a long line of getThis methods, or as a sequence of temps. Navigating this way means the client is coupled to the structure of the navigation. Any change to the intermediate relationships causes the client to have to change.

Coupling? getThis? Ding ding ding. This smell sounds like it can guide me in recognizing where I can eliminate the use of getters as much as possible. The remedy Fowler prescribes is to Hide Delegate:

The move to use here is Hide Delegate (157). You can do this at various points in the chain. In principle, you can do this to every object in the chain, but doing this often turns every intermediate object into a middle man.

Ah – this can hopefully help me limit getters in my real world projects that aren’t perfectly simple, complex, convenient, and plain just right like the examples that are given.

In my recent re-implementation of core parts of The Project in C#, I unconsciously wrote code like this:

IEnumerable query = from d in db
where d.getTitleForInfra().Contains(title) &&
d.getLifecycleCommentsForInfra().Contains(comments) &&
d.getLifecycle().Contains(lifecycle) &&
d.getDirectorForInfra().Contains(director)
select d;

and this:

public bool isIn(Room r)
{
return this._history.getRoomForInfra().Equals(r);
}

One night, like the Satanic hallucinations the Marine in Doom 3 experiences, the vision came to me:

IEnumerable query = from d in db
where d.titleContains(title) &&
d.lifecycleCommentsContain(comments) &&
d.lifecycleContains(lifecycle) &&
d.directorContains(director)
select d;

public bool isIn(Room r)
{
return this._history.roomEquals(r);
}

I grant you that these improvements are relatively minor, but the benefits are the same: clients are now coupled to only the class they are using. Also, these lines of code kind of read like sentences too: “… where d’s title contains the title”. If I had made prodigious and conscious use of my variable names, the English would be even more pleasing.

As far as keeping the server objects from becoming middle men, I don’t run that risk here. In the first example, d is of type DVD, which is a domain object. Similarly, _history in the second example is of type EquipmentHistory, also a domain object. They both contain useful domain knowledge, so they aren’t mere forwarders of messages.

Know your refactoring catalog. Many seemingly hot design debates in the industry might actually be recycled from yesterday, and we can all use a reminder. It seems that fashion and bad design are both cyclical.

Method Regulator Pattern

Posted in Design Issues with tags , , , , , , , , , , on July 1, 2008 by moffdub

(skip ahead to download here)

This blog’s thusfar most popular post, The Getter Setter Debate, spawned this comment from Wolter:

“Just put them in the same package and set them to package-private.
Or put warnings in the API documentation that the getters/setters are for internal use only, and are likely to change.”

While this is a viable solution, I strive for maximal generality. Granted, Java and .NET support the concept of package or “friend” access. Even so, this is language and/or platform dependent, and it can force some awkward package organization in some situations.

Since the thought-provoking idea of avoiding getters occurred to me, I’ve come to the following conclusion: yes, they should be avoided as much as possible, but no, you can’t avoid them completely. They are needed for legitimate uses, such as UI mapping and persistence.

And now, the natural question arises: if you are on a large team, how do you enforce this policy?

There is the possibility of manual intervention. Indeed, many teams employ code reviews and walkthroughs, and every instance of an accessor method being used can be scrutinized. A small example of domain-driven design called TimeAndMoney takes this approach in the file Money.java:

 /**
* How best to handle access to the internals? It is needed for
* database mapping, UI presentation, and perhaps a few other
* uses. Yet giving public access invites people to do the
* real work of the Money object elsewhere.
* Here is an experimental approach, giving access with a
* warning label of sorts. Let us know how you like it.
*/
public BigDecimal breachEncapsulationOfAmount() {
    return amount;
}

public Currency breachEncapsulationOfCurrency() {
    return currency;
}

Certainly, as the author of this class, you are discouraging other developers from using a method in their code with the substring “breachEncapsulation”. And giving the power to the class author is a step in the right direction.

Approach

But you know me. I’m never happy with anything. I’m especially not content with relying on others to behave. It rarely works.

This got me thinking of a pattern-based solution, one where only certain classes are authorized to use another class’s getters and setters, and violation of such would cause run-time problems like exceptions.

I began tinkering around with this approach and came up with the Method Regulator Pattern:

The heart of this setup is the Regulator. It keeps track of a type that is being regulated and a list of types that are authorized to use regulated methods of that type.

What is a regulated method? It is up to the class that wants to be regulated to, first off, inherit from the Regulated Object class, and second, call the inherited authorizeCaller method as the first statement in every method that should be regulated.

The Regulated Object base class uses the process-global Regulated Types class to look up all of the Regulators for the class that called authorizeCaller, including those inherited from base classes and interfaces. Then, for each Regulator, if at least one of them authorizes the method call, the authorization succeeds. Otherwise, indicate failure somehow, like with an exception.

The burning question remains: how does authorizeCaller know the type of the caller? This will depend on your platform, but your platform must have some kind of reflective capability. Both Java and .NET support the ability to query the activation stack: getStackTrace in Java and StackTrace in .NET.

Example

I would never be one to just describe something very abstractly, wave my hands at it, and declare that it works. What am I, a professor? I went ahead and implemented this pattern in C#. You can download it here. Included is a set of NUnit tests.

Another burning question: what prevents some mischievous or misinformed coder from highjacking an instance of a Regulator, twiddling with it to allow access to a specific type to their new class, and rendering this scheme impotent?

This is an implementation issue, and I chose to deal with it by providing methods to close or “lock” a regulator once we’re finished authorizing types. I also do the same for trying to replace an entire Regulator for a given type with a new Regulator.

Some of you are confused. I’m confused as well. I think an example will clear up what is going on.

Suppose you have the following relatively complicated class hierarchy of five classes, each of which has some getters that you’d like to regulate:

Attached to each class in that diagram is a note indicating which classes are allowed to call getters on each class. It is important to note that authorized callers in a base class should be inherited by a subclass to preserve the Liskov Substitution Principle.

Speaking of which, suppose there are five calling classes like so:

Therefore, the classes authorized to use the accessors of class Sub1 are Caller2, Caller1, and Caller5, the latter two inherited. Also, since Caller3 and Caller4 implement Interface1, we could just as easily authorize Interface1 for Sub3 instead of the two classes individually.

Here is some code for setting up who can call accessors on who:

public void Setup()
{
MethodAccessRegulator superReg = new MethodAccessRegulator(typeof(Super));
superReg.addAuthorizedType(typeof(Caller1));
superReg.closeRegulator();

RegulatedTypes.addRegulatedType(superReg);

MethodAccessRegulator sub1Reg = new MethodAccessRegulator(typeof(Sub1));
sub1Reg.addAuthorizedType(typeof(Caller2));
sub1Reg.closeRegulator();

RegulatedTypes.addRegulatedType(sub1Reg);

MethodAccessRegulator subInterfaceReg = new MethodAccessRegulator(typeof(SubInterface));
subInterfaceReg.addAuthorizedType(typeof(Caller5));
subInterfaceReg.closeRegulator();

RegulatedTypes.addRegulatedType(subInterfaceReg);

MethodAccessRegulator sub2Reg = new MethodAccessRegulator(typeof(Sub2));
sub2Reg.addAuthorizedType(typeof(Caller2));
sub2Reg.closeRegulator();

RegulatedTypes.addRegulatedType(sub2Reg);

MethodAccessRegulator sub3Reg = new MethodAccessRegulator(typeof(Sub3));
sub3Reg.addAuthorizedType(typeof(Caller3));
sub3Reg.addAuthorizedType(typeof(Caller4));
sub3Reg.closeRegulator();

RegulatedTypes.addRegulatedType(sub3Reg);
}

Here is some code for Super:

public class Super: MethodAccessRegulatedObject
{
private int _x;

public int x
{
get
{
authorizeCaller(this);
return _x;
}
}
}

Boring; only one member to protect, but it gets the point across.

Now if, say, Caller2 is defined in this way:

public class Caller2
{
public void useSub1()
{
int i = (new Sub1()).x;
}

public void useSub2()
{
int i = (new Sub2()).z;
}

// inherited access
public void useSub3()
{
int i = (new Sub3()).x;
}

// should throw exception
public void useSuper()
{
int i = (new Super()).x;
}
}

and then we use Caller2 like this:

public void TestSingleInheritance()
{
(new Caller2()).useSub1();
(new Caller2()).useSub2();

try
{
(new Caller2()).useSuper();
Assert.Fail();
}
catch (MethodAuthorizationException ex)
{
Console.WriteLine(ex.Message);
}
}

the first two calls will succeed, while the second one will throw a MethodAuthorizationException with message “An object of type Caller2 attempted an unauthorized method call on class Super”.

Catches

  • First, we make use of this StackTrace class, and we kind of assume where we are in the stack. This approach, as I have implemented it, breaks if you build the download in Release mode, because calls to InvokeMethodFast are inserted instead of the actual calling class.
  • Second, a much better way for client classes to declare their desire to be regulated would be to use Aspect-Oriented Programming. I do not have experience with AOP yet, so this is a future enhancement. I also wanted a lightweight approach for the blog, instead of telling you to download something like NAspect.
  • The “locking” aspect mentioned above (all the code in that Setup() function) is only effective if you can, within your organization, centralize the creation and instantiation of the Regulated Types object and the constituent Regulators.
  • Finally, all of this has to happen within a single process, though I think if you are distributing your objects, you might have more pressing problems.
  • This entire scheme might be rendered impotent by reflection.

I look forward to some feedback on the practicality, usability, and security of this approach.

The Getter Setter Debate

Posted in Design Issues with tags , , , , , , , , on June 16, 2008 by moffdub

I’ll be honest. I never thought there was a debate as to whether getters and setters were good practice or not. It never occurred to me, directly at least, that I should avoid them. Then a couple of days ago, I was browsing Reddit and came across this article by Michael Feathers; of interest is this excerpt:

“John Nolan, gave his developers a challenge: write OO code with no getters. Whenever possible, tell another object to do something rather than ask. In the process of doing this, they noticed that their code became supple and easy to change. They also noticed that the fake objects that they were writing were highly repetitive, so they came up with the idea of a mocking framework that would allow them to set expectations on objects – mock objects.”

Then somehow I run across another article that happened to link to the Feathers post:

“Suppose that we want to print a value that some object can provide. Rather than writing something like statement.append(account.getTransactions()) instead we would write something more like account.appendTransactionTo(statement) We can test this easily by passing in a mocked statement that expects to have a call like append(transaction) made. Code written this way does turn out to be more flexible, easier to maintain and also, I submit, easier to read and understand. (Partly because) This style lends itself well to the use of Intention Revealing Names.”

Ah, this blog entry has a code example, something I respect. Now I do agree with the latter point on Intention Revealing Names; this is a key Evanism from DDD, named Intention Revealing Interfaces.

However, this example seems to imply a code smell. Yes, the account object is hiding how it is managing its transactions to the consumer, but now appendTransactionTo() is altering its argument. This is something I try to avoid.

Then again, this smell could be avoided by returning a new statement object with the transactions appended. This approach seems slightly contrived and possibly inefficient in an efficiency-sensitive environment.

Another way I can think of:


statement.appendTransactionsFrom(account);

This gets you the intention-revealing part, but you lose on killing getters because appendTransactionsFrom() has to get at the transactions of account somehow.

Noting now that this getter-setter topic is not a rogue argument, I did a search and came across this JavaWorld article, excerpt following:


double orderTotal;
Money amount = ...;
//...
orderTotal += amount.getValue(); // orderTotal must be in dollars

The problem with this approach is that the foregoing code makes a big assumption about how the Money class is implemented (that the “value” is stored in a double). Code that makes implementation assumptions breaks when the implementation changes. If, for example, you need to internationalize your application to support currencies other than dollars, then getValue() returns nothing meaningful.

The business-logic-level solution to this problem is to do the work in the object that has the information required to do the work. Instead of extracting the “value” to perform some external operation on it, you should have the Money class do all the money-related operations, including currency conversion. A properly structured object would handle the total like this:

Money total = ...;
Money amount = ...;
total.increaseBy( amount );

OK nice – here is an example that does not alter its argument. However, it begs the question: how does increaseBy() operate without a getter for the amount of money in amount, assuming all member variables are private?

Maybe we can get away with this by defining a MoneyInterface that does not use getters and setters, only intention-revealing methods. Then, implement the MoneyInterface with the necessary getters and setters. Then in increaseBy(), attempt a downcast in order to get at those methods and perform the increase.

OK, I know, the huge downside here is the downcast. There is also the abuse of the interface construct.

Following the spirit of the Feathers post, shouldn’t we ask the amount object, the one on which we used a getter, to perform the action, like this:

double orderTotal = ...;
amount.increaseBy(orderTotal);

Granted, this does not take a Money object, but here you side-step both the getter-is-evil idiom and my objection above.

Another question I have for this crowd: is this getter-is-evil dance even valid at layer boundaries? I think back to The Project and I had code like this:

Public Function storeNewPC(ByRef pcToStore As PC) As Integer

	Dim qryStr As String = ""

	qryStr = qryStr & "sp_STORE_PC '" & pcToStore.getMake().getID() & "', '"
	qryStr = qryStr & pcToStore.getModel().getID() & "', '"
	qryStr = qryStr & pcToStore.getCores() & "', '"
	...

	Return db.exec(qryStr)

End Function

Following this idiom, I shouldn’t be asking the pcToStore for all of its guts; I should instead tell it to generate the query string I need:

Public Function storeNewPC(ByRef pcToStore As PC) As Integer

	Dim qryStr As String = pcToStore.generateStoreStr()

	Return db.exec(qryStr)

End Function

Please excuse me as I vomit. Big no-no.

Possible side-steps:

  • use reflection. However, as I alluded to in the Nilsson book review, this sort of trickery is both a little too complex and also kind of weak; if all you’re doing is using it to access private members, then you are already violating the spirit of the getter-is-evil argument.
  • use the Friend keyword and keep repositories and domain objects in the same assembly. This works, but it carries with it the same “spirit” problem as above, and it is a .NET-specific approach.
  • eliminate getters and provide methods that return an agreed-to data structure whenever info like this is needed…an IList, an array, a struct…whatever. This way, you’re not tied to internal data types. On the other hand, this approach is somewhat klunky.
  • most industrial solutions will be using a third-party tool for this stuff anyway, so…use a third-party tool like NHibernate (which, by the way, uses reflection).

It could very well be the case that layer boundaries are special cases. Another JavaWorld article states that procedural boundaries (like my layer boundary; after all, the examples provided in these articles stay within the business logic (domain) layer) and getters that return interfaces are exceptions to the rule.

In the case of the latter, I have to wonder if I really need all of that bloat if I simply want to return an integer from an object. Do I really need to define an interface for some sort of holder, implement the interface, and then return the implementation? Even so, if I do that, won’t that interface have its own getter method to return the integer?

What’s even worse, all throughout the Nilsson book, .NET properties were used. And I’m now thinking of doing a re-make of The Project in C#, and I know I’ll come across this issue — now that I know it is an issue to start with. And I don’t know, but I think I had another thing to say about the statement/account example that I came up with when I was tossing in my sleep at 2 AM last night.

This is making my head hurt. You tell me. Am I thinking too hard?

Follow

Get every new post delivered to your Inbox.