生活之井项目

Question

我设计了一个java类来表示一个活井(垂钓)。寻求帮助，我如何可以使这更有效率，或如果有人看到我可以改进的设计，它将不胜感激。

package com.livewell.model;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import org.apache.commons.lang.StringUtils;

/**
 * Represents a live well on a fishing boat. Fish can be added to the live well. As fish are added,
 * only the largest n fish are kept, where n = the defined capacity. The BigFishFactor is used when
 * determining which fish is largest.
 *
 */
public class LiveWell {

    //default values
    private static final int defaultCapacity = 3;
    private static final int defaultMinimumFishLengthInInches = 12;
    private static final int defaultMinimumFishWeightInOunces = 6;
    private static final BigFishFactor defaultBigFishFactor = BigFishFactor.WEIGHT;

    //live well properties
    private final int capacity;
    private final int minimumFishLengthInInches;
    private final int minimumFishWeightInOunces;
    private final BigFishFactor bigFishFactor;

    //live well contents
    private List<Fish> contents = new ArrayList<Fish>();

    /**
     * Constructs a live well using the default values for minimum length, weight, and big fish factor
     */
    public LiveWell(){
        this(null, null, null, null);
    }

    /**
     * Constructs a live well.
     * @param capacity maximum capacity of the live well
     * @param minimumFishLengthInInches minimum length of a fish that will be accepted for addition to the live well
     * @param minimumFishWeightInOunces minimum weight of a fish that will be accepted for addition to the live well
     * @param bigFishFactor factor used when comparing 2 fish (for purposes of determining which fish will be kept)
     */
    public LiveWell(Integer capacity, Integer minimumFishLengthInInches, Integer minimumFishWeightInOunces, BigFishFactor bigFishFactor){
        this.capacity = capacityValidOrProvided(capacity) ? capacity : defaultCapacity;
        this.minimumFishLengthInInches = lengthValidOrProvided(minimumFishLengthInInches) ? minimumFishLengthInInches : defaultMinimumFishLengthInInches;
        this.minimumFishWeightInOunces = weightValidOrProvided(minimumFishWeightInOunces) ? minimumFishWeightInOunces : defaultMinimumFishWeightInOunces;
        this.bigFishFactor = bigFishFactor == null ? defaultBigFishFactor : bigFishFactor;
    }

    private static boolean capacityValidOrProvided(Integer capacity){
        return capacity != null && capacity >= 0;
    }

    private static boolean lengthValidOrProvided(Integer minimumFishLengthInInches){
        return minimumFishLengthInInches != null && minimumFishLengthInInches >= 0;
    }

    private static boolean weightValidOrProvided(Integer minimumFishWeightInOunces){
        return minimumFishWeightInOunces != null && minimumFishWeightInOunces >= 0;
    }

    public int getNumberOfFish(){
        return contents.size();
    }

    public List<Fish> getContents(){
        return Collections.unmodifiableList(contents);
    }

    /**
     * May add a fish to the live well.
     * @param fishLengthInInches the length of the fish being added to the live well
     * @param fishWeightInOunces the weight of the fish being added to the live well
     * @return true if and only if all of the following conditions are satisfied:
     * 
     * -the capacity of the live well is > 0
     * -the length and weight of the fish are >= the minimums defined
     * -the fish is larger than the smallest fish currently in the live well (where larger is defined
     *  by the BigFishFactor)
     */
    public boolean addFishToLiveWell(int fishLengthInInches, int fishWeightInOunces){
        boolean fishAddedToLiveWell = false;
        boolean fishIsKeeper = fishLengthInInches >= minimumFishLengthInInches && fishWeightInOunces >= minimumFishWeightInOunces;
        if(fishIsKeeper){
            Fish fishToAdd = new Fish(fishLengthInInches, fishWeightInOunces);
            int numberOfFish = getNumberOfFish();
            //if we have reached capacity
            if(numberOfFish == capacity){
                if(capacity > 0){
                    //if there are fish
                    if(numberOfFish > 0){
                        //if bigger than the smallest fish, replace the smallest fish
                        Fish smallestFish = contents.get(0);
                        if(fishToAdd.compareTo(smallestFish) > 0){
                            _replaceSmallestFish(fishToAdd);
                            fishAddedToLiveWell = true;
                        }
                    }
                    //if there are no fish, add the fish
                    else{
                        _addFishToLiveWell(fishToAdd);
                        fishAddedToLiveWell = true;
                    }
                }
            }
            //if we are not at capacity, add the fish
            else{
                _addFishToLiveWell(fishToAdd);
                fishAddedToLiveWell = true;
            }
        }
        return fishAddedToLiveWell;
    }

