15

I have a code block as follows and I'm using 3 nested using blocks.

I found that using try finally blocks I can avoid this but if there are more than two using statements, what is the best approach?

private FileStream fileStream = null; private Document document = null; private PdfWriter pdfWriter = null; using (fileStream = new FileStream("ABC.pdf", FileMode.Create)) { using (document = new Document(PageSize.A4, marginLeft, marginRight, marginTop, marginBottom)) { using (pdfWriter = PdfWriter.GetInstance(document, fileStream)) { document.AddAuthor(metaInformation["author"]); document.AddCreator(metaInformation["creator"]); document.AddKeywords("Report Generation using I Text"); document.AddSubject("Document subject"); document.AddTitle("The document title"); } } } 
5
  • 2
    I don't see a problem with this. Commented Apr 21, 2014 at 12:40
  • 1
    each using statement would translate into try-finally block. So It really depends how you are planning to replace your current structure with a try-finally. A single try/finally or multiple try/finally for each using block Commented Apr 21, 2014 at 12:41
  • 6
    Do you really need these to be instance variables? After the using statements they'll be disposed (and therefore probably useless) anyway - could you make them local variables instead, declared in the using statements? Commented Apr 21, 2014 at 12:41
  • 1
    Instantiating using new might fail but it would throw an exception. Instantiating using Class.GetInstance() static methods might fail but return null. The code ought to check - and recover - in either case. Commented Apr 21, 2014 at 12:59
  • Yap @ClickRick I will refactor it , Thank you Commented Apr 21, 2014 at 13:13

5 Answers 5

19

You can remove the indention and curly brackets this way:

using (var fileStream = new FileStream("ABC.pdf", FileMode.Create)) using (var document = new Document(PageSize.A4, marginLeft, marginRight, marginTop, marginBottom)) using (var pdfWriter = PdfWriter.GetInstance(document, fileStream)) { // code } 
Sign up to request clarification or add additional context in comments.

Comments

5

A little less verbose way to avoid the indenting:

 using (var fileStream = new FileStream("ABC.pdf", FileMode.Create)) using (var document = new Document(PageSize.A4, marginLeft, marginRight, marginTop, marginBottom)) using (var pdfWriter = PdfWriter.GetInstance(document, fileStream)) { document.AddAuthor(metaInformation["author"]); document.AddCreator(metaInformation["creator"]); document.AddKeywords("Report Generation using I Text"); document.AddSubject("Document subject - Describing the steps creating a PDF document"); document.AddTitle("The document title - PDF creation using iTextSharp"); } 

As Jon Skeet pointed out, there is no need for these variables to be instance variables, as they are disposed after the using blocks anyway.

You can use local variables as shown in the code above instead.

Comments

2

Maybe something conventional; best approach for choosing between two in my opinion would be;

  • Using : If you are going to use an instance within a context and need to Dispose it after you are done with it
  • try/finally : If you are expecting any issue and have something to do with the exception, catching it before you Dispose the instance you are using.

And as other comments / answers state; you don't need instance level variables;

using (FileStream fileStream = new FileStream("ABC.pdf", FileMode.Create)) using (Document document = new Document(PageSize.A4, marginLeft, marginRight, marginTop, marginBottom)) using (PdfWriter pdfWriter = PdfWriter.GetInstance(document, fileStream)) { // # Implementation here seems like a good approach } 

Comments

2

now on C# 8 you have using declarations

 using var fileStream = new FileStream("ABC.pdf", FileMode.Create); using var document = new Document(PageSize.A4, marginLeft, marginRight, marginTop, marginBottom); using var pdfWriter = PdfWriter.GetInstance(this.document, this.fileStream); 

Comments

0

In a single method where you don't need to process or change data; the option suggested by Jan Sommer would be my choice. However, in some circumstances the DisposableList is useful. Particularly, if you have many disposable fields that all need to be disposed of (in which case you cannot use using).

Requires you to remember to add the item to the list. (Although you could also say you have to remember to use using.) Aborts the disposal process if one of the disposes methods throws, leaving the remaining items un-disposed.

public class DisposableList : List<IDisposable>, IDisposable { public void Dispose() { if (this.Count > 0) { List<Exception> exceptions = new List<Exception>(); foreach (var disposable in this) { try { disposable.Dispose(); } catch (Exception e) { exceptions.Add(e); } } base.Clear(); if (exceptions.Count > 0) throw new AggregateException(exceptions); } } public T Add<T>(Func<T> factory) where T : IDisposable { var item = factory(); base.Add(item); return item; } } 

Now catch any exceptions from the Dispose calls and will throw a new AggregateException after going through all the items. I've added a helper Add method that allows a simpler usage:

using (var disposables = new DisposableList()) { var file = disposables.Add(() => File.Create("test")); // ... var memory = disposables.Add(() => new MemoryStream()); // ... var cts = disposables.Add(() => new CancellationTokenSource()); // ... } 

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.