java - Calling methods from the constructor -


i wrote below code, , see, in constructor call methods perform operations. , inquiring is, whether practice call methods constructor or declare methods public , instantiate object class info, , let object call methods? practice that?

code:

class info { public roadinfo(string cityname, double lat, double lng) throws filenotfoundexception, saxexception, ioexception, xpathexpressionexception {     // todo auto-generated constructor stub     this.cityname = cityname;     this.lat = lat;     this.lng = lng;      this.path = "c:"+file.separatorchar+this.cityname+".xml";      system.out.println(path);      this.initxpath();     this.method1()     this.method2()     ..      this.expr = "//node[@lat='"+this.lat+"'"+"]/following-sibling::tag[1]/@v";     this.xpath.compile(this.expr);     string s = (string) this.xpath.evaluate(this.expr, this.document, xpathconstants.string);     system.out.println(s); } 

tldr in opinion, using methods inside of constructor sign of bad design. if aren't looking design advice, answer "no there's nothing wrong it, technically speaking, long avoid calling non-final methods" should fine. if looking design advice, see below.

i think example code not practice @ all. in opinion, constructor should receive values relevant , should not need perform other initialization on values. there's no way test constructor 'works' of little steps - can construct object , hope ends in correct state. further, constructor ends more 1 reason change, violates srp.

class info { public roadinfo(string cityname, double lat, double lng) throws filenotfoundexception, saxexception, ioexception, xpathexpressionexception {     // todo auto-generated constructor stub     this.cityname = cityname;     this.lat = lat;     this.lng = lng;      this.path = "c:"+file.separatorchar+this.cityname+".xml";      system.out.println(path);      this.initxpath();     this.method1()     this.method2()     ..      this.expr = "//node[@lat='"+this.lat+"'"+"]/following-sibling::tag[1]/@v";     this.xpath.compile(this.expr);     string s = (string) this.xpath.evaluate(this.expr, this.document, xpathconstants.string);     system.out.println(s); } 

so, example, constructor loading file, parsing in xpath.. if ever want create roadinfo object, can loading files , having worry exceptions being thrown. class becomes hilariously difficult unit test because can't test this.initxpath() method in isolation, example - if this.initxpath(), this.method1() or this.method2() have failures, every 1 of test cases fail. bad!

i prefer more this:

class roadinfofactory {   public roadinfo getroadinfo(string cityname, double lat, double lng) {     string path = this.buildpathforcityname(cityname);     string expression = this.buildexpressionforlatitute(lat);     xpath xpath = this.initializexpath();     xdocument document = ...;      string s =  (string) xpath.evaluate(expression, document, xpathconstants.string);     // or whatever it..     return new roadinfo(s);   } } 

never mind fact have @ least 5 responsibilities here.

  • build os-neutral path
  • build xpath expression latitude/longitude
  • create xpath doocument
  • retrieve s - whatever is
  • create new roadinfo instance

each of these responsibilities (except last) should separated own classes (imo), , have roadinfofactory orchestrate them together.


Comments