Windows.Forms controls dont seem to be able to use implicitconversions

Z

Zorgoban

Hi all!

Since it doesnt seem possible to derive from System.Drawing.Image
class, I buillt following construct:

public class ImageBinary
{
private Image _image = null;

public static implicit operator Image(ImageBinary imageBinary)
{
return imageBinary._image;
}

public static implicit operator ImageBinary(Image image)
{
return new ImageBinary() { _image = image };
}

public static implicit operator Byte[](ImageBinary imageBinary)
{
MemoryStream memStream = new MemoryStream();
imageBinary._image.Save(memStream, ImageFormat.Jpeg);
return memStream.GetBuffer();
}

public static implicit operator ImageBinary(Byte[] bytes)
{
return Image.FromStream(new MemoryStream(bytes));
}
}

The conversions work perfectly in my own code. But as soon as I use
this class in my business objects and bind them to a GridView, I get
conversion errors during runtime. What am I doing wrong?

Greetings and Thanks!
Zorgoban
 
B

Bryan

I have a few updates to your code first:

public class ImageBinary
{
private Image _image; // dont set to null explictly, the CLR ensure this

public static implicit operator Image(ImageBinary imageBinary)
{
return imageBinary._image;
}

public static implicit operator ImageBinary(Image image)
{
return new ImageBinary() { _image = image };
}

public static implicit operator Byte[](ImageBinary imageBinary)
{
// MemoryStream is IDisposable
using (MemoryStream memStream = new MemoryStream())
{
imageBinary._image.Save(memStream, ImageFormat.Jpeg);
return memStream.GetBuffer();
}
}

public static implicit operator ImageBinary(Byte[] bytes)
{
using (MemoryStream stream = new MemoryStream(bytes))
{
return Image.FromStream(stream);
}
}
}


I think if you would have wrapped the MemoryStream's in using statements
right away you would have seen the problem right away. If you look at the
MDSN for Image.FromStream()

http://msdn.microsoft.com/en-us/library/93z9ee4x.aspx

You will see that you need to keep the Stream around with the Image. So you
will need another field for the MemoryStream (in which case you don't dispose
of it right away). ImageBinary should also implement IDisposable.
 
Z

Zorgoban

Thanks for your answer! Unfortunately your code does exactly the same
as mine and is not targeting the actual problem.
 
P

Peter Duniho

Thanks for your answer! Unfortunately your code does exactly the same
as mine and is not targeting the actual problem.

I don't think Bryan meant to fix the actual bug with his code example.
He's just cleaning up the code you posted a bit. You need to read his
actual post to see how to fix the bug; he describes the problem and the
solution, but does not show that in the code he posted.

Pete
 
Z

Zorgoban

he describes the problem and the solution, but does not show that in the code he posted.

Unfortunately he describes a problem that doesnt exist. I allready
mentioned, that my code works fine when I access it inside my own code
in the frontend application. Just external assemblies, like
System.Forms or NHibernate dont seem to be able to see the
implementation of the operators. Also implementation of IDisposable
doesnt fix this. To clarify my situation even more, another code
example.

The following code works perfectly in my app:

// our byte array. just fill it from the source of your choice
byte[] bytes;
// now we test the ImageBinary(byte[]) operator
ImageBinary imageBinary = bytes;
// now we test the Image(ImageBinary) operator
Image image = imageBinary;

Now lets assume that I have a business object with a property of type
ImageBinary. As soon as I bind this objects property to a grid views
column, I get the message that ImageBinary cant be converted to byte
[], wich is definitely wrong, since I implemented the implicit
operator.

Greetings!
Zorgoban

PS: Here the exception to prove the problem is not inside the code of
the operator since it is never called.

