Search2

[cream-cheeze]

Сделал новым PR, так как в предыдущем уже теряешься, ну и это экспериментальная версия на обсуждение.

[cream-cheeze]

@DCrow @ILAgent посмотрите, пожалуйста! Очень хочется с этим покончить, как можно скорее...

[cream-cheeze]

Хочется получить комментарии по принципиальным моментам, которые обозначил в этом ревью, ну или которые вы ещё увидите. Цель — договориться о финальной архитектуре. Получившееся решение не идеально, в предыдущие обсуждения не все нюансы были учтены. Мелочи вроде nullable предлагаю позже обсуждать.

ЭТО НЕ ФИНАЛЬНЫЙ ВАРИАНТ, А ВЕРСИЯ НА ОБСУЖДЕНИЕ.

[cream-cheeze]

Вообщем, пока ждал ответа, закончил эту версию – в результате в SearchSession сделал стрим results, на который можно подписаться и получать новые результаты по мере их поступления.

Честно говоря, всё это выглядит чрезмерно усложнённым, и стримы здесь кажутся явно лишними. Мой самый первый вариант с одной сессией на колбэках был самым простым в использовании и самым понятным... Здесь куча нюансов: нужно получить сессию, потом не забыть закрыть ёё, подписаться на стрим, потом отписаться. Короче каша какая-то, на мой взгляд.

Но если уж мультисессионность так принципиальна, то я бы лучше взял вот эту последнюю мою версию, выпилил бы оттуда нафиг этот стрим, и вернул бы колбэки с результами.

Варианты с колбэками:

1. Передавать один колбэк в searchByText и сохранять его в сессии - вызывать при каждом _onResponse. Получается, что этот единственный колбэк будет срабатывать: при первом вызове searchByText, при fetchNextPage и при retrySearch. Способ обработки будет зависеть от номера страницы в ответе.
2. Передавать разные колбэки в каждый из вышеперечисленных методов. Но смысла в этом мало - в каждом из случаев возвращается одна и та же структура данных, и обработка будет одна и та же.

Я за первый вариант.

@DCrow ответь, пожалуйста, давай уже решим что-нибудь.

[cream-cheeze]

На сколько я понял Android пока не имплементирован?

Да, Android не делал, чтобы зря время не тратить.

[DCrow]

В продолжение диалога https://github.com/Unact/yandex_mapkit/pull/119#issuecomment-890897257

Что касается ошибок - что в стримах, что во фьючерах есть механизм обработки ошибок: onError или catchError - я бы использовал его, как я и сделал в serach2. Это позволяет не изменять структуру searchResponse и не добавлять в него ошибку - более красивое решение, на мой взгляд.

Действительно, лучше его использовать.

По поводу последующих результатов уже не совсем понятно... Вот у нас есть сессия от searchByText и есть первая страница, которая содержится в results (который Future). А что дальше - дёргаем fetchNextPage - он должен вернуть тоже Future уже с новой страницей?

Да, думаю юзер сам должен хранить результаты первой страницы, ровно также как и нативная либа требует.

Да, future нельзя переиспользовать для разных результатов. Когда результатов много - это как раз стрим. Но хз, насколько это удобно.

Так никто же не запрещает поле делать final. Можно же новый future создать и его отдать. Стрим тут как-то все сильно усложняет.

[ILAgent]

Мне кажется, что стрим - это ок. Стрим эмитит очередную страницу после каждого фетча. При подписке сразу эмитится последняя полученная страница. Ничего лучше в голову не приходит. И вроде бы вполне понятное апи.

[cream-cheeze]

@DCrow так что в итоге, как делаем? Твоя репа - тебе решать.

[ILAgent]

Что касается ошибок - что в стримах, что во фьючерах есть механизм обработки ошибок: onError или catchError - я бы использовал его, как я и сделал в serach2. Это позволяет не изменять структуру searchResponse и не добавлять в него ошибку - более красивое решение, на мой взгляд.

Действительно, лучше его использовать.

