> (2) Why not refactor and pull-out methods? This then forces you to _name_
> the methods, instead of the above (anonymous blocks vs. commented
blocks.)
I did not pull out the code sections into separate methods because I had no
intention of re-structuring the whole class. I only wanted to fix a bug in
the class Fraction and add a test case in FractionTest that would have
failed
due to this bug, and in theprocess organize that which was already
present in
FractionTest a bit better, because it has been pointed out to me that new
contributions should not only strive to improve functionality but also
readability.
Introducing those code blocks seemed like a straightforward way of
making the
mess in the bodies of some of the test methods in FractionTest more
comprehensible –
I would think that adding new methods could be more controversial than
adding
code blocks.
Besides, should anyone in the future wish to extract these code sections
into
separate methods, I doubt that the code blocks would be a hindrance – if
anything,
I imagine that they would it easier, because with the code blocks, it is
a lot
clearer WHAT can be extracted to a method in the first place than
without them.
> (1) It is helpful to add a // comment for each block, otherwise, it
feels
> anonymous and weird to me.
I am not sure how adding a comment to the code blocks would be helpful in
this case. The blocks only serve to separate the test cases, and most of
the time,
these test cases differ only in the values that they use, and not in
some defining
characteristic. For example, the first three blocks in testReciprocal()
only test
some different arbitrary fractions, but are otherwise completely
analogous, so I
couldn't think of any other comment than something like "test case 1",
"test case 2",
etc. Granted, the fourth block tests failure with a zero-denominator, so
in this
case, I can understand your point about adding comments. By the way,
some of these
test cases were already commented before I edited the class.
On 6/12/19 3:08 PM, Gary Gregory wrote:
I've used code blocks in this style in the past but...
(1) It is helpful to add a // comment for each block, otherwise, it feels
anonymous and weird to me.
(2) Why not refactor and pull-out methods? This then forces you to _name_
the methods, instead of the above (anonymous blocks vs. commented blocks.)
Gary
On Wed, Jun 12, 2019 at 9:00 AM Heinrich Bohne <heinrich.bo...@gmx.at>
wrote:
I have been asked to request some feedback on this pull request:
https://github.com/apache/commons-numbers/pull/36– specifically, about
the introduction of code blocks in the commit "NUMBERS-100: Reduce scope
of local variables".
I had the idea with the code blocks when I wanted to add a test to the
method testAdd() but was intimidated by the huge wall of code contained
in the method. When taking a closer look, this code wall is actually
composed of several test cases that are completely independent of each
other, but because the local variables live throughout the whole method
and are re-used in almost every test case, this is not obvious. The more
variables are involved, the closer you have to look to figure out which
sections are independent of the rest.
I think that, with the code blocks, it is instantly obvious that a
specific section does not depend on anything that happened before it, or
that it does not affect anything that comes after it. So I think that
they are preferable to the previous version of the file.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
