Add methods to Set Interface #139
Conversation
Codecov Report
@@ Coverage Diff @@
## release-0.4.0 #139 +/- ##
=================================================
- Coverage 78.63% 78.00% -0.64%
=================================================
Files 13 13
Lines 1807 1823 +16
=================================================
+ Hits 1421 1422 +1
- Misses 253 260 +7
- Partials 133 141 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
|
srikartati
force-pushed
the
change_sets
branch
2 times, most recently
from
935e9ca
to
f61e2a8
Compare
| func (s *set) ResetSet() { | ||
| s.buffer.Reset() | ||
| s.setType = Undefined | ||
| s.records = s.records[:0] | ||
| } |
There was a problem hiding this comment.
Since we will probably encounter Undefined for setType, we should check whether setType is Undefined in AddRecords function to avoid adding record to invalid set.
pkg/entities/set.go
Outdated
| if !isDecoding { | ||
| } | ||
|
|
||
| func (s *set) PrepareSet(templateID uint16) error { |
There was a problem hiding this comment.
Is it possible that we PrepareSet after ResetSet? I think this is a valid use case for us to re-use the set. If so, we can consider moving the setType parameter to PrepareSet otherwise the setType will be Undefined
There was a problem hiding this comment.
Agreed it is good to have these checks. Yes, we can reuse the set for both data and template types instead of allocating again.
Added those checks.
Edit: still working on the change
There was a problem hiding this comment.
I think the problem is still there -- if we call ResetSet and then want to use PrepareSet, the setType will always be Undefined.
There was a problem hiding this comment.
And it's better to add another check here so that Undefined setType will not go through this function.
srikartati
force-pushed
the
change_sets
branch
2 times, most recently
from
5993fb3
to
82ec754
Compare
Add PrepareSet and ResetSet methods to Set interface. This is to avoid multiple allocations by the user, for example, when exporting sets user could allocate the Set only once and re-use it for exporting multiple records.
srikartati
force-pushed
the
change_sets
branch
from
82ec754
to
2a38936
Compare
|
Originally I was thinking of using separate sets for data and template, so did not take care of set type. Made the changes accordingly and addressed comments too. |
Add PrepareSet and ResetSet methods to Set interface. This is to avoid multiple allocations by the user, for example, when exporting sets user could allocate the Set only once and re-use it for exporting multiple records.
Add PrepareSet and ResetSet methods to Set interface. This is to avoid multiple allocations by the user, for example, when exporting sets user could allocate the Set only once and re-use it for exporting multiple records.
Add PrepareSet and ResetSet methods to Set interface. This is to avoid multiple allocations by the user, for example, when exporting sets user could allocate the Set only once and re-use it for exporting multiple records.
Add PrepareSet and ResetSet methods to Set interface.
This is to avoid multiple allocations by the user, for example, when
exporting sets user could allocate the Set only once and re-use it
for exporting multiple records.
PS: In Antrea, the main user of go-ipfix library, this change enables to write better unit tests.