Мы же договорились не превращать мапкитовые ошибки в дартовые эксепшены. Так-то у future тоже есть onError.

[ILAgent]

Мне кажется, что стрим - это ок. Стрим эмитит очередную страницу после каждого фетча. При подписке сразу эмитится последняя полученная страница. Ничего лучше в голову не приходит. И вроде бы вполне понятное апи.

Попроще вариант - fetchNextPage(): Future<Response>. Да, у сессии есть response и тут. Но зато сразу понятно, что к чему. Имхо) Переиспользовать свойство response сесии как-то очень неочевидно.

[cream-cheeze]

Мы же договорились не превращать мапкитовые ошибки в дартовые эксепшены. Так-то у future тоже есть onError.

Я может чего не знаю.. А в чём минусы? Удобно же - использовать для ошибок специально предусмотренный для этого механизм языка, благодаря чему можно не уродовать класс ответа.

[cream-cheeze]

Мне кажется, что стрим - это ок. Стрим эмитит очередную страницу после каждого фетча. При подписке сразу эмитится последняя полученная страница. Ничего лучше в голову не приходит. И вроде бы вполне понятное апи.

Попроще вариант - fetchNextPage(): Future<Response>. Да, у сессии есть response и тут. Но зато сразу понятно, что к чему. Имхо) Переиспользовать свойство response сесии как-то очень неочевидно.

Переиспользовать свойство response сесии - совсем неочевидно, согласен. Но для меня так же неочевидно первый результат получать из сессии, а последующие напрямую от выполнения функций. Помимо следующей страницы может быть такая ситуация: вызвали searchByText, создалась и вернулась сессия, а при получении результата возникла ошибка - на этот случай есть метод retry - он тоже будет Future возвращать, несмотря на то, что это будет первая страница? Крайне запутанно.

Все результаты от всех этих методов нужно возвращать одним способом - и самый лучший способ видится колбэками, т.к. стрим здесь не в кассу (он искусственный), а фьючи не получается сделать в одном ключе из-за того, что searchByText возвращает сессию, а не ответ. Не даром в mapkit все эти методы принимают абсолютно одинаковый responseHandler, который одинаковым образом обрабатывает результат.

Что касается одного колбэка на все эти методы - @DCrow всегда не нравилась эта идея - можно изловчиться, и в каждый из методов передавать свой, ради бога.. Только вот как это будет выглядеть в клиентском коде - 4 одинаковых колбэка будут писать (для searchByText, retry, fetchNexPage и resubmit)? Да вряд ли - проще написать один и обработать в нём номера страниц...

[DCrow]

Попроще вариант - fetchNextPage(): Future. Да, у сессии есть response и тут. Но зато сразу понятно, что к чему. Имхо) Переиспользовать свойство response сесии как-то очень неочевидно.

Сам же противоречишь себе, что

Мы же договорились не превращать мапкитовые ошибки в дартовые эксепшены. Так-то у future тоже есть onError.

Тогда надо их выдавать, иначе куда ошибку возвращать?

Со стримами @cream-cheeze тут описал проблему https://github.com/Unact/yandex_mapkit/pull/127#discussion_r679342503 что у нас race-condition, и если исправить ее, то может получится, что юзер подпишется на стрим, а ответ уже прошел и его юзер не получит.

Переиспользовать свойство response сесии - совсем неочевидно, согласен. Но для меня так же неочевидно первый результат получать из сессии, а последующие напрямую от выполнения функций. Помимо следующей страницы может быть такая ситуация: вызвали searchByText, создалась и вернулась сессия, а при получении результата возникла ошибка - на этот случай есть метод retry - он тоже будет Future возвращать, несмотря на то, что это будет первая страница? Крайне запутанно.

Да получается не совсем очевидно, но из-за того, что у нативной сессии есть разные методы, приходится так изварачиваться. Либо вообще другой вариант это сделать чтобы сессия была отдельно стоящий класс. Юзер сам инициализирует объект сессии(на native стороне при этом сессии еще нет, пока не вызовут searchByText), и на нем может использовать searchByText/retry/fetchNextPage. Тогда каждый из методов возвращает Future с результатом. В этом случае получаем более очевидный интерфейс, пропадает проблема race-condition и не приходится усложнять интерфейс стримами. И в таком варианте можно все остальные(Suggest/Routing) держать.

С ошибками похоже, что придется его в ответ запихивать, придется юзерам всегда проверять. Не нравится отбирать у пользователей возможность использовать await/async.

[ILAgent]

Мы же договорились не превращать мапкитовые ошибки в дартовые эксепшены. Так-то у future тоже есть onError.

Я может чего не знаю.. А в чём минусы? Удобно же - использовать для ошибок специально предусмотренный для этого механизм языка, благодаря чему можно не уродовать класс ответа.

Дело в том, что если мы можем не обработать ошибку future или стрима и получим крэш. Т.е. мы ответ мапкита превращаем в исключительную ситуацию (exception) - это не правильно. Апи мапкита же не роняет приложение, и мы не должны.

[ILAgent]

Помимо следующей страницы может быть такая ситуация: вызвали searchByText, создалась и вернулась сессия, а при получении результата возникла ошибка - на этот случай есть метод retry - он тоже будет Future возвращать, несмотря на то, что это будет первая страница? Крайне запутанно.

Да, плохой вариант. Всё-таки один стрим лучше.

[ILAgent]

Мы же договорились не превращать мапкитовые ошибки в дартовые эксепшены. Так-то у future тоже есть onError.

Тогда надо их выдавать, иначе куда ошибку возвращать?

в Response, как договаривались

[ILAgent]

может получится, что юзер подпишется на стрим, а ответ уже прошел и его юзер не получит.

стрим должен хранит последний вариант и возвращать при подписке (как Future)

[ILAgent]

С ошибками похоже, что придется его в ответ запихивать, придется юзерам всегда проверять.

Например, фильтровать стрим с помощью where )

[cream-cheeze]

Яснее не стало... Давайте сконцентрируемся на конкретных вещах, иначе ещё долго может здесь обсуждать.

Сейчас есть такой интерфейс:

YandexSearch

• searchByText(String searchText, Geometry geometry, SearchOptions options): Future

SearchSession

• Stream results // стрим с результатами

• cancelSearch(): void

• retrySearch(): void

• fetchNextPage(): void

• closeSession():void

• resubmit(...) - может появиться в будущем

SearchResponse

• int found
• List items
• int page
• bool hasNextPage

Вот пример кода, который сейчас работает:

var session = await YandexSearch.searchByText(
  searchText: query,
  geometry: Geometry.fromBoundingBox(
    BoundingBox(
      southWest: Point(latitude: 55.76996383933034, longitude: 37.57483142322235),
      northEast: Point(latitude: 55.785322774728414, longitude: 37.590924677311705),
    )
  ),
  searchOptions: SearchOptions(
    searchType: SearchType.geo,
    geometry: false,
  ),
);

_sessions[session.id] = session;

// Listen to results stream

StreamSubscription? subscription;

subscription = session.results.listen((res) {

  var resStr = res.toString();

  print('Page ${res.page}: $resStr');

  setState(() {
    responseByPages.add(res);
  });

  if (res.hasNextPage) {
    print('Got ${res.found} items, fetching next page...');
    session.fetchNextPage();
  } else {
    print('No more results available, closing session...');
    session.closeSession();
    _sessions.remove(session.id);
    subscription!.cancel();
  }

}, onError: (error) {

  if (error is PlatformException) {
    print('Error: ${error.message}');
  }

  subscription!.cancel();
});
}

Предлагаю @DCrow ещё раз оценить все приведённые выше аргументы и предложения, и написать, каким должен быть финальный интерфейс. И также переписать тот же самый пример под этот интерфейс - это поможет и тебе и нам, наглядно увидеть и убедиться, что там нет подводных камней.

