4

I've got a class member function that generates a large list and puts it into a private member variable so it can be accessed through a getter. The generation of this list is a rather intensive process (in my testing it's just under a second, but with larger datasets I expect it to take much longer), involving lots of recursion and going back and forth over sublists. This function is public so that the list can be re-generated at any time, whether or not 'natural' initialization has occurred yet.

Given all this, is it better to set things up so that the list is generated when the object is constructed, or to make it generate the list when the getter function is called if the list has not been generated yet?

Also, I should add that we assume that the list is up-to-date and doesn't need to be changed until the user specifically tells us to refresh it.

  • 1
    What does the re-generation or refreshing actually do? – whatsisname Nov 9 '17 at 20:15
  • @whatsisname it's exactly the same as regular generation. The function that does generation is public so it can be called at any time from outside. – SenorMeltyface Nov 9 '17 at 23:39
  • "until the user specifically tells us to refresh it" - that sounds like placing the burden of refresh-decisions for an internal entity onto external users. I've seen similar (anti-) patterns fail in nasty ways. But maybe I'm interpreting your architecture incorrectly. – Ralf Kleberhoff Nov 10 '17 at 9:39
7

It depends.

If the list should be constructed before, during or after construction of the other parts of the object depends on the context, how this influences testability and what a user of that class would expect about the behaviour of the class.

Generating the list when the getter function is called (if it has not been generated then) makes most sense if you have situations where objects of that class might be used without ever calling that getter function. Even a unit test case can justify to implement this kind of lazy loading, if generating the list always slows the test down too much. It comes for the price of making things more complex, and making the timing behaviour of the getter (and maybe its exception-throwing behaviour) less predictable.

Generating the list beforehand (and injecting it to the constructor) might be the best if one can see this as a reponsibility on its own, which might be tested or reused separately, and which does not require access to private fields of the class in stake. However, this is probably not a good idea if the list must be updated from the object automatically at a later point in time.

For the remaining cases, generating the list in the constructor (or at construction time, but utilizing an injected factory or builder object) is probably the way to go. If the list content must be updated at later point in time, the logic for this or parts of it can be most easily reused from the constructor and from the function which updates the content as well.

  • Thanks for this answer. I've decided that generating in the constructor (or rather calling the generation function from the constructor) seems like the way to go, as this class' whole purpose is basically to generate and smartly access this list. – SenorMeltyface Nov 9 '17 at 23:55
3

Neither!

Either way you'd be violating two important principles.

How do you achieve that? By making generating the large list something elses job. Once generated you pass a reference to the list into the constructor of object that would use it. Now nothing exists that can't be used and object construction is about as expensive as pointer addition.

The fancy word for this is dependency injection but us old schoolers remember by the name reference passing.

  • 5
    "Objects should be in a valid state, ready to use, once constructed" doesn't mean the class can't leverage lazy loading of data members. It's entirely possible that a consumer of an object may want to call other methods or properties on the object without ever needing the huge list. – 17 of 26 Nov 9 '17 at 20:36
  • 4
    "Constructors should not do real work" is generally a good idea, but not absolute and there are situations where it makes sense to do 'real work' in them. – whatsisname Nov 9 '17 at 20:59
  • 7
    Yeah, so you basically add another layer of abstraction without solving the actual problem in place. Now the same question arises for your newly created class. The issue hasn't disappeared. – GuardianX Nov 9 '17 at 23:50
  • By "pass[ing] a reference to the list into the constructor" you make that list part of the public API of the class, I don't think that's a good idea as a general solution. And I like e.g. the Java collections framework that happily violates the "Constructors should not do real work" rule in constructors like HashSet(Collection c), doing a lot of real work in setting up and populating the hash table. – Ralf Kleberhoff Nov 10 '17 at 9:32
  • 1
    There is nothing wrong with constructors doing real work. That "principle" is flat out wrong. The only problem with it is that people may not expect it to do real work. But this can be documented. – Frank Hileman Nov 10 '17 at 16:35
0

If you are using .net, it is generally considered bad practice to create a long running (or indeterminately long running) property get method:

  • API users expect property get methods to return in a short constant amount of time, and can be safely accessed in loops.
  • Debugger problems: when examining an instance in a locals, watch, or quick watch window, the long running code will be executed.

For the same reasons, property get methods generally should not throw exceptions. See also .net Framework Design Guidelines.

0

You don't specify the language, but here's a (slightly complex) answer, Java oriented. And I'm slightly stale on my Java so this may be slightly wrong.

  1. The constructor does not create the List. But it constructs something like a FutureTask that can calculate the list.

  2. Somebody TBD, probably the constructor is simplest, starts running that task on a separate thread. This is the slightly complex part. If the list needs to be recalculated, use a similar approach.

  3. The getList() method returns theFuture.get()

  • While this may be a good choice for an implementation when you're already working in a concurrent environment, you wouldn't want to make a general solution out of it. – Kayaman Nov 15 '17 at 5:32
  • @Kayaman Agreed. However, OP has a list that will take several seconds to create. He better already be thinking hard about concurrency. The approved answer of taking several seconds in a constructor is not necessarily wrong, but disconcerting. – user949300 Nov 19 '17 at 22:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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