我经常申请工作只是为了测试我的技能。我最近申请了一个,不得不提交一个Java问题,但是被拒绝了。有人可以简短地回顾我的应用程序,并告诉我代码中最严重的错误是什么吗?我对自我改善很感兴趣,但我很难找出我需要改进的地方。

这里是问题所在:


简介

假设您正在使用一个名为IMS的新仓库库存管理系统。 IMS将负责实地单站点仓库内的库存跟踪。 IMS将跟踪产品在仓库中的命名物理位置以及每个产品的库存水平。 IMS将部署到繁忙的仓库中,以支持许多与单个终端和客户合作的拣货员和补货员。
库存水平的更新将得到实时处理,以防止
拣货员试图拣选产品缺货。

假设

每个产品将被存储在仓库中的一个且只有一个指定的位置。库存调整可以是加性(补充)或减性(增量)。无需跟踪其他产品信息即可。

问题

在Java中,实现IMS的提货和补货例程
系统。 IMS接口将是第一个要实现的组件。所有相关领域对象都必须已经分发给依赖它的其他团队。


GitHub

以下是重要的部分:

IMS.java(提供给我们,用于创建清单)

public interface IMS {
  PickingResult pickProduct(String productId, int amountToPick);
  RestockingResult restockProduct(String productId, int amountToRestock);
}


Inventory.java(实现IMS及其方法)

import java.util.Hashtable;
import java.util.Set;

public class Inventory implements IMS {

    private Hashtable<String, Product> products = new Hashtable<String, Product>();
    private Hashtable<String, Location> locations = new Hashtable<String, Location>();

    public void addLocation(Location location) {
        locations.put(location.getName(), location);
    }

    public void addProduct(Product product, Location location, int amount) {
        products.put(product.getName(), product);
        location.restockProduct(product.getName(), amount);
    }

    /*
     * returns location that has the product when you pass a product ID to it
     */
    public Location getLocationByProductID(String productId) throws Exception {
        Set<String> keys = locations.keySet();
        Location result = null;
        for (String key : keys) {
            Location current = locations.get(key);

            if (current.hasProduct(productId)) {
                if (result == null) {
                    result = current;
                } else {
                    // oh uh, we found the product in two locations, warning the
                    // user
                    throw new Exception("product found in two locations");
                }
            }
        }

        return result;
    }

    public void displayInventory() {
        Set<String> keys = locations.keySet();
        for (String key : keys) {
            Location current = locations.get(key);

            System.out.println(current.getName());
            current.displayInventory();
        }
        System.out.println("");
    }

    @Override
    public PickingResult pickProduct(String productId, int amountToPick) {
        Location loc = null;
        Product product = products.get(productId);

        // transaction data
        boolean transactionSuccess = false;
        String transactionMessage = "";

        try {
            loc = getLocationByProductID(productId);

            if (loc == null) {
                throw new Exception("Product " + productId + " wasn't found in any location");
            }

            int amount = loc.getAmountOfProduct(productId);
            if (amount < amountToPick) {
                throw new Exception("We do not have enough products for this transaction (quantity available: " + amount
                        + "), please restock the product " + productId + "!");
            }

            loc.pickProduct(productId, amountToPick);

            transactionSuccess = true;
            transactionMessage = "We have successfully picked " + amountToPick + " items of " + productId;
        } catch (Exception e) {
            transactionMessage = e.getMessage();
        }

        return new PickingResult(transactionSuccess, transactionMessage, loc, product, amountToPick);
    }

    @Override
    public RestockingResult restockProduct(String productId, int amountToRestock) {
        Location loc = null;
        Product product = products.get(productId);

        // transaction data
        boolean transactionSuccess = false;
        String transactionMessage = "";

        try {
            loc = getLocationByProductID(productId);

            if (loc == null) {
                throw new Exception("Location wasn't found");
            }

            loc.restockProduct(productId, amountToRestock);

            transactionSuccess = true;
            transactionMessage = "We have successfully restocked " + amountToRestock + " items of " + productId;
        } catch (Exception e) {
            transactionMessage = e.getMessage();
        }

        return new RestockingResult(transactionSuccess, transactionMessage, loc, product, amountToRestock);
    }

}


Location.java

import java.util.Hashtable;
import java.util.Set;

public class Location {

    private String name;
    private Hashtable<String, Integer> amountByProductID = new Hashtable<String, Integer>();

