My Import
method takes some data
and writes it to a stream. If the CompressedStore
property is true, the contents of that stream should be compressed.
This code works, however I just don't like it. For one, I call serializer.Serialize()
twice. I feel this code can be made more concise. Any ideas?
public void Import(IProvisionSource source)
{
InitializeStore();
// Call source.Export and populate local data store
var data = source.Export();
var serializer = new XmlSerializer(data.GetType());
var file = CompressedStore ? "KPCData.gz" : "KPCData.xml";
var path = Path.Combine(DataDirectory, file);
using (var fileWriter = new FileStream(path, FileMode.Create))
{
if (CompressedStore)
{
using (var writer = new GZipStream(fileWriter, CompressionLevel.Optimal))
{
serializer.Serialize(writer, data);
}
}
else
{
serializer.Serialize(fileWriter, data);
}
}
}
1 Answer 1
By default, GZipStream
owns the underlying stream, so it disposes it when it's disposed itself. If you're okay with relying on that, then you could put the right Stream
into a variable and then have just one using
around it. Something like:
var writer = new FileStream(path, FileMode.Create);
if (CompressedStore)
writer = new GZipStream(writer, CompressionLevel.Optimal);
using (writer)
{
serializer.Serialize(writer, data);
}
This avoids the duplication and so it's shorter, but it's also less obviously correct (using
combined with new
is a good pattern, and it's not used here), so I think your original code is actually the better option. If there was more repetition than just one method call, it could make sense to extract that into a separate method, but that's not the case here.
-
\$\begingroup\$ Ah! Yea, that was the issue I couldn't get around; I could rewrite it with various conditionals, but every time something wasn't getting disposed. If disposing of
GZipStream
will also dispose of itsFileStream
, then I'm probably good. I'll keep this solution in mind! \$\endgroup\$Mike Christensen– Mike Christensen2014年01月29日 01:39:21 +00:00Commented Jan 29, 2014 at 1:39