Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
public static void streamReport(this Report report, Stream stream)
{
    using (var streamWriter = new StreamWriter(stream))
    {
       //some logic that calls streamWrite.Write()
       streamWriter.Flush();
    }
    //Should "return stream;" here ?         
}

I am writing an extension method to an object in which I would like to transform to a CSV which is generated and returned to a client (web).

My questions are:

  1. Should I return stream (change void to Stream) ?
  2. Should I use the out keyword in before the Stream parameter ? Is this a common way to let the caller know the parameter will be changed ?
  3. Should I change the method to generate a new stream (and not accept one) and trust the user to Dispose it ?
  4. Should I pass StreamWriter instead of Stream ?
share|improve this question

2 Answers 2

up vote 7 down vote accepted

Your questions imply that you might not be quite aware of how streams or references work in C#.

  • You pass in a reference to a Stream
  • You create a StreamWriter which writes to that Stream
  • This will automatically make changes visible to anyone holding a reference to the same Stream.

Therefor there is no need to try and return the Stream in any way from the method to make the changes visible to the caller - you just need to write to it. This is at least how I interpret your questions.

However there is a catch in your implementation: disposing the StreamWriter will automatically close the underlying Stream which I find hugely annoying at times. Only since .NET 4.5 there is a constructor for StreamWriter which allows you to leave the Stream open. So right now your extension method will close the Stream which I guess is not the intention. Your only option to avoid that is to use .NET 4.5 or do not wrap the StreamWriter in a using statement.

share|improve this answer
    
Nice comment, I am using 4.5.1, will switch for this constructor. –  Ofiris Feb 18 at 8:26
  1. Do you need the Stream reference in outer scope? I doubt it. After all, you already pass this same reference as parameter, so you should already have it. Also pay attention to StreamWriter. When you dispose it in using block - you dispose the underlying stream as well. Accessing it later on will lead to exceptions.
  2. No, you should not, if all you do is writing to the given stream. out implies, that you will assign a new value to passed reference parameter (meaning you will call stream = new FileStream() at some point or something). Which is not the case apparently.
  3. It depends on use cases. I guess the best approach is to create multiple methods to cover them all. For example you can create another method, which would accept string (file path) instead of a Stream. And another, which would accept a StreamWriter.

Also, you should use verbs as method names. Write(...) or WriteCsv(...) for example. Not streamReport.

share|improve this answer
    
Thanks, I added a question.. due to the point you mention in 1 –  Ofiris Feb 18 at 8:19

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.