Дальнейшее обсуждение, если ещё потребуется, будем вести только в таком формате - если кто-то не согласен, видит ошибку или проблему, то предлагает свой вариант какой-то части или всего интерфейса. Так, надеюсь, мы скорее придём к единогласному решению.

После этого, я берусь и реализовываю, включая Android.

P.S. Мне уже пора и обратный поиск делать - очень скоро нужен будет.

[ILAgent]

Всё-таки посмотрел оригинальный интерфейс мапкита. Там так.

public interface SearchManager {
    @NonNull
    public Session submit(
        @NonNull String text,
        @NonNull Geometry geometry,
        @NonNull SearchOptions searchOptions,
        @NonNull SearchListener searchListener);

}

public interface Session {

    public static interface SearchListener {       
        public void onSearchResponse(@NonNull Response response);    
        public void onSearchError(@NonNull Error error);
    }

    public void cancel();

    public void retry(@NonNull SearchListener searchListener);

    public boolean hasNextPage();

    public void fetchNextPage(@NonNull SearchListener searchListener);

}

Т.е. в каждый метод передаётся отдельный колбек. Кажется правильно было бы сделать так же, только вместо приёма колбеков возвращать future. Получается как-то так.

class Response {
    List items;
    ...
}

class ResponeOrError {
    Response? response;
    String? error;
}

abstract class SearchSession {

    void cancel();

    Future<ResponseOrError> retry();

    boolean hasNextPage();

    Future<ResponseOrError> fetchNextPage();
}

class ResponseWithSession {
   Future<ResponseOrError> responseOrError;
   SearchSession session;
}

Future<ResponseWithSession> searchByText(...)

[DCrow]

Я предлагаю следующий вариант(прототип)

search_session.dart

class SearchSession {
  late final MethodChannel _methodChannel;

  static const MethodChannel _systemChannel = MethodChannel('yandex_mapkit/yandex_search');

  static int _nextId = 0;

  static int _getNextId() => _nextId++;

  final int _id;

  int get id => _id;

  bool _active = true;

  bool get active => _active;

  SearchSession._(this._id) :
    _methodChannel = MethodChannel('yandex_mapkit/yandex_search_session_$_id');

  static Future<SearchSession> init() async {
    var id = SearchSession._getNextId();

    await _systemChannel.invokeMethod('initSearchSession', { 'id': id });

    return SearchSession._(id);
  }

  Future<SearchResponse> searchByText({
    required String searchText,
    required Geometry geometry,
    required SearchOptions searchOptions
  }) async {
    if (!active) throw('SearchSession has been closed');

    var params = {
      'searchText': searchText,
      'geometry': geometry.toJson(),
      'options': searchOptions.toJson(),
    };
    var result = await _methodChannel.invokeMethod('searchByText', params);

    return SearchResponse.fromJson(result['response']);
  }

  Future<void> cancelSearch() async {
    if (!active) throw('SearchSession has been closed');

    await _methodChannel.invokeMethod('cancelSearch');
  }

  Future<SearchResponse> retrySearch() async {
    if (!active) throw('SearchSession has been closed');

    var result = await _methodChannel.invokeMethod('retrySearch');

    return SearchResponse.fromJson(result['response']);
  }

  Future<SearchResponse> fetchNextPage() async {
    if (!active) throw('SearchSession has been closed');

    var result = await _methodChannel.invokeMethod('fetchNextPage');

    return SearchResponse.fromJson(result['response']);
  }

  Future<void> close() async {
    if (!active) throw('SearchSession has been closed');

    await _methodChannel.invokeMethod('close');
    await _systemChannel.invokeMethod('closeSearchSession', { 'id': id });

    _active = false;
  }
}

search_response.dart

class SearchResponse extends Equatable {

  final int                found;
  final String?            errorMsg;
  final List<SearchItem>   items;
  final int                page;
  final bool               hasNextPage;

  const SearchResponse({
    required this.found,
    required this.items,
    required this.page,
    required this.hasNextPage,
    this.errorMsg
  });

  factory SearchResponse.fromJson(Map<dynamic, dynamic> json) {
    List? items;

    if (json['errorMsg'] != null) {
      return SearchResponse(
        found: 0,
        items: [],
        page: 0,
        hasNextPage: false,
        errorMsg: json['errorMsg']
      );
    }

    if (json['items'] != null) {
      items = json['items'] as List?;
    }

    List<SearchItem>? mappedItems;

    if (items != null) {
      mappedItems = items.map((i) => SearchItem.fromJson(i)).toList();
    }

    return SearchResponse(
      found:        json['found'],
      items:        mappedItems ?? [],
      page:         json['page'],
      hasNextPage:  json['hasNextPage'],
    );
  }

  @override
  List<Object?> get props => <Object?>[
    found,
    items,
    page,
    hasNextPage,
    errorMsg
  ];

  @override
  bool get stringify => true;
}

search_example.dart (часть его)

void search(String query) async {

    print('Search query: $query');

    responseByPages.clear();

    var session = await SearchSession.init();

    _sessions[session.id] = session;

    var response = await session.searchByText(
      searchText: query,
      geometry: Geometry.fromBoundingBox(
        BoundingBox(
          southWest: Point(latitude: 55.76996383933034, longitude: 37.57483142322235),
          northEast: Point(latitude: 55.785322774728414, longitude: 37.590924677311705),
        )
      ),
      searchOptions: SearchOptions(
        searchType: SearchType.geo,
        geometry: false,
      ),
    );

    do {
      if (response.errorMsg != null) {
        print('Error: ${response.errorMsg}');
        await session.close();
        _sessions.remove(session.id);
        break;
      }

      var resStr = response.toString();

      print('Page ${response.page}: $resStr');

      setState(() {
        responseByPages.add(response);
      });

      if (!response.hasNextPage) {
        print('No more results available, closing session...');
        await session.close();
        _sessions.remove(session.id);
        break;
      }

      print('Got ${response.found} items, fetching next page...');
      await session.fetchNextPage();

    } while (response.hasNextPage);
  }

На мой взгляд тут получаем и возможность мульти-сессий, и используем один канал для каждой сессии, и можем получать последующие результаты вполне очевидным путем.

Какие моменты на мой взгляд решены

1. Нет усложнения работы с стримами(меньше кода, более ясная работа метода searchByText), ошибки заварачиваются в тот же ответ(в целом ошибка поиска это тоже вариант ответа).
2. Нет проблемы с калбэками для fetchNextPage, можно явно выдать результат и юзер что хочет, то и будет делать.
3. race-condition в этом случае не будет, так как ответ сразу передастся

Ошибка в данном случае в Response передается(можно и там усложнить, делать доп. класс где есть поля response и error, но мне кажется это каким-то излишеством) Через нативные ошибки Future, не нравится так как, там может смешаться с ошибками MethodChannel, + в таком случае не возможно использовать await.

Если что-то упустил, просьба напомнить, а то диалог слишком большой все сразу вспомнить не могу...

[DCrow]

Т.е. в каждый метод передаётся отдельный колбек. Кажется правильно было бы сделать так же, только вместо приёма колбеков возвращать future. Получается как-то так.

В целом тоже считаю, что можно после каждого метода future с ответом возвращать. В своем примере я также возвращаю их(только не на столько усложненно, как у тебя)

[cream-cheeze]

await _systemChannel.invokeMethod('initSearchSession', { 'id': id });

Этот вызов что на стороне ios/android делает и возвращает?

[DCrow]

Этот вызов что на стороне ios/android делает и возвращает?

Создает объект YandexSearchSession(без самой сессии YMKSearchSession), который будет работать с каналом, id которого будет переданный в параметрах id.

[cream-cheeze]

Этот вызов что на стороне ios/android делает и возвращает?

Создает объект YandexSearchSession(без самой сессии YMKSearchSession), который будет работать с каналом, id которого будет переданный в параметрах id.

На мой взгляд, это ошибка - так делать - ты создал объект сессии, которой по факту ещё нет. Можешь отправить ей команды, которые ни к чему не приведут.

[ILAgent]

Этот вызов что на стороне ios/android делает и возвращает?

Создает объект YandexSearchSession(без самой сессии YMKSearchSession), который будет работать с каналом, id которого будет переданный в параметрах id.

На мой взгляд, это ошибка - так делать - ты создал объект сессии, которой по факту ещё нет. Можешь отправить ей команды, которые ни к чему не приведут.

Мне тоже не очень нравится. Мапкит возвращает сессию при запросе, хорошо бы делать так же.

[DCrow]

На мой взгляд, это ошибка - так делать - ты создал объект сессии, которой по факту ещё нет. Можешь отправить ей команды, которые ни к чему не приведут.

Да такой момент есть, и в этом случае будет возвращаться ошибка в dart в SearchResponse.

[cream-cheeze]

Всё-таки посмотрел оригинальный интерфейс мапкита. Там так.

Т.е. в каждый метод передаётся отдельный колбек. Кажется правильно было бы сделать так же, только вместо приёма колбеков возвращать future. Получается как-то так.

Ну это, откровенно говоря, совсем не новость - я видел как там, иначе бы поиск не сделал. Пытался донести это не один раз до вас.

class ResponseWithSession { Future responseOrError; SearchSession session; }

Проблему возврата сессии и первого результата решает, но выглядит монструозно.

Никому не кажется, что мы просто зациклились на идее "сделать без колбэков", а колбэки здесь - самый естественный вариант?

searchByText - принял колбэк и вернул сессию, далее работа с сессией с передачей колбэка в каждый метод. По-моему максимально понятно. Уже и @ILAgent почти до этого же дошёл.

Все остальные предложенные до сих пор варианты предполагают некий компромисс: стримы, объекты типа ResponseWithSession, инициализация сессии - в каждом из вариантов есть неудобный нюанс, с которым приходится мириться.

[cream-cheeze]

Но если выбирать между вариантами @DCrow и @ILAgent я бы выбрал последний - с ResponseWithSession - он хоть и нагромождает структуру ответа, но нет такой проблемы, как пустая сессия.

Последнее слово за @DCrow - реализую уже любой из вариантов, сделав над собой усилие..

[ILAgent]

Проблему возврата сессии и первого результата решает, но выглядит монструозно.

Почему? По-моему сразу понятно, что к чему.

Никому не кажется, что мы просто зациклились на идее "сделать без колбэков", а колбэки здесь - самый естественный вариант? searchByText - принял колбэк и вернул сессию, далее работа с сессией с передачей колбэка в каждый метод. По-моему максимально понятно. Уже и @ILAgent почти до этого же дошёл.

Отчасти ты прав) Считаю, надо делать близкое к оригиналу апи. Заменить колбеки на фьючеры - получаем почти такое же, но более дартовое апи.

[ILAgent]

Придумывать что-то новое и натягивать на оригинальное апи не стоит, т.к. это сложно в реализации и неудобно для пользователей, знакомых с мапкитом.

[DCrow]

Если так не хочется создавать объект с не существующей сессией, то можно такой вариант.

search_result.dart

class SearchResult {
  SearchSession session;
  SearchResponse response;

  SearchResult({
    required this.session,
    required this.response
  });
}

yandex_search.dart (тут убрал suggest для наглядности)

class YandexSearch {

  static const MethodChannel _channel = MethodChannel('yandex_mapkit/yandex_search');

  static int _nextId = 0;

  static int _getNextId() => _nextId++;

  static Future<SearchResult> searchByText({
    required  String        searchText,
    required  Geometry      geometry,
    required  SearchOptions searchOptions
  }) async {

    var id = _getNextId();
    var params = {
      'id': id,
      'searchText': searchText,
      'geometry':   geometry.toJson(),
      'options':    searchOptions.toJson(),
    };
    var response = await _channel.invokeMethod('searchByText', params);

    return SearchResult(
      session: SearchSession(id),
      response: SearchResponse.fromJson(response['response'])
    );
  }
}

