Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Reachability to Fetcher #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

finder39
Copy link
Contributor

fixes #7

@Dimillian
Copy link
Owner

I really need to think about that.

@finder39
Copy link
Contributor Author

Ya, I was looking around and it seemed to still be the standard way to check if there is a valid internet connection. A lot of things still are only built in Objective-C.

If you look in my Resumod project I still have to import to use MD5 variables like CC_MD5_DIGEST_LENGTH.

https://github.com/finder39/resumod/blob/master/Resumod/Resumod-Bridging-Header.h
https://github.com/finder39/resumod/blob/master/Resumod/Constants.swift#L46

@Dimillian
Copy link
Owner

Well there is a lot of things with Reachbility that I don't like.

  • It's wrote in Objective-c & C (but that part is ok)
  • It's only mostly reliable, in some edge cases it won't work. I remember that sometime it report no connection but there is one or vice-versa
  • What about very slow edge/gprs connection (like in the subway), Reachbility will report that there is an active connection but you won't get any content because I'll timeout or failed. And we won't display the cache.

I think that maybe we could enhance the cache, and say, it's invalid after 1 hour, so we display it first if it's dated less than an hour, and don't display it if it's dated more than an hour. And if we're really offline, we could display it all the time, even if it's dated from more than hour. I think it's a better compromise.

What do you think?

@finder39
Copy link
Contributor Author

I do like the idea of a dated cache, that makes a lot of sense to me. A couple thoughts:

  1. That could still lead to a double closure call, which seems like a bad practice.
  2. This would show the user the network activity indicator even after the cached version is given to them. This could confuse them as they would see content and not know why that is still showing.
  3. How would we tell if we are really offline? We need some alternative to Reachability. I totally understand the unreliability part of it, that is an obvious concern.

I don't think we have the solution just yet. hmmm...

@Dimillian
Copy link
Owner

  1. Why do you think double closure call is a bad practice. I use a system like that in a pro closed source app, it's quite powerful, you always have content and it's fast. Of course, it's not like HN where the content change so often. It's more close to a local file system than anything else. But I think it works well in our case here too.
  2. Really, we maybe could do some sort of UI here, and if you have a fast connection the cached version will vanish quickly. I don't know we need to think about that. SwiftHN is all about get something fast, maybe we need to show that it's cached content. What about gray cells ? Different background color etc... Could really work well!
  3. Well the simple way, like rateability do (look at AFNetworking code) is to ping a website, then you can quickly get the NSURLSession/Connection NSError that tell you you are offline. But again, slow Edge/GPRS is offline too for me.

We need to think about it.

@finder39
Copy link
Contributor Author

  1. I guess just the only time I have ever seen that happen was in bugs. There aren't any explicit rules against it, I have just never seen it before and it seems strange to me. Bad practice was the wrong wording, it just seems strange to me.
    • 2a. I still think that there is something wrong on a UX level of showing stale data to a user. When I think Twitter, I think of things that are displayed in a chronological order. With that, instead of replacing data it is adding to it, so the cache seems appropriate even when online. If we we are showing the newest posts it would make sense. This is the issue of showing stale data, then removing and replacing it. I understand not wanting to show the user an empty app, but I feel like an activity indicator is enough for the user to understand that data is loading.
    • 2b. I do agree that if we are showing cached data we should make it clear to the user in some way. A different color cell doesn't seem intuitive enough, but a message (maybe a section header) that states all information below this message is cached or something is good. this could work many ways.
  2. This is a great point! That could work really well, I like it. I'll have to look more into the code of AFNetworking 2. I used to use the framework all the time.

@Dimillian
Copy link
Owner

Well you don't get many solutions when it come to have a local data and an remote data. You need to first display your local data and then display the remote one. True, we could gracefully merge both and rearrange cells maybe, but it would be too much work I guess. Twitter is nice because it's chronological, you could just add new items at the top and boom. In our case items can me moved and updated randomly :/

I think we should keep the actual system with an UI that explicitly say "cached data", in home pages and comments, etc...
A header may be nice, I'll mock something when I'll have time on Sketch. I think this is the ideal solution, so we don't mess with reachability or remove speed from the app.

What do you think?

@finder39
Copy link
Contributor Author

I think that could work pretty well and is a good plan to mock up.

As for the rearranging, I don't think it would be too bad to program. I think it could be overwhelming to a user though to see 30 items appear and then shift around all crazy like haha. Although I do think it would be a really cool experiment to see a flow chart of top items on HN over time.

@Dimillian
Copy link
Owner

Well I'll open issues in SwiftHN for the UI and the experiment then, if you want to try why not :)
I love those discussions, we did some great progress, too bad this pull request might me kind of useless now :/

@finder39
Copy link
Contributor Author

Sounds good to me! And I may try that out, if nothing else than for fun :)

Ya these kinds of discussions are always great! Makes you think about all different issues and solutions. No problem if this pull request becomes useless. It led to a good discussion and had us thinking outside the box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have cache only load when offline
2 participants