current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

I have multiple different implementations of an object, which implement this custom interface I made called Board.

Board contains a method that looks like the following

public void ConvertFromString(String formattedString);

Each object implementing Board calls ConvertFromString() in its constructor.

Looks like the following.

public void BoardImpl1 implements Board
{
    public Board(string B)
    {
        ConvertFromString(b);
    }

    public void ConvertFromString(String formattedString)
    {
        //do some parsing on string and set up the BoardImpl properties
    }
}

ConvertFromString being public causes a warning, so the best practice workaround would be to make BoardImpl final. Is there a better way to write this?

share|improve this question
4  
Voting to close, as I think this question is better suited on StackOverflow. While your code is technically working, in practice it is not. You are not really asking for a code review, you are asking for a solution to a problem; which belongs on SO. –  Lstor Jul 14 '13 at 16:36
    
Well I'm not sure if my OP wasn't clear. I stated that I can make my class Final, which gets rid of the warning. I was wondering if something should be restructured so that I don't end up making a bunch of final classes each time I want to implement the Interface. –  Rhs Jul 14 '13 at 16:58
add comment

closed as off-topic by Lstor, Peter Kiss, Jamal♦, Brian Reichle, Jeff Vanzella Jul 14 '13 at 22:09

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must contain working code for us to review it here. For questions regarding specific problems encountered while coding, try Stack Overflow." – Lstor, Peter Kiss, Jamal, Brian Reichle, Jeff Vanzella
If this question can be reworded to fit the rules in the help center, please edit the question.

1 Answer

up vote 1 down vote accepted

The warning you get, is because you call an overridable method from your constructor. This is bad because subclasses may change the implementation of the method to access other parts of the object, while it is not fully initialized yet.

So to get rid of the warning : don't call overridable methods from the constructor.

Then how to get around your predicament?

Well you could :

  • make the public method final.
  • make the class final (which you found yourself)
  • duplicate the logic (while possible, not the way to go)
  • have the public method delegate to a private method, and have the constructor call the private method.

Like this : 

public class BoardImpl implements Board {
    public BoardImpl(String b) {
        doConvertFromString(b);
    }

    public void convertFromString(String formattedString) {
        doConvertFromString(formattedString);
    }

    private void doConvertFromString(String formattedString) {
        //do some parsing on string and set up the BoardImpl properties
    }
}

I wouldn't worry about making all implementations final. final is your friend.

share|improve this answer
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.