Refactoring of Gilded Rose

前言

本文简单介绍一个重构到策略模式的小例子。
GitHub地址:https://github.com/janipeng/GildedRose.git

里面有三个branch,分别是我做了三次重构的过程,有一点点差别。重构过程中的视频如下。录制的是forVideo这个branch。视频地址如下:
http://v.youku.com/v_show/id_XMzQ2MTI0ODczNg==.html?spm=a2h3j.8428770.3416059.1

因为有朋友说视频中没有讲解过程,所有我又重新录制了一个视频。这个视频是录制的forVideoTwo 这个branch的,有一些简单的讲解和IntelliJ IDEA快捷键的说明,IDE快捷键的使用可以帮助我们提升写代码的体验和效率,也是我这里想要表达的,完成这个重构练习的过程中,完全可以不需要使用鼠标,所有的操作都可以用键盘快捷键来操作。视频地址如下:
http://v.youku.com/v_show/id_XMzQ2NDE4MDMxNg==.html?spm=a2h3j.8428770.3416059.1

需要重构的代码

package com.jani;

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

public class GildedRose {

    static List<Item> items = null;

    /**
     * @param args
     */
    public static void main(String[] args) {
        
        System.out.println("OMGHAI!");
        
        items = new ArrayList<Item>();
        items.add(new Item("+5 Dexterity Vest", 10, 20));
        items.add(new Item("Aged Brie", 2, 0));
        items.add(new Item("Elixir of the Mongoose", 5, 7));
        items.add(new Item("Sulfuras, Hand of Ragnaros", 0, 80));
        items.add(new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20));
        items.add(new Item("Conjured Mana Cake", 3, 6));

        updateQuality();
}


    
    public static void updateQuality()
    {
        for (int i = 0; i < items.size(); i++)
        {
            if ((!"Aged Brie".equals(items.get(i).getName())) && !"Backstage passes to a TAFKAL80ETC concert".equals(items.get(i).getName())) 
            {
                if (items.get(i).getQuality() > 0)
                {
                    if (!"Sulfuras, Hand of Ragnaros".equals(items.get(i).getName()))
                    {
                        items.get(i).setQuality(items.get(i).getQuality() - 1);
                    }
                }
            }
            else
            {
                if (items.get(i).getQuality() < 50)
                {
                    items.get(i).setQuality(items.get(i).getQuality() + 1);

                    if ("Backstage passes to a TAFKAL80ETC concert".equals(items.get(i).getName()))
                    {
                        if (items.get(i).getSellIn() < 11)
                        {
                            if (items.get(i).getQuality() < 50)
                            {
                                items.get(i).setQuality(items.get(i).getQuality() + 1);
                            }
                        }

                        if (items.get(i).getSellIn() < 6)
                        {
                            if (items.get(i).getQuality() < 50)
                            {
                                items.get(i).setQuality(items.get(i).getQuality() + 1);
                            }
                        }
                    }
                }
            }

            if (!"Sulfuras, Hand of Ragnaros".equals(items.get(i).getName()))
            {
                items.get(i).setSellIn(items.get(i).getSellIn() - 1);
            }

            if (items.get(i).getSellIn() < 0)
            {
                if (!"Aged Brie".equals(items.get(i).getName()))
                {
                    if (!"Backstage passes to a TAFKAL80ETC concert".equals(items.get(i).getName()))
                    {
                        if (items.get(i).getQuality() > 0)
                        {
                            if (!"Sulfuras, Hand of Ragnaros".equals(items.get(i).getName()))
                            {
                                items.get(i).setQuality(items.get(i).getQuality() - 1);
                            }
                        }
                    }
                    else
                    {
                        items.get(i).setQuality(items.get(i).getQuality() - items.get(i).getQuality());
                    }
                }
                else
                {
                    if (items.get(i).getQuality() < 50)
                    {
                        items.get(i).setQuality(items.get(i).getQuality() + 1);
                    }
                }
            }
        }
    }

}

这段代码很明显的坏味道就是if else 太多,而且很多嵌套。代码的可读性和可维护性非常差。

重构

有必要再啰嗦一下重构的定义:

名词定义:对软件内部结构的一种调整,目的是在不改变软件可观察行为的前提下,提高其可靠性,降低其修改成本。

