Closed goranche closed 7 years ago
I'd suggest that in devClientsViewController
we add "+" in the navigation bar.
When "+" is tapped we should do a modal presentation of a text field so that the user can enter the client's name. What do you think?
Additionally, I have some issues understanding the API on the server side. Are there more details available about how to understand the server side API? What should we look at?
the comment from the PR might help...
and yes, it should probably be added to the documentation... :hint: :hint*
regarding the + in the navigation bar... sure, that's one way of doing it 👍
I suggest we simply add another method in Client extension for new Client creation. We should probably also add toJSON
method into Client considering the fact that server expects JSON. @goranche any special reason why Client is class instead of a struct?
Of course also NetworkAPI
should be updated with performPOSTRequest
in a similar fashion as performGetRequest
. Maybe performPOSTRequest
and performGetRequest
should be used for appropriate URLRequest
construction which would call dataRequest(with urlRequest: URLRequest)
?
any special reason why Client is class instead of a struct?
🤔 I'm not completely sure, but I think not, as usually I'd use a struct for this, or at least add a comment why it's not a struct... who knows what I was thinking at the time 😅
regarding the model, I'd rather see it done the "standard" way... add a save()
method (with completion and what else not) that checks if the model object has been changed (creation means changed, of course) and stores it to the server...
it's a relatively simple change, add a changed
or isSaved
property (names need work), make the id
optional, and when constructing a new client, only set it's name and the changed value, and that's it... the save()
method can then look at the changed value and decide if it needs to store the data to the server, or not...
if storing is needed, the code can look at id
, and if it's nil, a new client is created, if it isn't nil, it was obviously retrieved from the server, so an update is needed...
ah, yes, I remember why a class... 😁 we want model objects to be passed by reference, so if we keep a list of them somewhere, and the name for a client is changed elsewhere, we actually want all the references to change... it will make change propagation much easier in this case (I think, as least)
isDirty
😅 that's the name I was looking for, not changed/isSaved, ...
't was too early... 😇
The API already exists on the server side