BOOLÃ-BOOLâ † 'ENUM mapping
Является ли эта часть достаточно чистой? Любые предложения о том, как сделать его более чистым?
if (isCheck)
{
if (isStuck)
{
return GameState.Mate;
}
else
{
return GameState.Check;
}
}
else
{
if (isStuck)
{
return GameState.Stalemate;
}
else
{
return GameState.Ok;
}
}
16 ответов
Я бы использовал таблицу решений здесь:
GameState[,] decisionTable = new GameState[2,2];
decisionTable[0,0] = GameState.Ok;
decisionTable[0,1] = GameState.Stalemate;
decisionTable[1,0] = GameState.Check;
decisionTable[1,1] = GameState.Mate;
return decisionTable[Convert.ToInt32(isCheck), Convert.ToInt32(isStuck)];
Из кода Complete 2nd Edition, глава 19: Общие проблемы управления, стр. 431:
Использовать таблицы решений для замены сложных условий
Иногда у вас сложный тест, включающий несколько переменных. Полезно использовать таблицу решений для выполнения теста, а не использовать ifs или cases. Поиск в таблице принятия решений легче кодировать изначально, имея только пару строк кода и никаких сложных структур управления. Эта минимизация сложности сводит к минимуму возможность ошибок. Если ваши данные изменяются, вы можете изменить таблицу решений без изменения кода; вам нужно только обновить содержимое структуры данных.
Я думаю, что ?
сделает ваш код более компактным и читаемым:
return isCheck
? isStuck ? GameState.Mate : GameState.Check
: isStuck ? GameState.Stalemate : GameState.Ok
В качестве альтернативного варианта решения таблицы решений для решения проблемы @palacsint ситуация в серии bool ^ n традиционно представляется с использованием флагов.
// in a global constants file somewhere
const Stuck = 1 << 0; // 1
const Check = 1 << 1; // 2
const ..... = 1 << n; // 2^n
int decisionTable[] = {
GameState.Ok, // 0b00
GameState.Stalemate, // 0b01
GameState.Check, // 0b10
GameState.Mate, // 0b11
};
// in code
int flags = 0;
if (isStuck(board)) flags |= Stuck;
if (isCheck(board)) flags |= Check;
return decisionTable[flags];
Существует также альтернативный способ построения таблицы решений, которая может быть более читаемой и менее подверженной ошибкам, хотя, возможно, более подробной:
int decisionTable[] = new int[4];
decisionTable[0] = GameState.Ok;
decisionTable[Stuck] = GameState.StaleMate;
decisionTable[Check] = GameState.Check;
decisionTable[Stuck | Check] = GameState.Mate;
Проблема с таблицей решений, используемой @palacsint, или с ее записью в виде функций с рядом аргументов (например, getState(isCheck, isStuck)
) заключается в том, что вы должны помнить точный порядок аргументы, которые становятся громоздкими, когда у вас большое количество флагов. Например, если вы случайно поменяли индексы, такие как: decisionTable[(int)isStuck][(int)isCheck];
, вы можете оказаться в длительном отладочном сеансе, не осознавая свопа. Побитовое ИЛИ является коммутативным, поэтому у него нет этой проблемы.
Прежде всего, я думаю, что ваше решение в порядке.
Сказав это, я могу понять, почему решение разочаровывает. Как показывают многие другие, различные варианты ответа, это одна из тех проблем, которые просто чувствует , как будто она должна иметь простое, элегантное однострочное решение. Но это просто не так. Есть более короткие решения, но они все жертвуют удобочитаемостью.
Как я уже говорил выше, я думаю, что ваш подход хорош, и если я натолкнулся на него в коде, который я просматривал на работе, я бы не стал комментировать его. Но я думаю, что его можно было бы очистить немного. При приближении к вложенному if
я думаю, что лучше всего начинать с самого широкого вопроса /условия во внешнем if
, а затем задавать более узкий вопрос (вопросы) ниже. Кроме того, я думаю, что лучше всего поставить наиболее распространенный случай в верхней части, потому что, когда кто-то читает код (возможно, пытается отладить проблему), именно в этой отрасли они, скорее всего, будут заинтересованы ... также мне это кажется более логичным. Наконец, не стоит бояться сбрасывать в хорошо именную переменную, чтобы сделать код более удобочитаемым.
Учитывая все это, вот мое решение:
var gameShouldContinue = ! isStuck;
if (gameShouldContinue) {
return isCheck ? GameState.Check : GameState.Ok;
} else {
return isCheck ? GameState.Mate : GameState.Stalemate;
}
Это зависит от того, что вы считаете более читаемым, но вы можете использовать двоично-подобный способ выражения переменных, как в таблице истинности:
isCheck isStuck Value ======================= T T Mate Проверка T F F T Stalemate F F Ok
Что бы перевести на нечто более компактное:
if (isCheck && isStuck) return GameState.Mate;
if (isCheck && !isStuck) return GameState.Check;
if (!isCheck && isStuck) return GameState.Stalemate;
if (!isCheck && !isStuck) return GameState.OK;
Каждый раз, когда у вас есть несколько независимых логических переменных, представляющих состояние, рассмотрите возможность использования битового поля с использованием перечисления с применением FlagsAttribute
. Таким образом вам не нужно поддерживать несколько переменных для представления одного состояния.
[Flags]
public enum BoardState
{
None = 0x0,
IsCheck = 0x1,
IsStuck = 0x2,
}
Затем вы можете отобразить эти состояния платы в соответствующее состояние игры. Просто убедитесь, что вы наметили все допустимые комбинации, если вы планируете использовать словарь.
private Dictionary<BoardState, GameState> transition = new Dictionary<BoardState, GameState>
{
{ BoardState.None, GameState.Ok },
{ BoardState.IsCheck, GameState.Check },
{ BoardState.IsStuck, GameState.Stalemate },
{ BoardState.IsCheck | BoardState.IsStuck, GameState.Mate },
};
public GameState Next(BoardState boardState)
{
GameState nextState;
if (!transition.TryGetValue(boardState, out nextState))
throw new ArgumentOutOfRangeException("boardState");
return nextState;
}
В противном случае используйте оператор switch, если отображение становится слишком непослушным или вы хотите сопоставить несколько комбинаций с одним значением (в этом случае это необязательно).
public GameState Next(BoardState boardState)
{
switch (boardState)
{
case BoardState.None:
return GameState.Ok;
case BoardState.IsCheck:
return GameState.Check;
case BoardState.IsStuck:
return GameState.Stalemate;
case BoardState.IsCheck | BoardState.IsStuck:
return GameState.Mate;
default:
throw new ArgumentOutOfRangeException("boardState");
}
}
Затем используйте регулярные манипуляции с битами для установки /очистки флагов. Позаботьтесь об этом, если у вас есть значения, которые представляют несколько значений
BoardState boardState = ...;
// to set a flag
boardState |= BoardState.IsCheck;
// to clear a flag
boardState &= ~BoardState.IsStuck;
// to test a flag
boardState.HasFlag(BoardState.IsCheck);
Вы можете очистить, сохраняя это красивым и чистым, выставляя это через логические свойства.
private BoardState internalBoardState;
public bool IsCheck
{
get { return internalBoardState.HasFlag(BoardState.IsCheck); }
set
{
if (value)
internalBoardState |= BoardState.IsCheck;
else
internalBoardState &= ~BoardState.IsCheck;
}
}
public bool IsStuck
{
get { return internalBoardState.HasFlag(BoardState.IsStuck); }
set
{
if (value)
internalBoardState |= BoardState.IsStuck;
else
internalBoardState &= ~BoardState.IsStuck;
}
}
Просто добавьте ответ @palacsint , если язык, с которым вы работаете, позволяет использовать литералы массива , вы можете сделать его похожим на таблицу:
var decisionTable = new GameState[][] {
new GameState[] { GameState.Checkmate, GameState.Check } // isCheck
new GameState[] { GameState.Stalemate, GameState.Ok }
// isStuck
}
return decisionTable[(int)isCheck][(int)isStuck];
inline, если оператор короче, но я не знаю о чистом
return isCheck
? (isStuck ? GameState.Mate : GameState.Check)
: (isStuck ? GameState.Stalemate : GameState.Ok);
И для расширения и просто побитового
int result = (isCheck ? 1 : 0) + (isStuck ? 2 : 0);
return (GameState) result;
enum GameState
{
Ok = 0,
Check = 1,
Stalemate = 2,
Mate = 3,
}
Вы можете использовать перечисление «как» дерево решений!
Я разместил их
Я думаю, что деревья решений не нужны и слишком сложны для случая, это простое - хотя у них есть явные преимущества, если он становится более сложным, чем это, лично я бы сделал это:
if (isCheck)
return isStuck ? GameState.Mate : GameState.Check;
else
return isStuck ? GameState.StaleMate : GameState.Ok;
Который я нахожу более ясным, чем чистое тернарное операторное решение, более компактным и, следовательно, более удобным для чтения, чем чистое решение if
.
Не нужно много говорить, я думаю,
stuck
|--------- mate (11)
check |
|---------|
| |
| |--------- check (10)
|
| stuck
| |--------- stalemate (01)
| |
|---------|
|
|--------- ok (00)
enum GameState
{
Ok = 0, // 0x00
Stalemate = 1, // 0x01
Check = 2, //0x10
Mate = 3 //0x11
}
return (GameState) (0x10 * isCheck + isStuck);
Если вам нравится идея таблиц решений (например, в ответе palacsint ), вы можете использовать более элегантные с помощью ValueTuple
и Dictionary
.
GameState GetGameState(bool isCheck, bool isStuck)
{
var decisionDictionary = new Dictionary<(bool, bool), GameState>
{
[(true, true)] = GameState.Mate,
[(true, false)] = GameState.Check,
[(false, true)] = GameState.Stalemate,
[(false, false)] = GameState.Ok
};
return decisionDictionary[(isCheck, isStuck)];
}
Я думаю, что таблицы решений достаточно хороши. Если вы хотите перейти на эзотерику, вы также можете определить значения перечисления в виде битовых значений, и отображение будет автоматическим (что очень специфично для данного примера, поэтому не является хорошей идеей в целом):
enum GameState
{
Ok = 0,
Stalemate = 1,
Check = 2
Mate = 3
}
код становится:
return (GameState)(((int)isCheck << 1)|(int)isStuck);
Вы можете просто сделать GameState переименованием флагов, который представляет собой просто более компактную версию предлагаемой таблицы решений.
[Flags]
public enum GameState
{
None = 0x0,
IsCheck = 0x1,
IsStuck = 0x2,
Ok = None,
Check = IsCheck,
Stalemate = IsStuck,
Checkmate = IsCheck | IsStuck,
}
При использовании перечисления вы можете установить два флага с поразрядными или операторами:
GameState current = GameState.Ok;
current |= GameState.IsStuck;
current |= GameState.IsCheck;
Если вы хотите проверить два битовых флага на нем, вы можете использовать побитовые операторы следующим образом:
var isStuck = ((current & GameState.IsStuck) == GameState.IsStuck);
или, в более поздних версиях .NET, вы можете использовать метод Enum.HasFlag:
var isStuck = current.HasFlag (GameState.IsStuck);
Конечным результатом является то, что все ваши чтения /записи флагов выполняются целиком с вашей переменной, хранящей GameState, вместо того, чтобы иметь отдельные булевы, плавающие вокруг, и мути-мерный массив для поиска вещей.
Так много сумасшедших ответов! Я добавлю еще один! : D
Самое важное свойство хорошего кода - это не дублирование кода. В исходном решении это условие if (isStuck)
, которое дублируется. Мы можем избежать дублирования кода, сделав умную (не столько в этом случае) абстракцию!
T if_<T>(bool b, Tuple<T,T> t) {
return b ? t.Item1 : t.Item2;
}
GameState choose (bool isCheck, bool isStuck) {
return if_(isStuck, if_(isCheck,
Tuple.Create(
Tuple.Create(GameState.Mate, GameState.Check),
Tuple.Create(GameState.Stalemate, GameState.Ok)
)));
}
Теперь if_(isStuck,...
больше не дублируется. Win!
Лично я просто разрушу дерево.
if (isCheck && isStuck)
{
return GameState.Mate;
}
else if (isCheck)
{
return GameState.Check;
}
else if (isStuck)
{
return GameState.Stalemate;
}
else
{
return GameState.Ok;
}
или даже
if (isCheck && isStuck) return GameState.Mate;
else if (isCheck) return GameState.Check;
else if (isStuck) return GameState.Stalemate;
else return GameState.Ok;
Я нахожу это гораздо более читаемым, чем попытка сделать преобразования в Enum, и более простой, чем решетка решений. При этом он не масштабируется, как и любой из них.
Не рекомендую это, но вы можете использовать тот факт, что вы всегда используете обе переменные для генерации строкового представления четырех состояний, а затем либо оператор словаря, либо оператор switch вернули правильное значение.
string checkStuck = isCheck.ToString() + isStuck.ToString();
switch(checkStuck) {
case "TrueFalse":
return GameState.Mate;
case "TrueTrue":
return GameState.Check;
case "FalseFalse":
return GameState.Ok;
case "FalseTrue":
return GameState.Stalemate;
}
Вы можете сделать то же самое с целыми числами, но в этот момент вы могли бы пойти с ответом Энди.