yandex_search.dart

class SearchSession {
  late final MethodChannel _methodChannel;

  final int _id;

  int get id => _id;

  bool _active = true;

  bool get active => _active;

  SearchSession(this._id) :
    _methodChannel = MethodChannel('yandex_mapkit/yandex_search_session_$_id');

  Future<void> cancelSearch() async {
    if (!active) throw('SearchSession has been closed');

    await _methodChannel.invokeMethod('cancelSearch');
  }

  Future<SearchResponse> retrySearch() async {
    if (!active) throw('SearchSession has been closed');

    var result = await _methodChannel.invokeMethod('retrySearch');

    return SearchResponse.fromJson(result['response']);
  }

  Future<SearchResponse> fetchNextPage() async {
    if (!active) throw('SearchSession has been closed');

    var result = await _methodChannel.invokeMethod('fetchNextPage');

    return SearchResponse.fromJson(result['response']);
  }

  Future<void> close() async {
    if (!active) throw('SearchSession has been closed');

    await _methodChannel.invokeMethod('close');

    _active = false;
  }
}

search_page.dart

void search(String query) async {

    print('Search query: $query');

    responseByPages.clear();

    var result = await YandexSearch.searchByText(
      searchText: query,
      geometry: Geometry.fromBoundingBox(
        BoundingBox(
          southWest: Point(latitude: 55.76996383933034, longitude: 37.57483142322235),
          northEast: Point(latitude: 55.785322774728414, longitude: 37.590924677311705),
        )
      ),
      searchOptions: SearchOptions(
        searchType: SearchType.geo,
        geometry: false,
      ),
    );

    var session = result.session;
    var response = result.response;

    _sessions[session.id] = session;

    do {
      if (response.errorMsg != null) {
        print('Error: ${response.errorMsg}');
        await session.close();
        _sessions.remove(session.id);
        break;
      }

      var resStr = response.toString();

      print('Page ${response.page}: $resStr');

      setState(() {
        responseByPages.add(response);
      });

      if (!response.hasNextPage) {
        print('No more results available, closing session...');
        await session.close();
        _sessions.remove(session.id);
        break;
      }

      print('Got ${response.found} items, fetching next page...');
      await session.fetchNextPage();

    } while (response.hasNextPage);
  }

В таком случае будет явно возвращаться результат с сессией, которая существует.

Мне тоже не очень нравится. Мапкит возвращает сессию при запросе, хорошо бы делать так же.

Тут же мы управляем нативным кодом, всегда можем в нем при первом запросе инициализировать. Мне лично больше нравится такой вариант, чем возвращать SearchResult.

Юзеру легче работать становится, а думать о чем-то дополнительно не приходится. И вроде как логично, что до поиска нет смысла вызывать другие методы. Конечно может я тут и не прав, и лучше иметь полное отражение ситуации на native стороне...

[DCrow]

Придумывать что-то новое и натягивать на оригинальное апи не стоит, т.к. это сложно в реализации и неудобно для пользователей, знакомых с мапкитом.

Не всегда, зависит от того на сколько сложное апи было. Например если у тебя для того чтобы получить данные надо 10 методов вызвать в одной последовательности, то лучше дать один метод юзеру, чем его тоже нагружать всем(часто во всяких нативные апи на C такое встречал)

В данном случае, не получается дать какой-то более удобный инструмент, чем то, что нам было дано в нативной либе, так что придется близко к нему и придерживаться...

[ILAgent]

@DCrow твой вариант почти какой мой) Я не стал использовать название SearchResult, т.к. имхо result, response - много терминов. hasNextPage - метод сессии в мапките, думаю стоит так и оставить. Если ошибка прям в респонзе , то все поля нулабельные, как-то некрасиво, т.к. их много.

