[metrics-bugs] #31558 [Metrics/CollecTor]: Process bridge pool assignments again

[Tor Bug Tracker & Wiki]

#31558: Process bridge pool assignments again
-------------------------------+------------------------------
 Reporter:  karsten            |          Owner:  metrics-team
     Type:  enhancement        |         Status:  needs_review
 Priority:  Medium             |      Milestone:
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  BugSmashFund       |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:  irl                |        Sponsor:
-------------------------------+------------------------------

Comment (by fava):

 Replying to [comment:7 karsten]:


 > Please review
 [https://gitweb.torproject.org/user/karsten/collector.git/commit/?h=task-31558&id=db2a6bdab1bfc12377c515577589aa67d34fa2ba
 commit db2a6bd in my task-31558 branch].

 Hi karsten,

 it is my first review so let me know if I could do better.

  1. Please order statement following
 [https://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141855.html#1852
 Java guidelines] . This guidelines was been written in 1999 but is a good
 point to start and it is used also in static code analysis

  2. Probably it make sense to create a constant for string
 "BridgePoolAssignments"

  3. Instead of evaluate each time `330L * 60L * 1000L` it is better to
 create a constant with meaningfull name

  4. Instead of evaluate each time `3L * 24L * 60L * 60L * 1000L` it is
 better to create a constant with meaningfull name

  5. startProcessing it seems to complex, is it possible to split in more
 separated and testeable methods?

  6. there are some for each loop, is it make sense use a functional
 approach using lambda?

 Please let me know if I could help you in some way,

 Best Regards

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31558#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online