alibaba / easyexcel

快速、简洁、解决大文件内存溢出的java处理Excel工具
https://easyexcel.opensource.alibaba.com
Apache License 2.0
32.09k stars 7.5k forks source link

尝试修复 issue #3905 #3926

Open WT-AHA opened 1 month ago

WT-AHA commented 1 month ago

3905

CLAassistant commented 1 month ago

CLA assistant check
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.


wangtong_b seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

psxjoy commented 1 month ago

Hi @WT-AHA , thank you for your open-source contribution. We really appreciate everyone’s questions and shares. However, regarding this PR, I have a few questions to ask (don’t worry, this isn’t a critique):

  1. Will using reflection impact performance when dealing with large files (or large amounts of data)?
  2. I think setter methods should focus on the single responsibility of setting properties. What advantages do the changes you made offer?

Also, it’s not recommended to use wildcard imports like com.alibaba.excel.util.*, as it can cause confusion for other developers.

短暂的看了下issue和PR内容,讨论几个事情(这不是质询,不用紧张):

  1. 使用反射是否会影响大文件(或者大量数据)下的性能?
  2. 我认为 setter 方法应该专注于设置属性的单一职责。更改之后有什么好处?

综合两个问题,我觉得这么修改,适得其反了。

另外,不建议使用 com.alibaba.excel.util.* 这种导入。

psxjoy commented 1 month ago

@zhuangjiaju Let’s temporarily close this PR for now. If you have better implementations or ideas in the future, feel free to reach out. 我们暂时关闭这个PR,如果有新的想法,欢迎随时沟通。