Ruby on Rails: Если у вас есть 50 операторов if-else в вашем действии after_create, это замедлит ваше приложение? - PullRequest
3 голосов
/ 04 февраля 2010

Использование 50 операторов if-else слишком ресурсоемко для одного действия?

Я делаю что-то вроде этого:

if team.players.count > 1
   assign_team_type(..)
elsif team.players.count > 3
   assign_team_type(..)
...
etc.
...
end

Кроме того, более эффективно разместить 50 операторов if-else в вашем действии create внутри вашего контроллера вместо метода after_create? Или было бы эффективнее вместо этого использовать оператор переключения регистра или просто вообще его избегать?

РЕДАКТИРОВАТЬ: Спасибо за очень быстрые ответы! Код для спортивного турнира сообщества назначает команды в зависимости от количества игроков в этой команде. Я пытаюсь написать что-то, что назначает тип команды для каждой команды в зависимости от того, сколько игроков добавлено в эту команду. Таким образом, есть команды для 1 игрока, 3 игроков, 5 игроков, 7 игроков и т. Д., До 200 игроков, что требует в общей сложности 50 операторов if-else.

Операторы выполняются в Players_controller, после того как пользователь посещает http://localhost/players/new,, добавляет игрока, а затем приложение решает, какой команде назначить его или ее команду, исходя из того, сколько игроков в настоящее время входит в эту команду. Это очень просто (базовое CRUD-приложение, которому просто нужны эти 50 операторов if-else)

models:

Team (has_many :players)
Player (belongs_to :team)

scaffold team name:string team_type:string
scaffold player team_id:integer name:string

Вот и все: :)

Ответы [ 6 ]

9 голосов
/ 04 февраля 2010

Вы можете попробовать переписать его как


assign_team_type(case team.players.count
                 when 2    then ...
                 when 3..5 then ...
                 else raise "Assignment failed"
                 end
)

, что, вероятно, должно быть быстрее, поскольку team.players.count оценивается только один раз. Кроме того, он чище и короче. Ориентир поможет.

7 голосов
/ 04 февраля 2010

Тьфу, это очень неприятный запах кода. Скорее всего, у вас есть существенное дублирование, которое можно поместить в модель или, по крайней мере, свернуть в другое.

Почему бы не поместить их в массив или хэш? Что-то вроде:

TeamTypes = { 1 => something, 2 => something_else, .. }

assign_team_type( TeamTypes[team.players.count] )
3 голосов
/ 04 февраля 2010

Исходя из вашей новой информации, я предлагаю добавить столбец max_players к вашей модели team_types и запросить у модели что-то вроде этого:

find(:first, :conditions => ["max_players < ?", self.player_count], :order => "max_players ASC")

Это делает его полностью динамичным, и вы можете управлять назначением типа команды через веб-интерфейс.

2 голосов
/ 04 февраля 2010

Мой первый инстинкт состоит в том, чтобы упаковать ваши данные в двумерный массив, например:

x=[[1, "arg 1"],
   [3, "arg 2"],
   [9, "arg 3"],
   ...
  ]

, где первый столбец - это количество команд, а второй - параметр, который вы должны передать assign_team_type.

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

func_arg = x.collect{|w| w[0] < team.players.count ? w[1] : nil}.compact.last
assign_team_type(func_arg)

Я допускаю, что это своего рода C-подобный подход, но он позволит вам на лету изменить параметры группировки, просто изменив массив.

1 голос
/ 04 февраля 2010

Как насчет присвоения каждой функции хешу / dict?

d={1:assign_team_type(1),2:assign_team_type(2)}
0 голосов
/ 06 февраля 2010

современные процессоры способны «предсказывать переходы»

когда есть анализ if-else, процессор попытается угадать, какой ему понадобится, если он угадает неправильно, что может замедлить ваше приложение.

Я бы порекомендовал рефакторинг любых слишком больших ветвящихся секций в красивые чистые операторы switch.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...