2dust / clashN

A clash client for Windows, support Mihomo
https://1.2345345.xyz
GNU General Public License v3.0
4.73k stars 588 forks source link

改了一堆屎山代码 (主题变为蓝色了) #241

Closed SlimeNull closed 1 year ago

SlimeNull commented 1 year ago

既然 yaml 有命名转换, 就直接用 yaml 的命名转换罢, 还是不要用小写名称了

2dust commented 1 year ago

您修改后是否有测试过?

SlimeNull commented 1 year ago

说起来, 您的程序自始至终, 在我电脑上就无法正常运行 (指添加订阅后, 主页面没办法刷新出来代理列表 至于更改的代码, 和 v2rayN 最近给您提交的代码类似, 主要是将一些低能效的方法, 改为了较好的实现, 以及删去了一些没有必要的工具方法 (例如您封装的 String.IsNullOrEmpty 之类的, 以及某些不必要的字典操作

令我记忆尤新的一个代码是, 您判断了 Dict 是否有指定 Key, 如果有, 则使用索引器来更改值, 如果没有, 在 Add(new KeyValuePair) 事实上, 这个过程完全没有任何必要, 因为在所引器内部, 会判断是否有指定 key, 您的判断完全是冗余的

还有就是关于 Yaml 的实体类定义, YamlDotNet 有自带的连字符命名转换器, 您不应该在类的声明处使用奇怪的命名方式, 请使用符合 C# 规范的大驼峰.

另外, 还有就是空值的判断, 在您的这个项目中, 似乎对空值很不注重, 很多地方完全不对空值进行判断, 以及某些方法的返回值也不规范, 我在这方面进行了一部分优化 (但不是全部)

我建议您看一下 commit 中的改动, 然后参照这个改动, 尽量重构一下项目代码.

不过, 测试的话, 至少编译是能过的.

SlimeNull commented 1 year ago

还有个小问题, 我注意到您在很多地方使用了 XxxHandler 的命名方式, 例如 ConfigHandler (? 它用来进行配置的加载, 保存等操作, 我认为使用 Proc 命名是更好的选择, 当然, 如果您觉得 Handler 更合适, 可以直接对我的 commit 进行更改, 改回之前的 Handler.

另外, 很多接口的设计是不优雅的, 例如在加载配置的时候, 您判断了某个配置项是否为 0 (默认值), 但是这是没必要的, 您可以在类的声明中为属性设置默认值, 这样在反序列化的时候, yaml 文件中用户没有写的属性, 会保持您声明的默认值. 这在我的 commit 已经帮你写了下 (但是判断配置项是否为 0 (默认值) 的逻辑我并未删去, 具体处理还是要看您的意见.

SlimeNull commented 1 year ago

还有一个问题, 您对 ref 的使用方式是不正确的. ref 对于引用类型是没有任何作用的. (除非您想对传入的对象所在的字段直接进行赋值, 但我并没有发现您有这种操作)

所以, 我建议您稍微学习一下 ref 的使用, 以及 out 的使用. 参考 int.TryParse, 写比较合适的配置加载方法.

下面是我之前写的一个简单的配置加载逻辑, 您可以做参考:

/// <summary>
/// 尝试加载配置文件
/// </summary>
/// <param name="config">配置</param>
/// <returns>是否加载成功</returns>
private static bool TryLoadConfig([NotNullWhen(true)] out AppConfig? config)
{
    if (!File.Exists("config.json"))
    {
        Console.WriteLine("找不到配置文件 config.json, 已创建初始配置文件, 请编辑后重启程序 (按任意键继续");
        File.WriteAllText("config.json", JsonSerializer.Serialize(new AppConfig()));
        Console.ReadKey(true);

        config = null;
        return false;
    }

    config = JsonSerializer.Deserialize<AppConfig>(File.ReadAllText("config.json"));

    if (config == null)
    {
        Console.WriteLine("配置文件为空, 请删掉它并重新运行程序以生成新的配置 (按任意键继续");
        Console.ReadKey(true);
        return false;
    }

    return true;
}

在 Main 函数中这样使用:

if (!TryLoadConfig(out var config))
    return;     // 如果加载失败, 则退出程序 (这里替换成你想处理的逻辑)

在上面中, 我用到了 [NotNullWhen] 特性, 它用于使 IDE 知道, 当方法返回值为 true 的时候, 传出的 Config 不为空

SlimeNull commented 1 year ago

在实现全局程序单例运行的时候, 我注意到您使用了类似于命名句柄的东西 (我之前是使用命名句柄来实现它的)

需要注意的是, 最好在命名句柄的名称中附上程序的名称, 否则, 和其他程序的命名句柄冲突, 那么后果是不堪设想的.

SlimeNull commented 1 year ago

需要注意的是, 在 C# 中, 没有对枚举类型添加 E 前缀的规范, 并且这种做法也是不推荐的. 如果可以, 请删去 E 前缀 (部分枚举我已经帮您删去了), 如果有命名冲突, 您可以采用这样的做法:

例如: ClashConfig 中有一个字段叫做 Kind, 为它声明枚举类型时, 可以使用 ClashKind 名称

SlimeNull commented 1 year ago

我也建议在 Clash 配置中, 对于 "枚举" 类型, 我建议不要声明为字符串, 应该声明成枚举类型. 鉴于 YamlDotNet 没有对枚举名称的转换, 您可以使用这个转换器:

/// <summary>
/// 用于 YAML 的枚举转换器
/// </summary>
private class YamlEnumConverter : IYamlTypeConverter
{
    private YamlEnumConverter(INamingConvention? namingConvention)
    {
        NamingConvention = namingConvention;
    }

    public INamingConvention? NamingConvention { get; }

    public static YamlEnumConverter Create(INamingConvention? namingConvention) =>
        new YamlEnumConverter(namingConvention);

    public bool Accepts(Type type) => type.IsEnum;

    public object? ReadYaml(IParser parser, Type type)
    {
        var scalar =
            parser.Consume<Scalar>();
        var scalarRealName =
            NamingConvention?.Apply(scalar.Value);
        if (!Enum.TryParse(type, scalarRealName, out object? enumValue) &&
            !Enum.TryParse(type, scalar.Value, out enumValue))
            throw new YamlException(scalar.Start, scalar.End, $"Value '{scalar.Value}' not found in Enum {type}");
        return enumValue;
    }

    public void WriteYaml(IEmitter emitter, object? value, Type type)
    {
        if (value == null)
        {
            emitter.Emit(new Scalar("null"));
            return;
        }

        string write =
            NamingConvention?.Apply($"{value}") ?? $"{value}";

        if (string.IsNullOrWhiteSpace(write))
            throw new YamlException($"Cannot write {value} to yaml");

        emitter.Emit(new Scalar(write));
    }
}
2dust commented 1 year ago

感谢您的建议 有时间慢慢来学习检查您的代码