Raymond Lewallen

mmm... you should not throw exceptions in the contructors?

I'm all into good OO design, but that sounds like some nit-picky shit.

I've been thinking about it some more and I just think it has to be some oversight in editing...there's maybe missing information in the book. Because the "bad design" is really not apparrent in that snippet. And if it is the default parameterless constructor thing then certainly that should have been made more clear...but like the above guy said...that's hard-core nit-picky.

Or it could be that only losers use underscore prefixes.

Second example is bad design because the userId is marked as protected.
If it's protected, it means it can be safely modified by the interitor, thus the abstract should not depend on it or on having a valid/correct value.
By making the constructor require the userId you mandate the inheritor giving you a value and "you need and you depend on it", while you actually don't.
Making the member private, private readonly or protected readonly would make your design correct implying that userId is a mandatory value required in order to make your abstract class behave correclty.

I think the constructor with the least no of parameters should be the one that defines the minimum variables/values that are required in order to guarantee that your class works correctly.

to add to corneliu's comments:
With the 'bad' design, you can never add an empty constructor without cooking up a userid to pass to the base constructor. because adding
public MyFirstAbstract()
{
}

won't compile. In some scenario's you need an empty constructor (xmlserializer etc. ) and you don't know that in the abstract class, so you better be prepared for these kind of situations when you write your abstract class, hence it's better to add an empty constructor.

I agree with Frans, the problem is that there is no default constructor in the base class and we should add a protected one. I don't think the main reason is that in some situation, such as serialization, you need a no-param constructor. The main reason is that this framewiork is publishing an abstract class that will be inherited by users of the framework. Having only a parametrized constructor, we force all inheritors to supply this parameter, thus the signature of their constructors should include the parameter required by base class. That way we couple inheritors with the implementation details of the base class and we create an inflexible situation.

exactly, xanaris, very well worded :)

I now wonder though, if it wouldn't be better to replace the protected constructor with a method like
protected void InitClass(int userId)
{
...
}

Because due to the parameter less constructor, initializing the class isn't mandatory anyway.

So if initializing the class is mandatory, I'm now not so sure anymore if the design is that bad, as how else are you going to enforce calls to the initialization code without a constructor like the 'bad design' example.

Raymond,
If the domain requires that every initialization provides this parameter, it sounds like it is mainly needed by derived classes and the base class may or may not have significant use for it. So why not transferring the responsibility to derived classes but in a way that does not limit their options. Something like the following class enables inheritors to choose how to initialize this parameter (constructor supplied, AppConfig, IoC etc.).
public abstract MyFirstAbstract()
{
protected MyFirstAbstract()
{
}

protected abstract object MyRequiredParam { get; }
}

Raymond,
I see a number of options.

If the parameter is not needed at all by base class and we just want to enforce validation rules when it is set, we could do:
public abstract MyFirstAbstract()
{
protected MyFirstAbstract()
{
}

private _userId;
protected int UserId
{
get { return _userId; }
set
{
if (value < 0) throw new ArgumentOutOfRangeException("userId");
_userId = value;
}
}
}

If the validation is complex (also the initialization will be complex and chatty) we can implement validation methods:

public abstract MyFirstAbstract()
{
protected MyFirstAbstract()
{
}

protected abstract in UserId { get; set; }

protected void SelfValidate()
{
if (userId < 0) throw new MyCustomValidationException();

DoMoreValidation();
}

protected virtual void DoMoreValidation()
{ // inheritors add validation logic
}

public void SomeMethodThatRequiresValidState()
{
Selfvalidate();
....
}
}

Finally, I may choose to ignore this rule – depending on the situation - if it causes much more problems than it solves. If the base and derived classes are under my control and I am not publishing an API, I would most certainly use the approach you describe.

And what to do when userId is readonly Field?

public abstract class MyFirstAbstract

           {

                       protected READONLY int _userId;

                       // incorrect design??

                       protected MyFirstAbstract(int userId) // <--- this line is bad

                       {

                                   _userId = userId;

                       }

           }