System.InvalidCastException wurde nicht von Benutzercode behandelt.
Message="Das Objekt des Typs \"Questionnaire.Core.Base.ImageBinary\"
kann nicht in Typ \"System.Byte[]\" umgewandelt werden."
Source="NHibernate"
StackTrace:
bei NHibernate.Type.BinaryType.ToInternalFormat(Object bytes)
bei NHibernate.Type.AbstractBynaryType.DeepCopyNotNull(Object
value)
bei NHibernate.Type.MutableType.DeepCopy(Object value,
EntityMode entityMode, ISessionFactoryImplementor factory)
bei NHibernate.Type.TypeFactory.DeepCopy(Object[] values, IType
[] types, Boolean[] copy, Object[] target, ISessionImplementor
session)
bei
NHibernate.Event.Default.AbstractSaveEventListener.PerformSaveOrReplicate
(Object entity, EntityKey key, IEntityPersister persister, Boolean
useIdentityColumn, Object anything, IEventSource source, Boolean
requiresImmediateIdAccess)
bei
NHibernate.Event.Default.AbstractSaveEventListener.PerformSave(Object
entity, Object id, IEntityPersister persister, Boolean
useIdentityColumn, Object anything, IEventSource source, Boolean
requiresImmediateIdAccess)
bei
NHibernate.Event.Default.AbstractSaveEventListener.SaveWithGeneratedId
(Object entity, String entityName, Object anything, IEventSource
source, Boolean requiresImmediateIdAccess)
bei
NHibernate.Event.Default.DefaultSaveOrUpdateEventListener.SaveWithGeneratedOrRequestedId
(SaveOrUpdateEvent event)
bei
NHibernate.Event.Default.DefaultSaveEventListener.SaveWithGeneratedOrRequestedId
(SaveOrUpdateEvent event)
bei
NHibernate.Event.Default.DefaultSaveOrUpdateEventListener.EntityIsTransient
(SaveOrUpdateEvent event)
bei
NHibernate.Event.Default.DefaultSaveEventListener.PerformSaveOrUpdate
(SaveOrUpdateEvent event)
bei
NHibernate.Event.Default.DefaultSaveOrUpdateEventListener.OnSaveOrUpdate
(SaveOrUpdateEvent event)
bei NHibernate.Impl.SessionImpl.FireSave(SaveOrUpdateEvent
event)
bei NHibernate.Impl.SessionImpl.Save(Object obj)
bei REF.Database.NHibernate.DataObject`2.Save() in D:\Projekt
\Werner Amann\Questionnaire\REF.Database.NHibernate
\DataObject.cs:Zeile 134.
bei REF.Database.NHibernate.DataObject`2.EndEdit() in D:\Projekt
\Werner Amann\Questionnaire\REF.Database.NHibernate
\DataObject.cs:Zeile 217.
bei Questionnaire.EditDialogs.EditProductDlg.OnFormClosing
(FormClosingEventArgs e) in D:\Projekt\Werner Amann\Questionnaire
\Questionnaire\EditDialogs\EditProductDlg.cs:Zeile 27.
bei System.Windows.Forms.Form.CheckCloseDialog(Boolean
closingOnly)
InnerException:
 
P

Peter Duniho

Unfortunately he describes a problem that doesnt exist.

He made a guess and described a problem that could in fact exist with the
code you posted. Given the lack of a concise-but-complete code example
that reliably demonstrates the problem, and the lack of a specific problem
statement describing exactly what isn't working, I think he did a pretty
good job.

Maybe he didn't answer the question you wanted, but that has not been at
all clear from your previous posts.
I allready
mentioned, that my code works fine when I access it inside my own code
in the frontend application.

Yes, you did mention that whatever problem you had, it worked fine in one
setting but not another. However, that information might as well not have
been there because you never described the problem, nor provided examples
of each setting relevant to your problem.
Just external assemblies, like
System.Forms or NHibernate dont seem to be able to see the
implementation of the operators.

Your original post never even used the word "operators". Even now, you
are posting contradictory information. Your original post claimed an
error at run-time, but operators are a compile-time construct.
Also implementation of IDisposable
doesnt fix this. To clarify my situation even more, another code
example. [...]

