Simple... wandering of mind on a tiny bit of code style...
-
There was some piece of C# code I am refactoring (+ fixing small bug) where, at some stage, an expression like
a.b.cis replaced by (private method)GetBc(A a). When I refactored that, I ignoredGetBc(a)and wrotea.b.cinstead, because I like it better that way. One of the code reviewers complained I was not usingGetBc(a). Whatever, I updated my code to useGetBc(a)again. Now I wonder, mm.. how many people would care about that kind of detail and what kind of expression they would prefer?A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was some piece of C# code I am refactoring (+ fixing small bug) where, at some stage, an expression like
a.b.cis replaced by (private method)GetBc(A a). When I refactored that, I ignoredGetBc(a)and wrotea.b.cinstead, because I like it better that way. One of the code reviewers complained I was not usingGetBc(a). Whatever, I updated my code to useGetBc(a)again. Now I wonder, mm.. how many people would care about that kind of detail and what kind of expression they would prefer?A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
How would that work? GetBC(A a) => (a.b, a.c);? GetBC(A a) => new { a.b, a.c };? In both cases I'd prefer a.b.c. I don't see the added value of GetBC :~
Best, Sander Azure DevOps Succinctly (free eBook) Azure Serverless Succinctly (free eBook) Migrating Apps to the Cloud with Azure arrgh.js - Bringing LINQ to JavaScript
-
How would that work? GetBC(A a) => (a.b, a.c);? GetBC(A a) => new { a.b, a.c };? In both cases I'd prefer a.b.c. I don't see the added value of GetBC :~
Best, Sander Azure DevOps Succinctly (free eBook) Azure Serverless Succinctly (free eBook) Migrating Apps to the Cloud with Azure arrgh.js - Bringing LINQ to JavaScript
C GetBc(A a) => a.b.c;
The only "real advantage" (in the day of yore before intellisense) is that it's more like
SomeClass GetBc(AnotherClass a) => a.LongPropertyName.OtherLongPropertyName;
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was some piece of C# code I am refactoring (+ fixing small bug) where, at some stage, an expression like
a.b.cis replaced by (private method)GetBc(A a). When I refactored that, I ignoredGetBc(a)and wrotea.b.cinstead, because I like it better that way. One of the code reviewers complained I was not usingGetBc(a). Whatever, I updated my code to useGetBc(a)again. Now I wonder, mm.. how many people would care about that kind of detail and what kind of expression they would prefer?A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
I would ask the code reviewer, why not
GetC(GetB(a))as that is really the logical extreme he/she is going to and points out the ludicracy of not doinga.b.cLatest Article:
SVG Grids: Squares, Triangles, Hexagons with scrolling, sprites and simple animation examples -
I would ask the code reviewer, why not
GetC(GetB(a))as that is really the logical extreme he/she is going to and points out the ludicracy of not doinga.b.cLatest Article:
SVG Grids: Squares, Triangles, Hexagons with scrolling, sprites and simple animation examplesOnly problem with that is the reviewer might be one of those "letter of the law" people who would then demand that exact thing. Considering they had the problem in the first place, I wouldn't bet against their desire for the extreme.
I’ve given up trying to be calm. However, I am open to feeling slightly less agitated.
-
There was some piece of C# code I am refactoring (+ fixing small bug) where, at some stage, an expression like
a.b.cis replaced by (private method)GetBc(A a). When I refactored that, I ignoredGetBc(a)and wrotea.b.cinstead, because I like it better that way. One of the code reviewers complained I was not usingGetBc(a). Whatever, I updated my code to useGetBc(a)again. Now I wonder, mm.. how many people would care about that kind of detail and what kind of expression they would prefer?A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
Is this a coding standard to use regular Methods such as in your example? Is this really not up to the code reviewer, but more in line with enforcing consistency and standards that your dev shop has?
-
There was some piece of C# code I am refactoring (+ fixing small bug) where, at some stage, an expression like
a.b.cis replaced by (private method)GetBc(A a). When I refactored that, I ignoredGetBc(a)and wrotea.b.cinstead, because I like it better that way. One of the code reviewers complained I was not usingGetBc(a). Whatever, I updated my code to useGetBc(a)again. Now I wonder, mm.. how many people would care about that kind of detail and what kind of expression they would prefer?A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
Only the idgit what wrote
GetBcwould care. Mark the method and its writer as Obsolete. On the other hand, it may help when using "Find all references" if you ever want to see wherea.b.cis used. But, as presented, I see no logical reason to use the method. -
C GetBc(A a) => a.b.c;
The only "real advantage" (in the day of yore before intellisense) is that it's more like
SomeClass GetBc(AnotherClass a) => a.LongPropertyName.OtherLongPropertyName;
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
Super Lloyd wrote:
C GetBc(A a) => a.b.c;
This is just ridiculous :~ Send that reviewer over to this thread so he can read that I find him ridiculous. Judging from the other replies on this thread I'm not the only one :laugh:
Super Lloyd wrote:
The only "real advantage" (in the day of yore before intellisense) is that it's more like
In those instances I don't make a function, but I simply use a variable.
var shortName = a.LongPropertyName.OtherLongPropertyName;
// Use shortName here.Best, Sander Azure DevOps Succinctly (free eBook) Azure Serverless Succinctly (free eBook) Migrating Apps to the Cloud with Azure arrgh.js - Bringing LINQ to JavaScript
-
There was some piece of C# code I am refactoring (+ fixing small bug) where, at some stage, an expression like
a.b.cis replaced by (private method)GetBc(A a). When I refactored that, I ignoredGetBc(a)and wrotea.b.cinstead, because I like it better that way. One of the code reviewers complained I was not usingGetBc(a). Whatever, I updated my code to useGetBc(a)again. Now I wonder, mm.. how many people would care about that kind of detail and what kind of expression they would prefer?A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
GetBc is a wrapper. As posted, it could be doing something else, or has done something else in the past. Or it was a stub from development. Do you just clean it out or replace it when "done"? (and maybe introduce side effects). Mixing and matching isn't the solution.
"Before entering on an understanding, I have meditated for a long time, and have foreseen what might happen. It is not genius which reveals to me suddenly, secretly, what I have to say or to do in a circumstance unexpected by other people; it is reflection, it is meditation." - Napoleon I
-
C GetBc(A a) => a.b.c;
The only "real advantage" (in the day of yore before intellisense) is that it's more like
SomeClass GetBc(AnotherClass a) => a.LongPropertyName.OtherLongPropertyName;
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
I take the liberty to slightly defend that code reviewer. It is a matter of blackboxing: When you reference that bc property of a, you should be unaffected by how the a object structures its attributes, i.e. whether it has a single bc attribute (and maybe bc1, bc2, bc3, ...) or groups all the b<*> attributes into a b object with subfields. C# lets the a object provide a .bc property, so that is can map it to a .bc or a b.c field at its own discretion. Maybe that is nowadays available in C++ as well (it wasn't when I switched from C++ to C#). I have become very fond of blackboxing and need-to-know: You should need to know as little as possible about the inner structure of the objects you interact with. a.b.c does reveal an inner structure that you shouldn't care about. But I most certainly would prefer a .bc property to a GetBc(A a) method!
-
GetBc is a wrapper. As posted, it could be doing something else, or has done something else in the past. Or it was a stub from development. Do you just clean it out or replace it when "done"? (and maybe introduce side effects). Mixing and matching isn't the solution.
"Before entering on an understanding, I have meditated for a long time, and have foreseen what might happen. It is not genius which reveals to me suddenly, secretly, what I have to say or to do in a circumstance unexpected by other people; it is reflection, it is meditation." - Napoleon I
Gerry Schmitz wrote:
Mixing and matching isn't the solution.
True, consistency is desirable, but in this case, having the method means the method may be used in some places and not in others, which is inconsistent. Remove the method and all code will have to use use the
a.b.csyntax consistently. Writing a method and then having to root out all of the instances of the old syntax is less than ideal. -
I take the liberty to slightly defend that code reviewer. It is a matter of blackboxing: When you reference that bc property of a, you should be unaffected by how the a object structures its attributes, i.e. whether it has a single bc attribute (and maybe bc1, bc2, bc3, ...) or groups all the b<*> attributes into a b object with subfields. C# lets the a object provide a .bc property, so that is can map it to a .bc or a b.c field at its own discretion. Maybe that is nowadays available in C++ as well (it wasn't when I switched from C++ to C#). I have become very fond of blackboxing and need-to-know: You should need to know as little as possible about the inner structure of the objects you interact with. a.b.c does reveal an inner structure that you shouldn't care about. But I most certainly would prefer a .bc property to a GetBc(A a) method!
trønderen wrote:
When you reference that bc property of a, you should be unaffected by how the a object structures its attributes
If you use the data of the object outside the object then you have exposed the internal details of the object. The specifics of how the data is exposed is not relevant. This comes about because people confuse the idealization of 'object' and 'message' and the implementation to 'class' and 'method'. They then think incorrectly that it must mean that it goes the other way so 'method' => 'message'. And certainly ignore that the first is a ideal which can never and should never be rigidly enforced.
-
I would ask the code reviewer, why not
GetC(GetB(a))as that is really the logical extreme he/she is going to and points out the ludicracy of not doinga.b.cLatest Article:
SVG Grids: Squares, Triangles, Hexagons with scrolling, sprites and simple animation examples -
Super Lloyd wrote:
C GetBc(A a) => a.b.c;
This is just ridiculous :~ Send that reviewer over to this thread so he can read that I find him ridiculous. Judging from the other replies on this thread I'm not the only one :laugh:
Super Lloyd wrote:
The only "real advantage" (in the day of yore before intellisense) is that it's more like
In those instances I don't make a function, but I simply use a variable.
var shortName = a.LongPropertyName.OtherLongPropertyName;
// Use shortName here.Best, Sander Azure DevOps Succinctly (free eBook) Azure Serverless Succinctly (free eBook) Migrating Apps to the Cloud with Azure arrgh.js - Bringing LINQ to JavaScript
In most cases it's ridiculous, yes. It's not a construct I think I've used myself. However... What if
bis null? Supposebis a lazy-loaded property which is only instantiated as/when referenced, butadoes not support that functionality? Or - in the circumstances where you use theGetBCfunction you need a default value returned ifborcare null. I'd agree that ifGetBCdoes nothing more than returnb.cthen yes, it's pointless; but if there's code in that function (even just exception handling) then maybe... though a more descriptive/specific name than simplyGetBCmight be appropriate.Telegraph marker posts ... nothing to do with IT Phasmid email discussion group ... also nothing to do with IT Beekeeping and honey site ... still nothing to do with IT
