Is this code right?

K

Koliber (js)

I feel i still do not understand maybe a bit a dispose pattern
So I have a question - is this code right? Is fs.Close() there where
it is right?
If I do understand it in place 'BBB' there can be a situation where
such member
obiect as fs can be finalised - so it may be wrong. Am I right or not?
But if i would close fs in place 'AAA' then I am not sure if a user of
this class
called a dispose() on it ? Am I wrong or right?

Thanks for Answer
JS



class JpgFileDecoder : IDisposable
{
private FileStream fs = null;
private ProgressEventHandler progressEventHandler = null;
private JpegDecoder jpegDecoder = null;

private AtalaImage atalaImage = null;
private ImageInfo imageInfo = null;

private bool isDisposed = false;



public JpgFileDecoder(string filePath,
ProgressEventHandler progressEventHandler)
{
this.progressEventHandler = progressEventHandler;
fs = new FileStream(filePath, FileMode.Open,
FileAccess.Read);
fs.Seek(0, SeekOrigin.Begin);
}

public AtalaImage AtalaImage
{
get
{
if (atalaImage == null)
{
atalaImage = jpegDecoder.Read(fs,
progressEventHandler);
}
return atalaImage;
}
set { }
}

public ImageInfo ImageInfo
{
get
{
if (imageInfo == null)
{
imageInfo = jpegDecoder.GetImageInfo(fs);
}
return imageInfo;
}
set { }
}


~JpgFileDecoder()
{
Dispose(false);
}

protected void Dispose(bool disposing)
{
if (disposing)
{
// AAA
// Code to dispose the managed resources of the class

}
//BBB
// Code to dispose the un-managed resources of the
class

fs.Close();
fs = null;

//

isDisposed = true;
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
}
 
C

Christof Nordiek

Koliber (js) said:
I feel i still do not understand maybe a bit a dispose pattern
So I have a question - is this code right? Is fs.Close() there where
it is right?
If I do understand it in place 'BBB' there can be a situation where
such member
obiect as fs can be finalised - so it may be wrong. Am I right or not?
But if i would close fs in place 'AAA' then I am not sure if a user of
this class
called a dispose() on it ? Am I wrong or right?

Thanks for Answer
JS



class JpgFileDecoder : IDisposable
{
private FileStream fs = null;
private bool isDisposed = false;

public JpgFileDecoder(string filePath,
ProgressEventHandler progressEventHandler)
{
this.progressEventHandler = progressEventHandler;
fs = new FileStream(filePath, FileMode.Open,
FileAccess.Read);
fs.Seek(0, SeekOrigin.Begin);

You shouldn't open your filestream an the get-accessor. If this property is
assigned twice, the former filestream remains open. Also calling programmers
wouldn't expect the object to enter open state on property-assignment.
Better is an Open method.
}


~JpgFileDecoder()
{
Dispose(false);
}

Actually it's not needed here, to have a finalizer, because you don't have
any unmanaged resources. It will raise performance issues (though maybe
small.)
This pattern can be handy, if a deriving class will reference unmanaged
resources directly, but maybe it is better to make your class sealed.
protected void Dispose(bool disposing)
{
if (disposing)
{
// AAA
// Code to dispose the managed resources of the class

}
//BBB
// Code to dispose the un-managed resources of the
class

fs.Close();
fs = null;

fs is a managed resource (=~ managed class that implements IDisposable). So
this should appear under // AAA.

So far
Christof Nordiek
 
K

Koliber (js)

You shouldn't open your filestream an the get-accessor. If this property is
assigned twice, the former filestream remains open. Also calling programmers
wouldn't expect the object to enter open state on property-assignment.
Better is an Open method.



Actually it's not needed here, to have a finalizer, because you don't have
any unmanaged resources. It will raise performance issues (though maybe
small.)
This pattern can be handy, if a deriving class will reference unmanaged
resources directly, but maybe it is better to make your class sealed.






fs is a managed resource (=~ managed class that implements IDisposable). So
this should appear under // AAA.

So far
Christof Nordiek
I do not understand some things.

1. If i put fs closing under //AAA
then it will only be called when user calls Dispose() on my
class
so

What if class-user will forget about calling dispose?
a) fs.close() will never be called and fs will remain opdened forever?

Shouldn't I write code in this way that will make sure that
even if user will forgot about calling dispose finaliser
will do it - if so I think I should put filestream-closing code
under // BBB - so it can be called by finalisation time
so it will make sure that filestream will be closed

OR shouldn't I do that And must I belive that class-caler
will call dispose?

I am not sure how it should be done and it is real hard to
understand

thanx for answer
JS
 
C

Christof Nordiek

Koliber (js) said:
I do not understand some things.

1. If i put fs closing under //AAA
then it will only be called when user calls Dispose() on my
class
so

What if class-user will forget about calling dispose?
a) fs.close() will never be called and fs will remain opdened forever?

Then, when the instance is eligible for collection also fs is eligible for
collection, and the finalizer will be called on fs (maybe even prior to the
finalizer of the instance).
That's the idea of the Dispose(bool) pattern and that is, why a class that
doesn't directly reference an unmanaged resource may not have a finalizer,
even if implements IDisposable (because it references an unmanaged resource
directly.)

Christof
 

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