search

kgashok / elmBox

Using Elm to access Dropbox
2 stars 0 forks source link

Make use of Impossible States approach #52

Open kgashok opened 7 years ago

kgashok commented 7 years ago

http://fsharpforfunandprofit.com/posts/designing-with-types-making-illegal-states-unrepresentable/#series-toc

to fix the smothering of the Dropbox file with little or no contents.

Also see Feldman's video - https://www.youtube.com/watch?v=IcgmSRJHu_8

kgashok commented 7 years ago

And: if the app is about displaying a list if lines, I would expect some List in the model?

kgashok [5:39 PM] status is wrongly named -> it is actually post and linked to the textbox field where the user enters his updates.

[5:40]
appendContents must actually be a list, but write now it is a concatenation of one of more posts. It is used if the dropbox file has not been downloaded at all... contents is the actual string that contains the contents of the dropbox file when it has been already downloaded (edited)

wintvelt [5:41 PM] Oh I see, the user types something, the text is stored in the status field. (in the UpdateStatus branch of your update tree)

kgashok [5:42 PM] yes

[5:45]
When the user clicks the "Upload" button, for now,

If a file had already been downloaded (downloadSuccess boolean is true), then I just simply upload the contents to the Dropbox file. That is because contents already has been updated which each status update.

Like I said, this "design" has evolved incrementally, so it is kind of messy. (edited)

[5:46]
How and where would I use your recommended RemoteData type?

wintvelt [5:46 PM] :thinking_face:

kgashok [5:49 PM] I did not how to chain two Tasks( first Download and then Upload)...so I used downloadFirst as a flag

wintvelt [5:49 PM] I see.

kgashok [5:51 PM] How and where would I use your recommended RemoteData type?

wintvelt [6:05 PM] Maybe you do not need RemoteData. Maybe something like this is enough:

  { dropboxContent : Maybe String -- only for data which is for sure on dropbox
  , appendData : String -- only for submitted data which is not yet on dropbox
  , post : String -- the post the user is currently editing
  ...
  }

[6:05]
So you would have:

kgashok [6:09 PM] BTW, I am also thinking about changing appendData into a List post where post is { timestamp: Time, message: String} - probably after factoring in your suggestions (edited)

wintvelt [6:10 PM] yeah, you could do that.

kgashok [6:12 PM] Is there an alternative approach I can adopt for doing the "Download->Upload" sequence? It is only when I started doing that, my design / code started getting unwieldy...

wintvelt [6:13 PM] Thinking about that: Maybe you need a real status for the app: like type DropboxStatus = Idle | DownloadingData | UploadingData

[6:15]
Then when the user clicks UPLOAD, you check the status: if it is already downloading, you do nothing or give an error like "please wait"

[6:15]
If the contents are Nothing, you need to do downloading first, and set the status to Downloading (edited)

[6:16]
If there are contents, you do a merge and upload, and change the status to Uploading. (edited)

[6:17]
When the download task finishes, you check if there are any appendData that need uploading, and if so, you merge the data and do an upload, and also set status to Uploading. (edited)

[6:18]
When the upload task finishes, you update the contents field, empty the appendData field, and change the status back to Idle.

[6:20]
One benefit of this is, that you can use the status in your view, to disable stuff: so that user cannot click UPLOAD when your app is still waiting for dropbox to come back with a response.

kgashok [6:21 PM]

disable stuff As it stands, my app is unresponsive or in some kind of blocking state (not by design :grin:) while the download happens. I cannot enter anything into the textbox at least. (edited)

[6:23]
But I will use your suggestions and refactor. How and where do I weave the DropboxStatus into my code? In the Update section?

wintvelt [6:23 PM] mm, I'm not too familiar with Http in Elm, so I can't help there I am afraid.

[6:23]
yeah, you include a dropboxStatus in your model.

[6:24]
And then in your update branches, you check the value of the field.

kgashok [6:27 PM] Currently, I have not been able to debug the situation where the app smothers all the previous existing Dropbox content with just some of the latest status updates. That's what prompted me to explore "impossible states" to disallow this from happening... Will your suggested model changes eliminate this situation? or is that something I still have to ensure that through my logic? (edited)

