Repeatedly open and close a PDF document to perform various operations [closed]

Question

I am trying to implement the Execute Around pattern described in Kent Beck's Smalltalk Best Practice Patterns. An example in Java could be found here.

Basically, I am repeatedly opening and closing a pdf document while performing various operations, something like,

public void Parse()
{
    // Open the document
    PdfLoadedDocument loadedDocument = new PdfLoadedDocument("plan.pdf");

    List<string> annotations = Build(loadedDocument);
    // work with annotations. 

    // Close the document
    loadedDocument.Save();
    loadedDocument.Close();
}

I would like to move the opening and closing the document in a centralized place, as I have tens of similar methods. All these methods open the document, perform an action, and close the document, and it's easy to forget to close the document.

Here is what I tried:

public void BuildAnnotations()
{
    List<string> annotations = null;

    ExecuteAction("plan.pdf", (PdfLoadedDocument loadedDocument) =>
    {
        annotations = Build(loadedDocument);
        // work with annotations
    });
}

private void ExecuteAction(string path, Action<PdfLoadedDocument> perform)
{
    PdfLoadedDocument loadedDocument = new PdfLoadedDocument(path);

    try
    {
        perform(loadedDocument);
    }
    catch(Exception e)
    {
        Console.WriteLine($"An error occured. {e}");
    }

    loadedDocument.Save();
    loadedDocument.Close();
}

My question is, is passing a lambda to an Action delegate a good idea? I am not that familiar with delegates, Actions, and lambdas (other than using them in linq queries). Are there any other better alternatives?

Answer 1

IMO there is a problem with the try...catch statement. If an exception is thrown you'll have to deal with communicating the error to the client. You can't rethrow it because then the Save() and Close() aren't called, so you'll have to have a kind of logging or event to notify through and that could make it all unnecessarily complicated. Besides that: if an exception is thrown, you may not want to save the document in an unknown state, but you should close it anyway.

You can handle that in the following way:

  try
  {
    perform(loadedDocument);
    loadedDocument.Save();
  }
  finally
  {
    loadedDocument.Close();
  }

Here the document is only saved if the process went well, but closed in any case. And the caller can handle the exception as needed.

---

Alternatively you can make a wrapper like:

  class PdfDocumentExecuter : IDisposable
  {
    private PdfLoadedDocument document;

    public PdfDocumentExecuter(string fileName)
    {
      document = new PdfLoadedDocument(fileName);
    }

    public void Execute(Action<PdfLoadedDocument> action)
    {
      action(document);
      document.Save();
    }

    public T Execute<T>(Func<PdfLoadedDocument, T> function)
    {
      T result = function(document);
      document.Save();
      return result;
    }

    public void Dispose()
    {
      if (document != null)
      {
        document.Close();
        document = null;
      }
    }
  }

called as:

  using (PdfDocumentExecuter executer = new PdfDocumentExecuter("path"))
  {
    annotations = executer.Execute(Build);
  }