Request: optimize library using Lint

[AndroidDeveloperLB]

It seems to show a lot of suggestions and warnings, to improve the code.

Here's a tiny example :

[jdkoren]

We can make this change, but in general it's not critical that the code in squidb-processor be optimized or adhere to Android's Lint inspections because this code should not be compiled into your APK (so in theory Android Lint shouldn't even inspect it). It should only run in order to generate other files which are then compiled into your APK. If Android Lint is reporting this to you during your builds, then there might be an issue with how your build script is setup.

[sbosley]

I did a pass with lint on all the core modules, and although I found a few places here and there where things could be touched up, the vast majority of things it flagged will probably be considered WONTFIX, as most of them were either false positives or places where lint didn't like our chosen code style, rather than actual problems. I'd recommend configuring lint to ignore the squidb modules if lint is causing build problems with them, or opening specific issues/pull requests if you see other places in the code that you feel are actually incorrect (as opposed to just stylistically different than what Android expects).

[AndroidDeveloperLB]

How do I set Lint to ignore all modules of Squidb? Also, I don't see why not follow its suggestions. Have you looked at what I've shown, for example?

[sbosley]

Yes, we've looked closely at its suggestions, and as I said most of them are false positives or stylistic suggestions only. For example, in the example you showed us, the two pieces of code have equivalent results, so the suggestion is purely a style thing, not indicative of a bug or any actual problem. In that case, we did make a code change in the above PR, but we've adopted slightly different style guidelines from Android's suggested guidelines, so lint may suggest things that we don't want to change. We don't have any intentions to migrate our codebase to different conventions at this time.

As for how to configure lint, that is probably something you'll need to figure out in the context of your own project. This stackoverflow post may be helpful to you.

[sbosley]

Or perhaps using a lint.xml config file as described here.

[AndroidDeveloperLB]

The example I've shown isn't stylistic. It's more efficient. Instead of comparing to another object (""), you access the object you already have. You could also use TextUtils.isEmpty, if you wish to make it short.

[sbosley]

Technically true -- an equality check takes a couple extra bytecode instructions than a length check does -- but since this piece of code only ever runs at compile time on a workstation instead of a mobile device, it's very unlikely to have any sort of measurable difference. Also note that TextUtils isn't available, as the squidb-processor module is a compile time java-only project, not a runtime android library (which is why we are confused as to why lint is analyzing it in the first place).

Regardless, the code was changed in #173. That PR updates all the things I saw from lint that I felt were worth addressing, but if you see other things that you feel have a compelling reason that they should be changed, we'd welcome any additional pull requests you wanted to submit!

[AndroidDeveloperLB]

Since the library is supported to work for Android, it is always recommended to run Lint.

[jdkoren]

1. We have indicated that SquiDB 3.0 (in beta) will support iOS as well, so we do not consider SquiDB to be solely an Android project any longer.

2. As was mentioned before, the code in squidb-processor is for annotation processing and is used at compile time only. It should only be running on your workstation, so it neither needs to be heavily optimized nor adhere to Lint's inspections. It is not supposed to run on the device or even exist in the APK, therefore it is pointless to run Android Lint on it. The generated code should be Lint compliant, but the annotation processor code that generates it does not need to be.

We are happy to accept pull requests for changes that fix actual bugs or incorrect behaviors in the annotation processing code, but we see no compelling reason to make the squidb-processor module pass all of the Android Lint checks.

[AndroidDeveloperLB]

1. A lot of SDKs out there are supported on multiple platforms. It doesn't mean they cannot be optimized for each platform.
2. Lint helps with both potential-bugs handling and optimizing code. I see no reason to avoid using it.

[jdkoren]

A lot of SDKs out there are supported on multiple platforms. It doesn't mean they cannot be optimized for each platform.

The code that is not Lint compliant (in the squidb-processor module) does not run on either platform, it runs on your workstation. The code that does run on the device (e.g. the code in the squidb module) is Lint compliant.

Lint helps with both potential-bugs handling and optimizing code. I see no reason to avoid using it.

Again, we will happily accept pull requests that fix demonstrable bugs in squidb-processor. No one is suggesting you not to use Android Lint entirely. We are saying that Android Lint does not need to run on squidb-processor and it is pointless to do so.

None of our projects run Android Lint on this module during builds; in fact, Gradle doesn't appear to be able to do so anyway...

jdkoren ~/squidb > ./gradlew :squidb-processor:lint                                                                                             
Parallel execution is an incubating feature.
Incremental java compilation is an incubating feature.

FAILURE: Build failed with an exception.

* What went wrong:
Task 'lint' not found in project ':squidb-processor'.

...which leads me to wonder what's producing those messages in your project. If they are being generated by the IDE based on your Java inspection settings, that's different from Android Lint.