Is it an anti-pattern if a class property creates and returns a new instance of a class?

Question

I have a class called Heading that does a few things, but it should also be a able to return the opposite of the current heading value, which finally has to be used via creating a new instance of the Heading class itself.

I can have a simple property called reciprocal to return the opposite heading of the current value and then manually create a new instance of the Heading class, or I can create a method like createReciprocalHeading() to automatically create a new instance of the Heading class and return it to the user.

However, one of my colleagues recommended me to just create a class property called reciprocal that returns a new instance of the class itself via its getter method.

My question is: Isn't it an anti-pattern for a property of a class to behave like that?

I particularly find this less intuitive because:

1. In my mind, a property of a class should not return a new instance of a class, and
2. The name of the property, which is reciprocal, doesn't help the developer to fully understand its behaviour, without getting help from the IDE or checking the getter signature.

Am I being too strict about what a class property should do or is it a valid concern? I have always tried to manage the state of the class via its fields and properties and its behaviour via its methods, and I fail to see how this fits to the definition of the class property.

Answer 1

Its not unknown to have things like Copy() or Clone() but yes I think you are right to be worried about this one.

take for example:

h2 = h1.reciprocal
h2.Name = "hello world"
h1.reciprocal.Name = ?

It would be nice to have some warning that the property was a new object each time.

you might also assume that:

h2.reciprocal == h1

However. If your heading class was an immutable value type then you would be able to implement these relationships and reciprocal might be a good name for the operation

Answer 2

An interface of a class leads the user of the class to make assumptions about how it works.

If many of these assumptions are right and few are wrong, the interface is good.

If many of these assumptions are wrong and few are right, the interface is rubbish.

A common assumption about Properties is that calling the get function is cheap. Another common assumption about properties is that calling the get function twice in a row will return the same thing.

---

You can work around this by using consistency, in order to change expectations. For example, for a small 3D library where you need Vector, Ray Matrix, etc you can make it such that getters such as Vector.normal and Matrix.inverse are pretty much always expensive. Unfortunately even if your interfaces consistently use expensive properties, the interfaces will at best be as intuitive as one that uses Vector.create_normal() and Matrix.create_inverse() - yet I know of no strong argument that can be made that using properties creates a more intuitive interface, even after changing expectations.

Answer 3

Properties should return very quickly, return the same value with repeated calls, and getting their value should have no side effects. What you describe here should not be implemented as a property.

Your intuition is broadly correct. A factory method does not return the same value with repeated calls. It returns a new instance each time. This pretty much disqualifies it as a property. It's also not fast if later development adds weight to the instance creation, e.g. network dependencies.

Properties should generally be very simple operations that get/set a part of the current state of the class. Factory-like operations don't meet this criteria.

Answer 4

I don't think there's a language-agnostic answer to this since what constitutes a “property” is a language-specific question, and what a caller of a “property” expects is a language-specific question as well. I do think the most fruitful way to think about this is to think about what it looks like from the point of view of the caller.

In C#, properties are distinctive in that they are (conventionally) capitalized (like methods) but lack parentheses (like public instance variables). If you see the following code, absent documentation, what do you expect?

var reciprocalHeading = myHeading.Reciprocal;

As a relative C# novice, but one who’s read Microsoft’s Property Usage Guidelines, I would expect Reciprocal to, among other things:

1. be a logical data member of the Heading class
2. be inexpensive to call, such that there’s no need for me to cache the value
3. lack observable side effects
4. produce the same result if called twice in succession
5. (maybe) offer a ReciprocalChanged event

