taskadapter / redmine-java-api

Redmine Java API
Apache License 2.0
269 stars 162 forks source link

Some tiny problems in User class #309

Closed LeiXuan6 closed 5 years ago

LeiXuan6 commented 6 years ago

I found 2 problems in method getStatus of the User class .

1、Now that you have defined the constant for status, I think the user do not want and should not know what is the value of the status. My suggestion is remove the "1" and "3" from the comment. 2、If the redmine change its status in 4.x, 5.x, x.x and user want to use this project the access two redmines, and the one is 3.x and the another one is 4.x. This project may not work well. My suggestion is defining a interface too decide what the status is. And We could specify all the implements for all the version of redmine. And User can also add their implements of this interface if a latest redmine publised and you have no time to process.

https://github.com/LeiXuan6/redmine-java-api/blob/master/img/user%20class.png

alexeyOnGitHub commented 5 years ago

I see some hardcoded constants in User class indeed. I improved javadoc to make it clear that these numbrs are not guaranteed, although will probably work. added couple constants.

using an Enum instead of integer would be nice in theory, but that means we would need to update this class every time Redmine changes user statuses, plus it would break backward compatibility. I think it is okay for now