Whats wrong with this code? creating a deleting variable in different scopes

Question

I've recently seen some code similar to that outlined below.

public void someMethod() {
  Lecture lect = createLecture();

  ...

  lect.getLectureSeries().delete();
}


public Lecture createLecture() {
  LectureSeries series = new Series();
  Lecture lect = new Lecture(series);

  ...

  return lect;
}

The point being that some object (in this case the LectureSeries) which needs to be deleted at the end of the call to someMethod() is actually created in the call to another method. I'm trying to explain why it should be created in the same scope it will eventually be deleted. ie

public void someMethod() {
  LectureSeries series = new Series();
  Lecture lect = createLecture(series);

  ...

  series.delete();
}


public Lecture createLecture(LectureSeries series) {
  Lecture lect = new Lecture(series);

  ...

  return lect;
}

The original code has caused some complication tidying things up when things fail so hopefully the benefits will be evident, but has anyone got any ideas on how I can explain the more general principle behind this refactoring? or anyone want to explain to me why I'm wrong?

== Edit ==

The case in question was a test method so cleaning up anything which had been created during the execution of the test was important. I think though that the unwanted side effect of a LectureSeries being created as a result of the call to createLecture() is still something to try and avoid in most cases.

Answer 1

You can use the argument of "Inversion Of Control". The Lecture object needs a LectureSeries object and it is better this object to be injected into it rather than creating it by itself.

Injecting dependencies into objects is always a good practice. Specific to your case you can also say that the creation and deleting statements should be as close as possible. This certainly increases readability. In the first case I see an item being deleted, but I don't see where the item is created. I need to guess that the item is created in the creation method or search the whole code to find where this happens. Error handling (in case the LectureSeries creation fails) is also easier.

Answer 2

There is nothing inherently wrong with creating object in a method that lives beyond the scope of the method.

In garbage collected languages, the tiding up takes care of itself.

In languages where the programmer must manage the cleanup themselves, the programmer must understand the ownership of the object they receive from the method and their duties regarding releasing it.