    private void _replaceSmallestFish(Fish fishToReplaceSmallestFishWith){
        contents.remove(0);
        _addFishToLiveWell(fishToReplaceSmallestFishWith);
    }

    private void _addFishToLiveWell(Fish fishToAdd){
        contents.add(fishToAdd);
        Collections.sort(contents);
    }

    public class Fish implements Comparable<Fish> {
        private int lengthInInches;
        private int weightInOunces;

        public Fish(int lengthInInches, int weightInOunces){
            this.lengthInInches = lengthInInches;
            this.weightInOunces = weightInOunces;
        }

        public int getLengthInInches() {
            return lengthInInches;
        }

        public int getWeightInOunces() {
            return weightInOunces;
        }

        @Override
        public int compareTo(Fish o) {
            int compare = 0;
            if(o != null){
                switch(bigFishFactor){
                case COMBO:
                    compare = new Integer(lengthInInches * weightInOunces).compareTo(new Integer(o.lengthInInches * o.weightInOunces));
                    break;
                case LENGTH:
                    compare = new Integer(lengthInInches).compareTo(o.lengthInInches);
                    break;
                case WEIGHT:
                    compare = new Integer(weightInOunces).compareTo(o.weightInOunces);
                    break;
                }
            }
            else{
                compare = -1;
            }
            return compare;
        }

        @Override
        public String toString(){
            return lengthInInches+"in, "+weightInOunces+"oz";
        }

    }

    /**
     * Factor used when determining which of 2 fish will be kept in the live well (which is 'larger')
     */
    public enum BigFishFactor{
        WEIGHT("Fish with the highest weight is deemed to be the largest."),
        LENGTH("Fish with the longest length is deemed to be the largest."),
        COMBO("Fish with the largest (length * width) is deemed to be the largest.");

        private String displayValue;

        private BigFishFactor(String displayValue){
            this.displayValue = displayValue;
        }

        public String getDisplayValue() {
            return displayValue;
        }

    }

    @Override
    public String toString(){
        StringBuilder sb = new StringBuilder();
        sb.append("Maximum Capacity: ").append(capacity);
        sb.append("\n");
        sb.append("Minimum Length: ").append(minimumFishLengthInInches).append(" in.");
        sb.append("\n");
        sb.append("Minimum Weight: ").append(minimumFishWeightInOunces).append(" oz.");
        sb.append("\n");
        sb.append("Big Fish Determining Factor: ").append(bigFishFactor.getDisplayValue());
        sb.append("\n");
        sb.append("*****Live Well Contents****");
        sb.append("\n");
        if(contents.size() == 0){
            sb.append("Live Well Empty");
        }else{
            sb.append(StringUtils.join(contents, "\n"));
        }
        sb.append("\n");
        sb.append("***************************");
        return sb.toString();
    }

}

Answer 1

如果有人看到我在设计上可以改进的地方，我们将不胜感激。

使用null引用.

只有当null是值集的有效元素时，才应该使用它。(因此不需要对该单位进行特殊处理.)

但在您的情况下，输入是无效的，这是一个“带内信号”。

您不需要这样做，因为与调用参数化构造函数中的“空处理”静态方法不同，您可以从无参数构造函数而不是nulls传递默认值。

