help on generic space
GS
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
===================
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;
Alberto Poblacion
news:uhIGmBK8...@TK2MSFTNGP04.phx.gbl...
>
> 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
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);
Peter Duniho
<gsmsnews.mic...@msnews.nomail.com> wrote:
>
> 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
Ben Voigt [C++ MVP]
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.
GS
compare to less than 1. worked like a charm..
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...
Jeff Johnson
"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.
Ben Voigt [C++ MVP]
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
Peter Duniho
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