Comments 19
@Injectable({ providedIn: 'root' })
export class GeolocationService {
private geolocation?: Geolocation = this.document.defaultView?.navigator?.geolocation;
constructor(@Inject(DOCUMENT) private document: Document) {}
isSupported(): boolean {
return !!this.geolocation;
}
getCurrentPosition(options?: PositionOptions): Observable<Position> {
const geolocation = this.getGeolocationOrThrowError();
return new Observable<Position>((subscriber) =>
geolocation.getCurrentPosition(
(position) => {
subscriber.next(position);
subscriber.complete();
},
(positionError) => subscriber.error(positionError),
options,
),
);
}
private getGeolocationOrThrowError(): Geolocation {
assert(this.geolocation, 'Geolocation is not supported in your browser');
return this.geolocation;
}
watchPosition(options?: PositionOptions): Observable<Position> {
const geolocation = this.getGeolocationOrThrowError();
return new Observable<Position>((subscriber) => {
const watchPositionId = geolocation.watchPosition(
(position) => subscriber.next(position),
(positionError) => subscriber.error(positionError),
options,
);
return () => geolocation.clearWatch(watchPositionId);
});
}
}
На его основе я бы смог расшарить observable дальше как мне нужно (да, может быть сделать один глобальный position$).
Но через применение наследования вы смешали абстракции. Первая абстракция — адаптер над АПИ. Вторая абстаракция — расшаренный observable. (Я так считаю)
Сорри за оффтоп, просто мысли вслух.
Интересно, что в последнее время так много разговоров о том, что наследование — это зло и намного выгодней заменять его композицией и/или абстракцией. Есть подозрение, что скоро его настигнет судьба оператора goto… Но если подумать, наследование — это единственное, что качественно отличает ООП от других парадигм. Остальные "киты" так или иначе реализуемы в рамках не-ООП языков. Не хочу сказать, что ООП ожидает та же судьба, что и наследование, оно так и останется отличным инструментом структурирования кода. Но сколько новых холиваров это породит — сложно представить :)
через применение наследования вы смешали абстракции
Если расшаренный observable добавляет лишню ответственность (с этим можно подискутировать отдельно, но я тут про другое), то мы его просто уберем из конструктора GeolocationService. Переложим ответственность на консумеров. А наследование-то при этом остается. То есть в данном случае наследование как таковое ничего не смешало.
Признаюсь: наследование от observable, используемое в посте автора, для меня тоже выглядит чужеродно, непривычно. Но немного поразмыслив, я так и не нашел логического обоснования своей неприязни. Возможно, просто вкусовщина.
А, подумал еще немного :). Там наверху запостили про избыточные токены. Насколько я вижу, у топикстартера они тут используются для конфигурации.
Так вот, конфигурировать через DI сложнее, чем через параметры метода. Допустим, конфигурацию нам надо получить в рантайме с нашего сервера. Может, ключ API вообще разный у каждого залогиненного юзера. В случае топикстартера, это будет обертка типа
fetchConfigFromApi().pipe(switchMap(config =>
new GeolocationService(
this.geolocation, // OOPS - where should we get it from?
this.geolocationSupported, // leaked implementation details
config
)
))В случае, когда GeolocationService не является observable, а возвращает его из публичного метода, получается чище:
fetchConfigFromApi().pipe(switchMap(config =>
this.geolocationService.getGeolocation(config)
))
Конфигурация через DI хороша там, где оно не меняется. Через параметры вызова там, где меняется часто. Можно и совмещать эти два подхода. Что-то типа persistent config с возможностью переопределить в отдельных случаях. Пример такого подхода можно увидеть у нас в ng-dompurify — либы для использования DOMPurify в качестве санитайзера Angular:
https://habr.com/ru/company/tinkoff/blog/459396/
А зачем у вас @Inject(GeolocationService) при внедрении сервиса? Так вам приходится дублировать тип для внедряемой переменной. Почему не просто
constructor(private readonly position$: GeolocationService)navigator.geolocation.watchPosition(this.success, this.error);
Дальше читать не стал. Как эти откровения попали в топ? Извините, наболело, болезнь кодревьюера.
Это пример того, как работает НАТИВНЫЙ API.
А что конкретно в примере автора не будет работать как есть, без байнда/стрелки?
Суть вопроса в том, что, насколько я вижу, код в примере полностью рабочий.
Зачем же так резко? Не Вам этот код поддерживать. А чтоб уменьшить шансы того, что подобный код придется поддерживать, можно было бы и потрудиться дать развернутый ответ. Часто из комментов получаешь больше важной информации, чем из самой статьи.
Постараюсь ответить на коммент выше. Проблема в том, что при передаче метода в качестве аргумента теряется его контекст. То есть это будет работать, пока в передаваемом методе нет использования this. Если вдруг Вы захотите в методе success использовать что-то вроде this.message('success'), то получите ошибку. И без соответствующих тестов ловить ее придется в рантайме. Именно поэтому часто можно увидеть костыли типа this.handleClick = this.handleClick.bind(this); (строка из документации к реакту). Больше информации здесь https://learn.javascript.ru/bind
Сказанное выше справедливо для ES объектов. На счет TS — не уверен, может в TS эта проблема решена (хотя я в этом очень сомневаюсь). Проверить возможности нету, пишу с электрочайника. Если кто-то знает — просветите, плз.
PS: то, что код "полностью рабочий", не делает его хорошим.
В ТС, кончено, всё точно так же. Код этот написан так, просто чтобы кратко показать, что оно работает на коллбэках. Автору статьи, уверен, и в голову не пришло, что кто-то станет читать статью про Ангуляр и обзёрваблы не зная, как работает this в таких ситуациях. Это всё равно, как докопаться, что там в коде функция doSomething в итоге глобальная, а значит либо оно совершенно бесполезное, либо адовый сайдэффект.
Observable сервисы в Angular