动词定义:使用一系列软件重构手法,在不改变软件可观察行为的前提下,调整其结构。

重构的几个简单规则:

去除代码坏味道(Remove Code Smell)
代码总是工作(Always Work)
可以随时停止(Can Stop at Anytime)
可以随时开始(Can Continue at Anytime)
可以随时恢复(Can Revert at Anytime)

重构的一个有趣的口诀:

旧的不变(Keep Old)
新的创建(Create New)
一步切换(Switch to New)
旧的再见(Bye to Old)

在这个重构练习中,我会尽量遵守上面的规则。并且做到“小步提交,频繁测试”。

重构有一个非常重要的前提: 需要有足够的自动化测试。
在这次重构练习之前,我先给需要重构的代码加上了单元测试。加单元测试的过程不在录制视频中。单元测试的代码可以去GitHub上面看。

重构过程分析

GildedRose 中的 updateQuality 方法,if else 条件分支和嵌套很多。代码的可读性非常差,想要通过看代码理解代码逻辑会非常困难。简单分析一下,可以发现,updateQuality 方法是更新item的quality和sellIn的,并且不同的item会有不同的更新规则。不同的item有不同的策略进行更新,比较容易可以想到用策略模式。

根据上面提到的重构的口诀,我们先不去修改旧的代码,而是添加一个新的function。

public static void updateQuality2() {
        ItemStrategy itemStrategy;
        for (Item item : items) {
            switch (item.getName()) {
                case "Aged Brie":
                    itemStrategy = new AgedBrie();
                    break;
                case "Backstage passes to a TAFKAL80ETC concert":
                    itemStrategy = new BackstagePasses();
                    break;
                case "Sulfuras, Hand of Ragnaros":
                    itemStrategy = new Sulfuras();
                    break;
                default:
                    itemStrategy = new NormalItem();
            }
            itemStrategy.update(item);
        }
}

在这个function里面先创建一个ItemStrategy的接口类。里面有一个update的function。然后根据item的name去创建不同的实现类。
接着,把updateQuality里面的代码拷贝到每一个具体的实现类里面的update方法。然后把updateQuality2替换updateQuality。跑一下所有的单元测试。

接着,看每一个ItemStrategy的具体实现类。如下面的NormalItem。把所有跟NormalItem没关系的代码都删掉。就变成下面的样子:

public class NormalItem implements ItemStrategy {
    @Override
    public void update(Item item) {
        if (item.getQuality() > 0) {
            item.setQuality(item.getQuality() - 1);
        }

        item.setSellIn(item.getSellIn() - 1);

        if (item.getSellIn() < 0) {
            if (item.getQuality() > 0) {
                item.setQuality(item.getQuality() - 1);
            }
        }
    }
}

当然,这个里面的代码还是可以继续在进行一些重构的,但不是这次重构的重点。

更多更详细的内容可以去GitHub上面看,这里就不过多阐述了。

小结

本文介绍的一个比较简单的重构练习题,但是如果没有想到用这样的方式去重构的话,其实也并不是很容易。在实际工作的生产代码里面,假如有根据不同的类型有不同的处理逻辑,可以考虑用这个的方式去做重构。
同时,在反复多次完成这个重构练习的过程中,对IntelliJ IDEA的使用会有很好的练习。
如果想成为画家,可以先从练习画鸡蛋开始。就像这样。

最后,这份代码的需求是一个Coding Dojo的题目。大家有兴趣可以看下图中的需求去用TDD的形式做一个练习。后期,我也会尝试写一些Coding Dojo的文章分享出来。希望可以自己练习和给大家一定的帮助。

Gilded Rose.JPG
最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 214,313评论 6 496
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 91,369评论 3 389
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 159,916评论 0 349
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 57,333评论 1 288
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 66,425评论 6 386
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 50,481评论 1 292
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 39,491评论 3 412
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 38,268评论 0 269
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 44,719评论 1 307
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 37,004评论 2 328
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 39,179评论 1 342
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 34,832评论 4 337
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 40,510评论 3 322
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 31,153评论 0 21
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 32,402评论 1 268
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 47,045评论 2 365
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 44,071评论 2 352

推荐阅读更多精彩内容