remove unused code of TestJavassistUtils and make it readable

[wldandan]

• remove unused method definition.
• update gitignore file for IDEA project
• make it readable about {}

[coveralls]

Coverage decreased (-0.02%) to 86.965% when pulling 8ddee43 on wldandan:master into 7ddf345 on ServiceComb:master.

[wujimin]

deleted "getMethod" is to ensure setter is exist and has correct signature
if has any problems, will throw exception.

[WillemJiang]

We need to use assert the verify the call result.

[wldandan]

If "getMethod" has logic as you mentioned, I think it should be verified rather than just let it be a caller. For example, we can add the following assert for the logic if we really care about it :

method = cls.getMethod("setListStringField", List.class);
assertNotNull(method);

[WillemJiang]

@wldandan Please add a new commit for it.

[wujimin]

infact, if cls.getMethod return null, will throw exception inside getMethod
so the assert is invalid.

[wujimin]

if you care this so much, you can invoke org.springframework.util.ReflectionUtils.findMethod(Class, String, Class...)
then if method not exists, will get a null result
but this method invoked getDeclaredMethod inside it, not invoked getMethod, so after assert null, you must assert if it's public.

[wldandan]

Ok. my thought is straightforward. From my point of view, the test case is best entrance for beginner to get familiar with the functionality.

If some methods are called in test case, it means that we expect it to be verified.

Back to the discussion,
As you mentioned, "deleted "getMethod" is to ensure setter is exist and has correct signature if has any problems, will throw exception."

The logic is implicit, I absolutely think it should be verified if we call the method.
For the case, what is your suggestion if we want to verify it?

method = cls.getMethod("setListStringField", List.class);

[WillemJiang]

We still need to verify the method, if the getMethod throw the exception JUnit can still know about and tell us something is wrong. As a client we want to make sure is the method is not null and we can still use it. If we want to verify the non-exist method, we also need to add expected exception into JUnit test case to make sure the exception is thrown.

[wldandan]

@WillemJiang +1. To keep the scope limited, I think we can close the loop and add another PR to verify "getmethod" for the scenario about throwing exception.
I will do that soon.
Thanks guys.

[wujimin]

this comments is what the target method will be.

[wldandan]

It is easy to implement that if we need it in one day, and I do not think it is worth to highlighting it just for recall someone that the method will be in future.

[wujimin]

here, we create a new class in memory, and add a new method in the class
if we did not comments the targt method signature, it's hard to know what's the purpose

so i thinks, comments can be better, but delete it is not good.

[wldandan]

Is it reasonable if we define another method to hold the structure or use test case to clarify it?
Or should we add more detailed comments to mention that rather than just a method definition?

[wujimin]

after format code, it will rollback to be the deleted style.

[wldandan]

OK.
It might not be an issue. However, to make it human readable, I suggest that removing the line break if there is nothing inside.

[wujimin]

agree.
but it seems that hard to write this rule both in IntelliJ and eclipse

[wldandan]

Leave it as an issue and put it in the queue. :)