Should object properties be filled in the constructor

I'm designing a new application and I'm undecided if I should fill my object properties on the constructor

Public Sub New(UserId as integer)
    ' get database values
    dr = DataReader
    Me.FirstName = dr.fields(0)
    Me.LastName = dr.fields(1)
End Sub

Or create a factory with a method for each object type?

Public Function getUser(UserId as integer) as User
    Dim myUser as new User
    ' get database values
    dr = DataReader
    myUser.FirstName = dr.fields(0)
    myUser.LastName = dr.fields(1)
    return myUser
End Function

I realize that the code is rough, but I'm hoping it's enough to get my point across. I've see both implemented and not sure what the long term pros and cons are.

Specifically, I'm using VB.NET, if it matters.

From stackoverflow

• I like types to be immutable where that doesn't create other problems. For a "person" type it may not be suitable, but it's worth considering. You should also try to construct objects so that after the constructor has been called, the object is in a valid state.

  On the other hand, you should try to avoid doing too much work in the constructor itself - this is where static factory methods tend to be nice, as they can do significant work to grab all the data needed to construct the object, then pass it on to a simple constructor to create a complete, valid and potentially immutable object.

• It really depends on the language because some languages cannot guarantee the state of the object until the constructor and all base constructors have finished executing.

  I find it best to populate whatever members you need and then call methods to do the rest of the work. This tends to make the code more readable as you won't have lots of code in your constructors.

• My opinion:

  A constructor creates an object. The constructor of a User class should create a new user.

  Static member functions like your getUser retrieve an object which already exists.

  I would use the latter form.

  flq : I agree. That way there is also no need to reference a DataReader in your User class. That way you may be able to play with your User object in other secnarios and populate its values simpley by setting them.

• 1. You are instantiating the userhandler-object and using that to get different users, and let you work with all users. Have a GetUser-function and nothing in the constructor.

  2. You are instantiationg the userobject and working only with THE user, have the constructor handeling witch user to work with.

  My 5 cents.

• I would use a Factory-Pattern and make the constructor private/protected and make a static NewObject() method. The NewObject-method can then call the appropriate constructor. The pro on this is that if you need to change the behaviour of the Object during runtime you can plug-in (override) the NewObject-method and have your own NewObject-method "mixed in". The downside of this is that it can get very complex very fast - especially if you're debugging it.

• An object should be 100% ready for use when it's constructed. A default constructor should have sensible, non-null default values for all references.

  If you find that your constructor has "too much" code, perhaps it's time to consider refactoring that object.