代码命名_肮脏的代码问题:通过良好的命名习惯来改善您的游戏

代码命名

规则:每当您命名变量,函数或类时,都要询问它是否简洁诚实富有表现力完整

说明

一贯地写好名字很难,但是阅读和理解坏名字却很难。 使用好名字意味着我们允许读者停止阅读更高级别的抽象内容,而无需深入研究细节以了解事物的工作原理。 在一个小的代码库中,深入了解内部结构并不太昂贵,但是一旦代码库增长到成千上万行或更多行,我们期望读者阅读所有内容就变得不合理了。

命名变量,函数和类有一些不同的规则,但也有4个基本的共同点。 名称应为:

  1. 简洁:它们不应该浪费。 这意味着我们不会添加无意义的词或不必要的上下文。
  2. 诚实:他们不应该说谎。 这意味着名称不应隐藏或在其所代表的任何部分上误传。
  3. 表达力:他们应该表达意图。 这意味着名称应准确,清晰地描述其代表的含义。
  4. 完成:不应使用简短形式或首字母缩写词。 这意味着我们不希望读者了解与我们相同的编码。

话虽如此,我们应该讨论变量名称,名称和类之间的区别。

变量名

变量名称应基于变量的使用方式,而不是其类型(我将在后面的文章中进一步介绍不将类型编码为名称)。 这不仅有助于提高可读性,还使读者可以使用该变量完成操作。

一个可怕的变量名的示例可能看起来像:

final List<Integer> valueList = new ArrayList<>();

在这里,作者使用了没有意义的value单词(简明违反规则1)和对type进行编码的List单词。 这些词加在一起并没有表达意图,违反了规则3,表现力强。 此外,对这样的类型进行编码会使我们面临以下风险:最终,我们得到如下代码:

final Set<Integer> valueList = new HashSet<>();

因为有人认为使用组合列表更好,所以我们有一个毫无意义的词和一个谎言(违反我的第二条规则;诚实)。 记住也要重命名该变量会很不错,但这是一个容易犯的错误。

一个更好的名字看起来像:

final List<Integer> accountIdsToLookup = new ArrayList<>();

此名称表示通过多个accountIds存在多个值,但也没有将我们绑定到基础数据结构。 对于所有可迭代类型的对象,我都建议这样做。 此外,该名称是可表示的,因为它指示变量包含什么以及应将其用于什么。

对于键值数据结构(例如地图(字典等)),我建议使用诸如keysToValuesvaluesByKeys命名方案。

final Map<Integer, Account> accountsMap = new HashMap<>(); // bad
final Map<Integer, Account> accountsById = new HashMap<>(); // good

请注意第二行如何告知您键和值的含义。 第一种方法还将映射类型编码为名称,如果我们切换到其他键值存储方案(不要将编码类型编码为名称),则必须更改名称。

功能名称

函数名称应该是动作( 动词 )而不是事物( 名词 )。 函数执行更改,并在事情不诚实之后将其命名。 此外,功能是故事或您的程序的驱动步骤,因此,当我们阅读每个步骤时,我们希望它看起来就像是正在发生变化。 行动代表变化。

在此类中可以看到错误的函数名称:

public class Company  {
    private final String name;
    private final int taxId;

    public Company ( final String name, final int taxId)  {
        this .name = name;
        this .taxId = taxId;
    }

    public String json ()  {
        return new StringBuilder()
            .append( "{" )
            .append( "'name': " + this .name)
            .append( "," )
            .append( "'taxId': " + this .taxId)
            .append( "}" )
            .build();
    }
}

此处的函数json看起来好像是类的属性而不是函数。 它没有被命名为行动者,而是被称为不诚实的东西(违反规则2)。 更好的名称是类似于to_json名称,它表示该函数正在对其所属的Company类进行某些操作。

附带说明一下,作者在这里使用StringBuilder将此类转换为JSON(JavaScript对象表示法)。

这举了一个简单的例子,我确实已经在现实世界中看到了,但这不是推荐的方法。

相反,我建议使用序列化/反序列化库,例如FasterXML的JacksonGoogle的Gson

类名

有两种类型的类, 数据结构参与者 (我将在后面的文章中讨论不构建混合对象)中对此进行进一步讨论,每种类型的名称应不同。

演员班

应该以演员的意图来命名演员。 如果他们只能做一件事,这会容易得多(我将在稍后关于做一件事情的文章中对此进行讨论)。

一个名字不佳的演员的例子可能是:

public class Authentication  {
    ...
    public boolean validateSession ( final Session session)  {
        ...
    }
}

在这种情况下,该类似乎表示身份验证数据结构,因此,我们希望它具有一些访问器和/或属性,但没有任何功能。 当我们看到函数调用站点validateSession它的读取效果很差:

final Authentication authentication = new Authentication(...);
...
if (authentication.validateSession(session)) {
    ...
}

如果我将类名换成Authenticator这将使尴尬的读取变得少得多:

final Authenticator authenticator = new Authenticator(...);
...
if (authenticator.validateSession(session)) {
    ...
}

用它的名字,很明显, Authenticator是一个演员,在其中承担责任是有意义的。

数据结构

数据结构可以是通用的也可以是特定的。 通用数据结构可以保存并提供对任何其他类型数据的访问(例如,列表可以是整数列表)。 特定的数据结构包含一个非常精心设计的信息方案(例如Customer对象)。