Of these assumptions, (3) and (4) are probably correct (assuming Heading is an immutable value type, as in Ewan's answer), (1) is debatable, (2) is unknown but also debatable, and (5) is unlikely to make semantic sense (though whatever has a heading should perhaps have a HeadingChanged event). This suggests to me that in a C# API, “get or calculate the reciprocal” should not be implemented as a property, but especially if the calculation’s cheap and Heading is immutable, it’s a borderline case.

(Note, though, that none of these concerns have anything to do with whether calling the property creates a new instance, not even (2). Creating objects in the CLR, in and of itself, is pretty cheap.)

In Java, properties are a method naming convention. If I see

Heading reciprocalHeading = myHeading.getReciprocal();

my expectations are similar to those above (if less explicitly set out): I expect the call to be cheap, idempotent, and lacking in side effects. However, outside the JavaBeans framework the concept of a “property” is not all that meaningful in Java, and particularly when considering an immutable property with no corresponding setReciprocal(), the getXXX() convention is now somewhat old-fashioned. From Effective Java, second edition (already more than eight years old now):

Methods that return a non-boolean function or attribute of the object on which they’re invoked are usually named with a noun, noun phrase, or a verb phrase beginning with the verb get …. There is a vocal contingent that claims that only the third form (beginning with get) is acceptable, but there is little basis for this claim. The first two forms usually lead to more readable code… (p. 239)

In a contemporary, more fluent API, then, I would expect to see

Heading reciprocalHeading = myHeading.reciprocal();

-- which would again suggest that the call is cheap, idempotent, and lacks side effects, but would say nothing about whether a new calculation is performed or a new object is created. This is fine; in a good API, I shouldn’t care.

In Ruby, there's no such thing as a property. There are “attributes”, but if I see

reciprocalHeading = my_heading.reciprocal

I have no immediate way of knowing whether I’m accessing an instance variable @reciprocal via an attr_reader or a simple accessor method, or whether I’m calling a method that performs an expensive calculation. The fact that the method name is a simple noun, though, rather than say calcReciprocal, suggests, again, that the call is at least cheap and probably doesn’t have side effects.

In Scala, the naming convention is that methods with side effects take parentheses and methods without them don’t, but

val reciprocal = heading.reciprocal

could be any of:

// immutable public value initialized at creation time
val reciprocal: Heading = … 

// immutable public value initialized on first use
lazy val reciprocal: Heading = … 

// public method, probably recalculating on each invocation
def reciprocal: Heading = …

// as above, with parentheses that, by convention, the caller
// should only omit if they know the method has no side effects
def reciprocal(): Heading = …

(Note that Scala allows various things that are nonetheless discouraged by the style guide. This is one of my major annoyances with Scala.)

The lack of parentheses tells me the call doesn’t have side effects; the name, again, suggests that the call should be relatively cheap. Beyond that, I don’t care how it gets me the value.

In short: Know the language you’re using, and know what expectations other programmers will bring to your API. Everything else is an implementation detail.

Answer 5

As others have said, it is a rather common pattern to return instances of the same class.

The naming should go hand in hand with the language's naming conventions.

For example, in Java I'd probably expect it to be called getReciprocal();

That being said, I'd consider mutable vs immutable objects.

With immutable ones, things are pretty easy, and it won't hurt if you return the same object or not.

With mutable ones, this can get very scary.

b = a.reciprocal
a += 1
b = what?

Now what is b referring to? The reciprocal of the original value of a? Or the reciprocal of the changed one? This might be a too short of an example, but you get the point.

In those cases look for a better naming which communicates what happens, for example createReciprocal() might be a better choice.

But it really depends on the context as well.

Answer 6

In the interests of single-responsiblity and clarity I would have a ReverseHeadingFactory that took the object and returned its reverse. This would make it clearer that a new object was being returned and would mean the code to produce the reverse was encapsualated away from other code.

Answer 7

It depends. I usually expect properties to return things that are part of an instance, so returning a different instance of the same class would be a bit unusual.

But for example a string class could have properties "firstWord", "lastWord", or if it handles Unicode "firstLetter", "lastLetter" which would be full string objects - just usually smaller ones.

Answer 8

Since the other answers cover the Property part, I'll just address your #2 about reciprocal:

Do not use anything other than reciprocal. It is the only correct term for what you're describing (and it is the formal term). Do not write incorrect software to save your developers from learning the domain they're working in. In navigation, terms have very specific meanings and using something as seemingly innocuous as reverse or opposite could lead to confusion in certain contexts.

Answer 9

Logically, no, it is not a property of a heading. If it were, you could also say that negative of a number is property of that number, and frankly that would make "property" lose all meaning, almost everything would be a property.

Reciprocal is a pure function of a heading, same as negative of a number is a pure function of that number.

As a rule of thumb, if setting it does not make sense , it probably shouldn't be a property. Setting it can still be disallowed of course, but adding a setter should be theoretically possible and meaningful.

Now, in some languages, it might make sense to have it as a property as specified in the context of that language anyway, if it brings some advantages due to mechanics of that language. For example, if you want to store it to a database, it's probably sensible to make it as property if it allows automatiic handling by ORM framework. Or maybe it is a language where there is no distinction between a property and a parameterless member function (I don't know such a language, but I'm sure there are some). Then it is up to the API designer and documenter to distinguish between functions and properties.

---

Edit: For C# specifically, I'd look at existing classes, especially those of Standard Library.

• There are BigInteger and Complex structures. But they are structures, and have only static functions, and all properties are of different type. So not much design help there for you Heading class.
• Math.NET Numerics has Vector class, which has very few properties, and seems to be pretty close to your Heading. If you go by this, then you'd have a function for reciprocal, not a property.

You might want to look at libraries actually used by your code, or existing similar classes. Try to stay consistent, that's usually the best, if one way isn't clearly right and another way wrong.

Answer 10

Very interesting question indeed. I see no SOLID+DRY+KISS violation in what you're proposing, but, still, it smells bad.

A method that returns an instance of the class is called a constructor, right? so you're creating a new instance from a non-constructor method. It's not the end of the world, but as a client, I wouldn't expect that. This is usually done in specific circumstances: a factory or, sometimes, a static method of the same class (useful for singletons and for... not-so-object-oriented programmers?)

Plus: what happens if you invoke getReciprocal() on the returned object? you obtain another instance of the class, that could very much be an exact copy of the first one! And if you invoke getReciprocal() on THAT one? Objects sprawling anyone?

Again: what if the invoker needs the reciprocal value (as scalar, I mean)? getReciprocal()->getValue()? we are violating Law of Demeter for little gains.