Но в целом вроде пришли к одному подходу)

[cream-cheeze]

Адаптировал свою последнюю имплементацию под интерфейс на фьючах, предложенный @ILAgent и @DCrow, но с учётом комментария, оставленного мной в соседнем PR– считаю, что так будет правильнее.

• пофиксил все оставшиеся замечания по этому ревью.

Android запилю позже – когда получу подтверждение, что всё устраивает.

[cream-cheeze]

Ещё добавил searchByPoint - для поиска адреса по точке.

[cream-cheeze]

Адаптировал свою последнюю имплементацию под интерфейс на фьючах, предложенный @ILAgent и @DCrow, но с учётом комментария, оставленного мной в соседнем PR– считаю, что так будет правильнее.

• пофиксил все оставшиеся замечания по этому ревью.

Android запилю позже – когда получу подтверждение, что всё устраивает.

Ладно, парни, не смотрите, если ещё не успели. @ILAgent вроде убедил, что не должно быть проблем с его реализацией в плане получения сессии - переделаю так же, тогда посмотрите.

[cream-cheeze]

Всё, flutter+ios можно смотреть. @DCrow @ILAgent

[cream-cheeze]

Android подъехал

[cream-cheeze]

Добьём? Чуть-чуть осталось...

[ILAgent]

извини, но туго со временем( постараюсь асап

[DCrow]

Ладно, парни, не смотрите, если ещё не успели. @ILAgent вроде убедил, что не должно быть проблем с его реализацией в плане получения сессии - переделаю так же, тогда посмотрите.

Да, там все выполняется по FIFO. Тут можно почитать, не много устарело, но в целом хорошо рассказывает о том как выполняется код в дарте(включая async).

[DCrow]

Надо бы еще с мастером конфликты разрешить

[cream-cheeze]

Мастер подмержу в свою ветку

[cream-cheeze]

Ага... Я вижу, правки @ILAgent по suggestions уже попали в master в рамках работы над роутингом... Это, конечно зря было сделано, в отдельной ветке надо было... Ладно, посмотрим, как будет мержиться...

[cream-cheeze]

@ILAgent, не подскажешь, в чём может быть проблема?

Почему-то не находит этот класс: import io.flutter.embedding.engine.plugins.lifecycle.FlutterLifecycleAdapter;

/Developer/Projects/flutter/github/cream-cheeze/yandex_mapkit/android/src/main/java/com/unact/yandexmapkit/YandexMapkitPlugin.java:13: error: cannot find symbol import io.flutter.embedding.engine.plugins.lifecycle.FlutterLifecycleAdapter; ^ symbol: class FlutterLifecycleAdapter location: package io.flutter.embedding.engine.plugins.lifecycle

Это в твоей ветке появилось, и почему-то не находится у меня..

[cream-cheeze]

В общем, запушил ветку со смерженным мастером, с разнесёнными suggest и search и всеми правками по ревью. Сумел проверить на iOS - всё ОК, на Android столкнулся с вышеуказанной проблемой - приложение не собирается, потому что ругается на отсутствие FlutterLifecycleAdapter.

[cream-cheeze]

@ILAgent, не подскажешь, в чём может быть проблема?

Почему-то не находит этот класс: import io.flutter.embedding.engine.plugins.lifecycle.FlutterLifecycleAdapter;

/Developer/Projects/flutter/github/cream-cheeze/yandex_mapkit/android/src/main/java/com/unact/yandexmapkit/YandexMapkitPlugin.java:13: error: cannot find symbol import io.flutter.embedding.engine.plugins.lifecycle.FlutterLifecycleAdapter; ^ symbol: class FlutterLifecycleAdapter location: package io.flutter.embedding.engine.plugins.lifecycle

Это в твоей ветке появилось, и почему-то не находится у меня..

Всё, разобрался с этим.

[cream-cheeze]

Можно смотреть, на Android тоже всё ОК.

[DCrow]

Теперь все супер Смерджил в мастер

[cream-cheeze]

Ура