organicmaps/organicmaps-tmp

Archived

Cuisine UI speed improvement #2924

Merged

root merged 6 commits from issue-285-suisine-search into master

2022-07-08 22:31:40 +00:00

strump commented

2022-07-05 11:49:23 +00:00

Member

Moved osm::Cuisines::Instance() out of the loop for Editor.nativeTranslateCuisines(...) method.

After PR #2921 is closed I found that Editor.cpp could be improved a little too.

Is it possible to create new PR from a branch which was previously merged to master?

Moved `osm::Cuisines::Instance()` out of the loop for `Editor.nativeTranslateCuisines(...)` method. After PR #2921 is closed I found that Editor.cpp could be improved a little too. Is it possible to create new PR from a branch which was previously merged to master?

vng commented

2022-07-05 12:37:14 +00:00

(Migrated from github.com)

Just rebase your branch behind new master.

Just rebase your branch behind new master.

vng commented

2022-07-05 13:38:49 +00:00

(Migrated from github.com)

BTW, previous change was in util_StringUtils_nativeFilterContainsNormalized.
Don't you think that it's very common function to call Cuisines.Translate? I'm not familiar with editor flow in Android, but now I see that there is a jni Editor.cpp

Is it possible to incapsulate all Cuisines.Translate routine in Editor context?

BTW, previous change was in util_StringUtils_nativeFilterContainsNormalized. Don't you think that it's very common function to call Cuisines.Translate? I'm not familiar with editor flow in Android, but now I see that there is a jni Editor.cpp Is it possible to incapsulate all Cuisines.Translate routine in Editor context?

strump commented

2022-07-06 11:53:28 +00:00

Author

Member

@vng you are right. I shouldn't change util_StringUtils_nativeFilterContainsNormalized in PR #2921. This function should work for all strings (no cuisine translation). So I added new function Editor.nativeFilterCuisinesKeys implemented in C++ function ..._editor_Editor_nativeFilterCuisinesKeys. This new function searches in keys and in translated cuisine labels.

Reverted util_StringUtils_nativeFilterContainsNormalized.

@vng you are right. I shouldn't change `util_StringUtils_nativeFilterContainsNormalized` in PR #2921. This function should work for all strings (no cuisine translation). So I added new function `Editor.nativeFilterCuisinesKeys` implemented in C++ function `..._editor_Editor_nativeFilterCuisinesKeys`. This new function searches in keys and in translated cuisine labels. Reverted `util_StringUtils_nativeFilterContainsNormalized`.

vng (Migrated from github.com) approved these changes 2022-07-06 13:11:27 +00:00

biodranik (Migrated from github.com) approved these changes 2022-07-06 20:15:50 +00:00

biodranik (Migrated from github.com) left a comment

LGTM with minor comments.

LGTM with minor comments.

android/jni/com/mapswithme/maps/editor/Editor.cpp

biodranik (Migrated from github.com) commented

2022-07-06 20:10:43 +00:00

  std::string const substr = jni::ToNativeString(env, jSubstr);

```suggestion std::string const substr = jni::ToNativeString(env, jSubstr); ```

biodranik (Migrated from github.com) commented

2022-07-06 20:12:23 +00:00

    std::string const str = jni::ToNativeString(env, static_cast<jstring>(env->GetObjectArrayElement(src, i)));

```suggestion std::string const str = jni::ToNativeString(env, static_cast<jstring>(env->GetObjectArrayElement(src, i))); ```

biodranik (Migrated from github.com) commented

2022-07-06 20:14:06 +00:00

    std::string const key = jni::ToNativeString(env, static_cast<jstring>(env->GetObjectArrayElement(jKeys, i)));

```suggestion std::string const key = jni::ToNativeString(env, static_cast<jstring>(env->GetObjectArrayElement(jKeys, i))); ```

android/jni/com/mapswithme/util/StringUtils.cpp

					
				@ -37,15 +37,14 @@ Java_com_mapswithme_util_StringUtils_nativeContainsNormalized(JNIEnv * env, jcla

				JNIEXPORT jobjectArray JNICALL

				Java_com_mapswithme_util_StringUtils_nativeFilterContainsNormalized(JNIEnv * env, jclass thiz, jobjectArray src, jstring jSubstr)

biodranik (Migrated from github.com) commented

2022-07-06 20:14:34 +00:00

  std::string const substr = jni::ToNativeString(env, jSubstr);

```suggestion std::string const substr = jni::ToNativeString(env, jSubstr); ```

biodranik (Migrated from github.com) commented

2022-07-06 20:15:08 +00:00

    std::string const str = jni::ToNativeString(env, static_cast<jstring>(env->GetObjectArrayElement(src, i)));

```suggestion std::string const str = jni::ToNativeString(env, static_cast<jstring>(env->GetObjectArrayElement(src, i))); ```

strump reviewed 2022-07-07 08:07:54 +00:00

android/jni/com/mapswithme/maps/editor/Editor.cpp

strump commented

2022-07-07 08:07:16 +00:00

Author

Member

Fixed according to review comments.

Fixed according to review comments.

android/src/com/mapswithme/maps/editor/CuisineAdapter.java

strump commented

2022-07-07 08:06:45 +00:00

Author

Member

@biodranik I think maybe instead of 2 native calls I should copy nativeGetCuisines implementation into nativeFilterCuisinesKeys ?
And this line would look like:

Editor.nativeFilterCuisinesKeys(filter);

@biodranik I think maybe instead of 2 native calls I should copy `nativeGetCuisines` implementation into `nativeFilterCuisinesKeys` ? And this line would look like: ``` Editor.nativeFilterCuisinesKeys(filter); ```

biodranik (Migrated from github.com) reviewed 2022-07-07 21:26:47 +00:00

android/src/com/mapswithme/maps/editor/CuisineAdapter.java

biodranik (Migrated from github.com) commented

2022-07-07 21:26:46 +00:00

It's a good idea if the code can be reused and made simple. An empty filter returns everything, a non-empty filter returns filtered results.

It's a good idea if the code can be reused and made simple. An empty filter returns everything, a non-empty filter returns filtered results.

strump reviewed 2022-07-08 07:43:17 +00:00

android/src/com/mapswithme/maps/editor/CuisineAdapter.java

strump commented

2022-07-08 07:43:16 +00:00

Author

Member

Ok, I've simplified Editor.nativeFilterCuisinesKeys(String) function. Please review.

Ok, I've simplified `Editor.nativeFilterCuisinesKeys(String)` function. Please review.

vng (Migrated from github.com) approved these changes 2022-07-08 08:25:55 +00:00

biodranik (Migrated from github.com) approved these changes 2022-07-08 22:31:14 +00:00

biodranik (Migrated from github.com) left a comment

Super, thanks! Code became cleaner and faster ;-)

Super, thanks! Code became cleaner and faster ;-)

This repo is archived. You cannot comment on pull requests.

No reviewers

No labels

Android Automotive (AAOS)

Android Automotive (AAOS)

Battery and Performance

Battery and Performance

Bookmarks and Tracks

Bookmarks and Tracks

External Map Datasets

External Map Datasets

Frequently User Reported

Frequently User Reported

Good first issue

Good first issue

Linux packaging

Linux packaging

NLnet 2024-06-281

NLnet 2024-06-281

No Feature Parity

No Feature Parity

Public Transport

Public Transport

Track Recording

Track Recording

Walk Navigation

Walk Navigation

No milestone

No project

No assignees

2 participants

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#2924

No description provided.