应该以如何访问泛型命名。 诸如“地图”,“列表”或“集合”之类的单词以这种方式很有帮助。 大多数语言都带有内置的泛型,例如Java中的HashMapTreeSetLinkedList

特定的数据结构对象应以其封装的对象而不是Actor命名。

命名错误的特定数据结构可能类似于:

public class Authenticator  {
    ...
    public String getSessionKey ()  {
        return this .sessionKey;
    }
}

从名称上看,该类似乎是一个actor,这意味着对getSessionKey使我们认为此函数正在执行某些操作(例如访问数据库)。 取而代之的是,更好的类名称为Authentication以便呼叫站点的读法如下:

final Authentication authentication = new Authentication(...);
...
final String sessionKey = authentication.getSessionKey();

请注意,此功能仅访问authentication数据结构的sessionKey属性是多么明显。

这个例子

在我们的示例中,我们正在查看旨在计算给定保龄球比赛得分的代码。 保龄球比赛被定义为一系列的十帧。 玩家每获得一帧,都会掷两次掷骰子来尝试击倒尽可能多的别针。 通常,共有十个引脚。 如果玩家在第一卷击倒4个别针,在第二卷击倒4个别针,则该帧的得分为8。

如果玩家击倒第一卷的所有针脚,则称为“罢工”。 一次罢工可得10分,再加上接下来两轮的价值。 如果玩家没有在第一局中击倒所有的别针,而是在第二局中继续这样做,则称为备用。 一个备用零件的价值为10点,再加上下一轮的价值。

游戏的最后一帧(第十帧)的工作方式略有不同。 玩家最多可以掷三下,如果碰到或备用,则获得一组新的销。 如果玩家在第10帧中既没有罢工也没有备用,他们只会掷出两次。

为了将这个问题的输入建模到我们的示例中,我们获取一个框架列表,每个框架都由一个被击倒的引脚数列表表示。 输入示例(以JSON表示)可能类似于:

[[4 , 4 ], [ 10 ], [ 3 , 7 ], [ 10 ], [ 10 ], [ 10 ], [ 10 ], [ 10 ], [ 10 ], [ 2 , 8 , 5 ]]

在此游戏中,用户在第一帧中总共击倒了8个图钉。 然后,他们在第二帧中获得罢工,在第三帧中获得空缺。 然后,玩家打出一系列完美的击球(所有击球),直到获得备用球的最后一帧,然后将5针击倒。 该游戏的总得分是225。

由于编写此代码的目的是为保龄球馆的游戏板显示器提供动力,因此该示例还可以处理不完整的游戏。 不完整游戏的输入可能如下所示:

[[4 , 4 ], [ 10 ], [ 3 ]]

这只是上面完成的游戏的开始,但此时比分是24。

首先,我将向您展示解决该问题的完整代码。 该代码有效,并且具有良好的设计,但是命名很糟糕。 然后,我将迭代遍历代码的各个部分,并通过讨论为什么进行更改来清理命名。 最后,我将完整显示清理后的代码。

希望是,当您将原始代码与清理后的代码进行比较时,您会同意,尽管我只是在应用本章中的良好命名规则,但遵循起来要容易得多。

肮脏的代码

public static class Consts  {
    public static int PPF = 10 ;
    public static int NOF = 10 ;
}

public class BowlingScore  {

    private final FrameBuilder frameBuilder;
    private final RollPropagation rollPropagation;

    public BowlingScore (
            final FrameBuilder frameBuilder, 
            final RollPropagation rollPropagation
    )  {
        this .frameBuilder = frameBuilder;
        this .rollPropagation = rollPropagation;
    }

    public static BowlingScore build ()  {
        return new BowlingScore(
            new FrameBuilder(), 
            new RollPropagation());
    }

    public int calcScore ( final List<List<Integer>> rollPairList)  {
        final List<Frame> values = new ArrayList<>();
        for ( final List<Integer> rollPair : rollPairList) {
            this .rollPropagation.add(rollPair);
            values.add(
                this .frameBuilder.func(
                    rollPair, 
                    this .rollPropagation)
            );
        }
        return values.stream().reduce( 0 , (s, f) -> s + f.score());
    }
}

public class RollPropagation  {

    private List<Subscriber> strikes = new ArrayList<>();

    public void add ( final Subscriber subscriber)  {
        this .strikes.add(subscriber);
    }

    public void callbacks ( final List<Integer> rollValues)  {
        for ( final int r : rollValues) {
            for ( final Subscriber strike : this .strikes) {
                strike.run(r)
            }
        }
    }
}

public class FrameBuilder  {

    private int count = 0 ;

    public Frame func (
            final List<Integer> rollValues, 
            final RollPropagation rollPropagation
    )  {
        this .count += 1 ;

        if ( this .lf()) {
            return new Final(rollValues);
        }

        if ( this .ifr(rollValues)) {
            return new Incomplete(rollValues[ 0 ])
        }

        if ( this .sf(rollValues)) {
            final Strike strike = new Strike();
            rollPropagation.add(strike);
            return strike;
        }

        if ( this .sf2(rollValues)) {
            final Spare spare = new Spare();
            rollPropagation.add(spare);
            return spare;
        }

        return new NonTen(rollValues[ 0 ], rollValues[ 1 ])
    }

    private boolean lf ()  {
        return this .count == Consts.NOF;
    }

