Details
-
Sub-task
-
Status: Resolved
-
Blocker
-
Resolution: Fixed
-
None
-
None
Description
I already commented in it HDDS-5097, but it seems that majority of the new functions are implemented in just copying existing classes with V1 postfix.
It's not clear what is V1 (or what is V0). And I already quoted the clead code recommendation:
It turned out that a lot of tests just copied with V1 prefix with small modification (or original test extended with V1 which means the methods of the old and new tests are executed) instead of improving the original test to cover both of the cases (simple/prefix-ed).
Also: the same functionality seems to be tested on multiple levels (FileSystemInterface, OMRequest, acceptance test...)
This copy can make the maintenance of the tests slightly harder, and the V1 prefix is quite meaningless and confusing.
From the legendary "Clean code" book:
Programmers create problems for themselves when they write code solely to satisfy a compiler or interpreter. For example, because you can’t use the same name to refer to two different things in the same scope, you might be tempted to change one name in an arbitrary way. Sometimes this is done by misspelling one, leading to the surprising situation where correcting spelling errors leads to an inability to compile.2
[...] It is not sufficient to add number series or noise words, even though the compiler is satisfied. If names must be different, then they should also mean something different.
Number-series naming (a1, a2, .. aN) is the opposite of intentional naming. Such names are not disinformative—they are noninformative; they provide no clue to the author’s intention....
I totally agree with this section, I think we should avoid using V1 prefixes everywhere and have some more meaningful class names.
Attachments
Issue Links
- is related to
-
-
- Resolved
-
- links to