Make Intercom Credentials configurable on a Per - Object Basis about intercom-java

[martina-if]

Also, using static methods is generally bad practice. It makes things like this ^ and testing very hard.

From an API point of view it would be easier to use if you could create instances, here is an idea:

IntercomSession intercom = new IntercomSession(apiKey, apiId,...);
UserResource user = intercom.userResource();
user.list()
user.create(new User());

With this approach consumers of your library could:

• Pick which intercom app to talk to
• Mock the connection to intercom making testing easier

[esiqveland]

The current very static-based api makes it nearly impossible to use this SDK with multiple app-ids, requiring a separate java application for each api key.
This makes it very awkward to use, so please consider giving a builder-based api.

Something like:

Intercom app1 = Intercom.create()
        .withToken(API_TOKEN_APP1)
        .withAppId(appid1)
        //... whatever more
        .build()

Intercom app2 = Intercom.create()
        .withToken(API_TOKEN_APP2)
        .withAppId(appid2)
        //... whatever more
        .build()

or maybe require the non-optional settings directly as paramters to the create or build method.

[dehora]

@prismec thanks for raising this. This looks like a good idea at first glance; will want to think through the API signatures a bit - guessing based on the example, your preference is to supply an app context object?

[prismec]

The example uses the session (IntercomSession) as a factory for further resource accessors (e.g. UserResource). This is a common pattern found in many APIs. From a testing point of view it would be good if the factory, as well as all resources are encapsulated using interfaces, so that it’s much easier to mock it for testing purposes.

---

Smarter Ecommerce GmbH

Spittelwiese 15
4020 Linz

T [phone]
[email protected]
http://www.smarter-ecommerce.com http://www.smarter-ecommerce.com/

UID: ATU 63760825

smec – we automate ecommerce success

On 21 Dec 2015, at 12:22 , Bill de hÓra [email protected] wrote:

@prismec https://github.com/prismec thanks for raising this. This looks like a good idea at first glance; will want to think through the API signatures a bit - guessing based on the example, your preference to supply an app context object?

—
Reply to this email directly or view it on GitHub #89 (comment).

[choran]

Hi @prismec,
We were just reviewing some older issue and wanted to check in on this one.
Unfortunately we have not been able to make any specific changes with regard to this issue.
So as far as I can see I think this is still a valid issue to remain open.
I just wanted to check in with you to see if you had any other thoughts on it in the meantime or were there any changes that might have been related to this issue?
Like I said, it sounds like it is still valid to leave open as an issue but wanted to confirm with you.

Thanks
Cathal

[danielcompton]

This still seems like a good feature to have to me, it's something I'd find very useful.

[xgp]

I also had the problem of using this lib for multiple appIDs and tokens (on behalf of OAuth'd customers). I made a version of the library which eliminates the static config, and passes the config object to every service method that requires it.

My fork is here: https://github.com/xgp/intercom-java

You can use it like this example:

Intercom intercom = new Intercom();
intercom.setAppID("put_app_id_here");
intercom.setToken("put_token_here");

AdminCollection admins = Admin.list(intercom);
while(admins.hasNext()) {
    Admin admin = admins.next();
    System.err.println("Admin: "+admin.toString());
}

Note, this is not a drop-in replacement for the official library from Intercom, as all of the service methods require the Intercom config object.

[thewheat]

Thanks so much for sharing @xgp 👏 Definitely a good starting point but as you mentioned it isn't a drop-in replacement due to a breaking change in the code

I've tagged this issue as "help wanted" as I don't think I'd have the resources to look into this right now but definitely open to ideas and PRs to see how we can get this moving forward 👍