|
How about a linq'ish approach? BTW: "linq'ish" sounds like "inept" in German
public static bool AllTheSame<T>(this IEnumerable<T> source)
{
return AllTheSame<T>(source, EqualityComparer<T>.Default);
}
public static bool AllTheSame<T>(this IEnumerable<T> source, IEqualityComparer<T> comparer)
{
var it = source.GetEnumerator();
if (it.MoveNext())
{
var first = it.Current;
while (it.MoveNext()) if (!comparer.Equals(first, it.Current)) return false;
}
return true;
}
Cheers
Andi
|
|
|
|
|
You need to read the original post -- it shows Linq, and many others have posted various Linq alternatives.
|
|
|
|
|
I like that a lot. Thanks.
IMO there is no LINQ involved, it is pure generics.
|
|
|
|
|
Ok, I might have to elaborate why I say "linq'ish": It is an extension method that has a signature in the spirit of linq functions as found in the Enumerable namespace:
- it is an extension method for
IEnumerable<T> - it provides comparator parameter to let the client pass an approriate equality functionality
- it provides an overload which defaults to the default comparer of the given generic type
That's why I consider it "in the spirit of Linq", respectively "linq'ish".
Using it:
original post (simplified) | this suggestion |
---|
if (myList.Distinct().Count()>1)
{ ... }
if (myList.Distinct(Pressure.Equality).Count()>1)
{ ... }
|
if (myList.AllTheSame())
{ ... }
if (myList.AllTheSame(Pressure.Equality))
{ ... }
|
Even if I would leave away the word "linq'ish", I still consider putting the whole functionality into a function is my preferred way of doing this. I do that even for single liners to show the intent in a kind of self exlaining way.
I consider the use of Distinct() in the original post a bit an overshot since it calculates too many data.
There is a similar trap: if (SomeExtensivDataAquisition().Count()>0) . This calculates all the data to simply check if there is any data at all. If the problem is such that there is most of the time some data, one has far better direct methods to do so: e.g. Contains(...) or FirstOrDefault(...) . I hesitate to use constructs if (...Count() > 0) and alike in connection with IEnumerable<T> objects.
Cheers
Andi
|
|
|
|
|
Andreas Gieriet wrote: I still consider putting the whole functionality into a function is my
preferred way of doing this
Totaly agree. It applies a descriptive name to a technique, makes it more easily re-usable, and more readily maintainable. I don't think the OP understands that.
Andreas Gieriet wrote: a bit an overshot
Yes, very much. As well as dupicating data (even if it's just references).
|
|
|
|
|
Thanks. I see what you meant now. Still I don't even need to know about LINQ to appreciate your code, but yes it does fit in the LINQ way of doing things.
And I agree about the possible overkill by LINQ, when using Distinct, Count, and such when there is no real need for them.
|
|
|
|
|
I don't see that devolving what should be a foreach into its component GetEnumerator, MoveNext, and Current parts improves the "elegance" any. If it's simply so you don't break or return out of it, then I think you need to reevaluate your coding values* -- it's like replacing a goto with something that works like a goto simply so you don't have a goto .
At any rate, I chose to use IList so I could use an index and a for loop rather break out of a foreach .
* I'm not saying mine are better. I also understand that some times we post code that was whipped up quickly just to illustrate a point. Personally, I don't like multiple returns in a method and I prefer not to use break and continue if they can easily be avoided.
Andreas Gieriet wrote: "linq'ish" sounds like "inept"
That about sums up how I feel about Linq.
|
|
|
|
|
Well, I personally don't agree with my word game: "linq'ish" --> German "linkisch" ~ "inept"...
I find Linq and the respective C# functions (Where, Select, etc.) quite handy: you can chain them and they do late evaluation for iterating.
And I'm also a supporter for using iterators instead of indexed access whenever adequate. And I prefer many small functions over larger functions. And I employ idiomatic style where useful. And, and, and, ...
We both seem to prefer different coding styles.
Regrading "elegant": The title is badly chosen and misleading in my opinion - I would rather go for "expressive" or alike. What is elegant coding anyways (Straight to the point? Simple? ...)? I don't know. I have other expectations on coding style than being "elegant": correct, complete, robust, maintainable, documented, concise, expressive, not copy-paste, no code-bloat, universally usable, reasonable performant, "good-enough", little complexity per function...
The problem at hand is not a plain foreach problem; it is rather "get the first element (if there is any) and compare to the rest of the elements (if there are any)", so I chose to go the iterator way. See also alternatives to that further down in this comment.
To my suggestion: No, this function is programmed intentionally as it is. The changes I would agree to get the same level as linq functions: check the null source and allow for a null comparer (takes the default comparer of that type) - and maybe the formatting (adding blocks instead of the while-if-single-liner). The rest is ok for my standards.
So, I'm ok with the following code for production (place into an appropriate extension class):
1 public static bool AllTheSame<TSource>(this IEnumerable<TSource> source)
9 {
10 return AllTheSame<TSource>(source, null);
11 }
12 public static bool AllTheSame<TSource>(this IEnumerable<TSource> source,
21 IEqualityComparer<TSource> comparer)
22 {
23
24 if (source == null) throw new ArgumentNullException("Value cannot be null.", "source");
25 var equalityComparer = comparer ?? EqualityComparer<TSource>.Default;
26
27
28 var it = source.GetEnumerator();
29 if (it.MoveNext())
30 {
31 var first = it.Current;
32 while (it.MoveNext()) if (!equalityComparer.Equals(first, it.Current)) return false;
33 }
34 return true;
35 }
You can always improve or modify to your taste (but maybe for the cost of runing twice over the first element):
27 ...
28 var first = source.FirstOrDefault();
29 foreach (var item in source) if (!equalityComparer.Equals(first, item)) return false;
30 return true;
31 }
Or:
27 ...
28 var first = source.FirstOrDefault();
29 return source.All(e => equalityComparer.Equals(first, e));
30 }
BTW: You can not easily combine IEnumerable<T> and index-based for loops, and thus, Linq and index-based for do not easily combine neither.
If you stick on index-based for loop, then you have to implement different functions for the different more specific kind of collections (e.g. IList<T> , IDictionary<K, V> , HashSet<T> , etc.). You may not use any of the Linq functions since they return IEnumerable<T> that allows to chain them easily. An index-based loop needs an end-condition up-front, (something like Count() ) and IEnumerable<T> is not random-access (i.e. does not allow to access an element by index). IEnumerable<T> goes hand in hand with iterators, but not with indices.
You can of course call Count() , but that already iterates over all elements to simply find out how often to move ahead in the sequence of elements...
As I said further above, we both have different coding styles and approaches. Luckily, there is no "one and only right" (dogmatic?) solution
Cheers
Andi
|
|
|
|
|
Andreas Gieriet wrote: You can of course call Count()
Except for that there's no guarantee that a given IEnumerable will ever terminate.
|
|
|
|
|
Sorry to say, however I did read the tip again, and now I bumped into a real problem: if T is a reference type, first could be null, hence first.Equals(...) could go kaboom. And I'm curious to learn what the elegant solution will be...
|
|
|
|
|
Pfft... details...
Does null == null ?
And what does the Linq do?
I suppose one could iterate to the first non-null and go from there.
|
|
|
|
|
PIEBALDconsult wrote: Does null == null ?
yes null==null , however null.Equals(whatever) is over and out.
I came up with two approaches so far, none very elegant as they basically double the amount of code:
1. test first for null; if so reference-compare the list elements with null; otherwise do what you had already.
2. split the generic method in two overloads, one for value types (as is); one for ref types; make the requirement IComparable<T> rather than IEquatable<T> , and implement Object.CompareTo(T obj1, T obj2)
I would hope there is a better approach though, something that hides most everything about the null problem.
PIEBALDconsult wrote: And what does the Linq do?
I can't help you there, however you could subject it to some Reflectoring...
I wonder whether the elegant will be upheld in the title...
|
|
|
|
|
Luc Pattyn wrote: I wonder whether the elegant will be upheld in the title...
Oh, that was shattered by the OP.
I considered IComparable<T>, but I was hurrying. I also was thinking of an overload that takes an IComparer<T>.
|
|
|
|
|
This simple test and alternate loop seems to work.
if ( (object) first == null )
{
for ( int i = 1 ; result && ( i < List.Count ) ; i++ )
{
result = (object) List [ i ] == null ;
}
}
I also seem to recall that there are contrived cases where a value type may be null. By the way, I also tried to test with a List<object> containing ints and strings, but "There is no implicit reference conversion from 'object' to 'System.IEquatable<object>'."
|
|
|
|
|
Yes, that is one of the things I suggested.
In the mean time Andreas has come up with an approach I'd call definitive. BTW: Him mentioning LINQ seems to have confused you, as his code isn't LINQ at all.
|
|
|
|
|
Luc Pattyn wrote: I'd call definitive
I wouldn't. He didn't even check for null or even write a comment as to that fact!
|
|
|
|
|
Checking for null or empty would be shorter than commenting about it, so why didn't you?
|
|
|
|
|
|
That looks OK. Counts zero and one should return "all equal" indeed. I wouldn't mind null returning "all equal" also, however an exception is better. And you're right about the parameter order anomaly.
|
|
|
|
|
Well, seeing as you approve, I guess I'll update it.
|
|
|
|
|
Isn't that the same thing?
-bruce
|
|
|
|
|
I've no idea; maybe, maybe not. I can't read that.
Maybe show that to the original poster?
|
|
|
|
|
q == q is a tautology (i.e. it'll always be true).
One of the comments on the original suggested a variant on
All(q => q==list[0])
which is what I think you're suggesting.
John
|
|
|
|