47

Albeit a general question my scope is rather C# as I am aware that languages like C++ have different semantics regarding constructor execution, memory management, undefined behaviour, etc.

Somebody asked me an interesting question which was for me not easily answered.

Why (or is it at all?) regarded as bad design to let a constructor of a class start a never ending loop (i.e. game loop)?

There are some concepts that are broken by this:

  • like the principle of least astonishment, the user does not expect the constructor to behave like this.
  • Unit tests are harder as you cannot create this class or inject it as it never exits the loop.
  • The end of the loop (game end) is then conceptually the time where the constructor finishes, which is also odd.
  • Technically such a class has no public members except the constructor, which makes it harder to understand (especially for languages where no implementation is available)

And then there are technical issues:

  • The constructor actually never finishes, so what happens with GC here? Is this object already in Gen 0?
  • Deriving from such a class is impossible or at least very complicated due to the fact that the base constructor never returns

Is there something more obviously bad or devious with such an approach?

  • 61
    Why is it good? If you simply move your main loop to a method (very simple refactoring) then the user can write unsurprising code like this: var g = new Game {...}; g.MainLoop(); – Brandin Sep 8 '16 at 9:18
  • 61
    Your question already states 6 reasons not to use it, and I'd like to see a single one in its favor. You could also ask why it's a bad idea to put a while(true) loop in a property setter: new Game().RunNow = true? – Groo Sep 8 '16 at 13:20
  • 38
    Extreme analogy: why we say that what Hitler did was wrong? Except for racial discrimination, starting WWII, killing millions of people, violence [etc etc for 50+ other reasons] he did nothing wrong. Sure if you remove an arbitrary list of reason as to why something is wrong you may as well conclude that that thing is good, for literally anything. – Bakuriu Sep 8 '16 at 13:53
  • 4
    With respect to garbage collection (GC) (your technical issue), there would be nothing special about it. The instance exists before constructor code is actually run. In this case the instance is still reachable, so it cannot be claimed by GC. If GC sets in, this instance will survive, and at each occurrence of GC setting in, the object will be promoted to the next generation according to the usual rule. Whether or not GC takes place, depends on whether (enough) new objects are being created or not. – Jeppe Stig Nielsen Sep 8 '16 at 14:00
  • 8
    I would go a step further, and say the while(true) is bad, regardless of where it is used. It implies that there is no clear and clean way to stop the loop. In your example, shouldn't the loop stop when the game is exited, or loses focus? – Jon Anderson Sep 8 '16 at 16:00
249

What is the purpose of a constructor? It returns a newly constructed object. What does an infinite loop do? It never returns. How can the constructor return a newly constructed object if it doesn't return at all? It can't.

Ergo, an infinite loop breaks the fundamental contract of a constructor: to construct something.

  • 10
    Perhaps they meant to put while(true) with a break in the middle? Risky, but legit. – John Dvorak Sep 8 '16 at 16:15
  • 2
    I agree, this is correct - at least. from an academical point of view. Same holds for every function which returns something. – Doc Brown Sep 8 '16 at 16:18
  • 61
    Imagine the poor sod that creates a sub-class of said class... – Newtopian Sep 8 '16 at 16:25
  • 5
    @JanDvorak: Well, that's a different question. The OP explicitly specified "never ending" and mentioned an event loop. – Jörg W Mittag Sep 8 '16 at 16:28
  • 4
    @JörgWMittag well, then, yeah, don't put that in a constructor. – John Dvorak Sep 8 '16 at 16:29
44

Is there something more obviously bad or devious with such an approach?

Yes, of course. It is unneccessary, unexpected, useless, unelegant. It violates modern concepts of class design (cohesion, coupling). It breaks the method contract (a constructor has a defined job and is not just some random method). It is certainly not well maintainable, future programmers will spend a lot of time trying to understand what is going on, and trying to guess reasons why it was done that way.

Nothing of this is a "bug" in the sense that your code does not work. But it will likely incur huge secondary costs (relative to the cost of writing the code initially) in the long run by making the code harder to maintain (i.e., hard to add tests, hard to reuse, hard to debug, hard to extend etc.).

Many/most modern improvements in software development methods are done specifically to make the actual process of writing/testing/debugging/maintaining software easier. All of this is circumvented by stuff like this, where code is placed randomly because it "works".

Unfortunately, you regularly will meet programmers which are completely ignorant of all this. It works, that's it.

To finish with an analogy (another programming language, here the problem at hand is to calculate 2+2):

$sum_a = `bash -c "echo $((2+2)) 2>/dev/null"`;   # calculate
chomp $sum_a;                         # remove trailing \n
$sum_a = $sum_a + 0;                  # force it to be a number in case some non-digit characters managed to sneak in

$sum_b = 2+2;

