Consistency in return values

[IzzySoft]

During my recent implementations and rewrites, I've introduced some "error reporting" to make it easier for the caller to figure if and what might have gone wrong. On error, several methods now return an array like

[success:0, message:"reason"]

But not all of them – for example, most of the search/browse methods supposed to return a simple array of package names don't have this. There would be two options to reach (a sort of) consistency:

• returning [success:0, message:reason] instead (and if so, include success:1 with a "good result) also for those methods that currently do not, moving the "real results" into a "sub-array" (which then, on error, could be empty or just not present at all)
• simply returning an empty array, and have a getLastError() method for obtaining the reason for all search/browse methods while keeping the current behavior for the others.

I'd prefer the first approach (so it's completely consistent) with the "empty real result". This combines the best of two worlds, e.g.

$apps = $google->parseWhatever();
if ( empty($apps['data']) ) { // this could simply mean nothing found matching the criteria
  if ( $apps['success'] ) { log('nothing found'); }
  else { log ('ERROR occured: '.$apps['message']); }
} else { // do something with the data

I'd even go as far as to always include the message key, just leaving its value empty on success. That would make things most consistent.

What's your stance? If you agree, I'd go over the entire class another time and make it consistent:

• success:0 only on errors – not generally on "no results" for a user-specified search (I vaguely remember I accidentally made it such in one case). message then holds the reason (e.g. the HTTP response, or parse error when an expected pattern didn't match, etc)
• success:1 on success. message then is present but usually empty, but might eg hold a hint on why the result set is empty (like "no hits").

If we want to fix it, we want to do that as early as possible – before there are users whose code would otherwise break on some update.

[BaseMax]

Thanks, Yes, I did see some error message/warning.

We have another repository similar to this, We can take action these repos after this.
So no worry to apply minimal apply to structure. It's not much problem.
So can.

[BaseMax]

The difference between this repository and another repository is that the other one only focuses on the database.
https://github.com/BaseMax/GooglePlayDatabaseMirror

So I think it's best to use the final version of the library in the GooglePlayDatabaseMirror repository.
Comment?

[IzzySoft]

I'd prefer to finish it here. Using the latest version from the other project would mean starting all over again. Here I'm almost done now – what I described in this issue are the last remaining steps.

After reaching consistency, let's increase the version number here and declare it "RC". Next thing needed is what your TODO in the Readme says: "Unit test". In parallel, a set of "calls" can be set up for those methods available in both libraries, and the output compared so you can see what must be adjusted so your cron job won't break. Then the library from here can be taken to a branch of the other project (maybe as git submodule, to not have multiple copies to keep in sync), and adjustments be made there.

Funny coincidence: that other project has quite some parallels to what I want to use this library for. There are 14k5 apps in my database currently 😆 With the difference that my apps are from multiple sources (Play, F-Droid, my repo, Aptoide, Xposed), my cron job runs 4 times a day and merges those sources – each time a subset of the database to not hit some wall one day). For 2-3 days now, GooglePlayWebServiceAPI covers the permission part for Play Store. Once that's confirmed (had to fix a minor mapping issue yesterday, seems stable since), other details like ratings, price etc will follow to replace my "old code" grown since 2014.

Well, TL;DR: Do I have your "Go" to do the polishing here as lined out above? Maybe your last sentence was just ambiguous – and you did in fact mean using the final version from here, once completed, and bring it "over there"?

[BaseMax]

Yes, you can.

Thanks for your description.
It's good job: mix some different sources.

Maybe your last sentence was just ambiguous – and you did in fact mean using the final version from here, once completed, and bring it "over there"?

GooglePlayWebServiceAPI class used at GooglePlayDatabaseMirror repository.
link: https://github.com/BaseMax/GooglePlayDatabaseMirror/blob/main/src/google-play.php

[IzzySoft]

Done. Also increased version in the file's header (was tempted declaring it "1.0", which it probably should be: according to semantic versioning, this is a release "breaking backward compatibility" (e.g. $apps = $parseSearch('foobar') no longer has the result set directly in $apps but instead in $apps['data']).

Please review the PR thoroughly. If you consider it "finalizing", maybe set version to "1.0" – otherwise do not merge but point out what is missing, so that gets addressed first. Thanks!

[BaseMax]

Thanks,
I create a tag: v1.0.0

We still not have a good example file. Maybe need to adding some different example to example.php file.
Even wider than the wiki

[BaseMax]

Published: https://github.com/BaseMax/GooglePlayWebServiceAPI/releases/tag/v1.0.0

[IzzySoft]

I create a tag: v1.0.0

Cool!

We still not have a good example file.

I just thought to point to the wiki, which already has several examples and basically covers all use cases (and which I need to update again, now that the PR has been merged). I don't think it's necessary to have even more "examples" in some PHP file. Running a series of different calls isn't very helpful, so one needs to copy/paste anyway. I'd rather have 2 or 3 general examples there (for a quick test to see "ah, it's working!") and point to the wiki for further details. Duplicating everything doesn't ease maintenance – just the opposite.

But well, that's my opinion. I won't keep you from creating the "wider variant" 😉

[BaseMax]

Yes. You prepared the wiki very well.
We can copy examples code from wiki to examples.php file.

[IzzySoft]

You prepared the wiki very well.

Thanks!

I've just brought the wiki pages up-to-snuff now. You should take a look at them using Github's new dark theme – be prepared to be blown away (before reading 🙊)

We can copy examples code from wiki to examples.php file.

Be welcome to copy what you think should be copied 😉

Speaking of copy: with the entire framework of my engine using GPLv2, and this class here almost entirely being rewritten by me now: would you be OK with me declaring it GPLv2 in my framework when I make it public one day? There's no ETA for that (it would need a lot of cleanup before from all the embedded test cases and debug code), so currently the question is rather hypothetical. And should it one day happen, I'll of course point here, credits and all, incl. pointing out the different license.

Background: to my knowledge, MIT and GPLv2 have some "compatibility issues". So I just want to make sure not to fall into some trap then.

[BaseMax]

Be welcome...

Done. cee47f7

This repository is licensed under MIT right now.
Just curiosity, Can I ask your reasons for choosing GPLv2 license? And why not to MIT?