Office Hours — Today, November 8

← Tuesday, November 6

Mark M.
has entered the room
Nov 8
7:25 PM
Mark M.
turned on guest access
7:40 PM
Aaron
has entered the room
Mark M.
hello, Aaron!
how can I help you today?
Aaron
hey Mark, I have 3 questions today, and they are all follow-ups to our last conversation
here they are:
View paste (5 more lines) 
1. I implemented/fixed most everything we talked about last time, and learned enough Kotlin to read the event wrapper code and rewrite it in Java. However I am not totally sure I am using it correctly. First, do I understand correctly that how it works, is, getContentIfNotHandled() actually writes INTO the Event object currently being emitted by the LiveData? Such that hasBeenHandled becomes true until a new Event is emitted? I didn't even realize this was possible but as far as I can tell, this is how it works (?) Second, I am using a different event stream (i.e., a different MutableLiveData) per fragment, each of which is observed by my MainActivity, each of which wraps a different data structure to hold any needed data (i.e., variables that were formerly passed as method arguments when I was using listeners). Is this the right way to use the event wrapper? 

Relevant files:
https://github.com/ajh3/NoSurfForReddit/blob/master/app/src/main/java/com/aaronhalbert/nosurfforreddit/Event.java
https://github.com/ajh3/NoSurfForReddit/blob/8f6969df7bc4a073f07332a21de67b96c01a15f3/app/src/main/java/com/aaronhalbert/nosurfforreddit/activities/MainActivity.java#L211


