• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Long constructor: break up?

 
Ranch Hand
Posts: 86
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Ranch Hand
Posts: 1855
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
 
author and jackaroo
Posts: 12200
280
Mac IntelliJ IDE Firefox Browser Oracle C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks Andrew... I think I will probably split it up into some private methods.

Michal.
 
Ranch Hand
Posts: 118
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Remember to always leap before you look. But always take the time to smell the tiny ads:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic