RoboTutorLLC / RTFace_Login

1 stars 1 forks source link

2023 02 05 madelein new logging state changes #53

Closed madeleinvillegas closed 1 year ago

madeleinvillegas commented 1 year ago

Address the concerns from the previous pull request #52 :

The previous terminator is deleted before adding a new one. This can be seen in the appendToJSON method

madeleinvillegas commented 1 year ago

CreatePair was code already written that I just reused.

I believe the runtime of copying entire log is 2n. Since we do it n times, I see your concern of a possible n^2 runtime. I believe this should not be a problem for FaceLogin since its usage is just short and specific. I hope no one uses the app for an hour pressing various buttons and playing different audios, that can possibly slow down the program (but it should not be significant to the user) since the app is copying a long log.

JackMostow commented 1 year ago

The numerous calls to CreatePair just seemed like needlessly extra code for you to write and others to read. Replicated code often indicates a problem for maintaining code.

The O(N^2) time is more worrisome, especially if FaceLogin stays up for a long time. Does RoboTutor have the same issue? Did you reuse its logger?

Thanks. - Jack

madeleinvillegas commented 1 year ago

The last logger only uses a .txt file so the log just keeps getting appended to the existing file. Since it was decided to make the log file a JSON file, we need to worry about maintaining legal JSON. RoboTutor should not have that problem since we are not logging state changes in JSON there.

madeleinvillegas commented 1 year ago

If FaceLogin stays up a long time while idle, it should not be a problem because there is no state changes to log. It might be a problem if a session is not ended after a long while and students just keep using the app

JackMostow commented 1 year ago

@Madelein, @rohankulkz -

  1. Good point about the problem with logging json instead of plain text. Oh well, let's hope it's not a problem in practice -- and that if it is, we notice it.

  2. RoboTutor has long logs, especially verbose logs, but they're limited to a single session. Hopefully that suffices to keep log copying fast enough not to pose problems.

  3. To preserve student profiles across version updates (see #51), we can keep the same version number, but it's 1.8.1.3 on some tablets (e.g. 6111000955) and 1.8.2.2 on others (e.g. 6111000885). Other than using different apks on those tablets (:<), I don't see an easy way to preserve student profiles across version updates. We may have to just bite the bullet and sacrifice profiles on some of the tablets, e.g. the ones using 1.8.1.3. Otherwise we'd presumably need a way to dump and load sqlite databases.