init:.Net 6版本初始化 by Komi-Thaw · Pull Request #70 · dotnet-campus/AsyncWorkerCollection

Komi-Thaw · 2022-10-20T01:03:48Z

No description provided.

lindexi · 2022-10-20T01:55:39Z

@KomiSans 感谢贡献，似乎改格式的太多了，我没有找到核心的改动。如果可以的话，改格式的单独出来做一个 PR 如何？

lindexi · 2022-10-20T01:56:25Z

AsyncWorkerCollection/AsyncAutoResetEvent.cs

        private bool _isDisposed;

-        private readonly object _locker = new object();
+        private readonly object _locker = new();


由于采用源代码形式分发，考虑到源代码兼容性，不打算采用此语法特性

lindexi · 2022-10-20T01:57:00Z

AsyncWorkerCollection/AsyncTaskQueue_/AsyncTaskQueue.cs

-            task = null;
-            while (_queue.Count > 0)
+            task = default;
+            while (_queue.Count is 0)


这个逻辑是错误的

这里是不断获取队列内容

lindexi · 2022-10-20T01:57:36Z

AsyncWorkerCollection/AsyncTaskQueue_/AsyncTaskQueue.cs

            }

-            return false;
+            return default;


对于布尔来说，使用 false 更加表意

lindexi · 2022-10-20T01:58:01Z

AsyncWorkerCollection/AsyncTaskQueue_/AwaitableTask.cs

        public void SetNotExecutable()
        {
-            Executable = false;
+            Executable = default;


对于布尔来说，使用 true 和 false 更加表意

lindexi · 2022-10-20T01:58:50Z

AsyncWorkerCollection/DoubleBuffer_/DoubleBufferLazyInitializeTask.cs

-                }
+                // 如果不是空
+                // 那么设置任务完成
+                _waitForInitializationTask?.SetResult(true);


原本的写法是表示空和非空都在处理逻辑内，而且也方便加上断点进行调试。这个逻辑不做简化

lindexi · 2022-10-20T01:59:40Z

AsyncWorkerCollection/DoubleBuffer_/DoubleBufferLazyInitializeTask.cs

-// 根据 DoubleBufferTask 的设计，这个方法只有一个线程进入
-FirstCheckInitialized: // 标签：第一个判断初始化方法
+            // 根据 DoubleBufferTask 的设计，这个方法只有一个线程进入
+            FirstCheckInitialized: // 标签：第一个判断初始化方法


这里用到部分 Goto 的方式，似乎对标签的缩进格式有所不同。我比较喜欢使用不缩进的方式，用来表示标签就是和其他代码不同

lindexi · 2022-10-20T02:00:15Z

AsyncWorkerCollection/DoubleBuffer_/DoubleBufferLazyInitializeTask.cs

                {
                    await _waitForInitializationTask!.Task.ConfigureAwait(false);
                }
-                else


这个空 else 是用来表示另一个情况的，不简化哦

lindexi · 2022-10-20T02:01:54Z

AsyncWorkerCollection/LimitedRunningCountTask.cs

-                }
+                if (runningTaskList.Count <= MaxRunningCount) return;
+
+                if (WaitForFreeTask?.Task.IsCompleted is false) return;


这里的空逻辑是有意义的哦，不要省略了哦，表示两个情况都在逻辑预期内，且方便加上断点支持调试

这里是特意如此写的，还请不要简化

lindexi · 2022-10-20T02:02:11Z

