2017-02-13 13 views
-2

このコードを効率的にリファクタリングして2回実行するのが適切な方法であると思っています。2つの変数を一度に実行する方法

class Hamming 
    def compute (a, b) 

    a.to_a.split("") 
    b.to_a.split("") 

    end 
end 

一度 a, b = 1, 2のように2つの変数を割り当てることに類似したものはありますか?

+6

そのコードは意味をなさない。 'split'は配列ではなく文字列に対して機能し、結果とは何もしません。あなたが「効率性」と言うとき、どうやって自分自身を繰り返さないのですか? – Schwern

+0

はい。私は自分自身の繰り返しを避ける方法を尋ねるつもりだった。私はあなたが配列上で分割を実行することができないことを理解するとは思っていませんでした。 – Rich

答えて

1

コードが意味をなさないので、私はあなたがどうやって自分自身を繰り返すのを避けているかと思います。

シンプルに、別の方法を記述して呼び出します。ここでは、どのフレーズが長いのかを知りたいが、たくさんの空白を無視したいという例があります。したがって、foo bar12345678より長くはありません。

def longer_phrase(phraseA, phraseB) 
    normalizedA = normalize(phraseA) 
    normalizedB = normalize(phraseB) 

    return normalizedA.length > normalizedB.length ? phraseA : phraseB 
end 

def normalize(phrase) 
    normalized = phrase.gsub(/\s+/, ' '); 
    normalized.strip! 

    return normalized 
end 

puts longer_phrase("foo   bar ", "12345678") 

作業を行う前にすべてのデータを正規化する必要があります。これはあなた自身の繰り返しを避けます。文字列を正規化するために、すべての作業のポイントを知っているので、コードを理解しやすくします。また、他の場所で使用するための正規化機能を提供するので、データを同じ方法で正規化しています。

2

まず、コードが無効です。 #to_aは配列を返します。配列に#splitが定義されていません。あなたのコードだった場合、あなたのコードだけで、最後に実行されたステートメント(b.to_s.split(""))の値を返しますので、

第二に、有効(たとえば、a.to_s.split(""); b.to_s.split("")、それは実際には、多くのしないだろう。どちらも#to_s#splitは非-destructive、これは、彼らがaまたはbを変更しないことを意味します - あなたは、この関数から取得する唯一の効果は、それが返すものです、あなたはどのような方法でa.to_s.split("")の結果を返しません:それは忘れられ

あなたが意図した場合。このようなもの:

class Hamming 
    def compute(a, b) 
    [ 
     a.to_s.split(""), 
     b.to_s.split("") 
    ] 
    end 
end 

これはかなり読みやすいです。しかし、あなただけの.to_s.split("")よりも複雑な操作があった場合、独自の機能にそれを分離する方が良いでしょう:

class Hamming 
    def compute(a, b) 
    [ 
     list_chars(a), 
     list_chars(b) 
    ] 
    end 
    private def list_chars(str) 
    str.to_s.split("") 
    end 
end 

あなたはmapを使用して、さらにそれを簡略化することができますが、あなたが複数ある場合、それは本当にだけ必要になります2要素のケースはそのままの状態で完全に読みやすいため、しかし、ここに行く:

class Hamming 
    def compute(a, b) 
    [a, b].map { |x| list_chars(x) } 
    end 
    private def list_chars(str) 
    str.to_s.split("") 
    end 
end 

また、あなたが.split("")よりも、より読みやすいあなたにイテレータを与える方法#each_char、、、そして多くの場合正しい選択を確認したい場合があります。

EDIT:それについて少し考えた後、2つの文字列の間のハミング距離を評価する方法を開始しているようです。その関数が2つの文字列の文字を単に返すようにするつもりはないということです。あなたは絶対に文字そのものを、そしてないイテレータを持っている必要がある場合は、

def compute(a, b) 
    a_chars = a.to_s.each_char 
    b_chars = b.to_s.each_char 
    # ... 
end 

または多分この:その場合、私はちょうどこれを書い思い

def compute(a, b) 
    a_chars = a.to_s.each_char.to_a 
    b_chars = b.to_s.each_char.to_a 
    # ... 
end 

私はあなたが探していると考えているソリューションをこれは次のようになります:

def compute(a, b) 
    a_chars, b_chars = *[a, b].map { |x| x.to_s.each_char.to_a } 
    # ... 
end 

しかし、私は非DRYより読みにくいと考えています。あなたが本当にそれを乾燥させるためにしたい場合は、それはやり過ぎのビットが、この中であっても、上記のように、独自の機能にlistificationを抽出し、ちょうど実際に両方の長所である

a_chars = list_chars(a) 
b_chars = list_chars(b) 

を行いますケース:控えめで節約のために、保守が容易で、自己文書化で読みやすいです。

関連する問題