"Too many chained references" on fluent interfaces in general
egreiner opened this issue · 9 comments
var result = nhibernateSession.QueryOver<Employment>()
.Left.JoinAlias(x => x.Assistant, () => assistant)
.Where(x => x.EmploymentType.Id != 4)
.List<Employment>();
-> .List "Too many chained references"
makes no sence for me
Could you provide more detail, please? This doesn't give me anything I can work with.
I mean the same thing as stated in issue #15 but not only regarding to LINQ.
C# uses fluent programming extensively in LINQ to build queries...
https://en.wikipedia.org/wiki/Fluent_interface#C.23
I find nothing smelly on:
var result = nhibernateSession.QueryOver()
.Left.JoinAlias(x => x.Assistant, () => assistant)
.Where(x => x.EmploymentType.Id != 4)
.List();
so such code shouldn't be marked as "Too many chained references"
Without an real codebase I can't provide you any refactoring hints, but your code is kind of smelly ;)
IMHO this code actually should be marked as "Too many chained references".
Dear Toni,
Please refactor this code that i can see what you mean with not smelly code.
var result = nhibernateSession.QueryOver()
.Left.JoinAlias(x => x.Assistant, () => assistant)
.Where(x => x.EmploymentType.Id != 4)
.List();
thx
In your context your advise might be correct. Without knowing hibernate, it's still obvious to me what your code does.
But the real point is, that in general such method chains lead to code which is hard to understand and to maintain. Don't think of writing code, rather think of reading it later without background knowledge!
Because of the design of the underlying library your code isn't refactorable to me, without any knowledge of the surroundings.
I also suspect that I've got your question wrong. If so, I'm sorry.
"Without knowing hibernate, it's still obvious to me what your code does."
This is for me a perfect description of how code should be writen.
I don't know how it exactly works, but i understand what the code should do.
- hide details, i'm not interessted in details, i want to see details - how it's done - as late as possible
so that's why .Where(...) should be .Where(x => x.IsNot(EmploymentType.Freelancer)) ... - make details easily to UnitTest.
In this case i don't see it as violation of "Law of Demeter".
This type of "violation" of the the Law of Demeter is discussed in length by Phil Haack
He quotes Martin Fowler:
I’d prefer it to be called the Occasionally Useful Suggestion of Demeter.
http://blog.robustsoftware.co.uk/2010/04/linq-and-law-of-demeter.html
https://lostechies.com/derickbailey/2010/03/25/law-of-demeter-extension-methods-don-t-count/
All these articles seem to suggest that it's not just a "dot counting exercise".
This makes finding a one size fits all solution difficult to code into a CleanCode.Feature.
When is the correct time to ignore warning from the Clean Code extension?
Presumably when they are inappropriate.
Fluent interfaces which don't violate the Law of Demeter are generally implemented as static extension methods. I think it would be reasonable to (optionally) exclude them.
Not just extension methods.
I use builder classes quite a lot, using a similar fluent mechanism is really useful and IMHO very readable.
An example:
var myObject = builder
.SetPropertyA(A)
.SetPropertyB(B)
.SetPropertyC(C)
.SetPropertyD(D)
.Build()
This is a common pattern I use and I feel does not violate LoD