-
Notifications
You must be signed in to change notification settings - Fork 4
Implementation for removeAttendee with the beginnings of a test. #50
base: master
Are you sure you want to change the base?
Conversation
| public void deleteInactiveAttendees() { | ||
| throw new RuntimeException("Unimplemented"); | ||
| public void deleteInactiveAttendees(DatastoreClientInterface datastore, String sessionId) { | ||
| List<Attendee> attendees = datastore.getAttendeesInSession(sessionId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getAttendeesInSession returns List
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of AttendeeInterface i believe
| List<Attendee> attendees = datastore.getAttendeesInSession(sessionId); | ||
| for(int i = 0; i <= attendees.length(); i++){ | ||
| AttendeeInterface currentAttendee = attendees.get(i); | ||
| AttendeeeInterface screenName = currentAttendee.getScreenName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String screenName
| long currentTime = System.currentMillis(); | ||
| long twoMins = 120000; | ||
| if(currentTime - timePolled > twoMins){ | ||
| datastore.deleteAttendee(screenName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteAttendee take screenName and sessionId
ckdesir
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending
| datastore.deleteAttendee(screenName); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some weird formatting
| List<Attendee> attendees = datastore.getAttendeesInSession(sessionId); | ||
| for(int i = 0; i <= attendees.length(); i++){ | ||
| AttendeeInterface currentAttendee = attendees.get(i); | ||
| AttendeeeInterface screenName = currentAttendee.getScreenName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the type of screenName is a string
| for(int i = 0; i <= attendees.length(); i++){ | ||
| AttendeeInterface currentAttendee = attendees.get(i); | ||
| AttendeeeInterface screenName = currentAttendee.getScreenName(); | ||
| long timePolled = currentAttendee.getTimePolled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specify the units here, add Ms (I think they are in milliseconds?) to the ends of these long times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I think you want currentAttendee.getTimePolled().getTime();
ckdesir
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anith
| new LocalServiceTestHelper(new LocalDatastoreServiceTestConfig()); | ||
| BackgroundTaskManagerInterface backgroundManager = new BackgroundTaskManager(); | ||
| private final DatastoreClientInterface datastore = new DatastoreClient(); | ||
| String sessionId = "12345"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create test data in each individual test
| datastore.insertOrUpdateAttendee(attendee); | ||
| datastore.insertOrUpdateAttendee(attendee2); | ||
| datastore.insertOrUpdateAttendee(attendee3); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some formatting issues here as well
| backgroundManager.deleteInactiveAttendees(datastore,sessionId); | ||
| Assert.assertEquals(datastore.getAttendeesInSession, {"Taniece", "Chris"}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also i think you reversed this, you expect "Taniece" and "Chris" but you actually get datastore.getAttendeesInSession(parameters)
ckdesir
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pending
| BackgroundTaskManagerInterface backgroundManager = new BackgroundTaskManager(); | ||
| private final DatastoreClientInterface datastore = new DatastoreClient(); | ||
| String sessionId = "12345"; | ||
| long date = 239201; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
date should be of type Date()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chukwuemekagooglecorp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending
Some of this code doesn't compile and has syntax errors.
Run "mvn test" in the test directory to expose the errors and then fix them before putting out a PR.
| throw new RuntimeException("Unimplemented"); | ||
| public void deleteInactiveAttendees(DatastoreClientInterface datastore, String sessionId) { | ||
| List<Attendee> attendees = datastore.getAttendeesInSession(sessionId); | ||
| for(int i = 0; i <= attendees.length(); i++){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this produces an IndexOutOfBoundsException. I think you want "i < attendees.size()"
| for(int i = 0; i <= attendees.length(); i++){ | ||
| AttendeeInterface currentAttendee = attendees.get(i); | ||
| AttendeeeInterface screenName = currentAttendee.getScreenName(); | ||
| long timePolled = currentAttendee.getTimePolled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I think you want currentAttendee.getTimePolled().getTime();
| AttendeeeInterface screenName = currentAttendee.getScreenName(); | ||
| long timePolled = currentAttendee.getTimePolled(); | ||
| long currentTime = System.currentMillis(); | ||
| long twoMins = 120000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a constant at the top of the class:
// If an attendee last polled the server longer this amount of time ago, we assume the attendee
// left and we delete them from the database. < --- use appropriate javadoc format for this comment
private static final LAST_TIME_POLLED_DEADLINE_MS = 2 * 60 * 1000; // 2 minutes
No description provided.