Make it easier to read and maintain

[IzzySoft]

This is the announced "minor rewrite". I've again kept it in separate commits, so it's easier to review (simply walk top-down). Also managed to find the summary, which is kept in protobuf data at the end of the page. Luckily, with a little trick one can convert protobuf to JSON, which I did here. The protobuf structure rarely changes; I'm using that to fetch app permissions: set that up about 5 or 6 years ago, still works.

Speaking of which: permission list I might add with a separate PR later.

PS: Do you insist on tabs – or would you be OK having them replaced by spaces? I won't argue, whatever your answer is: it's your project, so your guidelines matter. Personally and in my projects, I use 2 spaces instead of 1 tab; especially with many nesting levels and long lines, that saves a lot of horizontal scrolling :wink: (don't worry to say you want to stick with tabs; I'd then simply convert back-and-forth as I did up to now)

[IzzySoft]

There's one minor issue with this specific protobuf (for summary), though: occasionally it seems to be missing (but is there again on a subsequent call for the same package). An idea to work around this is to retry-until-success (with a limit on retries, of course). Raw approach:

• moving the file_get_contents to a separate method, say getAppPage($packageName)
• replace the original code with a call to that
• placing the summary stuff into a loop, like while empty(summary) … if counter > 3 break, including a call to $this->getAppPage whenever summary was not found

Besides, this failure is one of the reasons why I didn't rewrite the code to pick all details from protobuf…

[BaseMax]

Dear Izzy, Thanks for your help. :+1:

[BaseMax]

I agree with you. Space is better from the tab and I do not mind if you change this. Good idea. @IzzySoft

[BaseMax]

Speaking of which: permission list I might add with a separate PR later.

Thank you in advance.

It may be better to change the == to === operator in some places.

Since I not have time to review all, Please if you have changed the structure of the applications. Apply this change to README as well. (Application Structure Section)

[IzzySoft]

It may be better to change the === to == operator in some places.

There's only one place where it's used: on deciding whether we got valid data, based upon whether the application name was set. Well, I agree: that could even be replaced by empty() – it doesn't matter whether it's explicitly null or an empty string. Changed it to == as requested.

if you have changed the structure of the applications. Apply this change to README

Oh – of course, apologies. Should have done so right from the start. Done now (with the commits I'll push now).

Once this is merged, I'd add a separate method for retrieving permissions. That's using my protobuf-to-JSON approach again, and seems to have the same issue as mentioned above: sometimes Google leaves out parts. I've no idea for the reason: sometimes the missing part is present on the next call, but for some apps (yes, bound to specific apps!) some permissions are always omitted (but shown when you look it up with the browser). Luckily, both occasions are rare.

Repeating my question from above: should I implement "iteration" (limited to 3 calls) on missing summary? For the permission part, iteration can be left to the user as it will be a separate method anyway.

[IzzySoft]

Funny. I've pushed 3 more commits, but they are not showing up here? – Argh, crossed with you merging it already… OK, let's see if I can push the branch for another PR…

[BaseMax]

Thanks again, I'm sorry I said the opposite.(=== is better because it's strict) I edit message right now.

[BaseMax]

About the things we talked about:

• protobuf-to-JSON Good idea, we can provide this as a extra feature. (Supporting both protobuf and JSON)

• iteration Yes, You can implenment a function for this, So users can get repeat iteration in loops. e.g: auto paging

[IzzySoft]

I'm sorry I said the opposite.(=== is better because it's strict)

I was already wondering (mostly because you sounded like there were multiple occurences and I only found one). In the place I've changed now (with the other PR) I'd leave it at ==, though, for the reasons given. Will take a look where === should be applied instead (separate PR then).

protobuf-to-JSON

I use that only when no alternatives are present, as the presence of the protobuf seems a bit "shaky" – and further it's more-or-less guess-work. You can dump the protobuf to cross-check, here's what I think I've identified in there (checking with com.twidere.twiderex and net.activitywatch.android):

AppName:    [0][0][0]
Desc:       [0][10][0][1]
Summary:    [0][10][1][1]
Screenshots: [0][12][0][$i] with dimensions in [2] (x,y=0,1) and URL in [3][2]
Icon:       [0][12][1]      with dimensions in [2] (x,y=0,1) and URL in [3][2]
featureGraphic: [0][12][2]  with dimensions in [2] (x,y=0,1) and URL in [3][2]

0-12-4: rating stuff
0-12-4-3-1: "Users Interact, Shares Location" (Twidere; ActivityWatch: 0-12-4-3 empty) => concerns?
0-12-5-1: developer
0-12-5-2: [0..n] of links? (Twidere, ActivityWatch: [0] = email)
0-12-5-3-5: [0..n] of urls? (ActivityWatch: 2=homepage)
0-12-5-4: [0..n] of addresses? (Twidere: [0])
0-12-5-5-4-2: developer market URL (relative)
0-12-7-2: privacy policy URL (ActivityWatch: 0-12-7 empty)
0-12-9: [0..3] different ways of telling the download count? (ActivityWatch: 1,000+ | 1000 | 1458 | 1k+ — Twidere: 100+ | 100 | 111 | 100+)
0-12-13-... category stuff
0-12-25: main category?
0-12-35-0: again developer name?
0-12-36: last update? (Twidere: completely missing)

we can provide this as a extra feature

For the stuff not present in HTML, yes. Maybe adding an optional parameter to parseApplication() – something like $proto=false (should stand for "include additional data via ProtoBuf"; I'm open to naming suggestions for the parameter as well as for the default to set – but would suggest false for the latter, given the "shakyness" of proto's presence, or at least document the corresponding keys might be empty).

Currently I'm puzzled why I always get a 404 for perms, using the same code that works in my class; so it might take a little longer before I can supply the new method.

iteration

OK, so I'll do that for summary (which we know should be present), looping for max 3 times with a delay of .3s in between. Comes with a separate PR then.

[IzzySoft]

Oh, speaking of 404: your link for Asrez Team from ReadMe gives a 403…

[IzzySoft]

I've checked all == if they should be turned to === – IMHO no need to. There are currently 3 of them:

• name (already discussed: type doesn't matter; if it's empty it's wrong)
• if(strtolower($values["category"]) == strtolower($game)) {: no need, as strtolower is applied on both sides, type must be identical
• if($link == "" || $link == null) { similar to "name" above: type doesn't matter. No link is no link.

So that task is off the list, if you agree.