    private boolean ifr ( final List<Integer> rollValues)  {
        return rollValues.length() == 1 && 
            rollValues[ 0 ] != Consts.PPF;
    }

    private boolean sf ( final List<Integer> rollValues)  {
        return rollValues.length() == 1 && 
            rollValues[ 0 ] == Consts.PPF;
    }

    private boolean sf2 ( final List<Integer> rollValues)  {
        return rollValues[ 0 ] + rollValues[ 1 ] == Consts.PPF;
    }
}

public interface Subscriber  {
    public void run ( final int r) ;
}

public interface Frame  {
    public int score () ;
}

public class Strike implements Subscriber , Frame  {

    private int score = Consts.PPF;
    private int count = 0 ;

    @Override
    public int score ()  {
        return this .score;
    }

    @Override
    public void run ( final int r)  {
        if ( this .count < 2 ) {
            this .score += r;
            this .count += 1 ;
        }
    }
}


public class Spare implements Subscriber , Frame  {

    private int score = Consts.PPF;
    private boolean off = false ;

    @Override
    public int score ()  {
        return this .score;
    }

    @Override
    public void run ( final int roll)  {
        if (! this .off) {
            this .score += roll;
            this .addedBonus = true ;
        }
    }
}


public class Incomplete implements Frame  {

    private final int fr;

    public Incomplete ( final int fr)  {
        this .fr = fr;
    }

    @Override
    public int score ()  {
        return this .fr;
    }
}

public class NonTen implements Frame  {

    private final int fr;
    private final int sr;

    public NonTen ( final int fr, final int sr)  {
        this .fr = fr;
        this .sr = sr;
    }

    @Override
    public int score ()  {
        return this .fr + this .sr;
    }
}


public class Final implements Frame  {

    private final int score;

    public Final ( final List<Integer> frame)  {
        this .score = frame.stream().reduce( 0 , Integer::sum);
    }

    @Override
    public int score ()  {
        return this .score;
    }
}

迭代改进

我将从Consts类开始,从上面的代码的顶部开始研究这些类。 对于每一个,我将确定它的作用,名称的问题,然后提出一个改进的版本。

Consts类

关于将常量与配置用于诸如此类中存储的值之类的说法存在争议,但是我在这里的任务不是讨论设计,而是讨论命名。 该类如下所示:

public static class Consts  {
    public static int PPF = 10 ;
    public static int NOF = 10 ;
}

类的名称Consts是一个简短的形式。 尽管此类很显然是为了保存常量,但简短形式并不能为我们节省太多(3个字母),那么为什么还要冒歧义的风险呢? 此名称违反了我的完整性规则,因此我将其更改为Constants

同样,作者在此类中使用常量的首字母缩写词。 这些首字母缩写词违反了我的完整性和表达规则。 在不深入研究其余代码的情况下,我们如何知道这些变量的含义? 我将PPF重命名为PINS_PER_FRAME ,将NOF重命名为NUMBER_OF_FRAMES来更正此问题。

public static class Constants  {
    public static int PINS_PER_FRAME = 10 ;
    public static int NUMBER_OF_FRAMES = 10 ;
}

保龄球得分班

这是使用示例的主要切入点。 用户可以通过编写BowlingScore.build().calcScore(...)将输入传递到calcScore函数中来使用其中之一。

该类当前看起来像:

public class BowlingScore  {

    private final FrameBuilder frameBuilder;
    private final RollPropagation rollPropagation;

    public BowlingScore (
            final FrameBuilder frameBuilder, 
            final RollPropagation rollPropagation)  {
        this .frameBuilder = frameBuilder;
        this .rollPropagation = rollPropagation;
    }

    public static BowlingScore build ()  {
        return new BowlingScore(
            new FrameBuilder(), 
            new RollPropagation())
    }

    public int calcScore ( final List<List<Integer>> rollPairList)  {
        final List<Frame> values = new ArrayList<>();
        for ( final List<Integer> rollPair : rollPairList) {
            this .rollPropagation.add(rollPair);
            values.add(
                this .frameBuilder.func(
                    rollPair, 
                    this .rollPropagation));
        }
        return values.stream().reduce( 0 , (s, f) -> s + f.score());

    }
}

BowlingScore的名称不表示该类是actor。 而是,此类听起来像一个数据结构,其中包含保龄球比赛的得分。 令人惊讶的是,发现此类具有职责(例如calcScore ),并且在呼叫站点读取用法非常尴尬。 从外部看, build方法是否会构建乐谱本身?

buildcalcScore之间的关注点分离是什么? 为了解决这个问题,我将把此类重命名为一个名为BowlingScorer的演员。 这是一个很小的修复,但是呼叫站点现在应该更加清晰,这表明build方法将构建BowlingScorer ,然后calcScore会计算游戏得分。

我要解决的下一个名称是build方法。 这个名称没有给我们任何上下文来说明为什么我应该在调用裸构造函数时使用它。 我们必须阅读定义才能看到它设置了默认的FrameBuilderRollPropagation对象。 我可以通过从fromDefaultBuilderAndPropagation调用此名称来提高其表达fromDefaultBuilderAndPropagation

我还需要解决函数名calcScore这是一个短形式calculateScore 。 这违反了我的完整性规则,因此我将其重命名。