What is wrong with the first approach? It returns 4 after a reasonably short amount of time; it is correct. The objections (together with all the usual reasons a developer may give to refute them) are:

  • Harder to read (but hey, a good programmer can read it as easily, and we have always done it like this; if you want I can refactor it into a method!)
  • Slower (but this is not in a timing critical place so we can ignore that, and besides, it's best to optimize only when necessary!)
  • Uses much more resoures (a new process, more RAM etc. - but dito, our server is more than fast enough)
  • Introduces dependencies on bash (but it will never run on Windows, Mac or Android anyways)
  • And all the other reasons mentioned above.
  • 5
    "GC could well be in some exceptional locking state while running an constructor" - why? The allocator allocates first, then launches the constructor as an ordinary method. There's nothing really special about it, is there? And the allocator has to allow new allocations (and these could trigger OOM situations) within the constructor anyways. – John Dvorak Sep 8 '16 at 16:17
  • 1
    @JanDvorak, Just wanted to make the point that there could be some special behaviour "somewhere" while in a constructor, but you are correct, the example I picked was unrealistic. Removed. – AnoE Sep 8 '16 at 16:22
  • I really like your explanation and would love to mark both replies as answers (have at least an upvote). The analogy is nice and your trail of thought and explanation of secondary costs is it too but I regard them as auxiliary requirements for code (same as my arguments). Current state of the art is to test code, therefore testability etc is a de-facto requirement. But @JörgWMittag 's statement fundamental contract of a constructor: to construct something is spot on. – Samuel Sep 9 '16 at 7:02
  • The analogy is not so good. If I see this I would expect to see a comment somewhere on why you are using Bash to do the maths, and depending on your reason it may be totally fine. The same wouldn't work for the OP, because you would basically have to put apologies in comments everywhere you used the constructor "// Note: constructing this object starts up an event loop that never returns". – Brandin Sep 9 '16 at 8:07
  • 4
    "Unfortunately, you regularly will meet programmers which are completely ignorant of all this. It works, that's it." So sad but so true. I think I can speak for all of us when I say the only substantial burden in our professional lives is, well, those people. – Lightness Races in Orbit Sep 10 '16 at 8:26
8

You give enough reasons in your question to rule this approach out but the actual question here is "Is there something more obviously bad or devious with such an approach?"

My first though here was that this is pointless. If your constructor never finishes, no other part of the program can get a reference to the constructed object so what's the logic behind putting it in a constructor instead of a regular method. Then it occurred to me that the only difference would be that in your loop you could allow references to partially constructed this escape from the loop. While this isn't strictly limited to this situation, it's guaranteed that if you do allow this to escape, those references will always point to an object that is not fully constructed.

I don't know whether the semantics around this kind of situation are well defined in C# but I would argue it doesn't matter because it's not something most developers would want to try to delve into.

  • I'd assume that the constructor is run after the object was created thus in a sane language leaking this should be perfectely well defined behaviour. – mroman Sep 10 '16 at 14:28
  • @mroman: it might be ok to leak this in the constructor - but only if you are in the constructor of the most derived class. If you have two classes, A and B with B : A, if the constructor of A would leak this, the members of B would still be uninitialized (and virtual method calls or casts to B can wreak havoc based on that). – hoffmale Sep 11 '16 at 0:27
  • 1
    I guess in C++ that can be crazy, yes. In other languages members receive a default value before they are assigned their actual values so while it's stupid to actually write such code the behaviour is still well defined. If you call methods that use these members you might run into NullPointerExceptions or wrong calculations (because numbers default to 0) or stuff like that but it's technically deterministic what happens. – mroman Sep 11 '16 at 6:07
  • @mroman Can you find a definitive reference for the CLR that would show that to be the case? – JimmyJames Sep 12 '16 at 16:16
  • Well you can assign members values before the constructor is run so the object exists in a valid state with members assigned default values or values assigned to them before the constructor was even run. You dont need the constructor to initialize members. Let me see if I can find that somewhere in the docs. – mroman Sep 12 '16 at 18:42
1

+1 For least astonishment, but this is a hard concept to articulate to new devs. At a pragmatic level, it is hard to debug exceptions raised within constructors, if the object fails to initialise it will not exist for you to inspect the state of or log from outside of that constructor.

If you feel the need to do this sort of code pattern, please use static methods on the class instead.

A constructor exists to provide the initialisation logic when an object is instantiated from a class definition. You construct this object instance because it as a container that encapsulates a set of properties and functionality that you wish to call from the rest of your application logic.

If you do not intend to use the object that you are "constructing" then what is the point of instantiating the object in the first place? Having a while(true) loop in a constructor effectively means you never intend for it to complete...

C# is a very rich object oriented language with many different constructs and paradigms for you to explore, know your tools and when to use them, bottom line:

In C# do not execute extended or never-ending logic within constructors because... there are better alternatives

  • 1
    his doesn't seem to offer anything substantial over points made and explained in prior 9 answers – gnat Sep 10 '16 at 14:38
  • 1
    Hey, I had to give it a go :) I thought it was important to highlight that while there is no rule against doing this, you shouldn't by virtue of the fact that there are simpler ways. Most other responses focus on the technicality of why it is bad, but that's hard to sell to a new dev who says "but it compiles, so can I just leave it like that" – Chris Schaller Sep 10 '16 at 15:08