Until you post a concise-but-complete code example that reliably
demonstrates the problem, along with a clear, specific description of the
problem (including _exactly_ the text of any error messages, exceptions,
etc. along with a specific statement as to when and where those messages,
exceptions, etc. occur), it's unlikely that you will get an answer
specifically useful to you.
The following code works perfectly in my app:

// our byte array. just fill it from the source of your choice
byte[] bytes;
// now we test the ImageBinary(byte[]) operator
ImageBinary imageBinary = bytes;
// now we test the Image(ImageBinary) operator
Image image = imageBinary;

Now lets assume that I have a business object with a property of type
ImageBinary. As soon as I bind this objects property to a grid views
column, I get the message that ImageBinary cant be converted to byte
[], wich is definitely wrong, since I implemented the implicit
operator.

If you are expecting your operator overloads to take effect in other code
that isn't compiled with your own code, then that other code must be using
reflection, and must have some specific rules you are required to follow
in order for the overloading to be useful. Those rules would be specific
to that other code though, and so you would have to consult a forum
specific to the other code.
Greetings!
Zorgoban

PS: Here the exception to prove the problem is not inside the code of
the operator since it is never called.

System.InvalidCastException wurde nicht von Benutzercode behandelt.
Message="Das Objekt des Typs \"Questionnaire.Core.Base.ImageBinary\"
kann nicht in Typ \"System.Byte[]\" umgewandelt werden."

I don't speak or read German.

Pete
 
Z

Zorgoban

Very nice. I prefer to stay on topic. Thanks for mentioning, that
those operators are compile time constructs. That explains a lot.

Greetings!
Zorgoban
 
B

Bryan

My point was that the Stream you pass to the Image(Stream) constructor needs
to stay open. If it worked in one case and not the other, maybe the Garbage
Collector didn't get rid of it in one case so it looks like it worked fine.
I'm sure it would only be a matter of time before it failed in that situation
as well. You could test that by doing a GC.Collect() from a different method.

Did you try to rewrite things so you keep that Stream around for the
lifetime of the Image?
 
Z

Zorgoban

Did you try to rewrite things so you keep that Stream around for the
lifetime of the Image?

No, since the operators were never called there was no need to do so.
I also couldnt find any information, that approved your statement.
If the image keeps a reference to the stream, it shouldnt be dropped
by the garbage collector.
Also keep in mind, that this code worked when I used it in my
application that was compiled with it.

Greetings!
Remo
 
N

not_a_commie

        public static implicit operator Byte[](ImageBinary imageBinary)
        {
                // MemoryStream is IDisposable
                using (MemoryStream memStream = new MemoryStream())
                {
                        imageBinary._image.Save(memStream, ImageFormat.Jpeg);
                        return memStream.GetBuffer();
                }
        }

The above method is clearly incorrect for two reasons. The buffer is
unallocated before you leave the function and will be unusable.
           public static implicit operator Byte[](ImageBinary imageBinary)
           {
                   MemoryStream memStream = new MemoryStream();
                   imageBinary._image.Save(memStream, ImageFormat.Jpeg);
                   return memStream.GetBuffer();
           }

This method is also incorrect. You are relying on the reference into
the MemoryStream class to keep it from being garbage collected --
fishy and poor coding on any account. But that's not the main problem.
The real problem is that the buffer pointed to by GetBuffer may or may
not be the exact size of the data in it. It's allocated in powers of
two, typically. You need to return an integer length as well as the
buffer. I think you should go with the above "using" method and return
memStream.ToArray() instead, as it makes a copy of the buffer into a
byte[] that is the right length -- it solves both the unallocation
problem and the length of buffer problem at the cost of more memory
and more time.
 
Z

Zorgoban


Yes and following is stated there:

- "You must keep the stream open for the lifetime of the Image."

I understand you must not call Close on the stream, since the Image
handles it.

- "The stream is reset to zero if this method is called successively
with the same stream."