AsyncWorkerCollection/Reentrancy/KeepLastReentrancyTask.cs

                    {
-                        throw new InvalidOperationException("在指定 KeepLastReentrancyTask 的任务时，方法内不允许返回 null。请至少返回 Task.FromResult<object>(null)。");
+                        throw new InvalidOperationException(
+                            "在指定 KeepLastReentrancyTask 的任务时，方法内不允许返回 null。请至少返回 Task.FromResult<object>(null)。");


奇怪的换行逻辑，感觉不需要换行

lindexi · 2022-10-20T02:02:49Z

AsyncWorkerCollection/Reentrancy/ReentrancyTask.cs

        /// 在派生类中执行重入任务的时候，从此处获取需要执行的可重入异步任务。
        /// </summary>
-        protected Func<TParameter, Task<TReturn>> WorkingTask { get; }
+        private Func<TParameter, Task<TReturn>> WorkingTask { get; }


这个是支持被业务方子类调用的哦，为什么这个要改动为私有？

lindexi

感谢贡献，似乎看起来有一点小问题，期望修改

Komi-Thaw · 2022-10-20T02:32:28Z

没问题的我这边再看下然后提出PR lindexi ***@***.***> 于2022年10月20日周四 10:03写道：

…

***@***.**** requested changes on this pull request. 感谢贡献，似乎看起来有一点小问题，期望修改 — Reply to this email directly, view it on GitHub <#70 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANLYJZQGXFWVEHF7RCEIPITWECR6XANCNFSM6AAAAAARJUQU2I> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Komi-Thaw · 2022-10-20T02:34:40Z

感谢指正

lindexi · 2022-10-20T11:40:58Z

AsyncWorkerCollection/AsyncWorkerCollection.csproj

        <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
        <GenerateDocumentationFile>true</GenerateDocumentationFile>
        <AssemblyName>dotnetCampus.AsyncWorkerCollection</AssemblyName>
+        <LangVersion>8</LangVersion>


这个是不加的，因为将会作为源代码包分发

好的我这边主要是为了使用Rider方便些加上去的

@KomiSans 好奇怪，居然你不用VS开发

Rider更智能一些其实

lindexi · 2022-10-20T11:41:34Z

AsyncWorkerCollection/DoubleBuffer_/DoubleBuffer.cs

-                    CurrentList = AList;
-                    return BList;
-                }
+                CurrentList = ReferenceEquals(CurrentList, AList) ? AList : BList;


这里的逻辑需要两次判断，不如原先的

好的,我去改过来

lindexi · 2022-10-20T11:42:18Z

AsyncWorkerCollection/LimitedRunningCountTask.cs

        /// </summary>
        /// <returns></returns>
-        public async ValueTask WaitForFree()
+        private async ValueTask WaitForFree()


这个看起来是非预期的

这个是提供客户方调用的么请问?

Komi-Thaw · 2022-11-07T12:05:15Z

AsyncWorkerCollection/AsyncTaskQueue_/AsyncTaskQueue.cs

        private readonly ConcurrentQueue<AwaitableTask> _queue = new ConcurrentQueue<AwaitableTask>();
        private readonly AsyncAutoResetEvent _autoResetEvent;
+
        // ReSharper disable once RedundantDefaultMemberInitializer


请问,这个地方的初始化语法镇压有什么特殊含义么?

@Komi-Thaw 只是让 Resharper 开心而已

这里明确给定 _autoCancelPreviousTask 的初始值为 false 的值，和默认 bool 行为相同，此时 Resharper 没有理解意图，毕竟执行逻辑是等价的，于是提示这个 false 可以删除。但是这里我期望是明确给定 false 初始值，以便在后续有期望更改为 true 的时候，可以看到，之前的设计就是明确要 false 值

CLAassistant · 2025-07-03T13:14:51Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

init:.Net 6版本初始化

5d7aef6

lindexi reviewed Oct 20, 2022

View reviewed changes

lindexi requested changes Oct 20, 2022

View reviewed changes

drawback:撤回问题部分

3ea4702

lindexi reviewed Oct 20, 2022

View reviewed changes

Komi-Thaw commented Nov 7, 2022

View reviewed changes

Merge branch 'dotnet-campus:master' into komi-dev

7687ed7

Conversation

Komi-Thaw commented Oct 20, 2022

Uh oh!

lindexi commented Oct 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lindexi Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lindexi left a comment

Choose a reason for hiding this comment

Uh oh!

Komi-Thaw commented Oct 20, 2022 via email

Uh oh!

Komi-Thaw commented Oct 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Jul 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lindexi Oct 20, 2022 •

edited

Loading