Remove recordType and reserved fields

[ckbaum]

This is a bit scary, but it seems that recordType and reserved fields don't really do anything. They are unexported, they are always set by each struct's corresponding New function, and they are effectively constants.

The existence of these fields causes issues when marshalling & unmarshalling ACH data structures. Because recordType is unexported, it isn't included in the JSON blob produced by marshal. Then when the blob is unmarshalled, the absence of recordType causes validation errors. This is addressed in a few different ways for different types:

• Functions like NewEntryDetail return a new struct with defaults set. The blob can be unmarshalled into that struct and the unexported defaults remain.
• The unexported function setEntryRecordType iterates through each of an EntryDetail's addenda fields and sets each recordTypes (this is called by FileFromJSON)
• Custom Unmarshal functions like batch.UnmarshalJSON populate fields with default structs before unmarshalling.

The particular issue I'm running into is marshalling and unmarshalling BatchHeaders. This could be addressed by exporting setEntryRecordType or adding another custom Unmarshal function. But I think it would be even better if we could remove recordType (and reserved) altogether, and replace references to them with appropriate constants.

[codecov-commenter]

Codecov Report

Merging #1070 (4d69e41) into master (626f584) will decrease coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1070      +/-   ##
==========================================
- Coverage   91.44%   91.28%   -0.17%     
==========================================
  Files          68       68              
  Lines        6499     6299     -200     
==========================================
- Hits         5943     5750     -193     
+ Misses        326      320       -6     
+ Partials      230      229       -1     

[adamdecaf]

You're right in that recordType (and reserved) breaks the typical marshal/unmarshal expectations. I'm going to look this over and likely merge it.

Have you tested this much?

[ckbaum]

I've tested it on the use cases I'm dealing with and it does solve the marashalling issues I was running into. And tests like testAddenda18String that parse and reoutput a raw ach record seem like good guards against anything changing with the parsing logic. But of course more eyes from people with more context are always good :)

[adamdecaf]

That sounds good to me. The change looks fine and you're correct we have a lot of tests which would cover this change. I'm feeling confident to merge. Thanks for the PR