2. We discussed how the
...
Mark M.
OK, let's take #3 first
Aaron
OK
Mark M.
"Actually, I am not sure if you meant one for all fragments, or one per fragment" -- the activity-scoped ViewModel is accessible by the activity and all of its fragment. A fragment-scoped ViewModel is accessible only by the one fragment
"I have seen the idea of multiple ViewModels suggested before but I am not sure I understand what the benefit is" -- in your app, there might not be a benefit, because your app is relatively small
sometimes, even a small app could benefit, though
for example, perhaps you are creating a wizard-style UI with several pages, each implemented as a fragment
7:45 PM
Mark M.
each fragment might have a "local" viewmodel for dealing with things unique to that page
each fragment would also have access to an activity-level viewmodel, for things that affect more than one page
Aaron
I see, so it would be for enforcing encapsulation of each fragment
Mark M.
yes
again, it's not always necessary, but it's a useful feature of the viewmodel system
Aaron
OK, yes, that makes sense but is probably not helpful in my case, but good to know
Mark M.
in terms of #2, your repository is a trifle big
a lot of that is comments, though, which is a good thing
7:50 PM
Mark M.
you might consider isolating things along storage lines
for example, you have a bunch of code for writing things into preferences, some of which appears to be duplicated
you might create a dedicated class for managing that sort of data, and have your repository work with an instance of that class
since you're going in a Dagger-y direction, you could inject the instance into the repository if you wanted
Aaron
that makes sense, would I want to call that PreferencesRepository or something like that, or would that be a bizarre convention?
Mark M.
first, focus on what you are storing, not where you are storing it
so, you might have a TokenStore interface with a PreferenceTokenStore implementation
since it seems like you're using SharedPreferences mostly for your OAuth tokens, though I may be missing something
Aaron
the tokens, plus night mode and a couple other boolean preferences
Mark M.
hmmm... OK, so there goes TokenStore...
7:55 PM
Mark M.
well, not necessarily -- are the "night mode and a couple other boolean preferences" being manipulated here in the repository?
Aaron
no
Mark M.
ah, OK, then TokenStore is still a reasonable choice
just because you happen to access some preferences through a TokenStore API does not mean that TokenStore is the *only* way to access data that you store in preferences
some of your repository code might get moved into your DTOs, such as the getCommentAuthor()-style methods
but on the whole, I wouldn't lose sleep over the size of this repository, at least right now
and in terms of #1, that looks better
8:00 PM
Mark M.
I've been working with Event in Kotlin, and I am using some extension functions that hide a bit of this
nearby where you found the Kotlin code for the Event class, you'll also find an EventObserver, I think, which basically safely "unwraps" the event and calls your supplied callback with the actual data (e.g., a NetworkErrors)
Aaron
what I said can't be right about writing into the Event being emitted, that's clearly not the case, I am just not quite putting together how it does work
Mark M.
oh, sorry, I was focused on the "Is this the right way to use the event wrapper?" part
hold on
Aaron
no worries, I am just thinking about that part, I am interested in what you are saying here
Mark M.
actually, your description seems pretty much on the money
an Event isn't magic, and neither is LiveData, when you get right down to it
while it is common to use LiveData with immutable objects, there is no requirement for that
and I don't think you're using AutoValue here anyway, so most of your objects are very mutable
all Event is doing is tracking whether or not we have already processed this particular bit of data or not
8:05 PM
Mark M.
on a configuration change, when we register an observer again, we will be given whatever the last object was that the LiveData emitted (if there was such an object)
for things that we want to handle only once, that becomes a problem if we do not use something like this Event wrapper
you could bake the "have I handled this thing already?" logic into the stuff that you're emitting, but a wrapper is usually cleaner for a cross-cutting concern like this
Aaron
OK yes - config changes - that is the part that I am not quite getting
so an Event is dispatched, and the observer is called
Mark M.
yup
Aaron
if we call getContentIfNotHandled() on that Event
it's only writing hasBeenHandled=true into the local copy of the Event
Mark M.
what does "local copy" mean?
8:10 PM
Aaron
um, my poor Java knowledge is showing here, but isn't Java pass-by-value, so the observer is receiving a "copy" of the Event that has been created, in its own scope? or does the Observer actually receive a reference to the original Event? sorry lol, that is probably gibberish...
Mark M.
the "pass-by-value" and "pass-by-reference" notation gets confusing
from your standpoint, think of it as pass-by-reference
your Frobozz creates an Event and call postValue() on the MutableLiveData
the MutableLiveData holds that Event plus passes it to observers
observers receive the Event and do work
that is the same Event object throughout
so, when your observer calls getContentIfNotHandled() on the Event, it is updating the same Event object that the MutableLiveData is still referencing
on a configuration change, the MutableLiveData hands that Event over to your brand-spankin'-new observer, and that's the same Event object that the previous observer observed
but, courtesy of the Event, we know this, and we know not to re-process that data, because the previous observer did that for us
8:15 PM
Aaron
wow, I did not realize that. I thought it was more akin to uh, when you use a local variable inside an anonymous inner class (which an Observer is), so you get a "copy" of the variable from its enclosing scope. I thought that's what happened when an Observer gets an updated LiveData...
I see
OK so I was accidentally right the first time
Mark M.
yay for happy accidents!
Aaron
Nice..
alright, well I learned something, I really need to go back and learn all that stuff much more solidly
OK then, I think that answers everything, let me see if I have any other followups
OK just one thing
Mark M.
no, I don't know how popular foldable phones will be
oh, no, wait -- that probably wasn't your question
:-)
Aaron
lol
it's about https://github.com/ajh3/NoSurfForReddit/blob/8f...
8:20 PM
Aaron
so here, I was previously passing a few variables as arguments back to MainActivity, a couple of strings and a boolean I think
I just made a data structure called LaunchPostLinkEvent that holds them, and that's what the Event wraps
is that the right way to do it, or is there some way I'm missing that could prevent having to create that extra data strucutre class?
Mark M.
your approach seems reasonable
you might consider passing the event along to the launchWebView() method, rather than unwrapping it all
Aaron
I might be able to simplify it some other way so that no "arguments" are required, I'll think about that, but good to know that makes sense
that's a possibility, OK
8:25 PM
Aaron
I thought of one more thing. So my plan is to probably spend another day addressing this stuff and commenting the code further, publish to play store this weekend, and then get to work on RxJava. Right now, the main areas I know I can apply it are 1) zip()ing disparate API results as we discussed, and 2) getting rid of the nested Retrofit callbacks. based on your review of the code so far, do you agree those are the main applications? does anything else stand out as potentially benefitting from its application?
Mark M.
anywhere you are doing work on background threads, think about where RxJava might apply
sometimes, those threads are provided for you (e.g., Retrofit)
similarly, you may wind up switching your Room DAO from returning LiveData to returning Single or Observable (RxJava types)
if you are forking your own threads anywhere, consider whether you should be letting RxJava handle the threading for you
Aaron
oh yes, that reminds me of another thing I forgot (sorry to keep you until the end here) - I wanted to ask you if my ThreadPoolExecutor (replacing AsyncTask) looks right: https://github.com/ajh3/NoSurfForReddit/blob/ma...
if you can check that in the next 2 minutes ;p
OK
Mark M.
usually, you can use methods on Executors to create your executors
and this is definitely something I would look to migrate into RxJava
Aaron
hm, OK, I'll look into it
alright - thanks a lot!!
Mark M.
you're welcome!
8:30 PM
Aaron
take care :)
Aaron
has left the room
Mark M.
you too!

← Tuesday, November 6

 

Office Hours

People in this transcript

  • Aaron
  • Mark Murphy