Azure / azure-cosmosdb-java

Java Async SDK for SQL API of Azure Cosmos DB
MIT License
54 stars 61 forks source link

rx namespace collision with io.reactivex #13

Closed sgireddy closed 6 years ago

sgireddy commented 6 years ago

Many rx components within com.microsoft.azure.documentdb.rx are hiding native rx components forcing me to use root in scala, perhaps its better to leave the term rx where it belongs avoiding confusion. It took me hours to realize that rx components are being hidden... I had to explicitly import the components I needed.

import _root_.rx.{Completable, SingleSubscriber, Subscriber, Observable => JObservable}
import _root_.rx.functions.{Action0, Action1, FuncN, Functions}
import _root_.rx.lang.scala.JavaConversions._
import _root_.rx.lang.scala.Observable
moderakh commented 6 years ago

Can you please explain more what you mean by namespace collision?

Get Outlook for iOShttps://aka.ms/o0ukef


From: Shashi Gireddy notifications@github.com Sent: Friday, December 1, 2017 3:44:38 PM To: Azure/azure-documentdb-rxjava Cc: Subscribed Subject: [Azure/azure-documentdb-rxjava] rx namespace collision with io.reactivex (#13)

Many rx components within com.microsoft.azure.documentdb.rx are hiding native rx components forcing me to use root in scala, perhaps its better to leave the term rx where it belongs avoiding confusion. It took me hours to realize that rx components are being hidden... I had to explicitly import the components I needed. I believe this causes similar issues in Java as well even though I didn't verified my self.

import root.rx.{Completable, SingleSubscriber, Subscriber, Observable => JObservable} import root.rx.functions.{Action0, Action1, FuncN, Functions} import root.rx.lang.scala.JavaConversions._ import root.rx.lang.scala.Observable

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fazure-documentdb-rxjava%2Fissues%2F13&data=02%7C01%7Cmoderakh%40microsoft.com%7Ca562b2ab7e8d4278940208d539157c67%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636477686807187178&sdata=Oc0YjvYFIFxDIWQxWwCgc9tOg3PoOrkb2fIUFRlzYiw%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAVP1-BGSNT9UlGWRgSV8-NKRYXn4XpJYks5s8I9lgaJpZM4QzDrY&data=02%7C01%7Cmoderakh%40microsoft.com%7Ca562b2ab7e8d4278940208d539157c67%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636477686807187178&sdata=z1wmilVsKuTWFr%2BnFfgJ0IWqnR50QcuuBQnldTa5%2Fjk%3D&reserved=0.

sgireddy commented 6 years ago

This issue is specific to Scala. The crux of the problem is with the following import statement:

 import com.microsoft.azure.documentdb._

Scala uses relative package naming which means above statement automatically imports the package rx from com.microsoft.azure.documentdb. This means it hides all packages within the real package "rx" from RxJava. You might argue that "isn't it a bad idea to to import everything from documentdb or even documentdb.rx", I would say "Yes, it is" but when we work on POCs we don't know many classes within the relatively new API so for the sake of convenience we import everything then clean up later. Also given the number of classes defined within azure.documentdb tools like IntelliJ automatically translate to . after seeing use of certain number of classes, so use of . is pretty common.

root is the construct that forces to use absolute package naming. So I had to use it this way in order to overcome the problem.

Here are some more details: https://www.scala-lang.org/old/faq/3.html

Given "rx" is the centerpiece of this library, I would recommend renaming com.microsoft.azure.documentdb.rx to something else, saving time for many naive programmers like me.

Update: I was able fix this by taking advantage of the import order by importing rx._ first. But I think this is going to be a nagging issue for many developers.

import rx._
import rx.functions._
import com.microsoft.azure.documentdb._

instead of

import com.microsoft.azure.documentdb._
import _root_.rx._
moderakh commented 6 years ago

@sgireddy I haven't done much scala programming myself so your comments on your experience when using this in Scala is helpful.

Two questions for you:

1) Please see mongo's rx driver classes, e.g, com.mongodb.rx.client.MapReduceObservable http://mongodb.github.io/mongo-java-driver-rx/1.1/javadoc/com/mongodb/rx/client/MapReduceObservable.html

Does mongo's rx driver have the same problem on Scala?

2) our rx driver is already used in a scala package: https://github.com/Azure/azure-cosmosdb-spark/blob/master/src/main/scala/com/microsoft/azure/cosmosdb/spark/CosmosDBSpark.scala#L30-L31

Can you follow the pattern used in the above link? do you see any problem with this approach?

Thanks