    public Location(String name) {
        this.name = name;
    }

    public void restockProduct(String name, Integer amount) {
        Integer previousAmount = getAmountOfProduct(name);

        amountByProductID.put(name, previousAmount + amount);
    }

    /*
     * returns false if we don't have enough product
     */
    public boolean pickProduct(String name, Integer amount) {
        Integer previousAmount = getAmountOfProduct(name);

        if (previousAmount < amount) {
            // not enough items
            return false;
        }

        amountByProductID.put(name, previousAmount - amount);

        return true;
    }

    public boolean hasProduct(String productId) {
        return amountByProductID.get(productId) != null;
    }

    public Integer getAmountOfProduct(String productId) {
        Integer amount = amountByProductID.get(productId);

        return amount != null ? amount : 0;
    }

    public String getName() {
        return this.name;
    }

    public void displayInventory() {
        Set<String> keys = amountByProductID.keySet();
        for (String productId : keys) {
            Integer amount = amountByProductID.get(productId);

            System.out.println("  " + productId + ": " + amount.toString());
        }
    }
}


(我重命名了一些变量,以便其他候选人无法搜索它。)

评论

您是否尝试要求公司提供有关您的代码的反馈?

Tpdi已经提到了它,但是我敢肯定,您不做进一步的主要原因是因为这里的关键是描述暗示着高度并发的情况-而且您的代码绝对不是线程安全的。

您的程序不是线程安全的。很容易以负数量的产品告终。鉴于文本,线程安全显然是必需的。

为什么产品在库存中,但数量在库存中?

#1 楼

Maps

Q4312079q或HashMap(如果需要线程安全性)优于旧版ConcurrentHashMap(如Javadoc中所述):


在Java 2平台v1.2中,对该类进行了改进以实现Hashtable接口,使其成为Java Collections Framework的成员。与新的集合实现不同,Map是同步的。如果不需要线程安全的实现,建议使用Hashtable代替HashMap。如果需要线程安全的高并发实现,那么建议使用Hashtable代替ConcurrentHashMap


此外,如果您需要遍历Hashtable的条目,有执行此操作的Map方法。因此,代替:

Set<K> keys = map.keySet();
for (K key : keys) {
    V value = map.get(key);
    // do something with key and value
}


这样做会更高效(更不用说紧凑了):
编辑:如果只需要遍历这些值,则@ njzk2的注释指出存在Map.entrySet()

for (Entry<K, V> entry : map.entrySet()) {
    // use temporary variables if required
    K key = entry.getKey();
    V value = entry.getValue();
    // do something with key and value
}


抛出values() s

例如:

for (V value : map.values()) {
    // do something with value
}


您可能要考虑使用自定义Exception,例如Exception,以便您可以以编程方式描述特定错误。这也将消除异常宽的LocationNotFoundException子句,该子句有时会被皱眉,因为它实际上是一个包罗万象的东西,并暗示代码库甚至不确定在这里可以抛出什么。同样,立即扔掉并立即捕获这种样式的catch (Exception e)也似乎是用来控制程序流程的。好像您正在使用它跳过最后几行一样,即用于构造一个成功的Exception有效负载。遇到无效位置后,最好返回一个“不成功”的RestockingResult有效载荷。 >编辑


RestockingResultPickingResult


相比RestockingResult,通常建议使用原始int,因为它可以防止Integer的使用(错误?),除非您确实需要一些东西来表示没有一个int。例如,在下面的方法中:

try {
    loc = getLocationByProductID(productId);
    if (loc == null) {
        throw new Exception("Location wasn't found");
    }
    loc.restockProduct(productId, amountToRestock);
    transactionSuccess = true;
    transactionMessage = "We have successfully restocked " + amountToRestock + 
                            " items of " + productId;
} catch (Exception e) {
    transactionMessage = e.getMessage();
}
return new RestockingResult(transactionSuccess, transactionMessage, 
            loc, product, amountToRestock);


当然,您实际上在Integer中进行了null检查,但是没有什么可以阻止呼叫者意外地将int传递为nullgetAmountOfProduct(String)将产生一个amount

此外,如果您恰巧是使用Java 8,则还有更好的null方法:

public void restockProduct(String name, Integer amount) {
    Integer previousAmount = getAmountOfProduct(name);
    amountByProductID.put(name, previousAmount + amount);
}


此使用方法参考0 + null为给定键添加任何现有值和新值。创建,又名钻石算子,可用于简化操作:

