如何重构这些相同的逻辑结构

Question

如何重构这两个函数的逻辑结构？随着这个逻辑越来越复杂，我不想让两个代码站点保持同步。IsValidDropTarget()是通过拖动代码来决定是否将一个有效的放置目标悬停(是否高亮显示(。OnDrop()发生在牌掉落时，掉落无效，或者牌以几种不同的方式玩。看起来我肯定可以从一个地方分享这个逻辑，但我不知道怎么做，因为IsValudDropTarget()在问一个布尔问题，而OnDrop()在执行一个更复杂的操作。

public bool IsValidDropTarget(Card card)
{
bool cardRequiresTarget = card.RequiresTarget();
if (cardRequiresTarget && targetType == TargetType.None) {
// Card requires a target but player didn't provide one.  Abort.
return false;
} else if (cardRequiresTarget && targetType == TargetType.Enemy) {
// Card requires a target and player targeted an enemy.
return true;
} else if (! cardRequiresTarget) {
// Target doesn't matter, play this card without a target.
return true;
} else {
// Shouldn't get here.
Debug.LogWarning("Unknown target conditions.  Card not played.");
return false;
}
}
public void OnDrop(PointerEventData eventData)
{
GameObject droppedCard = eventData.pointerDrag;
Card card = droppedCard.GetComponent<CardUI>().GetCard();
bool cardRequiresTarget = card.RequiresTarget();
if (cardRequiresTarget && targetType == TargetType.None) {
// Card requires a target but player didn't provide one.  Abort.
return;
} else if (cardRequiresTarget && targetType == TargetType.Enemy) {
// Card requires a target and player targeted an enemy.
EnemyUI enemyUI = GetComponent<EnemyUI>();
card.PlayCard(enemyUI.GetEnemy());
} else if (! cardRequiresTarget) {
// Target doesn't matter, play this card without a target.
card.PlayCard();
} else {
// Shouldn't get here.
Debug.LogWarning("Unknown target conditions.  Card not played.");
return;
}
}

Answer 1

您只需在获得card:之后，从OnDrop方法调用IsValidDropTarget方法

public void OnDrop(PointerEventData eventData)
{
GameObject droppedCard = eventData.pointerDrag;
Card card = droppedCard.GetComponent<CardUI>().GetCard();
if (!IsValidDropTarget(Card card)) {
return;
}
// rest of the code here
}

话虽如此，在这个特定的用例中，IsValidDropTarget方法中的所有代码都可以使用一行简单的代码来编写：

return !card.RequiresTarget() || targetType == TargetType.Enemy;

您只在两种情况下返回true：
要么卡不需要目标，要么目标是敌人
在任何其他情况下，您都会返回false，因此这是编写相同逻辑的更简洁的方法。

我会这样写：

public void OnDrop(PointerEventData eventData)
{
GameObject droppedCard = eventData.pointerDrag;
Card card = droppedCard.GetComponent<CardUI>().GetCard();
if (!IsValidDropTarget(Card card)) {
return;
}
// only two valid options here - either the target is an enemy or there's no target...
if (card.RequiresTarget()) {
EnemyUI enemyUI = GetComponent<EnemyUI>();
card.PlayCard(enemyUI.GetEnemy());
} 
else 
{
card.PlayCard();
}
}
public bool IsValidDropTarget(Card card)
{
return !card.RequiresTarget() || targetType == TargetType.Enemy;
}

Answer 2

例如，您可以使用一个枚举来提供状态

[Flags]
public enum CardTargetState // or whatever
{
NotPlayable = 0,
Playable = 1,
TargetEnemy = 2
// add more values to your liking
}

然后引入

CardTargetState GetTargetState(Card card)
{
bool cardRequiresTarget = card.RequiresTarget();
if (cardRequiresTarget && targetType == TargetType.None) {
// Card requires a target but player didn't provide one.  Abort.
return CardTargetState.NotPlayable;
} else if (cardRequiresTarget && targetType == TargetType.Enemy) {
// Card requires a target and player targeted an enemy.
return CardTargetState.Playable | CardTargetState.TargetEnemy;
} else if (! cardRequiresTarget) {
// Target doesn't matter, play this card without a target.
return CardTargetState.Playable;
} else {
// Shouldn't get here.
Debug.LogWarning("Unknown target conditions.  Card not played.");
return CardTargetState.NotPlayable;
}
}

这将使您的IsValidDropTarget

public bool IsValidDropTarget(Card card)
{
return GetTargetState(card) & CardTargetState.Playable == CardTargetState.Playable;
}

例如，您可以将OnDrop重构为

public void OnDrop(PointerEventData eventData)
{
GameObject droppedCard = eventData.pointerDrag;
Card card = droppedCard.GetComponent<CardUI>().GetCard();
var cardTargetState = GetTargetState(card);
var dropAction = dropActionFactory.CreateDropAction(card, cardTargetState);
dropAction.Execute();
}

CCD_ 11创建要针对当前卡和目标状态执行的动作。在不可能丢弃的情况下，它创建一个实现相应接口但不执行任何操作的NullObject，并且在涉及任何逻辑的情况下(就像Playable和Playable | TargetEnemy的情况一样(，它创建实现相应逻辑的对象。

如果您不喜欢为您创建逻辑对象的工厂(尽管您应该非常喜欢这个想法；(，您可以简单地使用if-else

public void OnDrop(PointerEventData eventData)
{
GameObject droppedCard = eventData.pointerDrag;
Card card = droppedCard.GetComponent<CardUI>().GetCard();
var cardTargetState = GetTargetState(card);

if (cardTargetState == CardTargetState.Playable)
{
card.PlayCard();
}
else if (cardTargetState == CardTargetState.Playable | CardTargetState.TargetEnemy)
{
EnemyUI enemyUI = GetComponent<EnemyUI>();
card.PlayCard(enemyUI.GetEnemy());
}
}

但这样，您可能会在分支中得到一个包含逻辑的大型if-else，这对可维护性和可读性是不利的。对于工厂，您可以将做什么的决定与如何分开。

Answer 3

由于这些方法的逻辑可能会增长，因此我们的想法是用以下签名将它们的逻辑封装在一个新方法中

private bool ValidateDrop(Card card, bool validateTaget)

参数validateTarget是一个标志，指示我们是否只验证我们的目标(来自public bool IsValidDropTarget(Card card)的调用(

public bool IsValidDropTarget(Card card)
{
return this.ValidateDrop(card,true);
}
public void OnDrop(PointerEventData eventData)
{
GameObject droppedCard = eventData.pointerDrag;
Card card = droppedCard.GetComponent<CardUI>().GetCard();
this.ValidateDrop(card,false);
}

这两种方法现在非常简单——

逻辑都封装在我们的私有方法中

private bool ValidateDrop(Card card, bool validateTaget)
{
bool cardRequiresTarget = card.RequiresTarget();
if (cardRequiresTarget && targetType == TargetType.None) 
{
// Card requires a target but player didn't provide one.  Abort.
return false;
} 
else if (cardRequiresTarget && targetType == TargetType.Enemy) 
{
if (!validateTaget)
{
// Card requires a target and player targeted an enemy.
EnemyUI enemyUI = GetComponent<EnemyUI>();
card.PlayCard(enemyUI.GetEnemy());
}
return true;
} 
else if (!cardRequiresTarget) 
{
if (!validateTaget)
{
// Target doesn't matter, play this card without a target.
card.PlayCard();
}
return true;
} 
else 
{
// Shouldn't get here.
Debug.LogWarning("Unknown target conditions.  Card not played.");
return false;
}
}