基本拍卖软件

Question

在用C#编写多年代码之后，我目前正致力于围绕对象进行一些基本实践，但我也开始尝试学习实心。我在代码中的主要关注点是拍卖类中的验证规则，并抛出许多异常，因为我觉得这是错误的，我觉得我应该实现某种List<ValidationResults>，甚至是PlaceBidResult class，但这感觉好像我没有封装在正确的类中出价背后的逻辑？

拍卖类-这基本上是整个系统的大脑，允许配售和取消出价，设定保留价格等。

    public class Auction
    {
        public string ItemName { get; private set; }
        public double ReservePrice { get; private set; } = 0;

        public DateTime StartDate { get; private set; }
        public DateTime EndDate { get; set; }

        List<Bid> bids;
        public IEnumerable<Bid> Bids
        {
            get
            {
                return bids.AsEnumerable();
            }
        }


        public int CurrentHighestBid
        {
            get
            {
                return bids.Max(c => (int?)c.Value) ?? 0;

            }

        }
        private const int REPUTATION_REQUIRED_TO_BID = 10;
        private const double MAXIMUM_ALLOWED_DURATION = 10;

        public Auction(string itemName, DateTime startDate, DateTime endDate)
        {
            if (string.IsNullOrEmpty(itemName))
                throw new NullReferenceException("Must provide a valid item name");

            this.ItemName = itemName;

            if (startDate < DateTime.Now.AddMinutes(-10)) //Gives a ten minute grace to prevent this failing if DateTime.Now is passed in.
                throw new ArgumentOutOfRangeException(nameof(startDate), startDate, "Auctions can't start in the past");

            this.StartDate = startDate;

            if (endDate < DateTime.Now || endDate > StartDate.AddDays(MAXIMUM_ALLOWED_DURATION))
            {
                throw new ArgumentOutOfRangeException(nameof(endDate), endDate, $"Auctions can't end in the past and the duration of the auction must be less than {MAXIMUM_ALLOWED_DURATION} days");
            }
            this.EndDate = endDate;


            bids = new List<Bid>();


        }

        public void PlaceBid(double value, User user)
        {
            //Check that the bid is valid. I.e are their bids that are higher than us already
            ValidateBid(value, user);
            bids.Add(new Bid(value, user));

            Console.WriteLine("Bid Added " + value);

        }

        private void ValidateBid(double value, User user)
        {
            if (CurrentHighestBid > value)
                throw new ArgumentException("There are already higher bids than this", nameof(value));

            if (value < 0)
                throw new ArgumentException("You cannot bid negative amounts", nameof(value));

            if (value < ReservePrice)
                throw new ArithmeticException("Bid value cannot be lower than the reserve price of an item");

            if (StartDate > DateTime.Now)
                throw new ArgumentException("Auction hasn't started yet!");

            if (user.Reputation < REPUTATION_REQUIRED_TO_BID) 
                throw new ArgumentException("User does not have enough reputation to bid on these", nameof(user.Reputation)); //Is throwing an exception here the correct way to go?
        }

        public void RemoveBid(double value, User user)
        {
            var bid = bids.Where(c => c.Value == value).FirstOrDefault();           

            if (bid != null)
                bids.Remove(bid);           
        }

        public void SetReservePrice(double value)
        {
            if (value < 0)
                throw new ArithmeticException("Reserve price cannot be below zero");

            this.ReservePrice = value;
        }       


    }
}

投标类-现在，这基本上是一个数据包或“贫血的领域模型”，这能用某种逻辑来改进吗？

    public class Bid
{
    public double Value { get; private set; }
    public User User { get; private set; }

    public Bid(double value, User user)
    {
        this.Value = value;
        this.User = user;
    }
}

Answer 1

您的问题是非常开放的，您可以在许多方面实现，因此不幸的是，我的答案也是相当开放的。首先，有几点一般性意见：

• 我通常使用string.IsNullOrWhitespace(blah)而不是string.IsNullOrEmpty(blah)
• 别扔NullReferenceException。扔ArgumentNullException(nameof(argumentName))。这就是它的意义所在。
• C#通常对常量名称使用PascalCase：ReputationRequiredToBid。
• ArithmeticException --您不应该像文档那样使用这个类。
• 虽然我不同意您使用异常(过度)，但是当当前状态破坏代码时，您应该使用InvalidOperationException而不是ArgumentExceptions，而不是其他有效和格式良好的论点(S)。例如，负value应该抛出一个ArgumentOutOfRangeException或简单的ArgumentException，但是CurrentHighestBid > value应该抛出一个InvalidOperationException或一个自定义异常(不过，我不喜欢自定义异常--继续阅读替代品)。

无论如何，在我看来，你肯定是过度使用了例外。考虑创建一个单独的验证类，并将验证代码移到那里--单一责任原则。考虑返回一个ValidationResult对象，其中包含关于哪里出错的详细信息，可能是在IEnumerable<string> Errors属性中。我可能还会把S( ArgumentExceptions )抛出一些荒谬的论调，比如否定的出价。

如果我需要抛出一个异常，在99%的情况下，这是ArgumentException的一个变体，有时是InvalidOperationException。最终的结果是，我编写的代码很少使用它们，并且在可预测的地方使用，比如方法的入口点等等。如果我查看一段抛出任何其他异常的代码，我希望看到或听到一个非常有说服力的原因。如果这个异常是一个自定义的异常，还有一个更强有力的理由。总之，特别是如果你没有经验，除了例外，不要做任何疯狂的事情。它们本身并不坏，但是有更优雅和可读性更强的解决方案。

可能是投标类参数验证？

Answer 2

您不需要检查StartDate > EndDate：

这一点并没有被抓住：

      Auction auction = new Auction("AAA", new DateTime(2019, 3, 2), new DateTime(2019, 3, 1));

公共int CurrentHighestBid { get {返回bids.Max(c => (int?)c.Value) ?？0；}

而不是return bids.Max(c => (int?)c.Value) ?? 0;，您可以编写return bids.Max(c => (int?)c.Value).GetValueOrDefault();。但你为什么要投int呢？

在ValidateBid(double value, User user)中，您检查开始日期，但不检查结束日期。

在……里面

公共无效RemoveBid(双值，用户){ var bid = bids.Where(c => c.Value == value).FirstOrDefault()；if (bid != null) bids.Remove(bid)；}

您没有对任何东西使用user参数。我希望条件类似于(您可以跳过Where-call)：

var bid = bids.FirstOrDefault(b => b.Value == value && b.User == user);

更进一步：Remove(...)返回一个布尔值来指示是否删除了某个内容。您可以从RemoveBid()中返回该值，以便让客户机知道.：

public bool RemoveBid(double value, User user)
{
  var bid = bids.Where(c => c.Value == value).FirstOrDefault();
  if (bid != null)
    return bids.Remove(bid);
  return false;
}

根据异常的使用和输入的验证，我认为您必须区分妨碍程序正常或有效执行的输入/状态(例如空引用或StartDate > EndDate )和违反业务规则的行为，这些规则不会导致程序停止工作，但会产生无效或不需要的结果--比如value < ReservePrice。第一种类型应作为异常抛出，而后者则应使用有用的返回值和/或消息来处理。看得很好。

但是很难确定区别(例如：StartDate > EndDate是业务规则还是异常？)，因此经常可以看到，在这两种情况下都使用了异常。无论您选择什么，都要始终如一地这样做，并记录哪些异常被抛出在何处以及为什么抛出。永远不要因为你觉得你有太多的异常而抛出一个异常:-)。如果你认为你必须抛出太多的例外，也许你应该考虑整体设计是否正确。