if else condition in class constructor ... is it good practice?

Question

I have written a constructor and passing one boolean flag to decide which value to set to class variable. Code is as follows

public PDFParagraph(PDFPhrase phrase,boolean isRtl) {
            super(phrase);
            if(isRtl)
                    this.setAlignment(Element.ALIGN_RIGHT);
            else
                    this.setAlignment(Element.ALIGN_LEFT);
    }

Now I am confused and not sure if I shall add if...else conditions in constructor. Is it good style to set the class varible value?

Thanks, Hanumant.

Answer 1

Conditionals in constructors aren't problematic per se. However, in this instance, I'd be inclined to write your constructor like this:

public PDFParagraph(PDFPhrase phrase, boolean isRtl) {
    super(phrase);
    setAlignment(isRtl ? Element.ALIGN_RIGHT : Element.ALIGN_LEFT);
}

Answer 2

There is no style issue with using an if / else to do that. However:

• You could write it more simply:

  setAlignment(isRtl ? Element.ALIGN_RIGHT : Element.ALIGN_LEFT);
  

• A lot of people (myself included) think that you should always put curly braces around the "then" and "else" statements.

---

On a related point: if you find yourself writing a constructor that looks like this:

public Thing(boolean cond, ...) {
    super(...);
    if (cond) {
        // Lots of statements
    } else {
        // Lots of different statements
    }
    ...
}

it is possibly an indication that you need to refactor your constructors. (Or possibly not ... it depends on the details.)