Noissue - move work with a raw jni pointer to a separate function

It helps to keep it in one place and reduces the number of changes to only that function if the pointer is being changed.

Review: https://codereview.adblockplus.org/29465626/

COLLABORATOR=anton@adblockplus.org

[anton, 2017-06-15 05:16:53]

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
File libadblockplus-android/jni/JniFilterEngine.cpp (right):

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
libadblockplus-android/jni/JniFilterEngine.cpp:48: namespace
can we have the same code style like we use in
https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockp...
like
`static AdblockPlus::FilterEnginePtr& GetFilterEnginePtrRef(jlong ptr) { ... }`

[sergei, 2017-06-16 10:44:36]

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
File libadblockplus-android/jni/JniFilterEngine.cpp (right):

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
libadblockplus-android/jni/JniFilterEngine.cpp:48: namespace
On 2017/06/15 05:16:53, anton wrote:
> can we have the same code style like we use in
>
https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockp...
> like
> `static AdblockPlus::FilterEnginePtr& GetFilterEnginePtrRef(jlong ptr) { ...
}`

Yes we can but this function is related to JniFilterEngine (another codereview)
and I thought that it should be in the same namespace. Should it be static?

In general, "static" here is used merely for internal linkage, as far as I
understand and I'm not sure how it behaves when there is the same method
declaration in a header, for instance. I personally prefer to have all code in
proper namespaces, e.g. "abp" and anonymous, because they are more descriptive,
explicit and can have some other benefits. So, my opinion is to continue to use
static to stay consistent with the existing code when it makes sense but
consider switching from it.

[anton, 2017-06-16 11:41:59]

On 2017/06/16 10:44:36, sergei wrote:
>
https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
> File libadblockplus-android/jni/JniFilterEngine.cpp (right):
> 
>
https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
> libadblockplus-android/jni/JniFilterEngine.cpp:48: namespace
> On 2017/06/15 05:16:53, anton wrote:
> > can we have the same code style like we use in
> >
>
https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockp...
> > like
> > `static AdblockPlus::FilterEnginePtr& GetFilterEnginePtrRef(jlong ptr) { ...
> }`
> 
> Yes we can but this function is related to JniFilterEngine (another
codereview)
> and I thought that it should be in the same namespace. Should it be static?
> 
> In general, "static" here is used merely for internal linkage, as far as I
> understand and I'm not sure how it behaves when there is the same method
> declaration in a header, for instance. I personally prefer to have all code in
> proper namespaces, e.g. "abp" and anonymous, because they are more
descriptive,
> explicit and can have some other benefits. So, my opinion is to continue to
use
> static to stay consistent with the existing code when it makes sense but
> consider switching from it.

LGTM

[sergei, 2017-07-03 07:55:43]

Could you please review it.

[jens, 2017-07-05 09:27:40]

On 2017/07/03 07:55:43, sergei wrote:
> Could you please review it.

LGTM

[jens, 2017-07-05 09:27:53]

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
File libadblockplus-android/jni/JniFilterEngine.cpp (right):

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
libadblockplus-android/jni/JniFilterEngine.cpp:368:
AdblockPlus::FilterEnginePtr& engine =
Minor one, but why don't have the definition of engine in only one line, like in
all other cases?

[anton, 2017-07-05 09:30:32]

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
File libadblockplus-android/jni/JniFilterEngine.cpp (right):

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
libadblockplus-android/jni/JniFilterEngine.cpp:368:
AdblockPlus::FilterEnginePtr& engine =
On 2017/07/05 09:27:53, jens wrote:
> Minor one, but why don't have the definition of engine in only one line, like
in
> all other cases?

yes, i believe previously the line was too long to fit into 100 characters.
now we can (like it's done in other places) and i'd say we should.

[sergei, 2017-07-05 13:46:20]

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
File libadblockplus-android/jni/JniFilterEngine.cpp (right):

https://codereview.adblockplus.org/29465626/diff/29465627/libadblockplus-andr...
libadblockplus-android/jni/JniFilterEngine.cpp:368:
AdblockPlus::FilterEnginePtr& engine =
On 2017/07/05 09:30:32, anton wrote:
> On 2017/07/05 09:27:53, jens wrote:
> > Minor one, but why don't have the definition of engine in only one line,
like
> in
> > all other cases?
> 
> yes, i believe previously the line was too long to fit into 100 characters.
> now we can (like it's done in other places) and i'd say we should.

Right, I guess it's been simply overlooked when I was reverting other changes.