MAINT - Use real enums and parent class members #351
Conversation
- moved repeatedly used `_ckSummer`, `_cstream`, and `_dstream` members to `ModuleBase`; - tidied `ModuleBase` parameter handling; - added a common method to set up module checksumming streams `ModuleBase.setupDataStream()`; - `BytestreamModule` uses `ModuleBase.setupDataStream()`; - replaced all types that extended `EnumerationType` with real Java `enum` types: - `AgentType`, - `AnalogDigitalFlagType`, - `ChecksumType`, - `DocumentType,` - `IdentifierType`, - `PropertyArity`, - `PropertyType`, - 'SignatureType', - `SignatureUseType`, and - removed now defunct `EnumerationType`.
Codecov Report
@@ Coverage Diff @@
## integration #351 +/- ##
=================================================
+ Coverage 45.21% 45.25% +0.03%
- Complexity 3452 3457 +5
=================================================
Files 367 366 -1
Lines 30151 30162 +11
Branches 5952 5944 -8
=================================================
+ Hits 13633 13649 +16
+ Misses 14073 14067 -6
- Partials 2445 2446 +1
Continue to review full report at Codecov.
|
| * CLASS CONSTRUCTOR. | ||
| ******************************************************************/ | ||
| /** | ||
| * Applications will never create SignatureTypes directly. |
There was a problem hiding this comment.
Incorrect *Type referenced in comment. An old copy-paste error, though we probably don't need any of these comments anymore anyway since all enum constructors must be private. I'd do away with the large "CLASS CONSTRUCTOR"-style sign-boards as well, at least for simple enums.
| _dstream = getBufferedDataStream (stream, _je != null ? | ||
| _je.getBufferSize () : 0); | ||
| } | ||
| setupDataStream(stream, info); |
There was a problem hiding this comment.
The comment preceding this call explains code now hidden inside setupDataStream, so it would make more sense there instead.
| @@ -129,14 +109,14 @@ public final int parse (InputStream stream, RepInfo info, int parseIndex) | |||
| if (_nByte == 0) { | |||
| info.setMessage (new InfoMessage (CoreMessageConstants.INF_FILE_EMPTY)); | |||
| } | |||
| if (ckSummer != null) { | |||
| info.setChecksum (new Checksum (ckSummer.getCRC32 (), | |||
| if (_ckSummer != null) { | |||
There was a problem hiding this comment.
I was going to suggest moving this checksum-related code to ModuleBase as well, but apparently there's already a setChecksums method there which you could replace this with. Might want to keep an eye out in other modules as well.
There was a problem hiding this comment.
I'm on the trail of a few Module similarities. The calculation of checksums is one, the proffering and checking of sigs is another. All of these were supposed to be abstract class tasks but cut and paste has prevailed.
- picked up a few more variable first `var.equals(const)` constructs; - improved and cleaned up comments in enumerated types; - tidied a few nested `if else` constructs; - `BytestreamModule` now uses `ModuleBase.setChecksums()` method; and - begun to add log statements for ingored exceptions.
_ckSummer,_cstream, and_dstreammembers toModuleBase;ModuleBaseparameter handling;ModuleBase.setupDataStream();BytestreamModuleusesModuleBase.setupDataStream();EnumerationTypewith real Javaenumtypes:AgentType,AnalogDigitalFlagType,ChecksumType,DocumentType,IdentifierType,PropertyArity,PropertyType,SignatureType,SignatureUseType, andEnumerationType.Closes #373