covid-sim icon indicating copy to clipboard operation
covid-sim copied to clipboard

Refactor DigitalContactTracingSweep

Open DavidVernest opened this issue 4 years ago • 6 comments

Refactor the DigitalContactTracingSweep function in Sweep.cpp.

Specifically, extract four functions:

  1. static void GetDctStartEndTimes(int infector, int contact, unsigned short contact_time, unsigned short& dct_start_time, unsigned short& dct_end_time);

  2. void AddToDctQueue(int tn, int ad, int i3, int ci, unsigned short ts);

  3. static void RemoveFromDctQueue(int j, int i, int k, int contact, int infector, unsigned short contact_time);

  4. static void RemoveContactFromDctQueue(int contact);

DavidVernest avatar Jun 23 '20 11:06 DavidVernest

Maybe this should be moved out of Sweep.cpp? The file is quite large, so it may be useful to break it down.

zlatanvasovic avatar Jun 23 '20 16:06 zlatanvasovic

@zdroid Yes - plans are afoot to break both Sweep.cpp and Update.cpp into separate files/classes. But not in this PR - to keep things simpler for the reviewers, I have not broken up these files.

DavidVernest avatar Jun 24 '20 10:06 DavidVernest

@weshinsley @dlaydon @matt-gretton-dann Is Digital Contact Tracing actively used? If so, do we have any regression tests for it? It might not be worth refactoring otherwise - and I'll move on to something else. Thanks.

DavidVernest avatar Jun 25 '20 10:06 DavidVernest

@weshinsley Hi - can I just confirm if Digital Contact Tracing is actively used? I can unit test and simplify the code - just looking to see if it is active before asking for a merge request.

DavidVernest avatar Jun 30 '20 15:06 DavidVernest

Hi @mstevens-uk, thanks for asking. We are using this code right now, but it isn't added to the formal regression tests. Therefore if you could refrain from changing or refactoring it for now that would be helpful. Thanks.

dlaydon avatar Jun 30 '20 16:06 dlaydon

Understood.

DavidVernest avatar Jun 30 '20 16:06 DavidVernest