2016-10-26 10 views
-1

以下のコードはできるだけ少ない行に減らそうとしています。私はそれを短縮するために使用することができる他のルビーのトリックですか?私は誰もが提供できるすべての助けに感謝します。ルビーコードカット(可能な限り数行)

条クラス:

class Article 
    attr_accessor :id, :price, :quantity 
    def initialize(id, price, quantity) 
     @id, @price, @quantity = id, Float(price), quantity.to_i 
    end 
end 

注文クラス:

class Order 
    def initialize(name) 
     @a, i = [], 0 
     input = File.open(name, "r") 
     while(id = input.gets.chomp) 
      j, price = 0, input.gets.chomp 
      while(j<@a.length) 
       if(@a[j].id.eql?(id.to_i)) 
        @a[j].quantity += 1  
       end 
      end 
      else 
       @a[i] = new Article(id,price,1) 
       i+=1 
      end 
     end 
    end 

    def orderCost 
     sum = 0 
     @a.each { |e| sum+=(e.price * e.quantity)} 
     sum = ((sum*1.07) + 2.99) 
    end 

    def displaySelectArticles 
     min, max = @a[0], @a[0] 
     @a.each do |e| 
      if(min.cost > e.cost) 
       min = e 
      end 
      if(max.cost < e.cost) 
       max = e 
      end 
      sum += e.cost*e.quantity and q += e.quantity 
     end 
     puts "Min: #{min.cost} | Max: #{max.cost} | Avg: #{Float(sum)/q}" 
    end 
end 
+0

また、このコードを作成するためのより良い方法が見つかった場合は、是非お知らせください。私はこのコードをできるだけ効率的にしようとしています。 –

+4

あなたがゴルフしていない限り、コードサイズについて心配するべきではありません。読みやすさについてもっと心配してください。 – Carcigenicate

+0

あなたはhttp://codegolf.stackexchange.com/を知っていますか?このタイプの質問をするより良い場所かもしれません。 – kristianp

答えて

0

私はここに私のベストを尽くしたが、あなたのinitializeメソッドは、私には任意の論理意味をなさないdidntの。うまくいけば、これは少なくとも正しい方向にあなたを導くことができます。 免責事項:これはどれもテストされておらず、私はそれをどのような方法で覚えているのか書きました。

class Order 
def initialize(name) 
    @a, i = [], 0 
    File.readlines(name) do |line| 
     # This while loop makes no sense to me 
     # Seems like a surefire infiniteloop because if id = 3, it is always true 
     # Maybe you meant to do an operator like == for comparison 
     while(id = line) 
      j, price = 0, line 
      while j < @a.length 
       @a[j].quantity += 1 if(@a[j].id.eql?(id.to_i)) 
      end 
     else 
      @a[i] = new Article(id, price, 1) 
      i += 1 
     end 
    end 
end 

def orderCost 
    # I would probably make this two lines because its unreadable 
    (@a.inject(0) { |accum, e| accum + (e.price * e.quantity) } * 1.07) + 2.99 
end 

def displaySelectArticles 
    min, max = @a[0], @a[0] 
    @a.each do |e| 
     min = e if min.cost > e.cost 
     max = e if max.cost < e.cost 
     sum += e.cost * e.quantity 
     q += e.quantity # I have no idea how you got a q 
    end 
    puts "Min: #{min.cost} | Max: #{max.cost} | Avg: #{Float(sum)/q}" 
end 
end 
-1

記事のクラスがあるため 三つの変数が同時に割り当てられているジャンクの寄せ集めの重大な注意が必要です。 3つの非常にシンプルな 割り当てにそのを分割:

class Article 
    attr_accessor :id, :price, :quantity 

    def initialize(id, price, quantity) 
    @id = id 
    @price = price.to_f 
    @quantity = quantity.to_i 
    end 
end 

x.to_fを使用すると、Float(x)に好適である、それは変換 に穏やかなアプローチがあります。あなたが財務計算を行っているなら、stronly BigDecimal のようなものを浮動小数点数として使用することをお勧めします。 $ 1.99は の方法が$ 1.989999423489になり、丸められたときにはここでセントを「失う」と表示される可能性があります。 またはそこにあります。 BigDecimalクラスは、値を厳密に表すことでこれを回避します。

残りの部分は基本的にシンプルであるべきものを行うために、独自のルーチンを書いて効果的にし、代わりにEnumerable を使用していないという問題を攻撃している:

class Order 
    def initialize(name) 
    @items = [ ] 

    File.open(name, "r") do |input| 
     while(id = input.gets.chomp) 
     price = input.gets.chomp 

     # find locates the first matching thing in the array, if any. 
     existing = @items.find do |item| 
      item.id == id 
     end 

     if (existing) 
      existing.quantity += 1 
     else 
      items << Article.new(id, price, 1) 
     end 
    end 
    end 

    def item_cost 
    # Inject is good at iterating and accumulating 
    @items.inject(0.0) do |sum, item| 
     sum + item.price * item.quantity 
    end 
    end 

    def item_count 
    @items.inject(0) do |sum, item| 
     sum + item.quantity 
    end 
    end 

    def order_cost 
    item_cost * 1.07 + 2.99 
    end 

    def display_select_articles 
    # minmax_by finds min and max entries based on arbitrary criteria 
    min, max = @items.minmax_by { |item| item.price } 

    sum = item_cost 
    count = item_count 

    puts "Min: %f | Max: %f | Avg: %f" % [ 
     min ? min.cost : 0.0, 
     max ? max.cost : 0.0, 
     count > 0.0 ? (sum/count) : 0.0 
    ] 
    end 
end 

可能な限り穀物で行く、そしてそれは、Rubyの構造体を使用することを意味それらのネイティブメソッド。あなたが書いているより多くのコードをこれからずらすほど、あなたのコードは醜いものになります。時には、難しい問題を解決していて選択肢がありませんが、この例はそのようなケースの1つではありません。 Rubyのやり方をすれば、ここのすべてがより簡単になります。

関連する問題