0

There's nothing inherently bad about it. However, what will be important is the question of why you chose to use this construct. What did you get by doing this?

First off, I'm not going to talk about using while(true) in a constructor. There is absolutely nothing wrong with using that syntax in a constructor. However, the concept of "starting a game loop in the constructor" is one which can cause some issues.

It's very common in these situations to have the constructor simply construct the object, and have a function which calls the infinite loop. This gives the user of your code more flexibility. As an example of the sorts of issues that can show up is that you restrict use of your object. If someone wants to have a member object that is "the game object" on their class, they have to be ready to run the whole game loop in their constructor because they have to construct the object. This might lead to tortured coding to work around this. In the general case, you cannot pre-allocate your game object, and then run it later. For some programs that's not an issue, but there are classes of programs where you want to be able to allocate everything up front and then call the game loop.

One of the rules of thumb in programming is to use the simplest tool for the job. It's not a hard and fast rule, but it's a very useful one. If a function call would have been enough, why bother constructing a class object? If I see a more powerful complicated tool used, I assume the developer had a reason for it, and I will begin exploring what sorts of odd tricks you might be doing.

There are cases where this might be reasonable. There may be cases where it is truly intuitive for you to have a game loop in the constructor. In those cases, you run with it! For example, your game loop may be embedded in a much larger program which constructs classes to embody some data. If you have to run a game loop to generate that data, it may be very reasonable to run the game loop in a constructor. Just treat this as a special case: it would be valid to do this because the overarching program makes it intuitive for the next developer to understand what you did and why.

  • If constructing your object requires a game loop, at least put it in a factory method instead. GameTestResults testResults = runGameTests(tests, configuration); rather than GameTestResults testResults = new GameTestResults(tests, configuration);. – immibis Sep 11 '16 at 7:45
  • @immibis Yes, that is true, except for the cases where that is unreasonable. If you're working with a reflection based system that instantiates your object for you, you may not have the option to make a factory method. – Cort Ammon Sep 11 '16 at 15:44
0

My impression is that some concepts got mixed up and resulted in a not so well worded question.

The constructor of a class may start a new thread which will "never" end (though I prefer to use while(_KeepRunning) over while(true) because I can set the boolean member to false somewhere).

You could extract that method of creating and starting the thread into a function of its own, in order to separate object construction and the actual start of its work - I prefer this way because I get better control over access to resources.

You could also have a look at the "Active Object Pattern". In the end, I guess the question was aimed at when to start the thread of an "Active Object", and my preference is a de-coupling of construction and start for better control.

0

Your object reminds me of starting Tasks. The task object has a constructor, but it there are also a bunch of static factory methods like: Task.Run, Task.Start and Task.Factory.StartNew.

These are very similar to what you are trying to do, and it's likely this is the 'convention'. The main issue people seem to have is that this use of a constructor surprises them. @JörgWMittag says it breaks a fundamental contract, which I suppose means Jörg is very surprised. I agree and have nothing more to add to that.

However, I want to suggest that the OP tries static factory methods. The surprise vanishes and there is no fundamental contract at stake here. People are used to static factory methods doing special things, and they can be named accordingly.

You can provide constructors that allow fine grained control (as @Brandin suggests, something like var g = new Game {...}; g.MainLoop();, that accommodate users who don't want to start the game immediately, and maybe want to pass it around first. And you can write something like var runningGame = Game.StartNew(); to make starting it immediately easy.

  • It means Jörg is fundamentally surprised, which is to say a surprise like that is a pain in the fundament. – Steve Jessop Sep 9 '16 at 14:28
0

Why is a while(true) loop in a constructor actually bad?

That's not bad at all.

Why (or is it at all?) regarded as bad design to let a constructor of a class start a never ending loop (i.e. game loop)?

Why would you ever want to do this? Seriously. That class has a single function: the constructor. If all you want is a single function, that's why we have functions, not constructors.

You mentioned some of the obvious reasons against it yourself, but you left out the very biggest one:

There are better and simpler options to achieve the same thing.

If there are 2 choices and one is better, it doesn't matter by how much it is better, you chose the better one. Incidentally, that's why we don't almost never use GoTo.

-1

The purpose of a constructor is to, well, contruct your object. Before the constructor has completed, it is in principle unsafe to use any methods of that object. Of course, we may call methods of the object within the constructor, but then each time we do so, we have to make sure that doing so is valid for the object in its current half-baken state.

By indirectly putting all logic into the constructor, you add more burden of this kind onto yourself. This suggests that you did not design the difference between initialization and use well enough.

  • 1
    this doesn't seem to offer anything substantial over points made and explained in prior 8 answers – gnat Sep 10 '16 at 9:49

protected by gnat Sep 10 '16 at 14:38

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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