Re: Some extras with Reason.java

0 views
Skip to first unread message

aeagl...@gmail.com

unread,
Jun 16, 2009, 1:03:38 PM6/16/09
to valk...@gmail.com, testability-...@googlegroups.com
thanks, few comments:


http://codereview.appspot.com/75049/diff/1/6
File core/src/main/java/com/google/test/metric/MethodInfo.java (right):

http://codereview.appspot.com/75049/diff/1/6#newcode175
Line 175: public boolean getIsFinal() {
should be named isFinal() to match javabeans convention

http://codereview.appspot.com/75049/diff/1/3
File core/src/test/java/com/google/test/metric/CostUtil.java (right):

http://codereview.appspot.com/75049/diff/1/3#newcode100
Line 100: }
can revert this file

http://codereview.appspot.com/75049/diff/1/2
File core/src/test/java/com/google/test/metric/MetricComputerTest.java
(right):

http://codereview.appspot.com/75049/diff/1/2#newcode587
Line 587: public final void staticFinalFunction() {
is it necessary to have the static variable in this class? it seems
confusing if you're just asserting that a final method can't be
overridden.

http://codereview.appspot.com/75049/diff/1/2#newcode598
Line 598: assertEquals("cost for calling static method
(non-overridable)", staticCall.getReason());
can you assert on the enum value rather than a string? Would be less
brittle.

http://codereview.appspot.com/75049/diff/1/2#newcode604
Line 604: assertEquals("cost for calling final method
(non-overridable)", staticFinalCall.getReason());
same

http://codereview.appspot.com/75049

Reply all
Reply to author
Forward
0 new messages