首页
学习
活动
专区
圈层
工具
发布
首页
学习
活动
专区
圈层
工具
MCP广场
社区首页 >问答首页 >使用事件的C#程序

使用事件的C#程序
EN

Code Review用户
提问于 2014-11-18 17:17:11
回答 2查看 324关注 0票数 6

我正在努力学习C#。请告诉我,以下代码是否尊重编码标准和命名约定,因为我想写漂亮的代码。这个项目的目的是帮助我理解我在过去几天里学到的不同概念,但是我很难完全理解它们(尤其是事件和代表)。请告诉我你会通过举例来改变什么,而不仅仅是推荐,因为我比代码更难理解短语。

例如,有人告诉我使用“异步/等待和TPL”。我用它们做了几个简单的程序,但我仍然不知道如何在这里应用这些概念。在等待你的回答的时候,我会继续尝试,在接下来的几个小时里,我将阅读从这里开始章节。

另外,我还被告知我应该使用Action。我想使用Actions,因为我喜欢ActionFunc,但是我不知道该在哪里使用Action

TProgram.cs:

代码语言:javascript
复制
#define DEBUG
#define TRACE
using System;
using System.Diagnostics;
using System.Text;
using System.Threading;

namespace LearnEventsAndOthers
{
    class Program
    {
        private static void OnTicked(object sender, TickingEventArgs e)
        {
            Console.WriteLine("Event triggered in object {0} at tick {1}", ((Metronom)sender).Id, e.TickCount);
        }

        static int Main(string[] args)
        {
            #region Standard Code - Init

            Console.InputEncoding = Console.OutputEncoding = Encoding.UTF8;
            Console.SetWindowSize(160, 56);
            Console.BufferHeight = 9000;
            Console.BufferWidth = 300;
#if DEBUG
            Console.WriteLine("The program is running in #DEBUG mode.");
#endif
#if TRACE
            Console.WriteLine("The program is running in #TRACE mode.");
#endif

            #endregion

            // Creating objects and testing overridden functions Equals and GetHashCode...
            Metronom m = new Metronom();
            Metronom n = new Metronom();
            Trace.Listeners.Add(new TextWriterTraceListener(System.IO.File.CreateText("TraceInfo.txt")));
            Trace.AutoFlush = true;
            Debug.Indent();
            Debug.WriteLineIf(!m.Equals(n), string.Format("Objects n and m have been created and they have different IDs: {0} vs {1}", m.GetHashCode(), n.GetHashCode()));
            Debug.Assert(!m.Equals(n), "Objects n and m have been created but they are equal! This shouldn't happen.");

            // Testing event subscription...
            m.TickedEvent += OnTicked;
            n.TickedEvent += OnTicked;

            // Testing destructors...
            Console.WriteLine(Environment.NewLine + "Press any key to destroy the objects!" + Environment.NewLine);
            Console.ReadKey();

            m.RequestStop();
            Console.WriteLine(Environment.NewLine + "The first object was destroyed. Waiting 5 seconds before destroying the second...");
            Thread.Sleep(5000);
            n.RequestStop();

            #region Standard Code - Exit

            Console.WriteLine("\nPress <enter> to exit!");
            Console.ReadLine();
            return 0;

            #endregion
        }
    }
}

Metronom.cs:

代码语言:javascript
复制
#define DEBUG
#define TRACE
using System;
using System.Diagnostics;
using System.Threading;

namespace LearnEventsAndOthers
{
    public class TickingEventArgs : EventArgs
    {
        public long TickCount { get; set; }
    }

    public class Metronom
    {
        public event EventHandler<TickingEventArgs> TickedEvent;

        public Guid Id { get; private set; }

        public int RandomStop { get; private set; } // Used to trigger the TickedEvent at random time spans.

        protected internal const int SpeedModifier = 17777777;  // Used to control the speed at which events are triggered. Higher means slower.
        // Why did I make it protected internal? I really wanted to use the access modifier "internal".

        private readonly Thread _t;

        private volatile bool _shouldStop;

        public Metronom()
        {
            this.Id = Guid.NewGuid();
            checked
            {
                int hashCode = Math.Abs(this.GetHashCode());
                while (hashCode > SpeedModifier)
                    hashCode -= SpeedModifier;
                RandomStop = hashCode;
            }
            _t = new Thread(Clock) { IsBackground = true };
            _t.Start();
            Debug.WriteLine("Object initialized with GUID HashCode " + this.GetHashCode());

        }

        ~Metronom()
        {
            _t.Join();
            Debug.WriteLineIf(_t.IsAlive == false, string.Format("Thread from object {0} has been stopped.", this.Id));
            Debug.Assert(_t.IsAlive == false, string.Format("Thread from object {0} is still running.", this.Id));
        }

        protected virtual void OnTickedEvent(long l)
        {
            EventHandler<TickingEventArgs> handler = TickedEvent;
            if (handler != null) handler(this, new TickingEventArgs() { TickCount = l });
        }

