Returning a new object or initializing member in constructor?

Question

We have a data provider class that returns repositories for each aggregate in our database.

Let's consider following scenario:

public class DataProvider {
    public IBookRepository Books { get { retun new BookRepository(); } }
    public IAuthorRepository Authors { get { retun new AuthorRepository(); } }
}

As you can see, we return a new Instance of the given object every time we call the Member: DataProvider.Books.DoBANANAS();

vs.

public class DataProvider {
    public IBookRepository Books { get; }
    public IAuthorRepository Authors { get; }

    public DataProvider()
    {
        Books = new BookRepository();
        Authors = new AuthorRepository();
    }
}

Will the call to `DataProvider.Books.BANANAS(); be less CPU / Memory heavy now?

As I just got around implementhing both version and they suprisingly worked!

But my experience tells me that Version 1 Sucks. Yet I've got more than enough time to fully optimize and implement the final product. (That's one of the benefits of working in a research group)

Answer 1

Your first example creates a new instance of the XXXRepository object each time, where the second is returning a handle to an already existing one, so the second should perform better and use less CPU as new memory does not need to be allocated.

The bigger problem is that unless the repository classes are entirely static or something, the 2 scenarios produce completely different results. If the repository classes are some sort of interface to a database, then option 1 does not need to be thread-safe where option 2 does.

Answer 2

The implementations shown are different in how they construct and manage objects. In the first example, a new repository is created every time the property is called on the DataProvider. If the repository holds any state (eg. a cache of fetched objects), you will get very different behaviour than with the second example, because this state will be reset to its default each time.

From the fact that you said both version work, I would hazard a guess that the repository objects do not hold state and are just proxies to the database calls. In this case, it probably will not have much difference.

However, the performance and the memory profile of the two approaches are very different. Whilst premature optimisation is a bad thing and often a waste of time, I would not class this as such because making a new repository on each property call could clearly be a performance problem later, or worse still introduce hard to track down bugs if you introduce state into the repositories. Creating less objects will ultimately put less of a strain on the garbage collector, but unless you are creating millions of objects, it will be a negligible difference.

In summary, the second example is the better one going forward, and I see no real problem considering that now.