駆け出しエンジニアが点字メーカープログラムに挑戦(銀座Rails#41)

自己紹介

はじめまして。
私はRailsで自社サービスの開発を行っている駆け出しエンジニアです。

2020年11月	エンジニアとしての学習を開始
2021年 5月	エンジニアへ転職

この1年間で自分ができるようになったこと、これから学ぶべきことを知る良い機会だと考え、
【オンライン開催】銀座Rails#41の企画に挑戦させて頂きました。

この記事では考えた過程を記載致します。

自分の実装が終わりましたので、これから他の方の実装も見ていきたいと思います。
至らない点は、優しくご指摘頂ければ幸いです。

※ 2022/1/12に一部補足となる説明を追加しました。

作成したPR

どうやって作ろう？ (設計)

まずはどのように実装するかを考えました。
他の人のコードを見ないようにしていましたが、
できれば他の人がやってなさそうな方法にしたいと思いました。

初めに思い浮かんだのは、母音と子音に分ける点字の仕様に合わせた方法でした。
しかし始めに思いついたものに飛びつかず、
この２つの分け方が相応しいのか、もっと良い方法がないか、
と自分への問いかけを続け、再考することにしました。

そこで今回の出力では改行コードで3段に分かれていることに気づき、
母音と子音の2つの分け方より、3段に分けることで表示までスムーズに行えるのではないかと考えました。

TenjiMaker.new.to_tenji("A HI RU")
=> o- o- --\n-- o- -o\n-- oo --
# o- o- --
# -- o- -o
# -- oo --

今回の実装では、母音子音で分けるのではなく、
表示方法を考慮し、3段に分ける方法で進めることにしました。
(結果的に拡張性の低さを感じてしまいましたが、その理由は苦労したところに記載しました)

(上段) => o- o- --
(中段) => -- o- -o
(下段) => -- oo --

実装開始 (STEP1)

最初のコミットへのリンクです。
まずはベタ書きでテストが通る状態を実装しました。
1列毎の表示方法は4パターンなので定数として保持しています。
upper などのそれぞれのメソッドでどこに点があるかを伝え、
tenji_format メソッドで点に変換しています。
N は上手く吸収できなかったので潔く特別扱いしました。
最後に \n で繋いで完成です。

tenji_maker.rb

class TenjiMaker
  TENJI_POINTS = { all: "oo", left: "o-", right: "-o", none: "--"}.freeze

  def to_tenji(text)
    upper_points_position  = upper(text)
    middle_points_position = middle(text)
    lower_points_position  = lower(text)

    [
      tenji_format(upper_points_position),
      tenji_format(middle_points_position),
      tenji_format(lower_points_position),
    ].join("\n")
  end

  private

  def upper(text)
    text.split(" ").map do |char|
      next :none if char =="N"

      # 省略しています
      if "--となる場合"
        :none
      elsif "-oとなる場合"
        :right
      ...
      end
    end
  end

  # middleとlowerも同様に定義しています
  ...

  def tenji_format(points_position)
    points_position.map do |position|
      TENJI_POINTS[position]
    end.join(" ")
  end
end

クラスに分離 (STEP2)

ひとまず動くものはできたので、リファクタリングを始めました。
幸い、テストは用意されていたので、このテストがグリーンであることを心の拠り所にしました。
(追加のテストを書いていないことについてはおまけに記載しました)

まずは先程のprivateメソッドが肥大化していたので別クラスに分離しました。
(この時点で同じものが並んでいたので、基底クラスからの継承を考えていました。これが正しかったかは…)

tenji_marker.rb

class TenjiMaker
  def to_tenji(text)
-   upper_points_position  = upper(text)
-   middle_points_position = middle(text)
-   lower_points_position  = lower(text)
+   upper_points_position  = Upper.new(text).position
+   middle_points_position = Middle.new(text).position
+   lower_points_position  = Lower.new(text).position
  # 以下同様
  ...
end

tenji_marker/upper.rb

class TenjiMaker
 class Upper
  attr_reader :text

  def initialize(text)
    @text = text.split(" ")
  end

  def position
    text.map do |char|
      next :none if char =="N"

      # 省略しています
      if "--となる場合"
        :none
      elsif "-oとなる場合"
        :right
      ...
      end
    end
  end
end

特有の処理を切り出す (STEP3)

STEP4に向けた下準備を行います。
それぞれのクラスでは同じようにtextに対してmapしています。
違うところは、それぞれの条件だけです。
ここで特有の処理と共通に処理とに分けることでリファクタリングを進めて行きます。

具体的にはNの時はどうなるか、noneとなる条件、rightとなる条件などを
privateメソッドとして切り出しました。

tenji_maker/upper.rb

def position
  text.map do |char|
-   next :none if char == "N"
+   next for_N if char == "N"
    # 省略しています
-   if "--となる場合"
+   if none?(char)
      :none
-   elsif "-oとなる場合"
+   elsif right?(char)
      :right
    ...
    end
  end
end

private

def for_N
  :none
end

def none?(char)
  "--となる場合"
end
# 以下、それぞれのパターンが続きます
...

基底クラスの作成 (STEP4)

ここまでで、クラスの特有の処理と共通の振る舞いを分離することができました。
ここで基底クラスを作って共通の処理を移動します。
私が最初に知ったデザインパターン「Template Method」を使用しました。

tenji_maker/base.rb

class Base
  attr_reader :text

  def initialize(text)
    @text = text.split(" ")
  end

  def position
    text.map do |char|
      next for_N if char == "N"

      if none?(char)
        :none
      elsif right?(char)
        :right
      #以下省略します
      ...
      end
    end
  end
end

tenji_maker/upper.rb

- class Upper
+ class Upper < Base

    private

    def for_N
      :none
    end

    # 以下省略
    ...
  end

初めに書いた点字組み立ての処理を整理 (STEP5)

最後の修正です。
初めに作った点字を組み立てる処理をメソッドにまとめます。
それぞれ作成したクラスは、共通の親クラスのインターフェースを継承しているので、
イテレータで同じように呼び出せます

tenji_maker.rb

+ require_relative "tenji_maker/base"
+ require_relative "tenji_maker/upper"
+ require_relative "tenji_maker/middle"
+ require_relative "tenji_maker/lower"

class TenjiMaker
  # 定数は省略しています。

  def to_tenji(text)
-   upper_points_position  = Upper.new(text).position
-   middle_points_position = Middle.new(text).position
-   lower_points_position  = Lower.new(text).position
-   [
-     tenji_format(upper_points_position),
-     tenji_format(middle_points_position),
-     tenji_format(lower_points_position),
-   ].join("\n")
+   tenji_build(Upper.new(text), Middle.new(text), Lower.new(text))

  private

+ def tenji_build(*rows)
+   rows.map do |row|
+     tenji_format(row.position)
+   end.join("\n")
+ end

  def tenji_format(points_position)
  ...
end

よかったこと

どうやって表示するかは to_tenji メソッドの責務として
どこに表示するかの判定は独自クラスの責務として分けることができました。
約束事はどこに表示するかをシンボルで渡すということだけです。

もし表示パターンが o- から !- などに変わっても独自クラスには影響しません。
また表示する点字が変わっても(変わらないとは思いますが)

例えばAの時は中段左に点を打つとなった場合
o- => --
-- => o-
-- => --

元のメソッドは伝えられた表示位置で表示するだけなので、
to_tenjiメソッドを修正する必要はありません。

苦労したところ

おそらくRailsだとやってくれているのだと思いますが、
素のRubyだと別ファイルを読み取る必要があることを理解していませんでした。

tenji_maker.rb

require_relative "tenji_maker/base"
require_relative "/tenji_maker/upper"
require_relative "/tenji_maker/middle"
require_relative "/tenji_maker/lower"

ロジックに関しては、特に中段の分岐が苦労しました。
基本的に左は母音、右は子音の領域にになっているので、あまりきれいには分けられませんでした。
return true if char == "A" || char == "U" のように先に母音だけ処理すべきだったかなと、書きながら思いました。

tenji_maker/middle.rb

def none?(char)
+ return true if char == "A" || char == "U"
- char.start_with?("A", "U", "K", "N", "H", "Y", "W") && char.end_with?("A", "U")
+ char.start_with?("K", "N", "H", "Y", "W") && char.end_with?("A", "U")
end

def right?(char)
  char == "YO" || (char.start_with?("S", "T", "M", "R") && char.end_with?("A", "U"))
end

def left?(char)
+ return ture if char == "I" || char == "E" || char == "O"
- char.start_with?("I", "E", "O", "K", "N", "H") && char.end_with?("I", "E", "O")
+ char.start_with?("K", "N", "H") && char.end_with?("I", "E", "O")
end

また、今回対象外の文字への拡張性が低くなってしまったと思いました。
新しい文字を追加する為には3つのクラスを変更しないといけません。

そして濁点や拗音への対応方法がすぐに思いつきませんでした。
条件を判定する為に独自クラスに実装を追加し、該当する点を追加する、とかでしょうか？
条件を追加して分岐が増えていくのは好ましくありませんね。

さらに、次に変更を行うのが自分ではないとしたら、どこに追加すべきかを考えないといけません。

今動くものという考えから、これからも動き続けるものという考えを身につけていくことは、
私にとって2022年の課題の1つだと思いました。

メッセージ

今回、このような素敵な企画をありがとうござました。
1年間で身につけたこと、これから身につけていかないといけないことを確認するきっかけとなりました。
まだ学習を始めてから日は浅いですが、毎日、楽しく勉強しています。
これからも、新しいことに挑戦するエンジニアとして活動できるように努めて参ります。
本当にありがとうございました。

おまけ

今回、追加したクラスのテストを書きませんでした。
エンジニア転職後、テストがあることの有り難さや大切さを感じ、
テストについて比較的多くの時間を費やしました。

書かないという選択をしたのは、追加したクラスが単独で、もしくは直接呼ばれることがないという点から書かないという判断をしました。
追加したクラスは必ず、to_tenji メソッドから呼ばれます。
この to_tenji メソッドのテストは既にあり、私の中では追加したクラスのテストを書くことが privateメソッドのテストを書くような感覚にありました。
追加したクラスは to_tenji を通してテストしていると考え、
現時点ではテストを書かないという判断に至りました。

しかし、クラス内の条件を変更や追加する可能性を考えると、書いておけば良かったのかなとも思います。
テストについては、もっと学んで行きたいと考えているので、学んだことをQiitaに書いてみようかなと考えています。

class UpperTest < Minitest::Test
  def test_a_hi_ru
    upper_position = TenjiMaker::Upper.new('A HI RU').position
    assert_equal [:left, :left, :all], upper_position
  end
end