patternjavaModerate
Trip Advisor - Train routes
Viewed 0 times
advisortriproutestrain
Problem
Gives advice to the user on which trains to take to reach a destination
public class TripAdvisor {
private Station source;
private Station destination;
private double cost;
/**
*
* @param path the shortest path returned by the shortestPath method implementation
*/
TripAdvisor(ArrayList path)
{
Lane color=null;
ArrayList changeStation =new ArrayList();//used to identify when the user needs to change lines
ArrayList trains=new ArrayList<>();//list of trains the user need to take to reach a station
for(int i=0;i<(path.size()-1);i++)
{
Station current=path.get(i);
Station next=path.get(i+1);
for (Connection e : current.adjacencies)
{
if(e.getTarget().equals(next))
{
Lane train=e.getLaneColor();
trains.add(train);
if(!e.getLaneColor().equals(color)&&(color!=null))
{
changeStation.add(current);
}
}
color=e.getLaneColor();
}
}
System.out.println("trains"+ "$" +trains);
if(trains.size()==1)
{
int q;
System.out.println("Take a "+ trains.get(0) +"liner from" + path.get(0) +"to reach" + path.get( q=path.size()));
}
else
{
System.out.println("Take a "+ trains.get(0) +"liner from" + path.get(0)+ "Get down at" + changeStation.get(0));
}
}
}
/**
*
* @author PrasannaAarthiB
* This has all the lines in the metro.They are represented by different colors
*/
public enum Lane{
RED,BLUE,GREEN,YELLOW,BLACK
}Solution
First of all, you have a possible bug in your code.
If
-
Don't code that much inside the constructor. The constructor should be used for initializing only.
-
Be consitent in the way you create objects
-
If you need comments to describe a variable, you have named the variable poorly
-
Let your code breathe, use some spacing
this would be easier to read like
-
use methods for calculations. You should add a method which calculates the shortest path and another one that would be responsible for the output of these calculated values.
-
code against interfaces instead of implementations so
should be
-
Return from methods what the methodnames implies and name the "receiving" variable accordingly
So the enum
If
trains is empty you will get a problem at the printing of the results.-
Don't code that much inside the constructor. The constructor should be used for initializing only.
-
Be consitent in the way you create objects
ArrayList changeStation =new ArrayList();
ArrayList trains=new ArrayList<>();-
If you need comments to describe a variable, you have named the variable poorly
-
Let your code breathe, use some spacing
for(int i=0;i<(path.size()-1);i++)this would be easier to read like
for (int i = 0 ; i < path.size() - 1; i++)-
use methods for calculations. You should add a method which calculates the shortest path and another one that would be responsible for the output of these calculated values.
-
code against interfaces instead of implementations so
ArrayList changeStation =new ArrayList();
ArrayList trains=new ArrayList<>();should be
List changeStation =new ArrayList<>();
List trains=new ArrayList<>();-
Return from methods what the methodnames implies and name the "receiving" variable accordingly
Lane train=e.getLaneColor();So the enum
Lane should be become LaneColor and train should become e.g. currentLaneColorCode Snippets
ArrayList<Station> changeStation =new ArrayList<Station>();
ArrayList<Object> trains=new ArrayList<>();for(int i=0;i<(path.size()-1);i++)for (int i = 0 ; i < path.size() - 1; i++)ArrayList<Station> changeStation =new ArrayList<Station>();
ArrayList<Object> trains=new ArrayList<>();List<Station> changeStation =new ArrayList<>();
List<Lane> trains=new ArrayList<>();Context
StackExchange Code Review Q#66989, answer score: 11
Revisions (0)
No revisions yet.