您可能会争辩说，您必须将该静态方法称为“空处理”，因为您希望用户能够传递nulls，但这是对“简单API”的误解。与其让用户传入nulls，不如以公共常量的形式提供默认值，这些常量可以由用户传入。这样，用户就知道通过读取自己的代码来使用默认值。在传递nulls时，她需要查找您的代码(至少是javadoc)来验证这一点。

命名约定

使用下划线前缀private (成员)方法。Java命名约定含蓄地劝阻了这一点。但是，如果您决定这样做，您应该这样做，因此，也应该使用前缀的static private方法。

immutability

您的类Fish只有getter。因此，我认为它注定是不可变的。那么，你为什么不通过制作鱼final的特性来表达这一点呢？

原始痴迷/关注点分离

公共方法addFishToLiveWell()有两个基本参数，它们是尚未构建的Fish对象的属性。既然您的类Fish是public，为什么不向用户说明她是通过一个类型为Fish的参数来添加fish呢？为什么创建一个鱼对象应该是这个方法的责任？

特性嫉妒/关注点分离

在您的方法addFishToLiveWell()中，在创建鱼之前检查它的属性。

为什么这是这种方法的责任？

既然类Fish是public，那么这个检查不应该也适用于代码的用户吗？

知道你的工具

你用ArrayList来抓鱼。

你自己记录一下名单上的鱼的顺序。

如果要实现这一点，我将使用一个TreeSet，它通过给定的Comparator实现或Comparable接口的元素实现自动地对内容进行排序。

这样做，addFishToLiveWell()方法将更改为：

public boolean addFishToLiveWell(Fish newfish){
    contents.add(newfish);
    if(capacity < contents.size()){
        Fish smallestFishInTank = contents.pollLast();
        return smallestFishInTank == newfish;
    }
    return true;
}

不合理地使用Java enums

您创建了一个Java enum类，并在该enum上使用了方法。

但是，您可以使用这个枚举来使用switch语句来分支执行流。为什么不把实际的比较操作放到枚举中呢？

public enum BigFishFactor {
    WEIGHT("Fish with the highest weight is deemed to be the largest.") {
        @Override
        int compare(Fish f1, Fish f2) {
            return new Integer(f1.getWeightInOunces()).compareTo(f2.getWeightInOunces());
        }

    },
    LENGTH("Fish with the longest length is deemed to be the largest.") {
        @Override
        int compare(Fish f1, Fish f2) {
            return new Integer(f1.getLengthInInches()).compareTo(f2.getLengthInInches());
        }
    },
    COMBO("Fish with the largest (length * width) is deemed to be the largest.") {
        @Override
        int compare(Fish f1, Fish f2) {
            return new Integer(f1.getLengthInInches() * f1.getWeightInOunces())
                    .compareTo(new Integer(f2.getLengthInInches() * f2.getWeightInOunces()));
        }
    };

    private String displayValue;

    private BigFishFactor(String displayValue) {
        this.displayValue = displayValue;
    }

    public String getDisplayValue() {
        return displayValue;
    }

    abstract int compare(Fish f1, Fish f2);
}

在课堂上，Fish：

    @Override
    public int compareTo(Fish o) {
        int compare = 0;
        if (o != null) {
                compare = bigFishFactor.compare(this, o);
        } else {
            compare = -1;
        }
        return compare;
    }

冗余/说谎注释

这些评论没有提供任何补充资料：

// default values
// live well properties
// live well contents

这句话实际上是对读者撒谎：

/**
 * Constructs a live well using the default values for minimum length,
 * weight, and big fish factor
 */

下面的代码传递的是nulls，不是默认值.

标识符名

一般来说，变量名应该以名词开头，但持有booleanS的变量也有例外:它们应该与is或have以及返回boolen的方法一起串。

还有一个方法addFishToLiveWell。它位于一个名为LiveWell的类中，这使得这个名称的这一部分过时了。此外，它还应该添加一个Fish对象(至少在我的重构建议之后)。因此，这个部分的名称也是过时的。

如果要保留两个参数版本，则可以保留Fish部分，但将名称更改为：

 addFishWith(int lengthInInches, int weightInOunces);