[05:07:19] Hi niedzielski , you around? :) [05:48:04] josephine_l: o/ [05:48:20] niedzielski, hi! [05:48:30] josephine_l: happy new year :) [05:49:00] niedzielski, happy new year to you too :) [05:49:31] I have been struggling with the repeat calls to the API that Nicolas requested, haha [05:51:40] josephine_l: just with the categories being greater than 10? i wasn't quite sure what you meant by that [05:52:13] niedzielski, you mean you aren't sure what the task involves, or I didn't describe the problem with my code clearly? [05:53:49] josephine_l: i wasn't really sure what the problem was. it sounded like a logic issue. did you try making the client supply the callback instead of keeping static state? [05:54:41] niedzielski, oh, yes, I've been meaning to talk to you about that bit. Uh, what do you mean by the 'client'? :) [05:55:32] josephine_l: for your class, MwVolleyApi. any code outside of that class that interacts with it is a client [05:55:41] And secondly, I'm a bit confused as to why we need some of the nested classes and methods in MwVolleyApi to be static. The mix of static and non static methods seems to make things a lot more difficult, and it doesn't seem like they need to be static, but there just might be something that I'm missing here [05:56:27] niedzielski, does that mean I should define the onResponse() method in the class that calls MwVolleyApi instead of inside MwVolleyApi? [05:56:39] josephine_l: in general, i try to avoid static methods except for utility classes [05:57:46] niedzielski, so basically I can convert all the nested classes and methods in MwVolleyApi to non-static except for the constants? [05:58:24] josephine_l: and i try to prefer static or unnested classes when the concerns can be separted [05:59:36] josephine_l: the classes may be static / non-static depending on their dependencies and responsibilities [06:00:24] josephine_l: yeah, i would recommend passing on a Listener from the client into MwVolleyApi so the client can choose how to act on it [06:01:02] niedzielski, what should I do if I need to call a static method (or a method of a static class) from a non-static method, or if they each need to reference the same variable? [06:02:04] niedzielski, perhaps I can define the Listener in CategorizationFragment.java then? [06:03:17] josephine_l: regarding static / non-static, that's too general a question so i would have to say refactor :) can you point me to the code block you're thinking about? [06:04:47] niedzielski, sure - in the repeat-calls branch, MwVolleyApi.java line 115 [06:05:25] There are 3 commented-out lines there, I was attempting to add a request to the queue in the onResponse() method [06:05:30] josephine_l: i think you want the Listener in ShareActivity since that where you initiate the API request [06:06:32] So the request(); was one attempt, and the next two lines was a separate attempt. Each one caused issues with mixing static and non-static methods [06:06:38] niedzielski, ah, okay, gotcha. [06:07:32] josephine_l: ok i'm checking this out now [06:07:40] niedzielski, thanks! [06:09:13] josephine_l: hm, why don't you just make a new MwVolleyApi object and make the request on it? [06:09:59] niedzielski, can't pass in the context [06:10:17] Because logResponseListener is a static class [06:10:47] But if I put all of that code in ShareActivity that might work... hm. [06:11:37] josephine_l: yeah, if you move the Listener outside of the class you could pass in context there [06:12:13] josephine_l: or i guess you could do the same thing without moving it too [06:12:45] josephine_l: on line 83, just pass in the context in the Listener constructor [06:13:44] niedzielski, so I would make an additional constructor for Listener that has a Context parameter? [06:14:02] Um, LogResponseListener I mean [06:14:31] josephine_l: yeah, just replace the default constructor with one expecting a Context [06:15:00] josephine_l: and add a member variable to store the context reference. you can then use it as needed in the onResponse method [06:15:47] niedzielski, oh, great, thanks! I hadn't thought of that >.< [06:16:27] josephine_l: np :) [06:17:53] niedzielski, are there any major changes I need to make to my repeat calls logic, or do you think I am roughly on the right track? [06:18:14] I've been reading about how multiple requests should use a singleton pattern [06:18:22] But we are only sending max 3 requests so not sure if that is needed [06:19:14] josephine_l: a Volley Request singleton or a RequestQueue singleton or something else? [06:20:13] niedzielski, RequestQueue. Apparently. According to https://developer.android.com/intl/es/training/volley/requestqueue.html [06:20:24] josephine_l: yeah, requestqueue should be a singleton [06:21:03] niedzielski, oh, okay. So I should try and refactor that, too [06:21:05] josephine_l: you can just make a separate class called DefaultRequestQueue, VolleyRequestQueue, or whatever is simplest [06:22:13] josephine_l: the refactoring should be minimal, i think, even for preexisting code unless they're manually pausing or clearing it which would surprise me [06:22:19] niedzielski, okay, I'll try to follow that example [06:22:56] josephine_l: for the repeat pattern, can you remind me the reason we can't use a large radius and sort by distance manually? [06:23:19] niedzielski, because the more cats received the longer the waiting time I think [06:23:32] To be fair I'm not entirely clear on why Nicolas recommended the repeats, just my guess [06:24:00] So for instance if you use a 10,000 radius on New York you would receive... a lot of categories [06:24:31] Whereas if you try 100 first, you wouldn't receive as many, and the ones you do receive would be the most relevant, likely [06:25:52] josephine_l: hm, did we find a way to get the data back sorted or not? [06:26:02] josephine_l: sorted by distance on the backend, i mean [06:26:11] niedzielski, uhhh... I don't think so. [06:26:37] josephine_l: i was just wondering if we could request a really large radius with a result set limit. i think it would save a bit of tricky code on the client side [06:27:18] niedzielski, that sounds good if we could get it pre-sorted, but AFAIK we tried and didn't succeed in finding out how to do that in Week 1 [06:27:24] josephine_l: darn, i think limiting the number of results can only work if they're prioritized by distance [06:27:32] niedzielski, yeah [06:28:45] niedzielski, it's all good, I think I can try the method you suggested and see if it works. Sounds like it will. [06:29:03] I have to start on the caching by Monday I think, heh. [06:29:10] That's a whole other confusing kettle of fish [06:30:47] josephine_l: i think the caching as described will take some thinking :) i recommend keeping all of that radius analysis / caching hit and miss logic in separate classes :) [06:31:47] niedzielski, okay, noted. I was hoping to reuse the existing Category.java class for the caching [06:31:57] That class already stores the 'recent categories' [06:32:23] So I thought if I could add a column to its table with the lat/long object, I could reuse its DB logic and content provider [06:34:01] josephine_l: you can store it there, i just mean to keep to keep geospatial logic for determining whether another request is necessary or not separate [06:34:56] niedzielski, oh, can't do that in ShareActivity? [06:35:38] josephine_l: it sounded like it would be a little involved but maybe i'm mistaking. just implement it however you think is best and i'll comment on the pull request [06:37:13] niedzielski, okay, sure. :) [06:38:57] niedzielski, you mentioned you reproduced some of the pom.xml errors? I need to import another 3rd party library for creating lat/long objects, but I'm not sure what's the best way to do it given that I can't seem to add Maven dependencies [06:39:20] With GSON I just downloaded the lib from the Maven repo, but this lib doesn't have a Maven repo it seems [06:39:39] And Android Studio's import module only seems to work for Gradle or Eclipse builds [06:40:20] josephine_l: android's Location object won't suffice? [06:41:07] niedzielski, I think it doesn't fit Nicolas's requirements [06:41:08] http://softwarerecs.stackexchange.com/questions/27569/java-data-structure-to-store-geographical-objects-and-retrieve-them-by-area [06:42:47] niedzielski, actually now that I look at the Location object again, it should work, yeah? [06:43:11] josephine_l: i think nicolas was wanting more than just location storage [06:43:33] niedzielski, but the Location object has distanceTo() [06:43:58] josephine_l: i think he wants a fast way to partition areas [06:44:06] niedzielski, ah, okay. [06:45:37] niedzielski, so, yeah... that Maven stuff. I guess worst come to worst I could just download the lib source code and put it in my project folder manually...? It's quite small. [06:46:04] josephine_l: i think what he wants is, given a lat / lng with a fixed radius, get the closest previously discovered lat / lng in that radius, if it exists. [06:46:18] josephine_l: and return its categories [06:46:56] niedzielski, yeah, I think so [06:47:47] josephine_l: you could try jitpack: https://jitpack.io/#varunpant/Quadtree [06:48:33] niedzielski, doesn't that still involve modifying pom.xml? [06:49:12] josephine_l: it does, but you also said it didn't have a maven repo. this would solve the first problem :) [06:49:35] niedzielski, oh, right :) [06:50:34] josephine_l: let me try it on my side. it's just some android studio issue [06:50:53] niedzielski, okay, thanks. I really wish this project had been Gradle :( [06:51:16] josephine_l: me too! [06:51:37] josephine_l: i didn't realize how poor the maven experience in android studio was now [06:52:04] niedzielski, haha, I had no idea AS even had Maven integration til I did this project... [07:12:19] niedzielski, hi, you still there? :) [07:12:32] josephine_l: yeah, not having much luck so far [07:13:41] niedzielski, don't worry about it I guess. I can import manually first and we can change later when we figure it out? [07:15:33] josephine_l: yeah, ok :/ [07:15:45] josephine_l: i'll let you know if i find anything on it [07:16:30] Thanks so much niedzielski :) [07:16:49] I submitted the modifications you suggested to the api-calls branch [07:17:21] But I had to open a 2nd branch to do the repeat calls so that isn't the most recent branch anymore [07:17:25] Sorry about the mess [07:21:07] josephine_l: ah checking [07:23:32] josephine_l: so this is the comparison i should use, right? https://github.com/misaochan/apps-android-commons/compare/master...api-calls [07:24:40] niedzielski, yep. I think technically it should be checked against nicolas-raoul/master? But my master branch is even with his now, so that works. [07:25:27] josephine_l: so this doesn't include moving the listener out, right? [07:26:22] josephine_l: er, is this just https://github.com/nicolas-raoul/apps-android-commons/pull/34 ? [07:28:48] niedzielski, uh, yeah, I meant to say that I made the modifications requested there even though I'm working on another branch now, heh. Sorry if I was unclear. [07:33:15] Thanks for the merge niedzielski ! [07:33:25] One more Q if you don't mind (apologies, I know it's late) [07:33:45] josephine_l: sure, i gotta hit the sack after this one though :) [07:34:32] niedzielski, okay. If I put the responseListener in another class, how do I cancel the request there? [07:34:40] I mean, after it's done [07:34:57] josephine_l: cancel the request or listen for a success / error response? [07:35:26] The current code keeps iterating from 100-10000, I thought it was because the request never ends [07:36:02] niedzielski, as in, I have to manually stop it somehow. [07:36:08] josephine_l: if you're spawning requests with new listeners, won't that continue forever? [07:36:29] niedzielski, yeah, exactly. I can call cancelAll() at the end of the loop [07:36:35] niedzielski, but I dunno if that's a good way to do it [07:36:50] josephine_l: i think the problem is that you're not extracting the current request's radius in the for loop [07:37:10] niedzielski, the for loop does end though [07:37:42] niedzielski, I pushed my current code in [07:37:53] niedzielski, so the 'Loop has ended' log does display [07:38:04] niedzielski, it still goes back to 100 anyway [07:38:19] josephine_l: you actually don't need a for loop if you're making a new request each time. old loops will die but you're making infinite requests [07:38:47] niedzielski, hmmm... so instead of a for loop, just check the radius each time? [07:39:08] niedzielski, how do I 'make it stop' without a for loop though? Is cancelAll() correct? [07:39:10] josephine_l: yeah, just pass the radius to the constructor, for example [07:39:53] josephine_l: i don't think you need cancel unless you want to expose a button to the user. either you meet your radius limit or you reach your category minimum? [07:40:26] niedzielski, ah, okay, so no need for manual cancelling at all. Just two 'if' checks [07:40:45] josephine_l: if you really want to cancel, you either clear request queue (dangerous if it's shared), or hold a reference to the request itself and call cancel on it IIRC [07:41:08] niedzielski, okay, thanks so much :) [07:41:57] josephine_l: right, once you make your initial request you have a couple outcomes: Listener.onResponse or ErrorListener.onResponse, right? if it succeeds but there are insufficient categories, you just make a new request if the radius is within limits. otherwise, you do nothing [07:42:21] niedzielski, gotcha, thanks! [07:42:53] josephine_l: np :) have a good one [07:43:01] niedzielski, have a good night :)