我还应该更改参数名称rollPairList 。 此名称可能是从更改的类型派生的。 作者可能从编写此代码开始,以便每个帧恰好包含两个元素,从而允许它们使用元组类型,它们称为对。 但是,一旦他们发现最后一帧可以包含三卷,他们便将类型更改为列表列表。 毫不奇怪,他们忘了更新名称。

这种故事经常发生,并导致违反我的诚实准则。 名称中使用List一词至少是诚实的,但这是一个毫无价值的词,使我们面临与Pair相同的风险。 相反,我将此参数重命名为rawFrames 。 多个Frames表示存在多个Frames ,而单词raw表示我尚未对其进行任何处理。

该论点的第二部分是基于预先阅读,并看到存在称为Frame的数据结构,并且不想使读者感到困惑。 我还将更新派生名称,例如rollPair ,以匹配新的rawFrames名称。

我发现此代码的另一个问题是名为values的变量。 这个名称没有任何意义,它违反了表达性,因此我将其重命名为frames ,因为这就是该集合中的内容。

最后,我应该解决lambda函数中作为reduce参数给出的单字母变量。 由于这些值的使用范围非常短,因此不应将单字母变量视为所有问题。 但是,单个字母变量始终会带来潜在风险,使后来的作者增加范围而不重命名。

同样,如果读者对此处的reduce方法不满意,则该名称不会为他们提供任何帮助。 让我们将这些变量重命名为currentSumframe以提供更多的表达能力。

现在,更新后的代码如下所示:

public class BowlingScorer  {

    private final FrameBuilder frameBuilder;
    private final RollPropagation rollPropagation;

    public BowlingScorer (
            final FrameBuilder frameBuilder, 
            final RollPropagation rollPropagation
    )  {
        this .frameBuilder = frameBuilder;
        this .rollPropagation = rollPropagation;
    }

    public static BowlingScorer fromDefaultBuilderAndPropagation ()  {
        return new BowlingScorer(
            new FrameBuilder(), 
            new RollPropagation());
    }

    public int calculateScore ( final List<List<Integer>> rawFrames)  {
        final List<Frame> frames = new ArrayList<>();
        for ( final List<Integer> rawFrame : rawFrames) {
            this .rollPropagation.add(rawFrame);
            frames.add(
                this .frameBuilder.func(
                    rawFrame, 
                    this .rollPropagation));
        }
        return frames.stream().reduce(
            0 , 
            (currentSum, frame) -> currentSum + frame.score());
    }
}

此类中的命名仍然存在问题,例如在frameBuilder上调用的func ,但是当我们进入该类时,我会解决这个问题。 您可以假设我会将这些更改传播回这些对象(并且您将在最终形式中看到其效果)。

RollPropagation类别

RollPropagation类允许将值从新的滚动传递回已经发生的帧。 这对于罢工和备用零件非常有价值,因为这些零件的得分不仅取决于这些帧的投掷,还取决于随后的帧。 当前看起来像这样:

public class RollPropagation  {

    private List<Subscriber> strikes = new ArrayList<>();

    public void add ( final Subscriber subscriber)  {
        this .strikes.add(subscriber);
    }

    public void callbacks ( final List<Integer> rollValues)  {
        for ( final int r : rollValues) {
            for ( final Subscriber strike : this .strikes) {
                strike.run(r)
            }
        }
    }
}

与此有关的第一个问题是,类名本身不是参与者,但读取时好像是某种数据结构。 为了解决这个问题,我将重命名该类以匹配它的功能,这将广播到订阅的帧。 我可以把这个RollBroadcaster其通信的功能,但我知道这是继称为观察者模式已知模式

如果读者熟悉此模式,并具有表示其使用方式的名称,则对他们来说很有价值。 在观察者模式中,有两个部分, 主题观察者 。 在这种情况下,该类为主题(观察者将观察的事物;滚动)。

我将重命名此类RollSubjectBroadcaster 。 重要的是要注意,作为作者,我们可能并不了解每种模式或其正确的命名约定。 我们不能指望我们不知道什么。 如果我没有发现这是“观察者模式”, RollBroadcaster命名为RollBroadcaster还是一个改进。 但是,不熟悉的读者可能要花几秒钟的时间才能了解我在做什么。

下一个问题是strikes变量。 我知道这个名字是个谎言,因为罢工并不是事实之后唯一需要知道的掷骰的类型(备用零件呢?)。 我猜想这个班级最初是用两个清单写的,一个清单用于罢工,另一个清单用于备件。

然后,作者正确地进行了更改,使两者之间具有公共接口,从而将列表简化为当前的单数形式。

问题在于他们从未调整过名称以匹配其更改。

为了更正此问题,我将该变量重命名为rollObservers ,该变量准确地描述了列表打算保留的内容。

通常使用函数名称add 。 例如,在Java的Collections接口上,它们使用名称来指示向集合中添加新成员的方法。

add的问题在于它也可能意味着“加总”,这肯定不是在这里做的。 通常,更好的名称是appendpushinsert等,因为它们更具表达性。 但是,在这种情况下,由于作者没有构建通用集合,因此他们本来可以(因此应该)更具表现力。 我将把此函数重命名为subscribe ,因为呼叫站点用户不必关心此类如何存储它们。

最后一个函数, callbacks是根据事物而不是动作来命名的,因此这是一个不好的名字。 即使我将其称为callback ,这可以是一个动作,但它仍不会表现得很好。 取而代之的是,我将其称为broadcastRolls ,它清楚地说明了此函数将要执行的操作。

