You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@dubbo.apache.org by "Duncan-tree-zhou (GitHub)" <gi...@apache.org> on 2019/04/02 14:00:58 UTC

[GitHub] [incubator-dubbo] Duncan-tree-zhou opened issue #3795: Found hard code Strings, which should be replace by constants.

- [ ] I have searched the [issues](https://github.com/apache/incubator-dubbo/issues) of this repository and believe that this is not a duplicate.
- [ ] I have checked the [FAQ](https://github.com/apache/incubator-dubbo/blob/master/FAQ.md) of this repository and believe that this is not a duplicate.

### Environment

* Dubbo version: master
* Operating System version: any
* Java version: any

### Steps to reproduce this issue

1. no step

Pls. provide [GitHub address] to reproduce this issue.

### Expected Result
```
package org.apache.dubbo.config;
public class RegistryConfig extends AbstractConfig {
......
    public void setUsername(String username) {
        checkName("username", username);
        this.username = username;
    }
......
    public void setPassword(String password) {
        checkLength("password", password);
        this.password = password;
    }
......
}
```
some property name use hard code String. actually they was defined in org.apache.dubbo.common.Constants.

the following is a list of hard code Strings in RegistryConfig and the correct Constrants.
"username" - >Constants.USERNAME_KEY
"password" - >Constants.PASSWORD_KEY
"file" - > Constants.FILE_KEY
"client" - > Constants.CLIENT_KEY


What do you expected from the above steps？

the hard code String should be replace by constants

What actually happens?

If there is an exception, please attach the exception trace:

```

```


[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] cvictory commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "cvictory (GitHub)" <gi...@apache.org>.

Could you commit a pr for this issue?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] Duncan-tree-zhou commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "Duncan-tree-zhou (GitHub)" <gi...@apache.org>.

I think relpace these hardcode strings with a string array is not a good way. because if replaced, the code would be harder to be understand.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "kezhenxu94 (GitHub)" <gi...@apache.org>.

> i had create the pull request. but i don't know how to attach it to this issue.
> can you tell me how to do that?
> 
> here is the pull request [#3803](https://github.com/apache/incubator-dubbo/pull/3803)

@Duncan-tree-zhou you can comment in your pull request like this:

> related issue #\<this issue number\>, OR resolve issue #\<this issue number\>

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] Ifdevil commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "Ifdevil (GitHub)" <gi...@apache.org>.

In the Wrapper Class has same issue ,like:
           
           private static final String[] OBJECT_METHODS = new String[]{"getClass", "hashCode", "toString", "equals"};

            if ("getClass".equals(mn)) {
                return instance.getClass();
            }
            if ("hashCode".equals(mn)) {
                return instance.hashCode();
            }
            if ("toString".equals(mn)) {
                return instance.toString();
            }
            if ("equals".equals(mn)) {
                if (args.length == 1) {
                    return instance.equals(args[0]);
                }
                throw new IllegalArgumentException("Invoke method [" + mn + "] argument number error.");
            }

Maybe these can be replaced by OBJECT_METHODS[i]?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] Duncan-tree-zhou commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "Duncan-tree-zhou (GitHub)" <gi...@apache.org>.

thanks, it looks like that the travis already attach the pr to this issue by reading it's title.



[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] Duncan-tree-zhou commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "Duncan-tree-zhou (GitHub)" <gi...@apache.org>.

i had create the pull request. but i don't know how to attach it to this issue.
can you tell me how to do that?

here is the pull request [#3803](https://github.com/apache/incubator-dubbo/pull/3803)

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "kezhenxu94 (GitHub)" <gi...@apache.org>.

> i had create the pull request. but i don't know how to attach it to this issue.
> can you tell me how to do that?
> 
> here is the pull request [#3803](https://github.com/apache/incubator-dubbo/pull/3803)

@Duncan-tree-zhou you can comment in your pull request like this:

> related issue #<this issue number>, OR resolve issue #<this issue number>

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] Ifdevil commented on issue #3795: Found hard code Strings, which should be replace by constants.

Posted by "Ifdevil (GitHub)" <gi...@apache.org>.

It's really unnecessary.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3795 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org