wintvelt [6:28 PM] One remark: the way I described above will ALWAYS try an upload of appendData after a download. If you do not want that (currently, your Refresh does only download, no upload), then you need to have two types of download tasks/ return Msg

[6:29]
My model changes will at least prevent firing multiple requests to dropbox. The response needs to be back before the next request can be sent.

[6:30]
One thing I did notice, is that your HTTP type is always POST, also when you download. This does seem strange..

kgashok [6:30 PM] I think that's how the dropbox API wants it, I think - I shall double check.

[6:31]
As it stands, the HTTP requests are "chained" one after the other and not simultaneous...A Download happens, and then a Upload is initiated.

[6:33]
Also, if the Download fails, the Upload will not be triggered. Although I haven't really tested whether that is the behavior.

[6:34]
Thanks a ton, @wintvelt - for your patience and very useful code review.

[6:36]
One quick question: You mentioned

  • a String for status: probably better to define a Union Type for this. I could not find if and where the status is updated in your code.

Should I do this as well?

wintvelt [6:37 PM] No need, your status really means post, so String is fine. Our newly defined dropboxStatus is now doing this..

kgashok [6:38 PM] And in ref: to your article, is my app design more suited to be in the 3rd option or the 4th option? (edited)

wintvelt [6:41 PM] The article on dropdowns is a bit different, but I think we just made it option 3 (Union Typed dropboxStatus), which makes sense. Going to option 4 means something like including all posts in the message, and that would really be taking it too far.

kgashok [6:43 PM] Noted, thanks again for your time and valuable inputs!

wintvelt [6:49 PM] You're welcome!

kgashok commented 7 years ago

So thinking about the flow, the types would look something like this:

  { dropboxContent : Maybe String -- data on dropbox
  , appendData : String -- data to be submitted but not yet on dropbox
  , post : String -- the post being edited
  , dropboxStatus : DropboxStatus
  ...
  }

type DropboxStatus = Idle | DownloadingData | UploadingData

type Msg = 
  RefreshClicked -- user wants to download
  | UploadClicked -- user wants to push to dropbox
  | Typed String -- user has typed stuff
  | GotDropboxData String -- successful download
  | UploadSuccess String -- successful upload
  | SomeError String

(edited)

wintvelt [10:43 PM] And then in your update something like this..

  (RefreshClicked, Idle) ->
    { model | dropboxStatus = DownloadingData }
    -- and send out download command through port

  (UploadClicked, Idle) ->
    case (model.dropboxContent, model.appendData) of
      (Nothing, _ ) ->
        { model | dropboxStatus = DownloadingData }
        -- and send out download command via port

      (Just content, "") ->
        -- no change
        (model, Cmd.none)

      (Just content, appendContent) ->
        -- change model.dropboxStatus to UploadingData
        -- then send out upload command, with merged appendContent and Content

  (Typed newContent, _) ->
    { model | post = newContent }

  (GotDropboxData newData, _) ->
    { model | dropboxContent = Just newData, dropboxStatus = Idle }
    |> update UploadClicked
    -- after download, simulate upload click

  (UploadSuccess uploadData, _) ->
    { model | dropboxContent = Just (model.dropboxContent ++ uploadData), appendContent = "", dropboxStatus = Idle }

...

(edited)

kgashok [11:11 PM] @wintvelt

    { model | dropboxContent = Just newData, dropboxStatus = Idle }
    |> update UploadClicked
    -- after download, simulate upload click

How to simulate the Upload click? Call update Upload model? (edited)

kgashok [11:48 PM] ..and what do you think about using this RemoteData type? http://package.elm-lang.org/packages/krisajenkins/elm-exts/25.1.0/Exts-RemoteData (edited)

[11:52]
http://blog.jenkster.com/2016/06/how-elm-slays-a-ui-antipattern.html and http://package.elm-lang.org/packages/krisajenkins/remotedata/latest

kgashok commented 7 years ago