        public void RequestStop()
        {
            _shouldStop = true;
        }

        private void Clock()
        {
            Stopwatch c = new Stopwatch();
            c.Start();
            Debug.WriteLineIf(_t.IsAlive, string.Format("Thread from object {0} has gone LIVE!", this.Id));
            while (!_shouldStop)
            {
                long l = c.ElapsedTicks;
                if ((l % this.RandomStop) == 0)
                {
                    OnTickedEvent(l);
                }
            }
        }

        public override bool Equals(object obj)
        {
            return obj is Metronom && (((Metronom)obj).Id.Equals(Id));
        }

        public override sealed int GetHashCode()
        {
            return Id.GetHashCode();
        }
    }
}
EN

回答 2

Code Review用户

发布于 2014-11-18 18:20:23

  • c
  • l
  • m
  • n
  • _t

这些变量名称很差,没有遵循大多数开发人员所接受的规范。

您需要更多地描述您的名称,即使在编写通用代码时也是如此。

  • c -> clocktimerstopwatch1

这让我想到了l..。你不需要它,也不应该使用它,它是一个神奇的变量,只是臭味。

你应该把它写成这样

代码语言:javascript
复制
private void Clock()
{
    Stopwatch clock = new Stopwatch();
    clock.Start();
    Debug.WriteLineIf(_t.IsAlive, string.Format("Thread from object {0} has gone LIVE!", this.Id));
    while (!_shouldStop)
    {
        if ((clock.ElapsedTicks % this.RandomStop) == 0)
        {
            OnTickedEvent(clock.ElapsedTicks);
        }
    }
}

编辑:

我明白你的观点,你需要那个神奇的变量来保存确切的值,但是如果时机正好,那么

代码语言:javascript
复制
l % this.RandomStop != 0

这只是个想法。

编辑:

现在布尔值_shouldStop也有一个问题,您没有将它初始化为一个值,它可能不会引起直接问题,因为如果一个布尔值没有初始化,它应该在引用时返回一个假值,但是这不是一个好习惯,如果它是另一种对象类型,它将在第一次调用Clock时返回一个错误。

简单的解决方案,因为您不希望它从get中停止,只需将变量设置为声明

代码语言:javascript
复制
private volatile bool _shouldStop = false;

mn是节拍器,所以它们的名字应该反映出,每一个都是一个Metronom。

由于这是测试您创建的类的简单方法,所以我将它们命名为metronome1metronome2

如果它们具有特定的用途,您可以将它们命名为更具描述性的东西。

这让我想到了最后一个

代码语言:javascript
复制
_t

好吧,这是一根线。

  • 是我的线吗?
  • 是你的线吗?
  • 是主线吗?
  • 范围具体吗?
  • 这个线安全吗?
  • 这个线程的目的是什么?

你应该对你的命名做更多的描述。

作为一名开发人员,你编写的代码应该读起来像一本书,所以要优雅地写,不要吝啬。

当我阅读代码并看到一个字母变量时,我必须停止查看代码,找出该字母代表什么,因为名称没有告诉我有关对象的任何信息。

附加信息:

我注意到您将Main方法声明为整数方法.不知道为什么你只对return 0;这么做

您应该将它声明为void,没有任何理由可以看出您需要一个整数方法。

票数 6
EN

Code Review用户

发布于 2014-11-19 22:38:50

代码中隐藏着一个微妙的bug。

int hashCode = Math.Abs(this.GetHashCode());

Math.Abs被传递给int.MinValue时,它会抛出一个OverflowException

否定两个补数的最小值是无效的。

GetHashCode屈从于Guid的S GetHashCode,后者没有保证不会归还int.MinValue

有多不可能?现在,我已经有一个程序运行了15分钟来找到一个Guid,它的哈希代码为int.MinValue,我只找到了一个,但这是可能发生的,很好地了解(并修复了!)。

修复这个错误的一个好方法是

代码语言:javascript
复制
int hashCode = this.GetHashCode() & int.MaxValue;

也就是说,假设您并不真正关心GetHashCode返回的值,那么您只需要一些非负整数。

这个回路

时间(!_shouldStop) {龙L= c.ElapsedTicks;if (L% this.RandomStop) == 0) {OnTickedEvent(符);}

固定CPU。两个节拍器使我的CPU使用率从~10%提高到50%;四个节拍器把它提高到100%。看看System.Timers.Timer,它可以在不消耗所有CPU的情况下获得相同类型的功能。

我强烈建议不要在这样的文件中使用#define DEBUG#define TRACE。要在没有它们的情况下进行编译,需要遍历项目中的每个文件并删除这些行。

这些符号可以在项目生成设置中的项目级别上在Visual中设置:

或者,如果您正在手工编译,可以使用/define选项设置它们。

票数 5
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/70208

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档