HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Trip Advisor - Train routes

Submitted by: @import:stackexchange-codereview··
0
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 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. currentLaneColor

Code 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.