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

修改数据源配置对象为Map #9

Closed
wants to merge 1 commit into from
Closed

修改数据源配置对象为Map #9

wants to merge 1 commit into from

Conversation

bohrqiu
Copy link
Contributor

@bohrqiu bohrqiu commented Feb 16, 2016

修改为Map的原因为:

  1. Properties对象太重,这里没有必要使用这个
  2. 修复bug
  3. 减少了点点测试代码

bug描述:

比如配置props.put("metrics.enable",true);,这行代码不会正确的执行,具体见java.util.Properties#getProperty(java.lang.String)

@terrymanu
Copy link
Member

先讨论一下:

  1. 如何理解Properties对象太重
  2. 这个bug,如果改成map<String, Object>,如果将true放置为"true",程序仍然解析不了。如果是properties,至少正确使用的人都会放置String,改成Object的泛型是否会导致理解成本增加?

@bohrqiu
Copy link
Contributor Author

bohrqiu commented Feb 17, 2016

  1. Properties对象提供的能力超过了场景需要的,

  2. 改成map可以解决的。

    Properties properties=new Properties();
    properties.put("metrics.enable",true);
    

properties#put方法参数签名为Object,Object,泛型的理解成本应该比较低,

@terrymanu
Copy link
Member

一些不成文的规定,prop是从prop文件中读取,不需要解释就会被认为是string,但map则不然。这里我们的本意也是想从prop文件加载配置,只是还没写

@terrymanu terrymanu closed this Feb 25, 2016
terrymanu pushed a commit that referenced this pull request Jun 23, 2018
KomachiSion pushed a commit to KomachiSion/incubator-shardingsphere that referenced this pull request Nov 25, 2019
@sunOnly sunOnly mentioned this pull request Oct 13, 2020
xuup added a commit to xuup/shardingsphere that referenced this pull request Jul 19, 2022
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.

2 participants