callbacks的参数称为rollValues 。 “ Values ”一词在这里是没有意义的,因此我将其删除(以遵循简洁的原则)。 我将其重命名为rawFrame ,它指示这是单个帧的滚动。

简短形式的变量r是安全的,因为它的使用范围很小。 但是,添加表现力是一项最小的工作,因此我将其重命名为roll 。 此外,范围还有未来增长的风险。 我们可以通过避免小范围争论作为懒惰的借口来证明自己的名字的未来。

经过这些更改,新类如下所示:

public class RollSubjectBroadcaster  {

    private List<Subscriber> rollObservers = new ArrayList<>();

    public void subscribe ( final Subscriber subscriber)  {
        this .rollObservers.add(subscriber);
    }

    public void broadcastRolls ( final List<Integer> rawFrame)  {
        for ( final int roll : rawFrame) {
            for ( final Subscriber rollObservers : 
                    this .rollObservers) {
                rollObservers.run(roll)
            }
        }
    }
}

FrameBuilder类

FrameBuilder类从单个原始帧获取滚动,并构建正确类型的新帧对象。 如果框架类型需要评分, FrameBuilder类还可以将框架预订给将来的FrameBuilder 。 其当前未更改的形式如下:

public class FrameBuilder  {

    private int count = 0 ;

    public Frame func (
            final List<Integer> rollValues, 
            final RollPropagation rollPropagation
    )  {
        this .count += 1 ;

        if ( this .lf()) {
            return new Final(rollValues);
        }

        if ( this .ifr(rollValues)) {
            return new Incomplete(rollValues[ 0 ])
        }

        if ( this .sf(rollValues)) {
            final Strike strike = new Strike();
            rollPropagation.subscribe(strike);
            return strike;
        }

        if ( this .sf2(rollValues)) {
            final Spare spare = new Spare();
            rollPropagation.subscribe(spare);
            return spare;
        }

        return new NonTen(rollValues[ 0 ], rollValues[ 1 ])
    }

    private boolean lf ()  {
        return this .count == Constants.NUMBER_OF_FRAMES;
    }

    private boolean ifr ( final List<Integer> rollValues)  {
        return rollValues.length() == 1 && 
            rollValues[ 0 ] != Constants.PINS_PER_FRAME;
    }

    private boolean sf ( final List<Integer> rollValues)  {
        return rollValues.length() == 1 && 
            rollValues[ 0 ] == Constants.PINS_PER_FRAME;
    }

    private boolean sf2 ( final List<Integer> rollValues)  {
        return rollValues[ 0 ] + rollValues[ 1 ] == 
            Constatns.PINS_PER_FRAME;
    }
}

此类的第一个问题是其名称是谎言。 在软件工程中,我们使用“ 模式 ”一词来描述针对常见问题的某些解决方案设计。 此类的作者可能不知道Builder是一种模式,但是确实存在。 我们不能期望知道我们不知道的东西,但是这会使读者感到受骗。

我建议每个人在职业生涯的某个时候都学习各种模式。 至少,首先要学习那里的模式名称。 然后,如果您不使用模式(或不确定),请避免使用模式名称。 模式被设计为通信工具。 如果有人将FrameBuilder命名为阅读器,那么我对如何使用该对象以及它将如何工作有一个很好的了解。 在这种情况下,作者实际上使用的是称为Factory模式的模式,因此我将此类重命名为FrameFactory

下一个问题是私人会员count 。 单词count通常在编程中使用,但很少表示所计数的内容。 由于这是到目前为止工厂已构建的帧数,因此我将重命名此成员builtFrames

函数func名称非常严格。 修复BowlingScorer时我们看到了它的用法,并在BowlingScorer引起了我们的问题。 不幸的是,我已经在整个行业的生产代码库中多次看到这一点,并且据我所知,只能将其归因于纯惰性。 我将重命名此方法来construct

对于传递给func的参数rollValuesValues一词毫无意义,因此违反了我的简洁规则。 取而代之的是,我将调用此rawFrame来表示我使用的是单帧胶卷来构造新的Frame对象。

FrameBuilder类包含一系列布尔函数,这些布尔函数封装了用于构建不同帧类型的各种条件的逻辑。 为了简洁起见,这些函数的名称被懒惰地命名为lfifrsfsf2 。 这些名称既不能视为完整名称也不可以表示。

我假设作者编写了第一个函数lf ,以表示识别最后一帧的逻辑,并在此过程中为其余这些函数设定了先例。 我将lf重命名为isLastFrame 。 使用前缀is表示返回布尔值的方法的常用方法。 这也使我的名字成为一个动作而不是事物(就像lastFrame一样)。 关于Command操作与Query操作( isLastFrame是后者)有一个有趣的对话,但是我将在以后的文章中讨论。

下一个条件函数ifr检查帧是否不完整。 如果可能,作者可能想调用此函数if但意识到这会与同名的保留字冲突,因此他们添加了r后缀。 这种命名方案的另一个问题是字母il之间的相似性。 前两个函数很容易被程序员错误键入,从而导致一个很难发现的重大错误。 我将此方法重命名为isIncompleteFrame

第三功能sf用于打击框。 同样,这是懒惰的短形式,我将其重命名为isStrikeFrame

