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?
-
1It'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– Robert Harvey2017年01月23日 19:40:29 +00:00Commented Jan 23, 2017 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– Gil Sand2017年01月23日 19:41:40 +00:00Commented Jan 23, 2017 at 19:41
-
2A field implies the ability to assign a value to it anywhere in the class. This "field" doesn't have that ability.Robert Harvey– Robert Harvey2017年01月23日 19:46:46 +00:00Commented Jan 23, 2017 at 19:46
-
Related: softwareengineering.stackexchange.com/a/340095Robert Harvey– Robert Harvey2017年01月23日 19:46:53 +00:00Commented Jan 23, 2017 at 19:46
-
Well Robert that's quite helpful, thank you. It's also quite logical and simple.Gil Sand– Gil Sand2017年01月23日 19:59:34 +00:00Commented Jan 23, 2017 at 19:59
2 Answers 2
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);
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.
-
2To keep it terse, you can keep the original lambda syntax for the method as well:
GenerateNewBeneficiary() => new Beneficiary(Iban, Name);
GHP– GHP2017年01月23日 20:27:27 +00:00Commented Jan 23, 2017 at 20:27 -
This is what I ended up doing, Graham ^^Gil Sand– Gil Sand2017年01月23日 20:43:14 +00:00Commented Jan 23, 2017 at 20:43
Explore related questions
See similar questions with these tags.