===================
List<Color> colors = new List<Color>(256);
Color c1, c2;
int i, j, nc = 0, ih = bmp.Height, iw = bmp.Width;
const int IMax = 32;
for (i = 0; i < Math.Min(iw, IMax); i++)
{
//find number of colors
for (j = 0; j < Math.Min(ih,IMax); j++)
{
c1 = bmp.GetPixel(i, j);
if (i == 0 & j == 0)
{
colors.Add(c1);
}
else
{
c2 = colors.Find(c1);
if (c2 == null)
{
colors.Add(c1);
if (nc > 256)
{
MessageBox.Show("Warning - input image has
more than 256 colors and maybe rejected by Visual Studio as application
icon");
return false;
}
else nc++;
}
}
}
}
return true;
The error message itself tells you what the argument to "Find" should
be: it is expecting a Predicate<Color>, which means "a delegate to a
function that takes a Color and returns a Boolean". You can provide such a
delegate by means of a Lambda:
Color c2 = colors.Find(c=>c==c1);
>
> I only dabble in C# once in a long while and am I lost. I got problem
> with
> the following code
> How should I reconcile
> Color c2 = colors.Find(c1)
> so I don't get
> Error 1 The best overloaded method match for
> 'System.Collections.Generic.List<System.Drawing.Color>.Find(System.Predicate
> <System.Drawing.Color>)' has some invalid arguments
In addition to what Alberto wrote...
For a simple lookup like that, you can also use the IndexOf() method. In
fact, in the code you posted, that's probably preferable. The code you
posted has a bug, in which your code will think it has always found the
desired color, because Color is a value type, and so if the Color value
you're looking for isn't found, the default Color value will be returned,
not "null".
Also because Color is a value type, obviously you don't need the actual
instance you're looking for. You just need to know the value exists in
your list. The IndexOf() method will return an index, -1 if the value
isn't found.
And yet another alternative would be the Exists() method. Like the Find()
method, it takes a Predicate<T>, but rather than returning the element
that was found, it simply returns a boolean indicating whether the value
is in the list or not. Again, this seems much more appropriate given your
usage.
I'd like to suggest that you abandon the List<T> class altogether. Even
though the list will never have more than 257 (*) elements, as it gets
larger the searches done at each pixel will get more and more expensive,
hurting performance significantly.
(*) (256, if you switch the "add, then check count" around so that it's
"check count, then add" instead...and if you do that, you avoid that last
reallocation of the List<T> buffer when you exceed the maximum allowed
number of colors :) )
Normally I argue against worrying about optimizations early in
development, but in this case there's such a high chance of the linear
search being prohibitive, I think you will probably want to use a faster
data structure. You can use HashSet<Color> instead of List<Color>. You
can use the Contains() method to check whether the set actually contains
the color you're looking for.
I'd also recommend getting rid of your special-case for the first pixel
("if (i == 0 & j == 0)"). Not only should you be using the "&&" operator
instead of the "&" operator, it's superfluous anyway and just detracts
from the readability of the code. Just call HashSet.Contains() for the
first pixel just like you do for every other one; it'll be practically as
fast and will keep the code more general.
Finally, I'll point out that you can also get rid of the "nc" counter.
Even List<T> has a Count property, and the HashSet<T> class does too.
Just check the Count property instead of keeping the counter.
In the end, this is what the code would look like if I had my way (well,
structurally anyway):
HashSet<Color> colors = new HashSet<Color>();
int iMax = Math.Min(bmp.Width, 32), jMax = Math.Min(bmp.Height, 32);
for (int i = 0; i < iMax; i++)
{
for (int j = 0; j < jMax; j++)
{
Color color = bmp.GetPixel(i, j);
if (!colors.Contains(color))
{
if (colors.Count == 256)
{
// This is a utility method...make the caller display
// the message box if it fails
return false;
}
colors.Add(color);
}
}
}
Pete
Or you could just use the right method. IndexOf is what most people want
when Find isn't working, for this particular case Contains would solve the
problem better. But using a better container, like a Set, would be even
better.
thank you again.
I must be too tired not able to think clearly.
I am somewhat better after some extra sleep although still not quite enough
quality sleep
"Ben Voigt [C++ MVP]" <bvo...@newsgroup.nospam> wrote in message
news:%23MxEHmS...@TK2MSFTNGP02.phx.gbl...
> But using a better container, like a Set, would be even better.
What is this "Set" of which you speak? The only thing I found in MSDN was a
C++ STL thing.
e.g. http://www.codeplex.com/PowerCollections
I thought .NET had added a set class.... apparently it's still in beta:
http://msdn.microsoft.com/en-us/library/bb397727(VS.100).aspx
No, the docs are still in beta. HashSet<T> has been in there since 3.5.
I actually posted a detailed reply to the OP when his question first
appeared, including describing HashSet<T> as a possible solution (with
code example). But, given recent problems with my posts being blocked by
Microsoft's servers, it's possible no one else saw the post:
Pete