Elegant code
I just like this code, so I thought I would publish it.
1: public static class ArrayExtension2: {
3: public static T[] GetOtherElementsFromElement<T>(this T[] array , T element)4: {
5: var index = Array.IndexOf(array, element);
6: if (index == -1)7: return array;8: return array.Skip(index + 1).Union(array.Take(index)).ToArray();9: }
10: }
And the unit test:
1: public class ReplicationUnitTest2: {
3: [Fact]
4: public void Will_distribute_work_starting_with_next_node()5: {
6: var nodes = new[] { 1, 2, 3 };7: Assert.Equal(new[] { 3, 1 }, nodes.GetOtherElementsFromElement(2));8: Assert.Equal(new[] { 1, 2 }, nodes.GetOtherElementsFromElement(3));9: Assert.Equal(new[] { 2, 3 }, nodes.GetOtherElementsFromElement(1));10: Assert.Equal(new[] { 1, 2, 3 }, nodes.GetOtherElementsFromElement(4));11: }
12: }
Comments
I'm not too sure what you're doing, but I assume you're getting other elements other than "2" in the first case, other than "3" in the second case, other than "1" in the third case and other than "4" in the last case.
Couldn't you just have done a negate intersect? In SQL terms, Except.
Notice the ordering of the elements there.
Ah. Indeed. Now I see. Elegant indeed.
I see what it is doing but can't see what you intend to use it for. Why when you remove the element do you want to swap the groups of numbers before/after its position in the source?
I think it's poorly named too :-) At first I'd have suggested
public static T[] Exclude <t(this T[] array , T element)
but seeing as it swaps the groups of numbers before and after I don't think that is sufficient, so without knowing your intent I can't think of a suitable name :-)
Check the test name.
and now imagine that instead of numbers, you would have urls of nodes in a grid
I read the test name, in fact the test told me more than the code. I don't know anything about workload distribution so your hint is lost on me :-)
I'd have expected items to be taken off the front, like a "round robin" approach, so I suspect the goal is to achieve something I am unfamiliar with.
PS: If you get a second :-)
stackoverflow.com/.../rhino-mocks-returning-a-d...
An Array just holds stuff. If you want some kind of queue-like behavior, then you should properly abstract that into an appropriately named structure. The code is not self descriptive of its behavior and it could be argued that you are violating the SRP for Array by bolting this on via an extension method.
This would be a good candidate for inclusion in Mono.Rocks. Do you know this library? It has a lot of useful extension methods.
I might be missing something but what are your reasons for not using IList(T) instead of array or even better IEnumerable(T)? As far as I can see this is an O(n) implementation and the same would be true for an IEnumerable-implementation.
I agree with TED. It looks like you are trying to make Array have behaviour that is relevant only to a specific case. What you really should be doing is creating a new class completely.
Not every problem requires extension methods as a solution. It reminds me of that common saying about a man with only a hammer treats every problem like it's a nail.
I like this extension method. It's unfortunate that arrays are second-class citizens in Java and C#, since arrays get special syntax that makes them easier to use. Java's by far the worse offender, lacking operator overloads.
I've only seen reasonable arrays in dynamic languages (Python and Boo; I don't know any others well enough) and D (which has a lot of other neat features, and I use at home all the time, but has more than its share of bugs).
GetOtherElementsFromElement could work with IEnumerable and could be an iterator IMO. Seems that would be more efficient.
As indicated by others this method is poorly named in regard that it works on any array. What about RoundRobinStartAfter ?
Another point is that semantic could be better. The method should take an index instead of an element. The current implementation doesn't support arrays that contain multiple instances of the same value.
On the other hand the concerns that I mention could be not important in your particular case.
Having thought about it you are dequeuing something aren't you? This to me just looks like a special queue implementation, so I think you should create a specialised queue class and the method should be (wait for it......) dequeue :-)
It's fluent IList.Remove(), but for arrays, nice. However, it's worth mentioning this link:
blogs.msdn.com/.../...idered-somewhat-harmful.aspx
Why Union and not Concat?
Nice, but does not look as an extension method, too tied to the specific usage.
This could be also expressed in pure LINQ, without the need to iterate more than once on any item :
list.Concat(list).SkipWhile( x => x != item)
(provided item is unique in the list)
Well, my one-liner sends an empty list if the item is not in the list. I'm trying to find a way to accomodate this in the expression, to no avail.
A ugly way to do this (without an indexOf :)
var res = array.Concat(array).SkipWhile( x => x != item)
.Skip(1).
TakeWhile(x => x!=item);
return res.Length == 0 ? array : res;
Dont you think if the index + 1 will fail if the index passed on is the last element ?
Vadi,
Check the test, that scenario is covered.
I don't care for the naming...
much better as GetElementsExcept(2) or perhaps just array.AllExcept(2);
var nodes = new[] { 1, 2, 3, 3,5,2 };
Assert.Equal(new[] { 3, 1 }, nodes.GetOtherElementsFromElement(2));
will not work.
+1 for Ben
array.AllExcept(2); // is a more better meaningful method name
Benny,
Duplicate elements cannot exists in the array, that is an implicit assumption.
Assuming that we remove the duplicate elements, it should result int:
5,1,2
Comment preview