pastk/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

[android] Fixed review notes

Browse source
This commit is contained in:
Dmitry Donskoy 2018-05-30 14:14:02 +03:00 • committed by Arsentiy Milchakov
parent 940ee0fb7a
commit b1fc7a742c
10 changed files with 160 additions and 193 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp
View file

@ -676,22 +676,22 @@ Java_com_mapswithme_maps_bookmarks_data_BookmarkManager_nativeGetBookmarkCategor
g_bookmarkCategoryClass,
categories,
[](JNIEnv * env, kml::MarkGroupId const & item) {
auto const & manager = frm()->GetBookmarkManager();
auto const & data = manager.GetCategoryData(item);
auto const isFromCatalog = manager.IsCategoryFromCatalog(item);
auto const tracksCount = manager.GetTrackIds(data.m_id).size();
auto const bookmarksCount = manager.GetUserMarkIds(data.m_id).size();
auto const isVisible = manager.IsVisible(data.m_id);
return env->NewObject(g_bookmarkCategoryClass,
g_bookmarkCategoryConstructor,
static_cast<jlong>(data.m_id),
jni::ToJavaString(env, kml::GetDefaultStr(data.m_name)),
jni::ToJavaString(env, data.m_authorId),
jni::ToJavaString(env, data.m_authorName),
static_cast<jint>(tracksCount),
static_cast<jint>(bookmarksCount),
static_cast<jboolean>(isFromCatalog),
static_cast<jboolean>(isVisible));
auto const & manager = frm()->GetBookmarkManager();
auto const & data = manager.GetCategoryData(item);
auto const isFromCatalog = manager.IsCategoryFromCatalog(item);
auto const tracksCount = manager.GetTrackIds(data.m_id).size();
auto const bookmarksCount = manager.GetUserMarkIds(data.m_id).size();
auto const isVisible = manager.IsVisible(data.m_id);
return env->NewObject(g_bookmarkCategoryClass,
g_bookmarkCategoryConstructor,
static_cast<jlong>(data.m_id),
jni::ToJavaString(env, kml::GetDefaultStr(data.m_name)),
jni::ToJavaString(env, data.m_authorId),
jni::ToJavaString(env, data.m_authorName),
static_cast<jint>(tracksCount),
static_cast<jint>(bookmarksCount),
static_cast<jboolean>(isFromCatalog),
static_cast<jboolean>(isVisible));
});
}
} // extern "C"

47
android/src/com/mapswithme/maps/bookmarks/data/AbstractCategoriesSnapshot.java
View file

@ -6,18 +6,18 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class AbstractCategoriesSnapshot
public abstract class AbstractCategoriesSnapshot
{
@NonNull
private final List<BookmarkCategory> mSnapshot;
public AbstractCategoriesSnapshot(@NonNull BookmarkCategory[] items)
AbstractCategoriesSnapshot(@NonNull BookmarkCategory[] items)
{
mSnapshot = Collections.unmodifiableList(Arrays.asList(items));
}
@NonNull
protected List<BookmarkCategory> items()
protected List<BookmarkCategory> getItems()
{
return mSnapshot;
}
@ -35,52 +35,21 @@ public class AbstractCategoriesSnapshot
@Override
@NonNull
public final List<BookmarkCategory> items()
public final List<BookmarkCategory> getItems()
{
return mStrategy.filter(super.items());
return mStrategy.filter(super.getItems());
}
public int indexOfOrThrow(@NonNull BookmarkCategory category)
{
int indexOf = items().indexOf(category);
int indexOf = getItems().indexOf(category);
if (indexOf < 0)
{
throw new UnsupportedOperationException(new StringBuilder("this category absent ")
.append("in current snapshot")
.append(indexOf)
throw new UnsupportedOperationException(new StringBuilder("This category absent in current snapshot ")
.append(category)
.toString());
}
return indexOf;
}
public static Default from(BookmarkCategory[] bookmarkCategories,
FilterStrategy strategy)
{
return new Default(bookmarkCategories, strategy);
}
}
public static class Private extends Default
{
public Private(@NonNull BookmarkCategory[] items)
{
super(items, new FilterStrategy.Private());
}
}
public static class Catalog extends Default
{
public Catalog(@NonNull BookmarkCategory[] items)
{
super(items, new FilterStrategy.Catalog());
}
}
public static class All extends Default {
public All(@NonNull BookmarkCategory[] items)
{
super(items, new FilterStrategy.All());
}
}
}

122
android/src/com/mapswithme/maps/bookmarks/data/BookmarkCategory.java
View file

@ -11,7 +11,7 @@ import android.text.TextUtils;
import com.mapswithme.maps.R;
import com.mapswithme.maps.bookmarks.BookmarksPageFactory;
import com.mapswithme.maps.content.TypeConverter;
import com.mapswithme.util.TypeConverter;
public class BookmarkCategory implements Parcelable
{
@ -25,14 +25,9 @@ public class BookmarkCategory implements Parcelable
private final int mTypeIndex;
private final boolean mIsVisible;
public BookmarkCategory(long id,
@NonNull String name,
@NonNull String authorId,
@NonNull String authorName,
int tracksCount,
int bookmarksCount,
boolean fromCatalog,
boolean isVisible)
public BookmarkCategory(long id, @NonNull String name, @NonNull String authorId,
@NonNull String authorName, int tracksCount, int bookmarksCount,
boolean fromCatalog, boolean isVisible)
{
mId = id;
mName = name;
@ -45,7 +40,6 @@ public class BookmarkCategory implements Parcelable
: new Author(authorId, authorName);
}
@Override
public boolean equals(Object o)
{
@ -105,47 +99,51 @@ public class BookmarkCategory implements Parcelable
public int size()
{
return getBookmarksCount() + getTracksCount();
return getBookmarksCount() + getTracksCount();
}
@PluralsRes
public int getPluralsCountTemplate()
@NonNull
public CountAndPlurals getPluralsCountTemplate()
{
return size() == 0
? R.plurals.objects
: getTracksCount() == 0
? R.plurals.places
: getBookmarksCount() == 0
? R.string.tracks
: R.plurals.objects;
if (size() == 0)
return new CountAndPlurals(0, R.plurals.objects);
if (getBookmarksCount() == 0)
return new CountAndPlurals(getTracksCount(), R.plurals.tracks);
if (getTracksCount() == 0)
return new CountAndPlurals(getBookmarksCount(), R.plurals.places);
return new CountAndPlurals(size(), R.plurals.objects);
}
public int getPluralsCount()
{
return size() == 0
? 0
: getBookmarksCount() == 0
? getTracksCount()
: getTracksCount() == 0
? getBookmarksCount()
: size();
}
public static class CountAndPlurals {
private final int mCount;
@PluralsRes
private final int mPlurals;
public static class Author implements Parcelable{
public static final String PHRASE_SEPARATOR = " • ";
public static final String SPACE = " ";
@NonNull
private final String mId;
public static String getRepresentation(Context context, Author author){
Resources res = context.getResources();
return new StringBuilder()
.append(PHRASE_SEPARATOR)
.append(String.format(res.getString(R.string.author_name_by_prefix),
author.getName()))
.toString();
public CountAndPlurals(int count, int plurals)
{
mCount = count;
mPlurals = plurals;
}
public int getCount()
{
return mCount;
}
public int getPlurals()
{
return mPlurals;
}
}
public static class Author implements Parcelable
{
private static final String PHRASE_SEPARATOR = " • ";
@NonNull
private final String mId;
@NonNull
private final String mName;
@ -198,6 +196,15 @@ public class BookmarkCategory implements Parcelable
return 0;
}
public static String getRepresentation(@NonNull Context context, @NonNull Author author)
{
Resources res = context.getResources();
return new StringBuilder()
.append(PHRASE_SEPARATOR)
.append(String.format(res.getString(R.string.author_name_by_prefix), author.getName()))
.toString();
}
@Override
public void writeToParcel(Parcel dest, int flags)
{
@ -227,15 +234,6 @@ public class BookmarkCategory implements Parcelable
};
}
public static class IsFromCatalog implements TypeConverter<BookmarkCategory, Boolean>
{
@Override
public Boolean convert(@NonNull BookmarkCategory data)
{
return data.isFromCatalog();
}
}
@Override
public String toString()
{
@ -295,20 +293,24 @@ public class BookmarkCategory implements Parcelable
}
};
public static class IsFromCatalog implements TypeConverter<BookmarkCategory, Boolean>
{
@Override
public Boolean convert(@NonNull BookmarkCategory data)
{
return data.isFromCatalog();
}
}
public enum Type
{
PRIVATE(
BookmarksPageFactory.PRIVATE,
FilterStrategy.Private.makeInstance()),
CATALOG(
BookmarksPageFactory.CATALOG,
FilterStrategy.Catalog.makeInstance());
PRIVATE(BookmarksPageFactory.PRIVATE, FilterStrategy.PredicativeStrategy.makePrivateInstance()),
CATALOG(BookmarksPageFactory.CATALOG, FilterStrategy.PredicativeStrategy.makeCatalogInstance());
private BookmarksPageFactory mFactory;
private FilterStrategy mFilterStrategy;
Type(@NonNull BookmarksPageFactory pageFactory,
@NonNull FilterStrategy filterStrategy)
Type(@NonNull BookmarksPageFactory pageFactory, @NonNull FilterStrategy filterStrategy)
{
mFactory = pageFactory;
mFilterStrategy = filterStrategy;

24
android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java
View file

@ -294,16 +294,15 @@ public enum BookmarkManager
@NonNull
public BookmarkCategory getCategoryById(long catId)
{
List<BookmarkCategory> items = getAllCategoriesSnapshot().items();
for (BookmarkCategory each : items){
if (catId == each.getId())
{
List<BookmarkCategory> items = getAllCategoriesSnapshot().getItems();
for (BookmarkCategory each : items)
{
if (catId == each.getId())
return each;
}
}
throw new IllegalArgumentException(new StringBuilder().append("category with id = ")
throw new IllegalArgumentException(new StringBuilder().append("Category with id = ")
.append(catId)
.append(" missing")
.append(" missed")
.toString());
}
@ -390,25 +389,28 @@ public enum BookmarkManager
@NonNull
public AbstractCategoriesSnapshot.Default getCatalogCategoriesSnapshot()
{
return new AbstractCategoriesSnapshot.Catalog(nativeGetBookmarkCategories());
BookmarkCategory[] items = nativeGetBookmarkCategories();
return new AbstractCategoriesSnapshot.Default(items, new FilterStrategy.Catalog());
}
@NonNull
public AbstractCategoriesSnapshot.Default getOwnedCategoriesSnapshot()
{
return new AbstractCategoriesSnapshot.Private(nativeGetBookmarkCategories());
BookmarkCategory[] items = nativeGetBookmarkCategories();
return new AbstractCategoriesSnapshot.Default(items, new FilterStrategy.Private());
}
@NonNull
public AbstractCategoriesSnapshot.Default getAllCategoriesSnapshot()
{
return new AbstractCategoriesSnapshot.All(nativeGetBookmarkCategories());
BookmarkCategory[] items = nativeGetBookmarkCategories();
return new AbstractCategoriesSnapshot.Default(items, new FilterStrategy.All());
}
@NonNull
public AbstractCategoriesSnapshot.Default getCategoriesSnapshot(FilterStrategy strategy)
{
return AbstractCategoriesSnapshot.Default.from(nativeGetBookmarkCategories(), strategy);
return new AbstractCategoriesSnapshot.Default(nativeGetBookmarkCategories(), strategy);
}
public boolean isUsedCategoryName(@NonNull String name)

38
android/src/com/mapswithme/maps/bookmarks/data/FilterStrategy.java
View file

@ -2,18 +2,18 @@ package com.mapswithme.maps.bookmarks.data;
import android.support.annotation.NonNull;
import com.mapswithme.maps.content.Predicate;
import com.mapswithme.util.Predicate;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public abstract class FilterStrategy
public interface FilterStrategy
{
@NonNull
public abstract List<BookmarkCategory> filter(@NonNull List<BookmarkCategory> items);
List<BookmarkCategory> filter(@NonNull List<BookmarkCategory> items);
public static class All extends FilterStrategy
class All implements FilterStrategy
{
@NonNull
@Override
@ -23,33 +23,23 @@ public abstract class FilterStrategy
}
}
public static class Private extends PredicativeStrategy<Boolean>
class Private extends PredicativeStrategy<Boolean>
{
public Private()
{
super(new Predicate.Equals<>(new BookmarkCategory.IsFromCatalog(), false));
}
public static FilterStrategy makeInstance()
{
return new Private();
}
}
public static class Catalog extends PredicativeStrategy<Boolean>
class Catalog extends PredicativeStrategy<Boolean>
{
Catalog()
{
super(new Predicate.Equals<>(new BookmarkCategory.IsFromCatalog(), true));
}
public static Catalog makeInstance()
{
return new Catalog();
}
}
public static class PredicativeStrategy<T> extends FilterStrategy
class PredicativeStrategy<T> implements FilterStrategy
{
@NonNull
private final Predicate<T, BookmarkCategory> mPredicate;
@ -67,11 +57,21 @@ public abstract class FilterStrategy
for (BookmarkCategory each : items)
{
if (mPredicate.apply(each))
{
result.add(each);
}
}
return Collections.unmodifiableList(result);
}
@NonNull
public static FilterStrategy makeCatalogInstance()
{
return new Catalog();
}
@NonNull
public static FilterStrategy makePrivateInstance()
{
return new Private();
}
}
}

6
android/src/com/mapswithme/maps/content/TypeConverter.java
View file

@ -1,6 +0,0 @@
package com.mapswithme.maps.content;
public interface TypeConverter<D, T>
{
T convert(D data);
}

15
android/src/com/mapswithme/maps/content/Predicate.java → android/src/com/mapswithme/util/Predicate.java
View file

@ -1,4 +1,4 @@
package com.mapswithme.maps.content;
package com.mapswithme.util;
import android.support.annotation.NonNull;
@ -7,21 +7,21 @@ public abstract class Predicate<T, D>
@NonNull
private final T mBaseValue;
protected Predicate(@NonNull T baseValue)
Predicate(@NonNull T baseValue)
{
mBaseValue = baseValue;
}
@NonNull
protected T getBaseValue()
T getBaseValue()
{
return mBaseValue;
}
public abstract boolean apply(D field);
public static class Equals<T, D> extends Predicate<T, D> {
public abstract boolean apply(@NonNull D field);
public static class Equals<T, D> extends Predicate<T, D>
{
@NonNull
private final TypeConverter<D, T> mConverter;
@ -32,12 +32,11 @@ public abstract class Predicate<T, D>
}
@Override
public boolean apply(D field)
public boolean apply(@NonNull D field)
{
T converted = mConverter.convert(field);
T value = getBaseValue();
return value == converted || value.equals(converted);
}
}
}

8
android/src/com/mapswithme/util/TypeConverter.java Normal file
View file

@ -0,0 +1,8 @@
package com.mapswithme.util;
import android.support.annotation.NonNull;
public interface TypeConverter<D, T>
{
T convert(@NonNull D data);
}

52
map/bookmark_manager.cpp
View file

@ -88,6 +88,17 @@ bool IsBadCharForPath(strings::UniChar const & c)
return false;
}
bool IsValidFilterType(BookmarkManager::CategoryFilterType const filter,
bool const fromCatalog) const
{
switch (filter)
{
case BookmarkManager::CategoryFilterType::All: return true;
case BookmarkManager::CategoryFilterType::Public: return fromCatalog;
case BookmarkManager::CategoryFilterType::Private: return !fromCatalog;
}
}
class FindMarkFunctor
{
public:
@ -1766,44 +1777,27 @@ bool BookmarkManager::IsUsedCategoryName(std::string const & name) const
bool BookmarkManager::AreAllCategoriesVisible(CategoryFilterType const filter) const
{
CHECK_THREAD_CHECKER(m_threadChecker, ());
bool visible = false;
bool fromCatalog = false;
for (auto const & category : m_categories)
{
visible = category.second->IsVisible();
fromCatalog = IsCategoryFromCatalog(category.first);
if (!visible && IsFilterTypeCorrected(filter, fromCatalog))
{
return false;
}
}
return true;
}
bool BookmarkManager::IsFilterTypeCorrected(CategoryFilterType const filter,
bool const fromCatalog) const
{
return (filter == CategoryFilterType::All ||
(filter == CategoryFilterType::Public && fromCatalog) ||
(filter == CategoryFilterType::Private && !fromCatalog));
return CheckVisibility(filter, true /* isVisible */);
}
bool BookmarkManager::AreAllCategoriesInvisible(CategoryFilterType const filter) const
{
return CheckVisibility(filter, false /* isVisible */);
}
bool BookmarkManager::CheckVisibility(BookmarkManager::CategoryFilterType const filter,
bool isVisible) const
{
CHECK_THREAD_CHECKER(m_threadChecker, ());
bool visible = false;
bool fromCatalog = false;
for (auto const & category : m_categories)
{
visible = category.second->IsVisible();
fromCatalog = IsCategoryFromCatalog(category.first);
if (visible && IsFilterTypeCorrected(filter, fromCatalog))
{
auto const fromCatalog = IsCategoryFromCatalog(category.first);
if (!IsValidFilterType(filter, fromCatalog))
continue;
if (category.second->IsVisible() != isVisible)
return false;
}
}
return true;
}

9
map/bookmark_manager.hpp
View file

@ -211,9 +211,9 @@ public:
enum class CategoryFilterType
{
Private = 0,
Public,
All
Private = 0,
Public,
All
};
struct SharingResult
@ -481,8 +481,7 @@ private:
void FinishConversion(ConversionHandler const & handler, bool result);
bool HasDuplicatedIds(kml::FileData const & fileData) const;
bool IsFilterTypeCorrected(CategoryFilterType const filter, bool const fromCatalog) const;
bool CheckVisibility(CategoryFilterType const filter, bool isVisible) const;
ThreadChecker m_threadChecker;
Callbacks m_callbacks;