Implementation for removeAttendee with the beginnings of a test.#50
Implementation for removeAttendee with the beginnings of a test.#50jcul22 wants to merge 1 commit intogoogleinterns:masterfrom
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.
getAttendeesInSession returns List
There was a problem hiding this comment.
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(); |
| long currentTime = System.currentMillis(); | ||
| long twoMins = 120000; | ||
| if(currentTime - timePolled > twoMins){ | ||
| datastore.deleteAttendee(screenName); |
There was a problem hiding this comment.
deleteAttendee take screenName and sessionId
| datastore.deleteAttendee(screenName); | ||
| } | ||
|
|
||
| } |
| 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.
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.
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.
Also I think you want currentAttendee.getTimePolled().getTime();
| new LocalServiceTestHelper(new LocalDatastoreServiceTestConfig()); | ||
| BackgroundTaskManagerInterface backgroundManager = new BackgroundTaskManager(); | ||
| private final DatastoreClientInterface datastore = new DatastoreClient(); | ||
| String sessionId = "12345"; |
There was a problem hiding this comment.
Create test data in each individual test
| datastore.insertOrUpdateAttendee(attendee); | ||
| datastore.insertOrUpdateAttendee(attendee2); | ||
| datastore.insertOrUpdateAttendee(attendee3); | ||
|
|
There was a problem hiding this comment.
some formatting issues here as well
| backgroundManager.deleteInactiveAttendees(datastore,sessionId); | ||
| Assert.assertEquals(datastore.getAttendeesInSession, {"Taniece", "Chris"}); |
There was a problem hiding this comment.
also i think you reversed this, you expect "Taniece" and "Chris" but you actually get datastore.getAttendeesInSession(parameters)
| BackgroundTaskManagerInterface backgroundManager = new BackgroundTaskManager(); | ||
| private final DatastoreClientInterface datastore = new DatastoreClient(); | ||
| String sessionId = "12345"; | ||
| long date = 239201; |
There was a problem hiding this comment.
date should be of type Date()
There was a problem hiding this comment.
chukwuemekagooglecorp
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.