我刚刚花了几天时间为我的游戏构建货币体系,我想知道你们是否有任何建议--如果有的话--我可以改进它。在我展示代码之前,让我解释一下目前的系统。它的工作原理很像“魔兽世界”,因为当你获得100英镑时,铜会自动转换成银。然后,白银被转换成黄金,黄金被转换成白金。对于那些玩过Everquest游戏的人来说,这个系统看起来更加熟悉。
我已经对所有货币的根结构进行了一些优化,比如我如何将所有货币分母保持在一个长时间内。用这么长的时间,我可以计算“伪造”其他面额。本质上,一切都以铜的形式保存在内部。
我还暗示了"999999999“的最大基本分母值。使用这个有点随意的数字允许我将货币总量限制在999白金、99黄金、99银和99铜。
我只是想知道你们是否有什么建议(除了“嘿,你们应该在那里完全使用var!”)这将有助于改进我目前的实现。
以下是目前的代码:
using System;
namespace Some.Arbitrary.Framework
{
public enum Coins
{
/// <summary>
/// Copper is the lowest denominator of currency.
/// It requires 100 Copper to make 1 Silver.
/// </summary>
Copper = 1,
/// <summary>
/// Silver is the second most common form of currency.
/// It requires 100 Silver to Make 1 Gold.
/// </summary>
Silver = 2,
/// <summary>
/// Gold is the most common form of currency. It takes
/// part in most expensive transactions.
/// It requires 100 Gold to make 1 Platinum.
/// </summary>
Gold = 3,
/// <summary>
/// Platinum is a coin which most people never see. A single
/// Platinum coin can purchase almost anything.
/// 1 Platinum Coin = 100 Gold.
/// 1 Platinum Coin = 10,000 Silver.
/// 1 Platinum Coin = 1,000,000 Copper.
/// </summary>
Platinum = 4
}
public class MoneyBag : IEquatable<MoneyBag>, IComparable<MoneyBag>
{
private long _baseDenomination;
public const string CopperName = "Copper";
public const string SilverName = "Silver";
public const string GoldName = "Gold";
public const string PlatinumName = "Platinum";
public const char CopperAbbreviation = 'c';
public const char SilverAbbreviation = 's';
public const char GoldAbbreviation = 'g';
public const char PlatinumAbbreviation = 'p';
public const long MaximumBaseDenomination = 999999999;
public static readonly MoneyBag FilledBag = new MoneyBag(MaximumBaseDenomination);
public static readonly MoneyBag EmptyBag = new MoneyBag();
public long BaseDenomination
{
get
{
return _baseDenomination;
}
set
{
_baseDenomination = value;
// Clamp if required.
if (_baseDenomination > MaximumBaseDenomination)
{
_baseDenomination = MaximumBaseDenomination;
}
if (_baseDenomination < 0)
{
_baseDenomination = 0;
}
}
}
/// <summary>
/// The total amount of Copper.
/// </summary>
public int Copper
{
get
{
return ComputeCopper(_baseDenomination);
}
}
/// <summary>
/// The total amount of Silver.
/// </summary>
public int Silver
{
get
{
return ComputeSilver(_baseDenomination);
}
}
/// <summary>
/// The total amount of Gold.
/// </summary>
public int Gold
{
get
{
return ComputeGold(_baseDenomination);
}
}
/// <summary>
/// The total amount of Platinum.
/// </summary>
public int Platinum
{
get
{
return ComputePlatinum(_baseDenomination);
}
}
public bool IsFull
{
get
{
return _baseDenomination == MaximumBaseDenomination;
}
}
public bool IsEmpty
{
get
{
return _baseDenomination == 0;
}
}
public bool HasPlatinum
{
get
{
return Platinum > 0;
}
}
public bool HasGold
{
get
{
return Gold > 0 || Platinum > 0;
}
}
public bool HasSilver
{
get
{
return Silver > 0 || Gold > 0 || Platinum > 0;
}
}
public bool HasCopper
{
get
{
return Copper > 0 || Silver > 0 || Gold > 0 || Platinum > 0;
}
}
public MoneyBag()
{
}
public MoneyBag(int platinum, int gold, int silver, int copper)
{
Add(platinum, gold, silver, copper);
}
public MoneyBag(long baseDenomination)
{
BaseDenomination = baseDenomination;
}
public void Add(int platinum, int gold, int silver, int copper)
{
BaseDenomination += platinum * 1000000;
BaseDenomination += gold * 10000;
BaseDenomination += silver * 100;
BaseDenomination += copper;
}
public void Add(int amount, Coins type)
{
if (amount <= 0) return;
switch (type)
{
case Coins.Copper:
Add(0, 0, 0, amount);
break;
case Coins.Silver:
Add(0, 0, amount, 0);
break;
case Coins.Gold:
Add(0, amount, 0, 0);
break;
case Coins.Platinum:
Add(amount, 0, 0, 0);
break;
}
}
public void Add(MoneyBag other)
{
BaseDenomination += other._baseDenomination;
}
public void Subtract(int platinum, int gold, int silver, int copper)
{
BaseDenomination -= platinum * 1000000;
BaseDenomination -= gold * 10000;
BaseDenomination -= silver * 100;
BaseDenomination -= copper;
}
public void Subtract(int amount, Coins type)
{
if (amount <= 0) return;
switch (type)
{
case Coins.Copper:
Subtract(0, 0, 0, amount);
break;
case Coins.Silver:
Subtract(0, 0, amount, 0);
break;
case Coins.Gold:
Subtract(0, amount, 0, 0);
break;
case Coins.Platinum:
Subtract(amount, 0, 0, 0);
break;
}
}
public void Subtract(MoneyBag other)
{
BaseDenomination -= other._baseDenomination;
}
public void Empty()
{
_baseDenomination = 0;
}
public void Fill()
{
_baseDenomination = MaximumBaseDenomination;
}
public static MoneyBag operator +(MoneyBag b1, MoneyBag b2)
{
return new MoneyBag(b1._baseDenomination + b2._baseDenomination);
}
public static MoneyBag operator -(MoneyBag b1, MoneyBag b2)
{
return new MoneyBag(b1._baseDenomination - b2._baseDenomination);
}
public bool Equals(MoneyBag other)
{
return _baseDenomination == other._baseDenomination;
}
public override bool Equals(object obj)
{
return (obj is MoneyBag) && Equals((MoneyBag)obj);
}
public override int GetHashCode()
{
return _baseDenomination.GetHashCode();
}
public static bool operator ==(MoneyBag a, MoneyBag b)
{
if (ReferenceEquals(a, null)) return false;
if (ReferenceEquals(b, null)) return false;
return a.Equals(b);
}
public static bool operator !=(MoneyBag a, MoneyBag b)
{
if (ReferenceEquals(a, null)) return false;
if (ReferenceEquals(b, null)) return false;
return !a.Equals(b);
}
public static bool operator <(MoneyBag a, MoneyBag b)
{
return a.CompareTo(b) < 0;
}
public static bool operator >(MoneyBag a, MoneyBag b)
{
return a.CompareTo(b) > 0;
}
public int CompareTo(MoneyBag other)
{
// The shit was null, dumbass!
if (other == null) return 0;
if (_baseDenomination > other._baseDenomination)
{
return 1;
}
if (_baseDenomination < other._baseDenomination)
{
return -1;
}
// They were equal.
return 0;
}
public static void ComputeWealth(long baseDenomination, out int platinum, out int gold, out int silver, out int copper)
{
platinum = ComputePlatinum(baseDenomination);
gold = ComputeGold(baseDenomination);
silver = ComputeSilver(baseDenomination);
copper = ComputeCopper(baseDenomination);
}
public static int ComputeCopper(long baseDenomination)
{
return (int)Math.Floor((double)Math.Abs(baseDenomination % 100));
}
public static int ComputeSilver(long baseDenomination)
{
return (int)Math.Floor((double)Math.Abs((baseDenomination / 100) % 100));
}
public static int ComputeGold(long baseDenomination)
{
return (int)Math.Floor((double)Math.Abs((baseDenomination / 10000) % 100));
}
public static int ComputePlatinum(long baseDenomination)
{
return (int)Math.Floor((double)Math.Abs(baseDenomination / 1000000));
}
public override string ToString()
{
return
"" + Platinum + PlatinumAbbreviation + "," +
Gold + GoldAbbreviation + "," +
Silver + SilverAbbreviation + "," +
Copper + CopperAbbreviation;
}
}
}简洁的用法示例(您可以自己多使用它):
using Some.Arbitrary.Framework;
namespace SomeGameOrSomething
{
static class Program
{
static void Main()
{
MoneyBag bag = new MoneyBag(1,22,44,55);
bag.Subtract(0,50,22,0);
Console.WriteLine(bag);
Console.Read();
}
}
}如果有人对我最后得到的建议感兴趣,那么代码可以在这里找到:http://pastebin.com/sqVjZYry。
发布于 2016-03-16 10:36:33
有几件事我觉得很突出:
public const string CopperName = "Copper";
public const string SilverName = "Silver";
public const string GoldName = "Gold";
public const string PlatinumName = "Platinum";..。名称和描述性文本应该(几乎)进入资源文件。为什么?国际化。这意味着,如果名称更改,则不必重新编译代码。
是的,你确实必须得到查找键,但它们并不真正属于这里-这是一个函数,无论你是如何输出给球员。
public long BaseDenomination
{
get
{
return _baseDenomination;
}
set
{
_baseDenomination = value;
// Clamp if required.
if (_baseDenomination > MaximumBaseDenomination)
{
_baseDenomination = MaximumBaseDenomination;
}
if (_baseDenomination < 0)
{
_baseDenomination = 0;
}
}
}在这里使用标准的max/min函数可能更常见:
public long BaseDenomination
{
get
{
return _baseDenomination;
}
set
{
// Clamp if required.
_baseDenomination = Math.Max(0, Math.Min(MaximumBaseDenomination, value));
}
} 此外,所使用的和返回的值是long,但是您的最大基数很适合于int,您可以考虑更改它。
public void Add(int platinum, int gold, int silver, int copper)
{
BaseDenomination += platinum * 1000000;
BaseDenomination += gold * 10000;
BaseDenomination += silver * 100;
BaseDenomination += copper;
}所有这些数字?这些叫做“魔法数字”,你不会想要它们的。相反,您应该定义和使用常量,如下所示:
private const int SilverInCopper = 100;
private const int GoldInCopper = SilverInCopper * 100;
private const int PlatinumInCopper = GoldInCopper * 100;然而,您的Add (和Subtract)方法有一个更严重的缺陷:您没有注意整数溢出。他们将接受1200枚白金硬币,并高兴地将钱包的内容设置为零(因为夹持-虽然这假设它是接近最大值的开始)。你也不会注意一些东西,比如正反两面的硬币,这可能很奇怪。
如果您将其更改为一个结构,那么无关紧要,但是这里没有空检查(将抛出NullReferenceException):
public bool Equals(MoneyBag other)
{
return _baseDenomination == other._baseDenomination;
}以下两项合并在一起:
public static bool operator ==(MoneyBag a, MoneyBag b)
{
if (ReferenceEquals(a, null)) return false;
if (ReferenceEquals(b, null)) return false;
return a.Equals(b);
}
public static bool operator !=(MoneyBag a, MoneyBag b)
{
if (ReferenceEquals(a, null)) return false;
if (ReferenceEquals(b, null)) return false;
return !a.Equals(b);
}这样做的效果是:
((MoneyBag)null) == ((MoneyBag)null)返回false。((MoneyBag)null) != ((MoneyBag)null)还返回false。同样,如果它变成了一个结构,这并不重要,但是您需要验证一个类的实例都不是空的。首先,这将打破平等的交换性质。实现这些运算符之间的相互关系也更为常见:
public static bool operator !=(MoneyBag a, MoneyBag b)
{
return !(a == b);
}比较运算符也没有空检查:
public static bool operator <(MoneyBag a, MoneyBag b)
{
return a.CompareTo(b) < 0;
}
public static bool operator >(MoneyBag a, MoneyBag b)
{
return a.CompareTo(b) > 0;
}同样,结构不是问题,但如果不是,则会产生不良影响:您将从其他代码中获得随机失败(NullReferenceException),这取决于为枢轴选择哪种元素排序。您还缺少了“大”、“等”和“少”或“等”,您可能希望将其余四项中的三项加起来(按照C/C++约定,使用<,小于)。
public int CompareTo(MoneyBag other)
{
// The shit was null, dumbass!
if (other == null) return 0;
if (_baseDenomination > other._baseDenomination)
{
return 1;
}
if (_baseDenomination < other._baseDenomination)
{
return -1;
}
// They were equal.
return 0;
}作为一种风格,在您为企业编写的代码中或计划向公众发布的代码中,咒骂或使用粗俗的语言通常被认为是糟糕的形式。
然而,更重要的是,null现在被认为等同于所有其他元素。这完全打破了平等的交换性质。如果您有一个包含空元素的集合,它将中断排序和搜索(当您期待其他东西时获得null,或者断言错误将取决于)。相反,您应该将空值排序到“底部”:
// If other is not a valid object reference, this instance is greater.
if (other == null) return 1;非泛型IComparable 引用还声明:
根据定义,任何对象都比较大于(或跟随) null,两个空引用比较相等。
(出于某种原因,这句话不在通用文档中,这是某人的严重疏忽)
此外,由于您是根据内部表示进行排序,所以可以使用long.CompareTo(...):
public int CompareTo(MoneyBag other)
{
if (other == null) return 1;
return _baseDenomination.CompareTo(other._baseDenomination);
}..。是否使用实际的基础字段或公共成员取决于您。
最后,关于所有的ComputeX方法:
public static int ComputeCopper(long baseDenomination)
{
return (int)Math.Floor((double)Math.Abs(baseDenomination % 100));
}
public static int ComputePlatinum(long baseDenomination)
{
return (int)Math.Floor((double)Math.Abs(baseDenomination / 1000000));
}..。你所做的所有数学都是整数数学。如果您不知道这是什么,我建议您阅读一下,但要点是:
int i1 = 10;
int i2 = 3;
Console.Out.WriteLine(10 / 3); // Prints '3'本质上,语言(以及几乎所有的计算机语言都是这样工作)正在截断结果(对于正数,这相当于地板)。
他们也会把负数变成正数!这在代码的其他地方可能是可利用的--要么夹紧到0,要么返回负值。
哦,ComputePlatinum也遇到了整数溢出:输入是long,但是输出是int。足够大的正数会变成..。其他的,很可能是负面的。您应该在这里返回一个long,或者一开始只接受一个int。(或者使用checked并抛出异常,但这可能会有问题)在任何情况下,我可能会按照以下代码编写方法:
public static int ComputeCopper(int baseDenomination)
{
return baseDenomination % SilverInCopper;
}
public static int ComputeSilver(int baseDenomination)
{
return baseDenomination % GoldInCopper / SilverInCopper ;
}
// I assume you can figure out ComputeGold
public static int ComputePlatinum(int baseDenomination)
{
return baseDenomination / PlatinumInCopper;
}发布于 2016-03-16 10:10:44
您确定您的业务逻辑真的需要了解所有这些不同类型的硬币吗?对我来说就像是不必要的并发症。维护代码应该要容易得多,它总是将货币简单地看作是long号(有一些额外的方法)。只有当涉及到UI层,你应该考虑什么是最好的方式显示您的钱袋。在那里,你可以有一个转换器,可以把钱转换成硬币,你的用户可以在屏幕上看到。但是它应该是一个类,独立于MoneyBag,它应该只作为UI层的一部分存在。
关于现有代码的其他一些事情:
a != b与!(a == b)相同,a - b与a + (-b)相同等等。例如,您可以实现相等:公共覆盖bool等于(对象obj) {返回等于(obj作为MoneyBag);}公共静态bool运算符==(MoneyBag a,MoneyBag b) {返回a.Equals( b);}公共静态bool运算符!=(MoneyBag a,MoneyBag b){a.Equals(B);//或返回!(a == b) }公共bool等于(MoneyBag other) {//实际实现,//您在所有其他方法中重复使用它,这样您就可以保证所有相等的方法总是返回相同的结果。修复等式逻辑中的错误就像修复单个方法一样容易。目前,将null传递给IEquatable.Equals方法将是throw,而使用==将运行得很好。FilledBag和EmptyBag),这也是个坏主意。考虑一下下面的代码: var myBag = MoneyBag.EmptyBag;//blah 100行代码myBag.Add(.);您可能会想:“嘿,我不是那个家伙!我不会那样使用EmptyBag!”但这些人中的每一个人都有相同的想法。platinum * 1000000这样的东西提取到专用的方法中,这样您就不必每次需要将一种货币类型转换为另一种货币时复制粘贴这些东西(或者手动计数零)。发布于 2016-03-16 21:18:47
已经有了很好的答案,重点是代码样式和跳转,所以我想从另一个角度来探讨:作为代码的潜在用户/调试器。
虽然限制这些值对于它给我们带来的安全是有吸引力的,但它也危及到最小惊奇原则。如果我有500 p,加上800 p,我将得到999p (和99g99s99c)。如果我再减去800便士,我就没有原来的500便士了,我只剩下不到200便士了。
如果您关心的是如何在有限的空间中显示大量的金钱,那么所讨论的组件就可以解决这个问题。也许,如果需要更多的空间,它可以逐步排除铜/银/金币。如果我有那么多钱,我就不会担心我有多少铜币了。;)
故障没有反馈
(我不是C#程序员,所以我可能错了。)
看来我可以从20枚硬币中减去50枚硬币,最后得到0枚硬币。这让我吃惊。如果一个函数不能完成它声称要做的事情(或者似乎声称要做一些棘手的事情),它应该以某种方式发出错误信号,比如抛出一个异常。
在有200 2sp或2SP(与这里相同)的情况下,HasCopper()将返回true,因为它还检查HasSilver(),但is ()将返回0。在我看来,HasCopper()应该意味着铜()> 0,所以HasCopper()应该返回false,或者铜()应该给出铜币(200)的总价值。
我的基本建议是:
毕竟,如果有四种不同的硬币类型没有意义,那有什么意义呢?(想想你将如何使用这门课:你会为商店里的白金、金银、铜币定价格吗?)或者你会设定一个“价值”基吗?)
https://codereview.stackexchange.com/questions/122959
复制相似问题