Ruby Ruby 菜鸟代码拍砖

tommylike · 2015年10月29日 · 最后由 tommylike 回复于 2015年10月30日 · 2368 次阅读

公司最近组织了一个技能鉴定,试题如下: 有一个采用 CP936 编码(也称为 GBK)的文本文件,名字为 a.txt,该文件中存在一些连续重复的内容,比如“HelloHello”,“国庆 60 周年国庆 60 周年”,这是因为作者在编辑的时候不小心而导致的错误,现在要求你编写一个程序来自动更正它,并且为了满足国际化的要求,必须使用小端模式的 UTF-16 编码来另存为新文件 (b.txt),具体的去重规则是:
程序对该文件进行分析处理,将上面提到的连续重复两次的内容删除掉重复部分,只保留原始内容。其它不涉及重复的部分保持原样不变。并且,只有被重复的字符串超过 3 个字符(注意是字符,不是字节)的时候,才算重复,小于或等于 3 个字符的则不需要处理

require 'pathname'
def magic_change(file)      
    begin   
        #read file      
        return unless File.exists? file
        original_string = IO.read(file,:encoding=>'GBK').encode!('UTF-8')
        #deduplicate characters     
        original_string =original_string.gsub(/([\S]{4,})\1/){Regexp.last_match[1]}
        #write file
        File.open(Pathname.new(File.join(__dir__,'b.txt')).cleanpath,"w:UTF-16LE") do |file|
            file.write original_string
        end
    rescue =>error
        p error
    end
end

magic_change(ARGV[0])

本题解法中的正则应该是没有问题的的,社区大神们看下对代码中的 ruby 规范,优化,异常处理,常见函数用法等方面有没有更好的建议或者问题,最终目标是写一份正统的 ruby 代码。

write 部分可以简化一点吧?

def magic_change(file)
  # read file
  return unless File.exist? file
  original_string = IO.read(file, encoding: 'GBK').encode!('UTF-8')
  # deduplicate characters
  original_string = original_string.gsub(/([\S]{4,})\1/) { Regexp.last_match[1] }
  # write file
  File.write('b.txt', original_string, external_encoding: Encoding::UTF_16LE)
rescue => error
  p error
end

magic_change ARGV[0]

变量名也可以更清晰,如果不关心内存占用的话。

final_text = source_text.gsub(/([\S]{4,})\1/) { Regexp.last_match[1] }
  1. 代码基本没有格式,每行也比较长,读起来很费劲。
  2. 函数的参数应该是 file_name 之类的,不是 file。
  3. original_string 的名字不对,既然要对它进行修改,就不要取这个名字。
  4. File.exists? 已经标记为 deprecate,换用 File.exist
  5. 已经用了 IO.read,能不能考虑用一下 IO.write,这样风格比较统一。
  6. 注释太啰嗦,无用,全都去掉。(正则表达式可以考虑写一下注释)
  7. 对 string 的修改可以单独写一个函数,比如什么 uniq(str) 之类的,然后可以写 UT。UT case 里面需要把 ABCDABCDABCD 这些特殊情况的处理结果列出来。
  8. 虽然你想写一个通用的函数,然而输入输出都已经限定了。如果非要写,那么对于各种 error 的处理要单独体现,比如非 GBK 编码的文件。
  9. 让老板把 Code Review 搞好,比技能鉴定管用。

@emayej 都说得非常漂亮了。

再补充一个异常捕捉的,异常捕捉应该是特定的,可控的。就是你应该知道这部分代码什么可能会出什么类型的异常然后用既定的手段去处理它。通用的rescue =>error一般来说是 anti pattern, 因为会掩盖其他地方不属于这部分代码的错误。当然做个 demo 这么写都是无所谓了。

#5 楼 @emayej 每个意见都细读了 特感谢~

需要 登录 后方可回复, 如果你还没有账号请 注册新账号