最近,我接受了编码测试以完成潜在客户的学习。这是FizzBu​​zz类型的事情,有两个小时的时间限制。我有一个要求编写基本的FizzBu​​zz,然后添加一个特例,然后添加一个报告。我没有接受采访,他们也没有返回任何反馈,所以我不知道为什么。你能看看这个,让我知道我哪里出问题了吗?在我看来,这一切都很好。编码测试要求我编写fizzbuzz,显示结果,进行更改,显示结果,然后进行另一次更改并显示结果。这就是为什么使用三种方法的原因-显示测试各部分的结果。我通常在代码中不会像这样重复。最好单独检查每个方法,就像它是代码中唯一的方法一样。

package com.engineerdollery;

import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static java.util.stream.Collectors.*;

public class FizzBuzz {
    private static final String NUMBER = "\d+";

    public String basic() {
        return IntStream.rangeClosed(1, 20)
                .parallel()
                .mapToObj(i -> i % 15 == 0 ? "fizzbuzz"
                        : i % 3 == 0 ? "fizz"
                        : i % 5 == 0 ? "buzz"
                        : Integer.toString(i))
                .collect(joining(" "));
    }

    public String lucky() {
        return IntStream.rangeClosed(1, 20)
                .parallel()
                .mapToObj(i -> Integer.toString(i).contains("3") ? "lucky" // this is the only change from basic()
                        : i % 15 == 0 ? "fizzbuzz"
                        : i % 3 == 0 ? "fizz"
                        : i % 5 == 0 ? "buzz"
                        : Integer.toString(i))
                .collect(joining(" "));
    }

    public String counter() {
        List<String> fizzBuzzList = IntStream.rangeClosed(1, 20)
                .parallel()
                .mapToObj(i -> Integer.toString(i).contains("3") ? "lucky"
                        : i % 15 == 0 ? "fizzbuzz"
                        : i % 3 == 0 ? "fizz"
                        : i % 5 == 0 ? "buzz"
                        : Integer.toString(i))
                .collect(Collectors.toList());

        Map<String, Long> countMap = fizzBuzzList
                .parallelStream()
                .collect(groupingBy(s -> s.matches(NUMBER) ? "integer" : s, counting()));

        // reports

        String fizzbuzz = fizzBuzzList.parallelStream().collect(joining(" "));

        String counts = countMap.entrySet().parallelStream()
                .map(e -> e.getKey() + ": " + e.getValue())
                .collect(joining("\n"));

        return fizzbuzz + "\n" + counts;
    }
}


测试

package com.engineerdollery;

import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class FizzBuzzTest {
    public static void main(String[] args) {
        System.out.println(new FizzBuzz().counter());
    }

    @Test
    public void basic() {
        FizzBuzz fizzBuzz = new FizzBuzz();
        String actual = fizzBuzz.basic();
        assertEquals("1 2 fizz 4 buzz fizz 7 8 fizz buzz 11 fizz 13 14 fizzbuzz 16 17 fizz 19 buzz", actual);
    }

    @Test
    public void lucky() {
        FizzBuzz fizzBuzz = new FizzBuzz();
        String actual = fizzBuzz.lucky();
        assertEquals("1 2 lucky 4 buzz fizz 7 8 fizz buzz 11 fizz lucky 14 fizzbuzz 16 17 fizz 19 buzz", actual);
    }

    @Test
    public void counter() {
        FizzBuzz fizzBuzz = new FizzBuzz();
        String actual = fizzBuzz.counter();
        assertTrue(actual.contains("1 2 lucky 4 buzz fizz 7 8 fizz buzz 11 fizz lucky 14 fizzbuzz 16 17 fizz 19 buzz"));
        assertTrue(actual.contains("fizz: 4"));
        assertTrue(actual.contains("buzz: 3"));
        assertTrue(actual.contains("fizzbuzz: 1"));
        assertTrue(actual.contains("lucky: 2"));
        assertTrue(actual.contains("integer: 10"));
    }
}


评论

