首页
学习
活动
专区
圈层
工具
发布
社区首页 >问答首页 >旅行顾问-火车路线

旅行顾问-火车路线
EN

Code Review用户
提问于 2014-10-17 09:50:14
回答 3查看 1.4K关注 0票数 5

向乘坐哪列火车到达目的地的用户提供建议

代码语言:javascript
复制
 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<Station> path)
        {
            Lane color=null;
            ArrayList<Station> changeStation =new ArrayList<Station>();//used to identify when the user needs to change lines
            ArrayList<Object> 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
    }
EN

回答 3

Code Review用户

回答已采纳

发布于 2014-10-17 10:05:31

首先,您的代码中可能有一个错误。

如果trains为空,则在打印结果时会出现问题。

  • 不要在构造函数中编写那么多代码。构造函数应仅用于初始化。
  • 在创建对象的方式上保持一致: ArrayList changeStation =新ArrayList();ArrayList trains=new ArrayList<>();
  • 如果需要注释来描述变量,则将变量命名为“错误”。
  • 让您的代码呼吸,使用一些间距(int i=0;i<( path.size() -1);i++) --这将更容易理解为for(int =0;i
  • 使用计算方法。您应该添加一个计算最短路径的方法,以及另一个负责这些计算值的输出的方法。
  • ArrayList changeStation =新ArrayList();ArrayList trains=new ArrayList<>();应该是List changeStation =新ArrayList<>();List trains=new ArrayList<>();
  • 从方法返回方法名称的含义,并相应地命名“接收”变量,Lane train=e.getLaneColor();因此枚举Lane应该变成LaneColortrain应该变成例如currentLaneColor
票数 11
EN

Code Review用户

发布于 2014-10-17 10:03:37

构造器

构造函数中似乎包含了所有内容。这不是很好的练习。您可以使用构造函数初始化所有变量,这样您就可以得到如下内容:

代码语言:javascript
复制
TripAdvisor()
{
    source = new Station();
    destination = new Station();
    cost = 0;
}

TripAdvisor(Station source, Station dest)
{
    this.source = source;
    this.destination = dest;
    cost = 0; //This isn't necessary, just nice to explicitly state 
}

在此之后,您会将您的方法分成不同的调用。您可能有一个输出路径的void Print()方法,以及一个重写的string toString()方法,可能吗?

“计算”方法

最重要的是,您可以有一个ArrayList<Station> calculatePath()和一个ArrayList<Station> calculatePath(Station source, Station dest),它们在算法中运行并返回应该走的路径。请注意,您必须检查站点是否已在no-args方法中设置。

未使用(但有用)变量

虽然,写完所有这些之后,你实际上并不是在使用源或目的地,所以有可能需要进一步的重新设计,但我现在无法帮助,因为我不知道您在本课程之外做什么。但是,我可以说,如果你像你一样代表网络,给这个类一个私有变量的拷贝(可能是在构造函数中传入),然后给它一个源和目的地是设计一个执行算法的类的好方法。

模式?

这种实现意味着,当您想要一个新的路由规划时,您不必为您的TripAdvisor类创建一个新实例,您可以重用旧的实例并节省内存。这与单例模式相当接近,因此您可能希望对它们的最佳实践以及使用它们的好方法进行一些研究。

票数 10
EN

Code Review用户

发布于 2014-10-17 11:08:09

构造函数的目的是创建一个对象。这里的TripAdvisor对象的用途是什么?作为一个对象,它似乎没有目的。您“创建”一个TripAdvisor对象只是为了获得输出上打印的内容。构造函数不是正确的元素,您需要的是一个方法,让我们称它为printShortestPath

sourcedestinationcost成员字段未使用。把他们移走。

在变量和方法参数声明中使用像List<>这样的接口类型而不是ArrayList<>,除非您对实现类型有特定的需求,如下所示:

代码语言:javascript
复制
public static void printShortestPath(List<Station> path) {
    List<Station> changeStation = new ArrayList<>();

这个变量非常混乱:

车道颜色=空;

莱恩是个颜色吗?再往下看,您也有一个Lane train。其中任何一个都是更好的选择:

代码语言:javascript
复制
Lane lane = null;
LaneColor laneColor = null;
Train train = null;

ArrayList<Object> trains不同,您应该使用Java的类型系统,并像下面这样声明:

代码语言:javascript
复制
List<Lane> trains = new ArrayList<>();

然而,与前面关于Lane color的观点一样,这是令人困惑的。如果这是一张车道列表,那么最好将其命名为lanes

我认为你需要重新考虑你的对象和它们之间的关系。很难找到正确的名称来描述程序应该如何工作,这表明设计中可能存在一个缺陷。当所有类和对象名称变得自然时,代码将变得更容易理解。

对于(连接e: current.adjacencies) { if (e.getTarget().equals(next)) {莱恩列车= e.getLaneColor();trains.add(列车);if (!e.getLaneColor().equals (color ) &( color = null)) {changeStation.add(当前);}.equals=e.getLaneColor()};

这里有几点需要改进:

  • connectionConnection对象的更好的名称,而不是e
  • 在这个循环中,要跟踪color变量发生了什么有点困难。也许可以用一种更自然、更容易遵循的方式重新组织代码
  • Lanecolortrain的命名不一致是读者的一个主要负担。你真的需要一些更一致、更清晰、更自然的东西。
  • (color != null)周围的括号是不必要的
票数 8
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/66989

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档