I have an alternative solution for family tree that have been posted some time ago and please forgive the similarity of the task but when i tried to post alternative solution in someone else's post and asked for review I got deleted.
Model the family tree such that:
Input:
Person=Alex Relation=Brothers
Expected Output should be
Brothers=John,Joe
Input:
Husband=Bern Wife=Julia
Expected Output should be
Welcome to the family, Julia
List of supported relations:
Father
Mother
Brother(s)
Sister(s)
Son(s)
Daughter(s)
Sibling
Assumption:
- Names are unique. Any new member added will also have a unique name.
TreeApp
import java.util.Scanner;
public class TreeApp {
public static Scanner sc= new Scanner(System.in);
public static RelationshipFactory rl = new RelationshipFactory();
public static void main(String[] args) {
Boolean quit = false;
printMenu();
try{
while(!quit){
System.out.println("Enter option:");
int operation = sc.nextInt();
sc.nextLine();
switch(operation){
case 1:
System.out.println("Menu:");
printMenu();
break;
case 2:
rl.printFamilyMap();
break;
case 3:
System.out.println("Enter query");
proceedWithQuerry();
break;
case 4:
System.out.println("Fill with test data");
rl.populateFamilyTree();
break;
default:
System.out.println("QUIT");
quit = true;
break;
}
}
}
catch(Exception e){
System.out.println("Error while getting option - quit");
}
}
private static void proceedWithQuerry() {
String query = sc.nextLine();
QueryParser qp = new QueryParser();
qp = qp.parse(query);
if (qp.com.equals(Command.ADD)){
rl.addRelation(qp.p1,qp.rel2,qp.p2,qp.rel1,0);
} else if (qp.com.equals(Command.RETRIEVE)){
rl.getSiblings(qp);
} else{
System.out.println("Process terminated");
}
}
private static void printMenu() {
System.out.println("1 - Print menu,"
+ "\n2 - Print familyTree,"
+ "\n3 - Enter query,"
+ "\n4 - Inject test data.");
}
}
RelationshipFactory
import java.util.HashMap;
import java.util.Map;
public class RelationshipFactory {
private static Map<String,HashMap<String,Relation>> familyMap = new HashMap<>();
public RelationshipFactory() {
}
public void addRelation(String p1, Relation rel1, String p2, Relation rel2, int mode){
HashMap<String, Relation> personalMap = new HashMap<>();
if (familyMap.get(p1) == null){
personalMap.put(p2, rel1);
familyMap.put(p1, personalMap);
System.out.println("Welcome to the family, " + p1 );
} else {
personalMap = familyMap.get(p1);
if (personalMap.get(p2) == null){
personalMap.put(p2, rel1);
}
}
if (mode == 0){
addRelation(p2, rel2, p1, rel1, 1);
}
}
public void printFamilyMap(){
System.out.println("Family Tree:\n");
for (Map.Entry<String,HashMap<String,Relation>> entry : familyMap.entrySet()){
System.out.println("Person: " + entry.getKey() + ":");
entry.getValue().forEach((p,role) -> { System.out.println("\t\t\t" + p + " - " + role.toString());});
}
}
public void getSiblings(QueryParser qp) {
Boolean foundMember = false;
if (familyMap.get(qp.p1) == null) System.out.println("Unregonised member of the family.");
else {
System.out.println(qp.rel2 + ": ");
for (Map.Entry<String,Relation> entry : familyMap.get(qp.p1).entrySet()){
if (entry.getValue().equals(qp.rel2)){
System.out.println("\t\t\t" + entry.getKey());
foundMember = true;
}
}
if (!foundMember) System.out.println("\tnot found." );
}
}
public void populateFamilyTree() {
addRelation("Bern", Relation.WIFE, "Julia", Relation.HUSBAND, 0);
addRelation("Bern", Relation.SON, "Boris", Relation.FATHER, 0);
addRelation("Bern", Relation.SON, "Adam", Relation.FATHER, 0);
addRelation("Bern", Relation.DAUGHTER, "Zoe", Relation.FATHER, 0);
addRelation("Julia", Relation.SON, "Adam", Relation.MOTHER, 0);
addRelation("Julia", Relation.SON, "Boris", Relation.MOTHER, 0);
addRelation("Julia", Relation.DAUGHTER, "Zoe", Relation.MOTHER, 0);
addRelation("Julia", Relation.SISTER, "Alicia", Relation.SISTER, 0);
addRelation("Bern", Relation.SIBLING, "Alicia", Relation.SIBLING, 0);
}
}
QueryParser
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
enum Command {ADD, RETRIEVE, ERROR}
enum Relation {MOTHER, FATHER, SON, DAUGHTER, BROTHER, SISTER, WIFE, HUSBAND, SIBLING};
public class QueryParser {
/*
Person=Bern Relation=Wife
Mother=Julia Son=Boris
*/
Command com;
String p1;
String p2;
Relation rel1;
Relation rel2;
public QueryParser() {
}
public QueryParser(Command com, String p1, String p2, Relation rel1, Relation rel2) {
this.com = com;
this.p1 = p1;
this.p2 = p2;
this.rel1 = rel1;
this.rel2 = rel2;
}
public QueryParser parse(String query) {
List<String> list = Stream.of(query.split("[ =]+")).map(elem -> new String(elem)).collect(Collectors.toList());
if (list.size()!=4) {
System.out.println("Error while parsing to list");
com = Command.ERROR;
} else if (list.get(0).equals("Person")){
com = Command.RETRIEVE;
p1 = list.get(1);
rel2 = adjustRelation(list.get(3));
} else {
com = Command.ADD;
p1 = list.get(1);
p2 = list.get(3);
rel1 = adjustRelation(list.get(0));
rel2 = adjustRelation(list.get(2));
}
return new QueryParser(com,p1,p2,rel1,rel2);
}
private Relation adjustRelation(String rel){
switch(rel){
case "Mother":
return Relation.MOTHER;
case "Father":
return Relation.FATHER;
case "Husband":
return Relation.HUSBAND;
case "Wife":
return Relation.WIFE;
case "Son":
return Relation.SON;
case "Sons":
return Relation.SON;
case "Daughters":
return Relation.DAUGHTER;
case "Daughter":
return Relation.DAUGHTER;
case "Sisters":
return Relation.SISTER;
case "Sister":
return Relation.SISTER;
case "Brother":
return Relation.BROTHER;
case "Brothers":
return Relation.BROTHER;
default:
return Relation.SIBLING;
}
}
}
1 Answer 1
QueryParser
Right now your query parser has two responsibilities. It parses the query (hence its name), but it also contains the result of the parse, which is kind of unintuitive and violates the Single Responsibility Principle.
I'd create a Query class that would contain the members com, p1, p2, rel1, rel2.
If you're not already confortable with polymorphism, I'd try using it to create ErrorCommand, RetrieveCommand and AddCommand instead of just a Query class. You could use the Command pattern to separate the responsibilities instead of using the if/else if/else from the proceedWithQuerry method.
RelationshipFactory
This class isn't really a Factory Pattern. In this situation I think you should try to rename it to something else. The only method in the class that kind of fits a Factory is the populateFamilyTree method. You could check out the Builder pattern to help build your family tree. It could be an interesting exercise though some people might argue that it's overkill. But as you tagged your question as beginner, I feel like you could learn something from trying to implement this :)
The int mode parameter to addRelation isn't intuitive. I think that it is used to "mirror" the relationship (so that Sibling(Harry,Emma) would also add Sibling(Emma,Harry)), this should be made clearer.
TreeApp
If I'm using your application, how can I know what I'm supposed to enter when this is prompted : System.out.println("Enter option:");. You should print the options with what they do so that users can use your application.
General
The variable names should be more explicit. Looking at this : String p1;, how can I know what it will contain? Naming it firstPerson or something of the like would be good. Short variable names bring nothing of value, especially with IDEs.