For me this is a clear hint, that the Image keeps a reference to the
Stream and messes with it, but also disposes of it.

Greetings!
Zorgoban
 
Z

Zorgoban

        public static implicit operator Byte[](ImageBinary imageBinary)
        {
                // MemoryStream is IDisposable
                using (MemoryStream memStream = new MemoryStream())
                {
                        imageBinary._image.Save(memStream, ImageFormat.Jpeg);
                        return memStream.GetBuffer();
                }
        }

The above method is clearly incorrect for two reasons. The buffer is
unallocated before you leave the function and will be unusable.
You are relying on the reference into
the MemoryStream class to keep it from being garbage collected --
fishy and poor coding on any account. But that's not the main problem.

I was not really thinking about the buffer that way. During debugging
it worked and I was expecting the reference to MemoryStream to be
cleaned up on exit. Maybe the buffer itself stays alive, due to the
additional reference in my code.
The real problem is that the buffer pointed to by GetBuffer may or may
not be the exact size of the data in it. It's allocated in powers of
two, typically. You need to return an integer length as well as the
buffer. I think you should go with the above "using" method and return
memStream.ToArray() instead, as it makes a copy of the buffer into a
byte[] that is the right length -- it solves both the unallocation
problem and the length of buffer problem at the cost of more memory
and more time.

Yeah, youre right. This would definitely be the cleaner solution. Even
if I dont think that the using statement is really necessary.
 
P

Peter Duniho

        public static implicit operator Byte[](ImageBinary imageBinary)
        {
                // MemoryStream is IDisposable
                using (MemoryStream memStream = new MemoryStream())
                {
                        imageBinary._image.Save(memStream,
ImageFormat.Jpeg);
                        return memStream.GetBuffer();
                }
        }

The above method is clearly incorrect for two reasons. The buffer is
unallocated before you leave the function and will be unusable.

Not correct. The buffer is a managed object, and so while
MemoryStream.Dispose() may release (emphasis on "may"...see parenthetical
below) its own reference to the underlying byte[] for the MemoryStream,
that wouldn't invalidate the byte[] object itself.
           public static implicit operator Byte[](ImageBinary imageBinary)
           {
                   MemoryStream memStream = new MemoryStream();
                   imageBinary._image.Save(memStream, ImageFormat.Jpeg);
                   return memStream.GetBuffer();
           }

This method is also incorrect. You are relying on the reference into
the MemoryStream class to keep it from being garbage collected --
fishy and poor coding on any account.

A) in the above code, there's no "reference into the MemoryStream class"
that would in fact prevent it from being garbage collected after the
underlying byte[] has been retrieved (even if that were an issue), and b)
as in the previous code example, the byte[] itself is safe to use, even
after the MemoryStream has been disposed.

(Actually, the current implementation of MemoryStream.Dispose(bool)
doesn't even release the reference to the byte[] -- so there's nothing
about disposing a MemoryStream instance that would have any effect on the
byte[] at all, never mind invalidate it -- but there's nothing in the docs
that promise it doesn't, so one would definitely want to get that
reference before disposing, just to be safe).
But that's not the main problem.
The real problem is that the buffer pointed to by GetBuffer may or may
not be the exact size of the data in it. It's allocated in powers of
two, typically. You need to return an integer length as well as the
buffer. I think you should go with the above "using" method and return
memStream.ToArray() instead, as it makes a copy of the buffer into a
byte[] that is the right length -- it solves both the unallocation
problem and the length of buffer problem at the cost of more memory
and more time.

I agree that ToArray() is better, for the reason you mention regarding the
array length. It's possible, depending on how the OP is using the byte[]
returned, that the extra unused bytes at the end of the byte[] would not
be a problem, other than perhaps excessive use of storage space (memory,
disk, transmitting on a network, etc.) But there's no good reason to not
address the issue, and in some cases fixing it could be a very big win.

There's no "unallocation problem" though. :)

Pete
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top