3

I've been bothered by this line of code I've written and I've been a bit confused into what should be written instead.

class SomeClass
{
    IBeneficiary _latestBeneficiary => new Beneficiary(Iban, Name);
}

In context, the field represents the latest version of a beneficiary object that is about to be created, and I want this variable to represent its latest possible version considering whatever is inside those public properties.

Here are my assumptions and thought process, I'm thinking there is something wrong in there otherwise I wouldn't have an issue.

So, clearly, this is a field. It's a field because it is a private variable I'm keeping inside my class, and to recognize it, I add an underscore as a prefix.

That field always returns the latest IBeneficiary possible consideringIban and Name (irrelevant here). Those properties are public and are classic MyProperty SomeProperty { get; set; } defined in the class.

I've defined a field with a property getter, the =>. This is confusing because it's not the expected behaviour of a field to always return something new. Or is it?

I feel like this then should be a function, something like

IBeneficiary CreateLatestBeneficiary (MyProperty param1, MyOtherProperty param2)
{ 
      return new Beneficiary(param1, param2);
} 

Or even name it GetLatestBeneficiary, but in both cases this looks and feels like a really simple getter, so I'd rather have a property with a single getter, that does exactly this, like the following. Right?

IBeneficiary LatestBeneficiary
{
   get
   {
       return new Beneficiary(Iban, Name);
   }
}

But a private property is pretty much a field. Isn't it?

And with that in mind we're back to square one, using a field.

I feel like somewhere in there, one of my statements is wrong.

Are private properties okay ? Or is a property by definition something public, with at least a getter ? Or is it okay for a field to not be a plain old variable ?

Ultimately, how would you write this line of code yourself?

  • It's not a field merely because you put an underscore in front of it. Semantically it's not a field at all. A field is just a variable that can hold something. – Robert Harvey Jan 23 '17 at 19:40
  • I said it was a field because it was a private holder of my object, and therefore I put the underscore (and not the other way around). Why is it not a field at all? – Gil Sand Jan 23 '17 at 19:41
  • 1
    A field implies the ability to assign a value to it anywhere in the class. This "field" doesn't have that ability. – Robert Harvey Jan 23 '17 at 19:46
  • Well Robert that's quite helpful, thank you. It's also quite logical and simple. – Gil Sand Jan 23 '17 at 19:59
7

So, clearly, this is a field. It's a field because it is a private variable I'm keeping inside my class, and to recognize it, I add an underscore as a prefix.

This is not correct. It is not a field; it is a property. And that property is just syntactic sugar for a method, which the compiler creates for you. In this case that method would be IBeneficiary get__latestBeneficiary() => ...

Just because it is private, doesn't make it a field. a field is a variable, not a method.

Or even name it GetLatestBeneficiary, but in both cases this looks and feels like a really simple getter, so I'd rather have a property with a single getter, that does exactly this, like the following. Right?

The code:

IBeneficiary LatestBeneficiary
{
   get
   {
       return new Beneficiary(Iban, Name);
   }
}

is semantically identical to:

IBeneficiary _latestBeneficiary => new Beneficiary(Iban, Name);

The expression body syntax of the second version is just a more compact way of expressing that getter.

I feel like this then should be a function

I would tend to agree. Even though this is a private read-only property, it's still a read-only property (ie, getter). They carry expectation baggage: most developers would expect read-only properties to return the same value each time (especially if the state of the rest of the object hasn't changed). So following the principle of least astonishment, you should avoid using properties in this way.

So make it a method:

IBeneficiary CreateLatestBeneficiary() => new Beneficiary(Iban, Name);
1
This is confusing because it's not the expected behaviour of a field to always return something new. Or is it?

Yes, that is quite confusing. I would expect a getter to return the beneficiary that was most recently created ("latest") before the function was called - I would not expect to create one immediately and then return the new beneficiary.

I think your function CreateLatestBeneficiary is the best way to go as the name makes it very clear that a new thing will exist as a result of the function call. I'd probably rename it to GenerateNewBeneficiary, but that's more my personal preference than anything else.

  • 2
    To keep it terse, you can keep the original lambda syntax for the method as well: GenerateNewBeneficiary() => new Beneficiary(Iban, Name); – Graham Jan 23 '17 at 20:27
  • This is what I ended up doing, Graham ^^ – Gil Sand Jan 23 '17 at 20:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.