Your question is composed from two completely separate parts:
Should I throw exception from constructor, or should I have method fail?
This is clearly application of Fail-fast principle. Making constructor fail is much easier to debug compared to having to find why the method is failing. For example, you might get the instance already created from some other part of code and you get errors when calling methods. Is it obvious the object is created wrong? No.
As for the "wrap the call in try/catch" problem. Exceptions are meant to be exceptional. If you know some code will throw an exception, you do not wrap the code in try/catch, but you validate parameters of that code before you execute the code that can throw an exception. Exceptions are only meant as way to ensure the system doesn't get into invalid state. If you know some input parameters can lead to invalid state, you make sure those parameters never happen. This way, you only have to do try/catch in places, where you can logically handle the exception, which is usually on system's boundary.
Can I access "other parts of the system, like DB" from constructor.
I think this goes against principle of least astonishment. Not many people would expect constructor to access a DB. So no, you should not do that.
When doing CQRS+ES I prefer not having public constructors at all. Creating my aggregate roots should be done via a factory (for simple enough constructions such as this) or a builder (for more complicated aggregate roots).
How to then actually initialize the object is an implementation detail. The OOP "Don't use initialize"-advice is imho about public interfaces. You should not expect that anyone that uses your code knows that they must call SecretInitializeMethod42(bool,int,string) - that's bad public API design. However if your class does not provide any public constructor but instead there is a ShoppingCartFactory with the method CreateNewShoppingCart(string) then the implementation of that factory may very well hide any kind of initialization/constructor magic which your user then don't need to know about (thus providing a nice public API, but allowing you to do more advanced object creation behind the scenes).
Factories get a bad rep from people thinking there's too many of them, but used correctly they can hide away a lot of complexity behind a nice easy-to-understand public API. Don't be afraid to use them, they're a powerful tool which can help you make complex object construction much easier - as long as you can live with some more lines of code.
It's not a race to see who can solve the problem with the least lines of code - it is however an ongoing competition as to who can make the nicest public API's! ;)
Edit: Adding some examples on how applying these patterns could look
If you just have an "easy" aggregate constructor that has a couple of required parameters you can go with just a very basic factory implementation, something along these lines
public class FooAggregate {
internal FooAggregate() { }
public int A { get; private set; }
public int B { get; private set; }
internal Handle(FooCreatedEvent evt) {
this.A = a;
this.B = b;
}
}
public class FooFactory {
public FooAggregate Create(int a, int b) {
var evt = new FooCreatedEvent(a, b);
var result = new FooAggregate();
result.Handle(evt);
DomainEvents.Register(result, evt);
return result;
}
}
Of course, exactly how you divide creating the FooCreatedEvent is in this case up to you. One could also make a case for having a FooAggregate(FooCreatedEvent) constructor, or having a FooAggregate(int, int) constructor that creates the event. Exactly how you choose to divide the responsibility here is up to what you think is the cleanest and how you have implemented your domain event registration. I often choose to have the factory create the event - but it's up to you since event creation is now an internal implementation detail that you can change and refactor at any time without changing your external interface. An important detail here is that the aggregate does not have a public constructor and that all the setters are private. You don't want anyone to use them externally.
This pattern works fine when you're just more or less replacing constructors, but if you have more advanced object construction this may become way too complex to use. In this case I usually forego the factory pattern and turn to a builder pattern instead - often with a more fluent syntax.
This example is a bit forced since the class it builds isn't very complex, but you can hopefully grasp the idea and see how it would ease more complex construction tasks
public class FooBuilder {
private int a;
private int b;
public FooBuilder WithA(int a) {
this.a = a;
return this;
}
public FooBuilder WithB(int b) {
this.b = b;
return this;
}
public FooAggregate Build() {
if(!someChecksThatWeHaveAllState()) {
throw new OmgException();
}
// Some hairy logic on how to create a FooAggregate and the creation events from our state
var foo = new FooAggregate(....);
foo.PlentyOfHairyInitialization(...);
DomainEvents.Register(....);
return foo;
}
}
And then you use it like
var foo = new FooBuilder().WithA(1).Build();
And of course, generally when I turn to the builder pattern it's not just two ints, it may contain lists of some value objects or dictionaries of some kind of maybe some other more hairy things. It is however also quite useful if you have many combinations of optional parameters.
The important takeaways for going this way is:
- Your main objective is being able to abstract object construction so that the external user doesn't have to know about your event system.
- It's not that important where or who registers the creation event, the important part is that it gets registered and that you can guarantee that - apart from that, it's an internal implementation detail. Do what fits your code the best, don't follow my example as some sort of "this is the right way"-thing.
- If you want to, by going this way you could have your factories/repositories return interfaces instead of concrete classes - making them easier to mock for unit tests!
- This is a lot of extra code sometimes, which makes many people shy away from it. But it's often quite easy code compared to the alternatives and it does provide value when you do sooner or later need to change stuff. There's a reason that Eric Evans talks about factories/repositories in his DDD-book as important parts of DDD - they are necessary abstractions to not leak certain implementation details to the user. And leaky abstractions are bad.
Hope that helps a bit more, just ask for clarifications in comments otherwise :)
Best Answer
This is not true when you're dealing with things like the factory pattern, where the instantiator of the object is not the handler of the object. Factory patterns quite often exist specifically because the object's construction is an implementation detail that should be abstracted away.
This applies to more cases than just the factory pattern. Essentially, it applies to any object that gets passed around at least once.
This isn't true for reference types. It's true that you can't change which object is being referenced, but you can still alter its content. For example:
As per your claim,
myBaz.Foo
can no longer be modified. Yet this code is perfectly legal:And that's still a risk you take.
These two don't quite follow. It doesn't require you to think about it, it requires you to default to
private
instead ofpublic
like you currently do. Unless there is a valid reason to expose it, don't.This is an oversimplification as there are cases where you shouldn't start out on
private
(e.g. DTO properties), but if you're still struggling with evaluating this, it's already better to default toprivate
instead ofpublic
.In my opinion, this is indicative of not quite understanding the class' responsibility and how it fits in the existing codebase.
In fact, that's sort of what you state in the question: you don't want to think about it. But you really should. For your example, what would ever be the purpose of a repository exposing its database connection? I can't think of any answer here that does not immediately violate good practice rules, can you?
Exposing the database connection is not part of the repository's purpose, which is all about providing access to a persistent data store.
In part, this is a matter of experience which will come over time. Every time you have to change the access modifier on an existing property/method is a time to learn why the previous choice was not the right one. Do it enough and you will improve at judging public contracts on the first design.
Don't forget OCP: "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification".
If you are inherently accounting for needing to change the internals of classes as time passes, you're taking a stance orthogonal to OCP.
That's not to say internals can't be changed when e.g. bugs are found or breaking changes are implemented; but it does mean you should try to avoid it as best as you can. Changing existing (often central) logic is a most common source of bugs, especially the crippling ones.
It really does matter. If your library is only being used in the same solution file, you can change things very quickly to your needs and can confirm it's working with a simple build.
But Nuget compounds the issue. If you change the contracts of your classes exposed in yout Nuget package, that means that every Nuget consumer will have to deal with breaking changes.
From personal experience, the issue is further compounded by Nuget servers not keeping a record of who has consumed your Nuget package, which makes it hard to figure out who all your consumers are and warn them ahead of time that breaking changes are about to be released.
Had you defaulted to making things
private
, and then selectively expose them, there would be less of a problem here. Adding to the contract without changing the existing parts does not break existing code.Removing things from the contract, which is what would happen if you default to
public
, would always be liable to breaking code that depends on the thing you're now removing from the contract.Yes. But it's not as complicated as you're making it out to be. Understanding what a certain class needs to expose or not is something you need to think about once per class. What is this class' purpose? How do I want this class to be used by its consumers?
After that, all properties/methods that you develop can easily be matched to the class' purpose, which is not a new evaluation but simply applying the decision you already made.
If you were using interfaces on all your classes and using interface-based dependency injection, it would really help you in understanding how to separate a class' contract (things in the interface) from its implementation (things not in the interface).
Take for example:
The internals of each vending machine is up to them. It doesn't matter how they have access to and dispense a drink to the consumer. To the consumer, that's an irrelevant implementation detail. The customer doesn't want to know how the sausage gets made.
What matters for the public contracts is that they dispense a drink to the consumer, and thus the
ISodaVendingMachine
interface is built specifically for that purpose.Notice how the interface doesn't care about anothing other than what it was designed to ensure.
When you have that interface, you can already see that anything in your class that isn't part of that interface should most likely be
private
as it is an implementation detail, not a contract.