I've been told few days ago that using is
is a code smell and anti-pattern.
I used it in an game to filter some elements of a collection (the inventory of the player).
Classes structure
The base class of an element of the collection is Item
:
public abstract class Item
{
public int Id { get; set; }
public string Name { get; set; }
public string Description { get; set; }
}
From that class are derived 3 classes, Potion
, Armor
and Weapon
A potion is IUsable
public interface IUsable
{
void Use(Character target);
}
And IStackable
public interface IStackable
{
int Quantity { get; set; }
int MaxQuantity { get; set; }
}
The class Potion
is, in example, defined that way :
public class Potion : Item, IUsable, IStackable
{
//Some properties and implementations of interfaces
}
The weapons and armors are IEquipable
public interface IEquipable
{
void TakeOn(Character target);
void TakeOff(Character target);
}
Here is a shortened example of Armor
public class Armor : Item, IEquipable
{
//Some properties and implementations of interfaces
}
And one for Weapon
public class Weapon : Item, IEquipable
{
//Some properties and implementations of interfaces
}
Collection access
Now, the part that smells, considering what was told to me :
To access some specific items in the collection, I'm using a generic method SpecificInventoryItems<T>()
, which contains this part to get the expected T
items :
foreach (Item item in player.Inventory)
{
if (item is T)
{
string stack = "";
if (item is IStackable)
{
var stackable = item as IStackable;
stack = "(" + stackable.Quantity + "/" + stackable.MaxAmount + ") ";
}
options += "[" + i++ + "] - " + item.Name + " " + stack + ": " + item.Description;
if (item is Armor)
options += " " + ArmorDetail(item as Armor);
else if (item is Weapon)
options += " " + WeaponDetail(item as Weapon);
options += "\n";
items.Add(item as T); //items is List<T>
}
}
What's wrong with it? How could (should) I redesign this structure ? I though using separates lists such as List<IUsable>
and so on, instead of just a List<Item> Inventory
Another example, after displayed the specific elements of the inventory, the user can select an item, and depending of its type, the item will be either used or taken on :
switch (selected) //selected type is T
{
case IEquipable eq:
eq.TakeOn(player);
break;
case IUsable us:
us.Use(player);
//Manage the deletion of the IUsable if not IStackable, or IStackable and quantity = 0
break;
default:
break;
}
}
Aucun commentaire:
Enregistrer un commentaire