(该评论本来是我的回答,对吗?)-IMO并不是要实施模式(尽管这是实现此目标的宝贵工具),而是要确定变更点,隐式假设以及企业所要从事的事情可以决定一时兴起...然后编写代码,这些代码仅在您的假设破灭且规格确实发生变化时才需要进行最小的更改:这就是我关于灵活性/可维护性的观点。请记住,FizzBu​​zz是一种练习,是对现实世界代码的隐喻。

请注意,答案尚未涵盖。关于格式化,有些行太长了。尽管不如答案中的问题那么重要,但这仍然是编写可读代码的一个因素。

好的程序员还应该编写自己的代码,使其易于理解并带有注释。与仅使用标准的For循环相比,使用Java 8的功能使其可读性较差,而标准的For循环已有较长的时间了。当然,FizzBu​​zz任务非常简单,但是注释不会受到损害。

@EngineerDollery注释并不总是意味着需要重构代码。说绝对是荒谬的。

blog.codinghorror.com/code-tells-you-how-comments-tell-you-why

#1 楼


然后添加一个特例


他们很可能在寻找使代码变得灵活和可维护的方法,并且向他们展示了一些原始内容。而是技术知识。

Fizzbuzz逻辑是在三个不同的地方编写的,对此我没有印象。

如果您要添加另一个“特殊情况”并保持相同的结构,因为时间紧迫,您可能需要将其复制并重新粘贴到第四个位置...然后当需求发生变化并且您要负责将所有37交换时,您有太多地方可以进行这些更改,并且结果仍然会散发复制粘贴代码。 br />
映射和并行性看起来像是使用Java 8功能的借口,并且坦率地说过分杀人且无用-您要迭代20个值(不是fizzbuzz应该是1-100吗?),而不是2千万。是否有在某处使用正则表达式的要求?如果不是这样,我将无法理解正则表达式在fizzbuzz提交中的功能。不过,我本可以以这种方式编写三元组,以更好地揭示其逻辑结构:

public String lucky() {
    return IntStream.rangeClosed(1, 20)
            .parallel().mapToObj(i -> 
                Integer.toString(i).contains("3") 
                    ? "lucky" // this is the only change from basic()
                    : i % 15 == 0 
                        ? "fizzbuzz"
                        : i % 3 == 0 
                            ? "fizz"
                            : i % 5 == 0 
                                ? "buzz"
                                : Integer.toString(i))
            .collect(joining(" "));
}


...然后我将对此进行研究并思考如何可以减少嵌套。也许只是我一个人,但是我发现这种格式化嵌套三元组的方式...


return condition.boolExpression()
       ? true
       : condition.boolExpression()
           ? true
           : false;



...使其更容易完全一目了然地掌握正在发生的事情,并且更容易发现值得被提取到自己功能中的地方。例如:

public String lucky() {
    return IntStream.rangeClosed(1, 20)
            .parallel().mapToObj(i -> fizzbuzz(i))
            .collect(joining(" "));
}


我不知道Java是否这样做,但是在c#中您可以使用方法组语法。在Java中可能看起来像这样:

public String lucky() {
    return IntStream.rangeClosed(1, 20)
            .parallel().mapToObj(fizzbuzz)
            .collect(joining(" "));
}


无论如何,这里的关键概念是抽象。这与“表面风格问题”和可读性的主观概念无关,而与从问题中提取抽象有关。您可以在excel中嵌套一堆3函数,并达到相同的抽象水平! >这也是一个四层嵌套的条件结构,需要为每个新的“特殊情况”提供新的层次。通过将循环的整个主体移到其自己的函数中,您的代码将更具表现力,并且感觉更加清晰。记录规格说明-他们测试输出,而不是规格-并且记录的只是他们正在测试的方法的名称。它们的命名方式不是标准的(即“ givenConditionThenSomething”),如果突然需要将5(或简单的迭代次数)更改为其他任何内容,则很难维护它们。

评论


\ $ \ begingroup \ $
这三种实现方法仅用于说明代码的演变。我已经在提交中对此效果进行了注释,并且通常只有一种方法随规范一起发展。我希望他们看到单独的步骤。
\ $ \ endgroup \ $
–软件工程师
16年4月27日在15:04

\ $ \ begingroup \ $
正则表达式用于counter()函数。
\ $ \ endgroup \ $
–软件工程师
16年4月27日在15:08

\ $ \ begingroup \ $
关于测试的观点-就是这样。我是根据技术规范专门编写它们的-实际上是复制示例。我不应该那么懒,但是有时间限制。
\ $ \ endgroup \ $
–软件工程师
16年4月27日在15:11

\ $ \ begingroup \ $
“我已经在提交的内容中对此进行了注释” ...作为一名面试官,我对此一点都不感兴趣。我希望看到您可以向FizzBu​​zz申请(这是微不足道的)我期望的纪律水平,并且需要针对真正复杂问题的防弹解决方案。我得到的消息是“这(复制粘贴,过度复杂)是我编写代码的方式,并且已经知道它并不是您真正想要的,所以我已经做到了”。
\ $ \ endgroup \ $
–朱莉娅·海沃德(Julia Hayward)
16年4月28日在8:41

\ $ \ begingroup \ $
感谢朱莉娅(Julia)–首先,我对这种方法感到厌倦。您已经确认这是个坏主意,我应该坚持使用脚本。感谢您的见解。
\ $ \ endgroup \ $
–软件工程师
16-4-29的0:44



#2 楼

Java 8在所有IDE中都具有良好的支持,但是仍然存在一些差异。例如,您的代码原样未使用最新的Eclipse版本(Mars.2)进行编译。在JDK 1.8.0_51的命令行上,它可以正常编译。我希望他们不会与您接触,因为他们使用Eclipse进行了测试。调用<String>时,请选择Stream管道。
硬编码值
您只能从1到20进行嗡嗡声。这些数字是硬编码的。最好使这些方法以最大值作为参数。
public String basic(int max) {
    return IntStream.rangeClosed(1, max) //...
}

这将使它们更通用。另外,您正在对fizzbuzz单词进行硬编码;考虑使它们恒定。您还应该尝试在专用方法中提取诸如以下映射方法:心里。但是,这并不意味着每个管道都应该并行化。例如,您当前对所有Stream管道都有:实际上,这可能会影响性能。在走并行路线之前,您应该始终测量差异以确定是否值得。微基准测试并非易事。如果要这样做,建议您使用有助于在Java中创建基准的JMH框架。最好将系统行分隔符与mapToObj一起使用。
通配符导入语句
您正在使用
i -> i % 15 == 0 ? "fizzbuzz"
    : i % 3 == 0 ? "fizz"
    : i % 5 == 0 ? "buzz"
    : Integer.toString(i)

导入所有收集器。这通常不是一个好习惯。您要导入与之交互的特定类。在这种情况下,您需要
IntStream.rangeClosed(1, 20).parallel() //...


评论


\ $ \ begingroup \ $
泰,这太好了。并行:我对此并不感兴趣,因为没有任何使其变得特别有效的方法,我对证明这一事实比改善性能更可记录这一事实更感兴趣。 import。*是一个严重的错误-我刚安装了IntelliJ的新副本,有时默认情况下会执行此操作,而我没有注意到-我讨厌该构造,并且如果我付钱就不会使用它注意。系统行分隔符非常有用-它将在我自己的系统上的任何地方中断。
\ $ \ endgroup \ $
–软件工程师
16-4-27在15:13



\ $ \ begingroup \ $
将数字和单词提取为常量有什么意义?有时,我们在采用最佳实践(例如避免使用幻数)方面走得太远了。硬编码作为业务逻辑一部分的常量通常是可以的。提取它们只会使它们脱离业务逻辑代码。这是过早概括的一种形式。
\ $ \ endgroup \ $
–约翰·库格曼(John Kugelman)
16 Apr 27 '21:56

\ $ \ begingroup \ $
@JohnKugelman在fizzbuzz案例中很难说,因为它是如此人为,但我认为3和5的确在这里并不意味着3和5。它们是“ fizz”和“ buzz”的当前数字表示形式,无论其含义如何。我同意声明final int FIVE = 5会很愚蠢;做最后的int FIZZ = 3;是相当合理和可取的。也许我不会在类中将其作为字段类,也许哈希表会更好,或者一些外部资源,或者只是局部变量,我不知道,但是标记一个带有数字意义的数字是有价值的值。
\ $ \ endgroup \ $
–萨拉
16年4月28日在7:26

\ $ \ begingroup \ $
但是提取FIZZ = 3的价格是多少呢?有什么帮助?它增加了一个间接层,在真空中它是负面的,因此应该有一些好处大于负面。我没看到无需使用代码“说明” 3是什么。代码本身对此进行了解释:i%3 == 0? “嘶嘶响”准确地告诉您3的意思。我们并不总是需要常量。代码也有含义。我想我已经对此进行了很多练习,但这是我的一个小烦恼:痴迷于应用最佳实践,而没有考虑这样做的实际原因。
\ $ \ endgroup \ $
–约翰·库格曼(John Kugelman)
16年4月28日在12:24

\ $ \ begingroup \ $
@JohnKugelman我的答案没有说出5。硬编码部分是IntStream.rangeClosed(1,20)的最大范围。
\ $ \ endgroup \ $
–Tunaki
16-4-28在15:34



#3 楼

如果您的语气有些刺耳,我深表歉意,但希望我能提供一些有价值的建议。

对我来说,Fizzbuzz涉及三件事:


有循环吗?
有功能吗?
代码可以运行吗?可为该输入打印的内容;那是fizzbuzz的核心。我了解您将转换埋入lambda并映射到其上,但是只能通过无参数接口访问,该接口无法独立访问或测试。然后,您将其复制/粘贴到report中,而不是将其分解成一个大的红色标记。如果我希望您从1迭代到21,则所有测试都将失败。这本来是应该重构到核心逻辑的接口的一个线索,但是相反,您很高兴地编写了许多脆弱的条件,这些条件并不能使人们对代码运行良好充满信心。

您显示出自己知道IntStream之类的奇特类型,但是在进行诸如计数fizz的数量之类的基本操作时,您选择对字符串actual.contains("fizz: 4")进行测试。做更多的事情。实际上,countMap是报告。将countMap移到其旁边,并使其作为返回值。在其他地方进行印刷。

平行印刷是一种令人难以置信的分散注意力的事物。完全不必要的任务,有点令人担忧。如果我是一名潜在的雇主,我想知道您还会在我的代码库中插入其他不必要的垃圾。以后很容易做到这一点,并且可以说它们无论如何都不应该移动,因为它们与特定的转换逻辑有着独特的联系。更改导入语句也很简单,因此我也不会太反对您。

我认为您应该争取KISS。您写了96行,我认为足够的行就足够了,而没有写出好的NUMBER。您表示与编写整洁的极简代码相比,您对编写许多具有奇特功能的行更感兴趣。或出于任何数量或原因再次与您联系。不要流汗太多。

评论


\ $ \ begingroup \ $
不要道歉。我来这里要求审查-我希望受到批评:)
\ $ \ endgroup \ $
–软件工程师
16年4月28日在0:51

#4 楼

您的解决方案可能看起来与他们自己编写代码的方式有很大不同,因此对于他们的要求来说似乎完全过头了。

您基本上已经告诉他们,您将遇到一个简单的问题,并找到一个复杂的,难以理解,难以调试且难以维护的解决方案。如果您的反应是“但并没有那么复杂,我只是在映射东西和一个正则表达式”,那就是问题所在:您无需为这个问题进行映射。

对于此任务,他们只是想看看您是否可以编写for循环并使用模运算符或跟踪2个计数器并将其重置。公司或某些面试官会对您使用该语言的更高级功能印象深刻。显然不是这个。

对于特殊的幸运盒,尚不清楚他们是否还希望您仍然打印嘶嘶声。也许他们想看看您是否要澄清。

#5 楼

其他人提到您的代码是过大的,但是我看到的最大问题是并行性。坦率地说,它无法始终提供预期的结果。对于观察者来说,看起来您使用的是您并没有真正理解的功能,以便看起来不错,结果您编写了错误的代码。您的并行性将不会总是产生正确的结果,并且希望您会理解,如果您使用该功能。问题。


编辑,根据评论:


看起来我实际上是错的。但是,问题在于,花了很多时间来挖掘Java 8实现的实质内容,才能解决这一问题。与提供的@Tunaki链接相比,以下内容提供了更好的细节:https://stackoverflow.com/questions/29710999/is-collect-guaranteed-to-be-order-on-parallel-streams?lq=1那么我该怎么办这个答案。也许应该删除它,但是在某些方面还是有意义的,因为这很可能是审稿人的想法。 – Necreaux


评论


\ $ \ begingroup \ $
我认为并行性不会破坏输出的顺序。 parallel()仅允许将映射函数应用于各个线程中的每个i,因此它们在顺序执行时不必彼此等待。映射完成后,收集器将按输入顺序收集所有结果。此处有更多详细信息。如果映射函数本身存在sysout,则该顺序将被破坏。
\ $ \ endgroup \ $
–醋
16-4-28在4:57



\ $ \ begingroup \ $
我必须同意@Vineet。这个答案似乎不正确。从理论意义上讲,他对并行的使用是完全可以的。并行映射可以并行工作。然后收集将保留订单。并行的主要问题是过大,尽管我没有对其进行基准测试,但考虑到首先需要使用串行的机器,我怀疑它比串行的版本慢。
\ $ \ endgroup \ $
–黛尔
16-4-28的7:19

\ $ \ begingroup \ $
@EngineerDollery不,这个答案是完全错误的。并行将起作用,并且始终会产生正确的结果。 Stream API保证收集时将保留相遇顺序。因此,无论是否并行,您的代码都可以使用。请参阅此处stackoverflow.com/questions/22350288/…
\ $ \ endgroup \ $
–Tunaki
16-4-28在8:21



\ $ \ begingroup \ $
看起来我实际上是错的。但是,问题在于,花了很多时间来探究Java 8实现的精髓细节才能弄清楚这一点。与提供的@Tunaki链接相比,以下内容提供了更好的详细信息:stackoverflow.com/questions/29710999 / ...因此,我该怎么办?也许应该删除它,但是在某些方面还是有意义的,因为这很可能是审稿人的想法。
\ $ \ endgroup \ $
– Necreaux
16-4-28在11:58



\ $ \ begingroup \ $
@Necreaux我已编辑您的答案以包含此评论。我不确定问题的作者是否会拒绝接受,但是至少这样对以后的读者来说是显而易见的。
\ $ \ endgroup \ $
– ran
16-4-28在16:40



#6 楼

正如其他人所指出的那样,您的代码有很多问题。

但是,我的猜测是您使用的功能是审阅代码的人员所不熟悉的。确定您是否被录用的人通常不是活跃的开发人员,而是曾经是活跃的开发人员的开发经理。

他们不太可能跟上最新的功能语言。显示您正在使用它们来解决问题将使您的代码对这位前开发人员而言更糟;他们可能会由于不了解您的代码做什么或如何解决问题而感到自我挑战(我不明白,我很聪明,因此这是不好的代码)。

他们是对的;演示的代码展示了您对Java功能的了解(在更复杂的问题中可能很有用),这些知识不适合此简单任务。没有炫耀的部分做得不好;复制粘贴,硬编码常量,易碎测试,无专一的库使用等。因此,即使他们不看您的代码,也可能消除了您的麻烦!

所以,如果有人不理解这些功能,那么代码看起来就像是噪音,而且是自负的。如果有人这样做,他们会说“好”,然后看看其余的。

他们更有可能在寻找具有最低能力的人来解决他们的问题:如果他们雇用某人编写Java来在UI中移动控件,那么他们不希望有人喜欢玩Java8。或者他们已经找到了候选人。或者他们因为分心而改变了关于招聘的想法。或者您在Java上被认为可以接受,但是您没有在简历中列出SQL,还有另一位候选人被Java接受并被标记为可以接受,因此人力资源部注意到需要SQL,并且您的简历被淘汰了。或应用程序是伪造的,他们只是在其中收集FizzBu​​zz实现,以馈送到FizzBu​​zz gar窃检测器。或者,他们收集FizzBu​​zz实现,这些实现可以避免Fizzbuzz窃检测器玩一些其他应用程序过程。或者,他们是为了保持稳定而收集简历和编码的人,然后他们找到合同并试图成为中间人。如果他们没有获得合同,则会丢弃申请人并重复。

经常发生与您的表现无关的事情,并且如果与您的事情无关,则不会提供反馈。或者,这与您有关,他们撒谎并说不是。

将来,FizzBu​​zz样式测试要记住的一点是,他们正在检查您完全可以编程。他们不是要你炫耀。该测试的目标是“您能否为一个简单的问题编写一个简单的解决方案”,并过滤掉根本无法编程的人员。使您的代码简单,无挑战性。尝试以最保守的方式遵循“最佳做法”。

评论


\ $ \ begingroup \ $
我否决了这个答案,因为我认为它花费太多的时间谈论访谈等,而花费的时间却很少谈论代码本身。
\ $ \ endgroup \ $
– ran
16年4月28日在22:07

\ $ \ begingroup \ $
此答案的涉及代码的部分不会对已经说过的内容添加任何内容。其他部分是猜测,不添加任何内容。当知道某项功能的人仍然认为这是一种不好的编写方式时,为什么要推测审阅者不知道所使用的功能呢?
\ $ \ endgroup \ $
– dan1111
16年4月29日在10:55

#7 楼

我意识到我们应该保留有关问题代码的评论。我从中得到一些突破,因为这不是我的代码有什么问题,而是为什么面试官不喜欢我的代码。

public class FizzBuzz {

    enum Special {

        Fizz {
            @Override
            boolean special(int n) {
                return (n % 3) == 0;
            }
        },
        Buzz {
            @Override
            boolean special(int n) {
                return (n % 5) == 0;
            }
        };

        abstract boolean special(int n);
    }

    static CharSequence encode(int n) {
        StringBuilder s = new StringBuilder();
        boolean normal = true;
        for (Special fb : Special.values()) {

            if (fb.special(n)) {
                s.append(fb.name()).append(" ");
                normal = false;
            }
        }
        if (normal) {
            s.append(n).append(" ");
        }
        return s;
    }

    private void fizzBuzz() {
        for (int i = 1; i < 20; i++) {
            System.out.println("" + i + " -> " + s);
        }
    }

    public static void main(String args[]) {
        try {
            new FizzBuzz().fizzBuzz();
        } catch (Throwable t) {
            t.printStackTrace(System.err);
        }
    }
}


是的,看上去更整洁,但现在当他们改变主意并要求增强功能时,会看到什么。我要做的就是添加:

        Lucky {
            @Override
            boolean special(int n) {
                return Integer.toString(n).contains("3");
            }
        }


一切仍然有效。

为什么

此解决方案演示了如果您以某种方式编写代码,则自然在某些方面具有延展性。在这种情况下,通过使用enum,可以轻松适应增加新特色的需求变化。你必须在多个地方重复代码的事实。在将其提交给面试官之前先进行修复。

评论


\ $ \ begingroup \ $
当n为15时,编码返回什么?我认为FizzBu​​zz问题的通常规范是它应该返回一个复合词,例如“ FizzBu​​zz”(“ Fizz”和“ Buzz”之间没有空格)。
\ $ \ endgroup \ $
– David K
16年4月29日在13:18

\ $ \ begingroup \ $
@DavidK-可以通过移动.append(“”); s来解决。
\ $ \ endgroup \ $
–OldCurmudgeon
16-4-29的13:21

\ $ \ begingroup \ $
也许更好,将它们完全省略,然后让调用者决定要使用的分隔符。是的,我知道这是一个很小的选择。
\ $ \ endgroup \ $
– David K
16-4-29的13:33

\ $ \ begingroup \ $
我在另一条评论中提到,但应该在帖子中指出重复不是真实的-这是一种伪造,它显示了代码如何根据我必须实现的三个需求而发展,而实际上我将仅具有第三个功能。我现在可以清楚地看到这是一个错误和混乱。
\ $ \ endgroup \ $
–软件工程师
16-4-29在21:32



\ $ \ begingroup \ $
这仍然是一个矫kill过正的超级复杂的FizzBu​​zz,值得的是,我会为此失败而进行编码面试。
\ $ \ endgroup \ $
–本杰明·格伦鲍姆(Benjamin Gruenbaum)
16年4月30日在6:57