第四个功能显示了这种命名方式的另一个问题。 作者想要一个函数来检查框架是否为备用框架,但是罢工框架函数已经使用了名称sf 。 短名称容易受到这些类型的冲突的影响。 作者的解决方案是将该名称后缀数字2。这让人感到很难过,这看起来似乎很可笑,但是我经常在人们的代码中发现这一确切的东西。 为了解决这个问题,我将重命名此方法为isSpareFrame

改进后的代码现在看起来像:

public class FrameFactory  {

    private int builtFrames = 0 ;

    public Frame construct (
            final List<Integer> rawFrame, 
            final RollBroadcaster rollBroadcaster
    )  {
        this .builtFrames += 1 ;

        if ( this .isLastFrame()) {
            return new Final(rawFrame);
        }

        if ( this .isIncompleteFrame(rawFrame)) {
            return new Incomplete(rawFrame[ 0 ])
        }

        if ( this .isStrikeFrame(rawFrame)) {
            final Strike strike = new Strike();
            rollBroadcaster.subscribe(strike);
            return strike;
        }

        if ( this .isSpareFrame(rawFrame)) {
            final Spare spare = new Spare();
            rollBroadcaster.subscribe(spare);
            return spare;
        }

        return new NonTen(rawFrame[ 0 ], rawFrame[ 1 ])
    }

    private boolean isLastFrame ()  {
        return this .builtFrames == Constants.NUMBER_OF_FRAMES;
    }

    private boolean isIncompleteFrame ( final List<Integer> rawFrame)  {
        return rawFrame.length() == 1 && 
            rawFrame[ 0 ] != Constants.PINS_PER_FRAME;
    }

    private boolean isStrikeFrame ( final List<Integer> rawFrame)  {
        return rawFrame.length() == 1 && 
            rawFrame[ 0 ] == Constants.PINS_PER_FRAME;
    }

    private boolean isSpareFrame ( final List<Integer> rawFrame)  {
        return rawFrame[ 0 ] + rawFrame[ 1 ] == Constants.PINS_PER_FRAME;
    }
}

订户界面

下一个要分析的代码是Subscriber接口。 接口允许我们根据类用法创建合同。 Subscriber接口声明一个名为run的方法,该方法采用整数,不返回任何值(无效)。 任何希望成为Subscriber必须为其自身实现此run方法。 您可以在上面的RollSubjectBroadcaster类中看到此方法的优势; 罢工和备件可以组合在一起,成为可以向其广播点播的对象。 当前界面如下:

public interface Subscriber  {
    public void run ( final int r) ;
}

该接口的名称并不可怕,只是它听起来太通用而不能用于这种专门用途。 通过将其命名为RollSubscriberFrame可以轻松提高此名称的表达RollSubscriberFrame 。 但是,如上所述,这是观察者模式的一部分,因此我想指出这一点。 因此,我将使用单词Observer代替单词Subscriber.

请注意,我的新名字RollObserverFrame不是演员的名字。 研究此接口的用法后,发现该接口实际上是用于数据结构对象的,而不是actor(如Subscriber指示的),使此名称更改适当。

函数run是常用的,但通常很糟糕。 在此接口的上下文中,很容易理解这是RollObserverFrame唯一要做的事情,但是在实际实现中使用时,它没有多大意义。 我将此函数重命名为rollCallback以表示这是该函数的实现要处理滚动时要采取的操作。

通常,作者会在其接口中使用单字母参数,因为名称实际上仅在实现中起作用(并且不必匹配)。 但是,这样做对界面的读取者没有帮助。 在这里付出一些额外的努力将有很长的路要走。 为了表达,我将参数r重命名为roll

改进后的代码如下所示:

public interface RollObserverFrame {
    publicvoid rollCallback(final int roll);
}

框架接口

Frame接口定义了一个约定,任何希望用作Frame都应实现返回整数的score方法。 在BowlingScorer中实现了该BowlingScorer ,该功能可以计算游戏的总得分。 它通过对帧的所有单个分数求和来实现。

public interface Frame  {
    public int score () ;
}

这里唯一的命名问题是方法score不是一个动作,而是可以表示一个动作。 相反,我将其重命名为getScore ,它清楚地将单词score定义为一个值而不是一个动作。

改进后的代码如下所示:

public interface Frame  {
    public int getScore () ;
}

罢工班

Strike类实现两个接口。 它实现了RollObserverFrame因为它需要在接下来的两帧中更新分数。 它还实现了Frame接口,以便可以从中提取分数。

当前版本如下:

public class Strike implements RollObserverFrame , Frame  {

    private int score = Constants.PINS_PER_FRAME;
    private int count = 0 ;

    @Override
    public int getScore ()  {
        return this .score;
    }

    @Override
    public void rollCallback ( final int r)  {
        if ( this .count < 2 ) {
            this .score += r;
            this .count += 1 ;
        }
    }
}

我的第一个评论是关于私人会员count 。 这个值是多少? 一份小调查表明,每次调用rollCallback ,它都会增加,直到达到2。我将其重命名为addedBonusCount ,以提高表达能力。

我还注意到参数r正在传递给rollCallback函数。 这可能被命名为单个字母,因为接口也以这种方式命名。 这是一个简单的例子,说明惰性命名可能具有传染性。 我将进行重命名, roll了澄清。

改进的Strike类如下所示:

public class Strike implements RollObserverFrame , Frame  {

    private int score = Constants.PINS_PER_FRAME;
    private int addedBonusCount = 0 ;

    @Override
    public int getScore ()  {
        return this .score;
    }

    @Override
    public void rollCallback ( final int roll)  {
        if ( this .addedBonusCount < 2 ) {
            this .score += roll;
            this .addedBonusCount += 1 ;
        }
    }
}

备用班级

Strike类相似, Spare类同时实现RollObserverFrameFrame接口。 在这一堂课中,只应将下一个掷骰值添加到分数中。

在改善命名之前,它看起来像:

public class Spare implements RollObserverFrame , Frame  {

    private int score = Constants.PINS_PER_FRAME;
    private boolean off = false ;

    @Override
    public int getScore ()  {
        return this .score;
    }

    @Override
    public void rollCallback ( final int roll)  {
        if (! this .off) {
            this .score += roll;
            this .addedBonus = true ;
        }
    }
}

在这里,我的抱怨是与私有成员做off这不是表现力。 这可能意味着“不在”,或者这是我不理解的简短形式。 如果确实表示“未启用”,则表示未启用什么? 我将其重命名为addedBonus以指示我是否还添加了奖励。

改进的版本如下所示:

public class Spare implements RollObserverFrame , Frame  {

    private int score = Constants.PINS_PER_FRAME;
    private boolean addedBonus = false ;

    @Override
    public int getScore ()  {
        return this .score;
    }

    @Override
    public void rollCallback ( final int roll)  {
        if (! this .addedBonus) {
            this .score += roll;
            this .addedBonus = true ;
        }
    }
}

未完成的班级

Incomplete类表示到目前为止仅制作一卷的框架。 它与其他框架对象一样实现Frame接口。 但是,它不关心额外的滚动,因此它不实现RollObserverFrame接口。

public class Incomplete implements Frame  {

    private final int fr;

    public Incomplete ( final int fr)  {
        this .fr = fr;
    }

    @Override
    public int getScore ()  {
        return this .fr;
    }
}

当作者命名SpareStrike框架时,我并不介意这些名称,因为它们是非常具体的保龄球术语。 但是, Incomplete是一个很难的名字,因为它不需要专门关于保龄球框架。 因此,我将此类重命名为IncompleteFrame 。 这会导致与其他框架名称不一致,因此我将类似地在它们StrikeFrame加上单词FrameStrike变为StrikeFrame等)。

此类中的另一个问题是私有成员fr 。 这是firstRoll的缩写形式(违反完整性规则),因此我将适当地重命名该值。

现在更新的Incomplete类是:

public class IncompleteFrame implements Frame  {

    private final int firstRoll;

    public IncompleteFrame ( final int firstRoll)  {
        this .firstRoll = firstRoll;
    }

    @Override
    public int getScore ()  {
        return this .firstRoll;
    }
}

NonTenFrame类

NonTenFrame类表示一个完整的框架,但是既不罢工也不是备用。

public class NonTenFrame implements Frame  {

    private final int fr;
    private final int sr;

    public NonTen ( final int fr, final int sr)  {
        this .fr = fr;
        this .sr = sr;
    }

    @Override
    public int getScore ()  {
        return this .fr + this .sr;
    }
}

这里的错误命名类似于IncompleteFrame类,因此我采用了相同的解决方案。 将fr重命名为firstRoll ,将sr重命名为secondRoll

NonTenFrame的改进版本如下所示:

public class NonTenFrame implements Frame  {

    private final int firstRoll;
    private final int secondRoll;

    public NonTen ( final int firstRoll, final int secondRoll)  {
        this .firstRoll = firstRoll;
        this .secondRoll = secondRoll;
    }

    @Override
    public int getScore ()  {
        return this .firstRoll + this .secondRoll;
    }
}

最终班

最后一个类是FinalFrame ,它代表保龄球游戏的第十帧。

public class FinalFrame implements Frame  {

    private final int score;

    public Final ( final List<Integer> frame)  {
        this .score = frame.stream().reduce( 0 , Integer::sum);
    }

    @Override
    public int getScore ()  {
        return this .score;
    }
}

此类中的名称已经可以接受,但这是正确的,因为我将其重命名为后缀为Frame 。 以前,该类以前(曾经)被命名为Final 。 单词final是Java中的保留字,具有非常具体的含义。 创建Final班级使保留的含义陷入困境。 将其重命名为FinalFrame是一个很大的改进。

改进代码

我现在经历了并清理了保龄球记分员的名字。 我没有涉及设计或任何其他方面,但我相信我在提高代码质量方面已取得了长足的进步。 以下是完整的改进版本。 将其与本章中的原始代码进行比较,以查看这些命名规则可以做出的改进。

public static class Constants  {
    public static int PINS_PER_FRAME = 10 ;
    public static int NUMBER_OF_FRAMES = 10 ;
}

public class BowlingScorer  {

    private final FrameFactory frameFactory;
    private final RollSubjectBroadcaster rollBroadcaster;

    public BowlingScorer (
        final FrameFactory frameFactory, 
        final RollSubjectBroadcaster rollBroadcaster
    )  {
        this .frameFactory = frameFactory;
        this .rollBroadcaster = rollBroadcaster;
    }

    public static BowlingScorer fromDefaultFactoryAndBroadcaster ()  {
        return new BowlingScorer(
            new FrameFactory(), 
            new RollSubjectBroadcaster());
    }

