• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Long constructor: break up?

 
Michal Charemza
Ranch Hand
Posts: 86
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi, my constructor for my lowest level data file access class is quite long, as it reads the header, then the schema, and then checks to make sure that all the flags in the file are either 0 or 1, and all the way it checks if there are any deviations from the format, and if so then it throws and exception.

Is it a good idea to break up the constructor into smaller methods? Is it bad programming practice?

Thanks,

Michal.
 
Roger Chung-Wee
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The most important purpose of the constructor is to initialize the newly created object to its initial, stable, safe state. So, ask yourself if all the code in your constructor is doing initialization. If some of it is not, then can it sensibly be moved elsewhere.

Sometimes an object always needs a certain initialization, and there could be addition initialization depending on other factors. So, you could put that certain initialization in the constructor and overload the constructor for the other stuff. The overloaded constructor would need to first invoke this() in order to call the first constructor.
 
Darya Akbari
Ranch Hand
Posts: 1855
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Michal,

better you avoid long argument lists and encapsulate the stuff you need.

This belongs to the Clarity and Maintainability section and to the General Programming Considerations criteria of the Marking Criteria section in our assignment.

General Programming Considerations is scored with 100 points

Regards,
Darya
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Michal,
my constructor [...] reads the header, [and] then the schema, and then checks to make sure that all the flags in the file are either 0 or 1, and all the way it checks if there are any deviations from the format, and if so then it throws and exception.
(Highlighting and additional "and" is mine)

I think you already know the answer to your question, but here goes anyway ...

Doesn't your description of your constructor sound like you might want private methods that would (say) readTheHeader(), then readTheSchema(), and then validateTheData()?

It is a good practice to break up methods when they start trying to do too much. One of the goals that we can try to achieve when programming, is to have our methods do one thing and do it well. The same thing applies to our classes (in fact some candidates consider this a reason for making the Data class a Fa´┐Żade - it is trying to handle file access and record locking: it is breaching the "do one thing only" suggestion).

The moment you start using the word "and" when trying to describe what your method (or class) is doing, then there is the possibility that it is trying to do too much (which then raises the possibility that the could may be hard to read / maintain).

In your initial question, you already identified different things that your constructor was doing. Imagine how much easier to maintain your program will be if you do break up the code based on those functions you identified. If the schema (and only the schema) changed in some way, would you prefer to try and modify one constructor that performs multiple functions, or would you prefer to jump right to the method that is clearly marked as reading the schems?

Regards, Andrew
 
Michal Charemza
Ranch Hand
Posts: 86
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Andrew... I think I will probably split it up into some private methods.

Michal.
 
Lara McCarver
Ranch Hand
Posts: 118
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There is no conceptual problem with calling methods from a constructor. In fact, in terms of a multithreaded application, doing more within the constructor (by calling methods from it) solves certain problems for you... obviously no other thread is going to be able to use your object until it is fully constructed. So constructors are always "thread-safe". (Although to be on the safe side, access member variable within your newly created object using synchronized or volatile, to make sure you get the up-to-date values.)
 
Roger Chung-Wee
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The only caveat is that you should never invoke a non-final method from a constructor as it can be overridden by a method that relies on instance fields of the subclass which would not yet have been initialized.

A private method is fine as all private methods are implicitly final.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic