TechnionYP5777 / Smartcity-Smarthouse

Smartcity-Smarthouse
11 stars 5 forks source link

Writing down verbal agreements #103

Closed EliaTraore closed 7 years ago

EliaTraore commented 7 years ago

Currently, I have 2 assumptions about the code -

Where should such things be written? Are they even correct?

(the issue will be closed when assumptions are corrected/approved and documented in some form)

inbalzukerman commented 7 years ago

It is all documented already and can be found in ListenableList and DatabaseHandler I will improve documentation asap 👍

SharonKL commented 7 years ago

@EliaTraore the DBHandler class has a getLastEntry method

EliaTraore commented 7 years ago
EliaTraore commented 7 years ago

I know that, but the AppTeam wants the abilitiy to get more then one entry (not in listeners, just as a general functionality)

inbalzukerman commented 7 years ago

The entire message is saved in the DB ( UpdateMessage to be exact ), this is what @RonGatenio asked in the last meeting.. This way, as I understand it, the application can parse the JSon msg back to the Message representation, and retrieve all the necessary data. If we'll change the data we're saving to be "less redundant", we'll have to parse the JSon msg, remove some data and then create another JSon msg to be saved in the DB. This new representation will be parsed by the application then...

EliaTraore commented 7 years ago

Actually, I think the idea was to keep only the data. The whole purpose of initializing objects for the app devs is to simplify the creation of sensor-representing objects, and allow easy access to the information. Whats the point Of that if we force them to work with UpdateMessage? They will still have to initialize the fields and cast them by themselves, which is tedious and not really needed. I don't think that parsing and getting the data itself in the ListenableList is such a hard job, and on the other hand I does come with a great benefit at the apps side.

inbalzukerman commented 7 years ago

I don't mind working this way 😅 It just not the way I understood it on first. Anyway I don't think the right place to do this is in ListenableList, it is supposed to be a generic class. It should be implemented in DatabaseHandler in my opinion.

SharonKL commented 7 years ago

The DBHandler receives a json message, meaning it will have to covnert it back to UpdateMessage to access the data. Considering the handlerUpdateMessage(...) method in SensorsHandler already receives an UpdateMessage, I think it should be implemented there.

inbalzukerman commented 7 years ago

Even better :smile:

EliaTraore commented 7 years ago

Awesome, I think we're almost done then. There's the small issue of where all of that should be written? This issue can't count as official documentation, or can it? I know its internal so it seems less important to keep track of, but this could lead to integration bugs in no time :sweat_smile: As for the ListenableList, @inbalzukerman , can you add getkentries(int k) method? it will surly clear any possible confusions

inbalzukerman commented 7 years ago

Sure, no problem 👍

inbalzukerman commented 7 years ago

Can this issue be closed?

EliaTraore commented 7 years ago

Yeah, any integration bugs are on Kunini anyway :tada:

SharonKL commented 7 years ago

hmm...