    public int calculateScore ( final List<List<Integer>> rawFrames)  {
        final List<Frame> frames = new ArrayList<>();
        for ( final List<Integer> rawFrame : rawFrames) {
            this .rollBroadcaster.broadcastRolls(rawFrame);
            frames.add(
                this .frameFactory.construct(
                    rawFrame, 
                    this .rollBroadcaster));
        }
        return frames.stream().reduce(
            0 , 
            (currentSum, frame) -> currentSum + frame.getScore());
    }
}

public class RollSubjectBroadcaster  {

    private List<RollObserverFrame> rollObservers = 
        new ArrayList<>();

    public void subscribe ( final RollObserverFrame observer)  {
        this .rollObservers.add(observer);
    }

    public void broadcastRolls ( final List<Integer> rawFrame)  {
        for ( final int roll : rawFrame) {
            for ( final RollObserverFrame observer : 
                    this .rollObservers) {
                observer.rollCallback(roll)
            }
        }
    }
}

public class FrameFactory  {

    private int builtFrames = 0 ;

    public Frame construct (
            final List<Integer> rawFrame, 
            final RollSubjectBroadcaster rollBroadcaster
    )  {
        this .builtFrames += 1 ;

        if ( this .isLastFrame()) {
            return new FinalFrame(rawFrame);
        }

        if ( this .isIncompleteFrame(rawFrame)) {
            return new IncompleteFrame(rawFrame[ 0 ])
        }

        if ( this .isStrikeFrame(rawFrame)) {
            final StrikeFrame strikeFrame = new StrikeFrame();
            rollBroadcaster.subscribe(strikeFrame);
            return strikeFrame;
        }

        if ( this .isSpareFrame(rawFrame)) {
            final SpareFrame spareFrame = new SpareFrame();
            rollBroadcaster.subscribe(spareFrame);
            return spareFrame;
        }

        return new NonTenFrame(rawFrame[ 0 ], rawFrame[ 1 ])
    }

    private boolean isLastFrame ()  {
        return this .builtFrames == Constants.NUMBER_OF_FRAMES;
    }

    private boolean isIncompleteFrame ( final List<Integer> rawFrame)  {
        return rawFrame.length() == 1 && 
            rawFrame[ 0 ] != Constants.PINS_PER_FRAME;
    }

    private boolean isStrikeFrame ( final List<Integer> rawFrame)  {
        return rawFrame.length() == 1 && 
            rawFrame[ 0 ] == Constants.PINS_PER_FRAME;
    }

    private boolean isSpareFrame ( final List<Integer> rawFrame)  {
        return rawFrame[ 0 ] + rawFrame[ 1 ] == Constants.PINS_PER_FRAME;
    }
}

public interface RollObserverFrame  {
    public void rollCallback ( final int roll) ;
}

public interface Frame  {
    public int getScore () ;
}

public class StrikeFrame implements RollObserverFrame , Frame  {

    private int score = Constants.PINS_PER_FRAME;
    private int addedBonusCount = 0 ;

    @Override
    public int getScore ()  {
        return this .score;
    }

    @Override
    public void rollCallback ( final int roll)  {
        if ( this .addedBonusCount < 2 ) {
            this .score += roll;
            this .addedBonusCount += 1 ;
        }
    }
}

public class SpareFrame implements RollObserverFrame , Frame  {

    private int score = Constants.PINS_PER_FRAME;
    private boolean addedBonus = false ;

    @Override
    public int getScore ()  {
        return this .score;
    }

    @Override
    public void rollCallback ( final int roll)  {
        if (! this .addedBonus) {
            this .score += roll;
            this .addedBonus = true ;
        }
    }
}

public class IncompleteFrame implements Frame  {

    private final int firstRoll;

    public IncompleteFrame ( final int firstRoll)  {
        this .firstRoll = firstRoll;
    }

    @Override
    public int getScore ()  {
        return this .firstRoll;
    }
}

public class NonTenFrame implements Frame  {

    private final int firstRoll;
    private final int secondRoll;

    public NonTenFrame ( final int firstRoll, final int secondRoll)  {
        this .firstRoll = firstRoll;
        this .secondRoll = secondRoll;
    }

    @Override
    public int getScore ()  {
        return this .firstRoll + this .secondRoll;
    }
}

public class FinalFrame implements Frame  {

    private final int score;

    public FinalFrame ( final List<Integer> frame)  {
        this .score = frame.stream().reduce( 0 , Integer::sum);
    }

    @Override
    public int getScore ()  {
        return this .score;
    }
}

希望您同意此新版本有所改进。 写好名字可能很困难,但是正如我们已经看到的那样,可以有很大的不同。 通过遵循本章概述的原则,命名也应该更容易一些。

结论

用代码编写名称时,最好使其简洁,诚实,富有表现力和完整。 如果不遵循良好的命名惯例,即使是设计合理的解决方案也可能难以理解。 这种做法具有挑战性,但可以使读者轻松理解我们的代码。

翻译自: https://hackernoon.com/the-dirty-code-problem-improve-your-game-with-good-naming-practices-wyds362d

代码命名

  • 0
    点赞
  • 1
    收藏
    觉得还不错? 一键收藏
  • 0
    评论

“相关推荐”对你有帮助么?

  • 非常没帮助
  • 没帮助
  • 一般
  • 有帮助
  • 非常有帮助
提交
评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值