Message Automatically Removed
-
Message Automatically Removed
At least in .NET 3.5, your first function will throw an exception, since you are modifying the collection being enumerated.
Software Zen:
delete this;
-
Message Automatically Removed
In addition to the comments above, each of those Remove statements will incur an array copy cost. Count the occurrence of each item in notRemove, clear the collection, then add the items in notRemove by their respective counts. Possibly something like this:
public static void RemoveAllBut<T>(this ICollection<T> source, params T[] notRemove)
{
Int32[] counts = new Int32[notRemove.Length];
EqualityComparer<T> comparer = EqualityComparer<T>.Default;foreach (T itm in source) { for (Int32 i = 0; i <= counts.Length - 1; i++) { if (comparer.Equals(notRemove\[i\], itm)) { counts\[i\] += 1; break; } } } source.Clear(); for (Int32 i = 0; i <= counts.Length - 1; i++) { for (Int32 j = 1; j <= counts\[i\]; j++) { source.Add(notRemove\[i\]); } }
}
-
Message Automatically Removed
Posting in this particular forum triggers a find-what-is-bad thinking. Sorry. ;) 1. It's O(n^3) which is not good. Did you consider working on sorted collections? Then it would be just O(n). (more precisely, O(max{n,m})). Why O(n^3): 1. foreach loop 2. Contains method which searches notRemove 3. Remove(itm) method which has to find index of itm (in worst case, n calls to Equals method) and, if it's an array list, it has to shift all elements by one which makes it even worse. Besides, I though that you cannot edit a collection inside foreach. 2. I method 2: Why copy all elements to a new array? Without it, it would be memory complexity of Ω(1), in situ operation. Now it's Ω(n) because it allocates a new array in the process. 3. Formatting horror, but maybe it was screwed up during pastying*.
if (notRemove.Contains(itm)) continue; source.Remove(itm);
Shouldn't it be:
if (notRemove.Contains(itm)) continue; source.Remove(itm);
IF it is known that all items in
notRemoves
appear in thesource
exactly once, then what about:public static void RemoveAllBut<T>(this ICollection<T> source, params T[] notRemove)
{
source.Clear();
foreach(var x in notRemove)
source.Add(x);
// because we don't have source.AddAll, unfortunately
}which is O(m) and Ω(1). * -- "pastying" - is it a correct spelling? (derived from a verb "paste").
Greetings - Jacek
-
Message Automatically Removed
Yeah, what they said. Plus I'd avoid ToArray. And see whether or not a HashSet might be of use: HashSet<T>.IntersectWith Method[^] Or not, whatever. :sigh:
-
Posting in this particular forum triggers a find-what-is-bad thinking. Sorry. ;) 1. It's O(n^3) which is not good. Did you consider working on sorted collections? Then it would be just O(n). (more precisely, O(max{n,m})). Why O(n^3): 1. foreach loop 2. Contains method which searches notRemove 3. Remove(itm) method which has to find index of itm (in worst case, n calls to Equals method) and, if it's an array list, it has to shift all elements by one which makes it even worse. Besides, I though that you cannot edit a collection inside foreach. 2. I method 2: Why copy all elements to a new array? Without it, it would be memory complexity of Ω(1), in situ operation. Now it's Ω(n) because it allocates a new array in the process. 3. Formatting horror, but maybe it was screwed up during pastying*.
if (notRemove.Contains(itm)) continue; source.Remove(itm);
Shouldn't it be:
if (notRemove.Contains(itm)) continue; source.Remove(itm);
IF it is known that all items in
notRemoves
appear in thesource
exactly once, then what about:public static void RemoveAllBut<T>(this ICollection<T> source, params T[] notRemove)
{
source.Clear();
foreach(var x in notRemove)
source.Add(x);
// because we don't have source.AddAll, unfortunately
}which is O(m) and Ω(1). * -- "pastying" - is it a correct spelling? (derived from a verb "paste").
Greetings - Jacek
Jacek Gajek wrote:
* -- "pastying" - is it a correct spelling? (derived from a verb "paste").
The word you're looking for is "pasting". However, I like the idea of using "pastying" when referring to anything connected to the Cornish foodstuff[^]. :)
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
Jacek Gajek wrote:
* -- "pastying" - is it a correct spelling? (derived from a verb "paste").
The word you're looking for is "pasting". However, I like the idea of using "pastying" when referring to anything connected to the Cornish foodstuff[^]. :)
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
Message Automatically Removed
-
Message Automatically Removed
There was no need to remove your message. Your idea was a good one but I am guessing you didn't test it. And with all of the egos on this site (not saying anyone who responded necessarily has one) if you say something wrong or that can be perceived as wrong there are many who jump at the opportunity to attack. Lots of vultures. :)
There are only 10 types of people in the world, those who understand binary and those who don't.
-
There was no need to remove your message. Your idea was a good one but I am guessing you didn't test it. And with all of the egos on this site (not saying anyone who responded necessarily has one) if you say something wrong or that can be perceived as wrong there are many who jump at the opportunity to attack. Lots of vultures. :)
There are only 10 types of people in the world, those who understand binary and those who don't.
-
There was no need to remove your message. Your idea was a good one but I am guessing you didn't test it. And with all of the egos on this site (not saying anyone who responded necessarily has one) if you say something wrong or that can be perceived as wrong there are many who jump at the opportunity to attack. Lots of vultures. :)
There are only 10 types of people in the world, those who understand binary and those who don't.