2014-11-19 5 views
2

я следующий код,Написание кратким множественным еще/если заявление

@checkedin=[] 
@accepted=[] 
@rejected=[] 
result.each do |parse_order| 
    orderId = parse_order['orderID'] 
    if parse_order['status'] == -1 
     @rejected << orderId 
    elsif parse_order['status'] == 1 
     @accepted << [orderId, parse_order['createdAt']] 
    elsif parse_order['status'] == 2 
     @checkedin << [orderId, parse_order['createdAt']] 
    elsif parse_order['status'] == 3 
     next  
    end 
end 

есть лучший способ его кратким. Спасибо.

ответ

5

Вы можете использовать однострочные условия. Кроме того, next не нужен в конце блока.

@checkedin = [] 
@accepted = [] 
@rejected = [] 

result.each do |parse_order| 
    orderId = parse_order['orderID'] 
    status = parse_order['status'] 

    @rejected << orderId if status == -1 
    @accepted << [orderId, parse_order['createdAt']] if status == 1 
    @checkedin << [orderId, parse_order['createdAt']] if status == 2 
end 
3
@checkedin=[] 
@accepted=[] 
@rejected=[] 
result.each do |parse_order| 
    orderId = parse_order['orderID'] 
    case parse_order['status'] 
    when -1 then @rejected << orderId 
    when 1 then @accepted << [orderId, parse_order['createdAt']] 
    when 2 then @checked_in << [orderId, parse_order['createdAt']] 
    end 
end 

Вам не нужно последний случай

Я рекомендовал бы вытаскивая каждый из этих случаев для читаемости

result.each do |parse_order| 
    reject(parse_order) || accept(parse_order) || check_in(parse_order) 
end 

def reject(parse_order) 
    @rejected << parse_order['orderId'] if parse_order['orderId'] == -1 
end 

def accept(parse_order) 
    @accepted << [parse_order['orderId'], parse_order['createdAt']] if parse_order['orderId'] == 1 
end 

def check_in(parse_order) 
    @checked_in << [parse_order['orderId'], parse_order['createdAt']] if parse_order['orderId'] == 2 
end 

я бы, вероятно, переименовать parse_order в order для краткости слишком

+0

В этом случае массивы инициализируются на уровне класса? – harshit

+0

Нет, вам придется приписывать все «себе». Я думаю, что решение Пита синтаксически лучше всего из всех размещенных решений –

+0

Я очень предпочитаю то, что у вас есть наверху. Вы сказали, что изменили его на удобочитаемость, но я думаю, что это гораздо менее читаемо. Это в основном потому, что у нас есть многолетний опыт чтения по горизонтали (вверху), и у него неплохо получается. Чтение по вертикали (внизу) просто неэффективно. Кроме того, чем компактнее код (в пределах, конечно), тем больше глаз может занять сразу. Когда я смотрю на ваш код сверху, я знаю, что вы делаете почти мгновенно. На дне я должен охотиться, прежде чем понять общую картину. При написании кода «скорость чтения» является одним из моих самых важных критериев. –

3

Я собираюсь бросить свою шляпу в кольцо. ИМХО это более человечно читаемо и будет легче поддерживать. В то время как она имеет больше строк кода (из-за класса Order), фактическая фильтрация заказов более лаконичен:

require 'order' 
orders = results.map{|result| Order.new(result['status'], result['createdAt'])} 

@checked_in_orders = orders.select {|order| order.checked_in?} 
@accepted_orders = orders.select {|order| order.accepted?} 
@rejected_orders = orders.select {|order| order.rejected?} 

Класс Заказ:

# orders.rb 
class Order 
    REJECTED_STATUS = -1 
    ACCEPTED_STATUS = 1 
    CHECKED_IN_STATUS = 2 

    attr_reader :created_at 

    def initialize(status, created_at) 
    @status, @created_at = [status, created_at] 
    end 

    def rejected? 
    @status == REJECTED_STATUS 
    end 

    def accepted? 
    @status == ACCEPTED_STATUS 
    end 

    def checked_in? 
    @status == CHECKED_IN_STATUS 
    end  
end 
2

Я хотел бы использовать Enumerable#group_by:

REJECTED = -1 
ACCEPTED = 1 
CHECKED_IN = 2 

h = result.group_by {|order| order['status']} 
@rejected = h[REJECTED ].map {|order| order['orderID']} 
@accepted = h[ACCEPTED ].map {|order| [order['orderID'] order['createdAt']]} 
@checked_in = h[CHECKED_IN].map {|order| [order['orderID'] order['createdAt']]} 
0

Вы можете использовать выражение случай:

@checkedin=[] 
@accepted=[] 
@rejected=[] 
result.each do |parse_order| 
    orderId = parse_order['orderID'] 

    case parse_order['status'] 
    when -1 
    @rejected << orderId 
    when 1 
    @accepted << [orderId, parse_order['createdAt']] 
    when 2 
    @checkedin << [orderId, parse_order['createdAt']] 
    when 3 
    next  
    end 
end 

Вы можете увидеть больше случаев использования в случае this blog post и в official documentation.

Смежные вопросы