public void restockProduct(String name, int amount) {
    amountByProductID.merge(name, Integer.valueOf(amount), Integer::sum);
}


评论


\ $ \ begingroup \ $
很高兴为您提供帮助! :)希望您继续在这里发表更多问题和答案!
\ $ \ endgroup \ $
– h.j.k.
15年9月28日在15:49

\ $ \ begingroup \ $
该键未使用,因此您甚至可以枚举values()
\ $ \ endgroup \ $
– njzk2
2015年9月29日,下午2:54

\ $ \ begingroup \ $
@ njzk2根据您的输入更新了我的答案,谢谢!
\ $ \ endgroup \ $
– h.j.k.
2015年9月29日,下午3:31

\ $ \ begingroup \ $
让我指出,当您开始定期枚举地图的值时,表明您做错了事
\ $ \ endgroup \ $
– njzk2
2015年9月29日在13:27

#2 楼

h.j.k对Hashtable非常有礼貌。不推荐使用Hashtable的Java 2于1998年问世。那是17年前了。实际代码的第一行以Hashtable开头。如果我是一名招聘经理,我将立即停止阅读。 IT的发展非常迅速,并且使用17年前不推荐使用的类确实非常糟糕。如果请求没有答案,我将其写在返回值中,而不是抛出异常。可能会在库存管理系统中提出不可能的请求,并且我不希望整个系统因为一个不可能的请求而停止。应该为真正的例外事件保留例外,并且我认为错误请求不是清单系统中的例外事件。使用标准返回值以外的异常也会产生性能损失。其他人可能会争辩说,使用异常是在此处进行处理的正确方法。他们甚至有可能要求您使用异常来符合其API。

评论


\ $ \ begingroup \ $
谢谢您的回答。我是一名PHP开发人员,只是出于业余爱好从事Java。这证明我还有很多要研究的地方:/
\ $ \ endgroup \ $
–xtrimsky
15年9月28日在13:07

\ $ \ begingroup \ $
请注意,不建议使用Hashtable(至少不支持Java 8)。在大多数情况下,HashMap可能是更好的解决方案,但是Hashtable可以在适当的地方不受限制地使用,并且受到完全支持。一个真正不推荐使用的方法的示例是`Thread.stop(...)。另请参阅Java 8中已弃用的API列表。
\ $ \ endgroup \ $
– siegi
2015年9月29日上午10:49

\ $ \ begingroup \ $
@siegi感谢您的澄清。我不是正式意义上的“弃用”,而是在建议使用HashMap的意义上。
\ $ \ endgroup \ $
–toto2
2015年9月29日在11:34

#3 楼

虽然提到了位置,但是在您指定的界面中不存在该位置,因此,在没有明确定义的要求的情况下,我不确定是否会麻烦它。也就是说,即使您完美地实现了位置,客户端程序也无法在界面中设置位置。 (可以在PickingResult和RestockingResult中返回一个位置,但是任何此类位置都是任意的-您最好为每个位置返回“ Someplace”或“ in the Warehouse”。)

有一个IMS界面中的其他提示;它返回PickingResult和RestockingResult的事实非常清楚地表明不需要抛出异常。当他们失败时?它们何时会失败?

PickingResult和RestockingResult应该是具体的叶子类,还是作为接口/抽象类更好? (即,像Scala的抽象Option一样,具有具体的子类Some()和None。)

要求之一是“该接口的实现和对共享数据的访问必须是线程安全的。”在某些地方代码不是线程安全的吗? (提示:是,有几个。)提示:请参见https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/atomic/AtomicInteger.html。

您需要确切地了解如何使用它(尤其是重试),它不能解决所有并发问题,但可以解决大多数并发问题。 (哪个问题不能解决,您可以怎么解决?)

评论


\ $ \ begingroup \ $
谢谢您的回答:)。我没有选择它作为“答案”,但它很有帮助!
\ $ \ endgroup \ $
–xtrimsky
15年9月28日在13:05

\ $ \ begingroup \ $
我认为atomicinteger在这里还不够。我们需要在一个原子操作中获取,测试,更改,设置它的值。我将在正在提货/进货的产品上有一个同步块
\ $ \ endgroup \ $
– njzk2
2015年9月29日,下午2:58

\ $ \ begingroup \ $
我们可以进行get / test / compareandset循环
\ $ \ endgroup \ $
–tpdi
2015年9月29日在6:05