Problem finding the design/implementation bugs in this code

[ali.jay110]

Hi
There is something about this code that does not seem right has anyone got any ideas? It does not matter what it does. I mean from a constructor/inheritance point of view. Some of the points I made were one constructor was passing a null value, which in turn was setting a final variable. This variable will always hold a null value. I also mentioned that ConceptA's constructor passes in a Concept instance to be set as ConceptA's parent. this did not seem correct as ConceptA extends Concept, so could pass in a ConceptB which is concrete class of Concept and set it as the parent of ConceptA, which is not good. Read the code and it will make sense.

Concept.java

public abstract class Concept {
	private String id;

	protected Concept(String anId) {
		if (anId == null) {
			throw new NullPointerException("id must not be null");
		}

		id = anId;
	}

	public String getId() {
		return id;
	}

	public void setId(final String id) {
		id = id;
	}

	public boolean equals(Object other) {
		return other != null && other.getClass().equals(getClass())
				&& id.equals(((Concept) other).id);
	}

	public String toString() {
		return "Concept(" + id + ")";
	}
}

Second class:

ConceptA.java

public class ConceptA extends Concept
	{
	  private final Concept parent;
	
	  public ConceptA( String anId, Concept aParent )
	  {
	    super( anId );
	
	    parent = aParent;
	  }
	
	  public Concept getParent()
	  {
	    return parent;
	  }
	
	  public String toString()
	  {
	    return "ConceptA{" + getId() + ", parent=" + parent + '}';
	  }
	}

Third class:

ConceptB.java

import java.util.Set;
import java.util.HashSet;
import java.util.Iterator;

public class ConceptB extends ConceptA {
	private final Set children;

	public ConceptB(final String anId, final Concept aParent) {
		super(anId, aParent);

		children = new HashSet();
	}

	public int getCount() {
		return children.size();
	}

	public void addChild(Concept aChild) {
		children.add(aChild);
	}

	public void removeChild(Concept aChild) {
		children.remove(aChild);
	}

	public Iterator getChildren() {
		return children.iterator();
	}

	public int getFamilySize() {
		int count = children.size();

		for (Iterator iter = getChildren(); iter.hasNext();) {
			count += ((ConceptB) iter.next()).getFamilySize();
		}

		return count;
	}

	public int getAncestorCount() {
		int count = 0;
		Concept ancestor = getParent();

		while (ancestor != null) {
			count++;
			if (ancestor instanceof ConceptA) {
				ancestor = ((ConceptA) ancestor).getParent();
			} else {
				ancestor = null;
			}
		}

		return count;
	}

	public String toString() {
		return "ConceptB{" + getId() + ", parent=" + getParent()
				+ ", children=" + children.size() + "}";
	}
}

Fourth class:

public class ConceptC extends ConceptA {
	private static int nextSerialNo = 0;

	public static int getNextSerialNo() {
		return nextSerialNo++;
	}

	private final int serialNo;

	public ConceptC(String anId) {
		super(anId, null);

		serialNo = getNextSerialNo();
	}

	public int getSerialNo() {
		return serialNo;
	}

	public String toString() {
		return "ConceptC(" + getId() + ", " + serialNo + ")";
	}
}

I know this bit of problem.

public void setId(final String id) {
		id = id;
	}

Could u find anything else in this code?

[stultuske]

no offence, but what exactly is your problem?
also, why do you declare your argument as final? and why, if one of them is a final, do you give it the same name as your instance variable?

yes, been up late ... yes, had a lot to drink ... no, didn't completely understand your question. could you please elaborate a bit further?

[JamesCherrill]

It's hard to criticise the functionality of arbitrary code, but...

constructor was passing a null value, which in turn was setting a final variable. This variable will always hold a null value

OK, so ConceptC is like ConceptA except that it has a serial no, and its parent is always null. No problem with that as Java code.

ConceptA's constructor passes in a Concept instance to be set as ConceptA's parent. this did not seem correct as ConceptA extends Concept, so could pass in a ConceptB which is concrete class of Concept and set it as the parent of ConceptA, which is not good

Why "not good"? It's perfectly valid Java. Without attaching any meaning to these classes why shouldn't a ConceptA have a ConceptB as its parent?

[JamesCherrill]

why do you declare your argument as final?

That's a great question. There was a stage in the early days when people argued that all Java parameters should be final by default - to prevent people treating them as call-by-reference and being surprised when assigning a new value to a parameter didn't have the expected results. Nowdays we assume that people know that, and so the final keyword is very rarely used. IMHO if you assign an new value to a parameter then you're either confused, or you want to confuse someone else. I wish all params were final.

[ali.jay110]

Thanks stultuske for your reply, well this question belongs to my test paper and I couldn't trace out the problems in it apart from one I mentioned in this post, I was to analyze the code and comment the code, my question here to get the hints so that I could learn something for future what to look for such problems.

Rgrds
-A.J