Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Implementing AntTable component #75

Closed
wants to merge 0 commits into from

Conversation

AlseinX
Copy link
Contributor

@AlseinX AlseinX commented Apr 14, 2020

No description provided.

value = value.ToLowerInvariant();

var index = value
.Select((c, i) => ((char c, int i)?)(c, i))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里能否使用 ([\d.]+) 正则表示?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

然后我又找了一个复杂一点的正则 ([\d]+[.]?[\d]*)\s*([\u4e00-\u9fa5]+|[a-z])

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

正则的性能是否优于手工算法,这个有比较过吗

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlseinX 这个其实都是半斤八两哈,从性能和可读性上都是差不多的渣

Fr,
}

public struct CssSizeLength
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

也许这里给一个 readonly 可以在后续传参数的时候加上 in 减少结构体复制,提升一丁点

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码库本身存在太多警告,看不到IDE的readonly建议有点难受


public RowSelectionType Type { get; set; }

public CssSizeLength ColumnWidth { get; set; } = DEFAULT_COLUMN_WIDTH;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

有点推荐同时也造一个 static readonly 的值,这样就不用每次创建这个对象的时候都进行一次转换,当然,这是内存换时间哈

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前想过,但是因为常量值本身就会驻留内存,如果存一个转换后的版本我觉得过度内存换速度了。


public bool HideDefaultSelections { get; set; } = false;

public Func<bool, TData, int, RenderFragment, RenderFragment> RenderCell { get; set; } = (c, d, i, o) => o;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我认为这里用 Func 有这样的坑,在于参数的含义的问题,在没有注释和文档的前提下,还是比较磨性的,我比较推荐此时创建一个抽象类或接口叫 Xx 创建器的,这样方便传入参数

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个我觉得待讨论,因为如果用不同的委托类型表示同一签名的函数,在直接传递时会不方便。所以个人取倾向于使用Func和Action作为标准,当然RenderFragment这类使用委托进行表达但是在语义上并不是一个“函数”的情况理应定义专门的委托类型。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlseinX 大概的问题在于参数有点多哈,此时参数的含义就有点诡异了


public IList<TData> SelectedRows { get; } = new List<TData>();

public IList<(RenderFragment text, Func<TData> onSelect)> Selections = new List<(RenderFragment, Func<TData>)>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

公开字段还是不太好,推荐修改为属性

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个地方是写到一半的。
因为这个库里面存在太多的警告,所以这种东西很难看到

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants