CODEWITHSUNDEEP
java
Mark Pinho
Mark Pinho

Reputation: 125

What is the best practice for logic inside a constructor? (Java)

I am learning Java and one of the resources I am using is Coursera. For one of the assignments, I am writing a constructor for a CaesarCipher class where I am initializing the alphabet and the "shifted" alphabet.

public CaesarCipher(int key) {
    this.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    this.shifted = alphabet.substring(key) + alphabet.substring(0, key);
}

The idea is that when you give the "key" in the constructor it will shift the alphabet accordingly. One possible issue I noticed is that if the key is longer then the alphabet string then you will get an index out of bounds error. To prevent this I would assume we could write logic inside the constructor as such:

public CaesarCipher(int key) {
    while(key > 26) {
        key = key - 26;
    }
    this.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    this.shifted = alphabet.substring(key) + alphabet.substring(0, key);
}

Having a bit of knowledge on Java before taking this course though I know you typically wouldn't include logic inside a constructor. For a case like this what is best practice?

Upvotes: 1

Views: 1622

Answers (3)

Gaurav kumar Singh
Gaurav kumar Singh

Reputation: 183

Here's how I would implement it.

public class CaesarCipher {
    private int key;
    private String alphabet;

    public CaesarCipher(int key) {
        this.key = key;
        this.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    }

    public String shift(){
        int key = getKey();
        return alphabet.substring(key) + alphabet.substring(0, key);
    }

    public int getKey(){
        return  key % 26;
    }

    public static void main(String[] args) {
        CaesarCipher cc = new CaesarCipher(31);
        System.out.println(cc.shift());
    }
}

EDIT: Based on above comment/answer, validation for the key can be done something like

    public int getKey(){
        if(key<0) {
            throw new IllegalArgumentException("Key < 0");
        }
        return  key % 26;
    }

Upvotes: 0

JCWasmx86
JCWasmx86

Reputation: 3583

I would say, you shouldn't have any logic in the constructor except validation. So for your example:

public CaesarCipher(int key) {
    if(key < 0){
       throw new IllegalArgumentException("key < 0");
    }
    key%=26;//Replaces the while loop
    this.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    this.shifted = alphabet.substring(key) + alphabet.substring(0, key);
}

The advantage of this method is, that it fails fast and doesn't show an error somewhere later. See this example:

public class Foo{
   private String bar;
   public Foo(String s){
      this.bar = s;
   }

   public int getLengthOfBar(){
       return this.bar.length();
   }
   
   public static void main(String[] args){
      String s=getStringFromSomewhere();//May return null
      Foo f=new Foo(s);
      ...Some code....
      int len = f.getLengthOfBar(); //BOOM, if s is null, it will fail with a NullPointer Exception.
      ...Do something with len....
   }
}

If you would add a check in the constructor, you would see really fast, that you shouldn't pass null. Yes, you could add a check to getLengthOfBar, too, but I really think, that you should validate in the constructor. (At least for small, checks like Nullchecks, ranges,...)

Upvotes: 3

Zack Macomber
Zack Macomber

Reputation: 6905

Just accept the key in your constructor (it's good practice to make it immutable by making it a private final int) and then make https://en.wikipedia.org/wiki/Single-responsibility_principle methods to do something with that key.

Also alphabet could be a public static final String

shifted looks like it should be a method to me.

Upvotes: 0

Related Questions