Modify Function `Room.create` return type, from `Promise<Room>` to `Promise<Room|null>` #616

lijiarui · 2017-06-28T06:23:23Z

Now, function Room.create() return type as follows:

public static create(contactList: Contact[], topic?: string): Promise<Room> {

I suggest it return Promise<boolean> to detect whether it can create room successfully

Related logs as follows:

If it can create room successfully, return as follows:

Bot createDingRoom() new ding room created: %s Room {
  domain: null,
  _events: {},
  _eventsCount: 0,
  _maxListeners: undefined,
  id: '@@356afb5f9c9491eb34158dd1f11b99e136d104c9055106545869cbba1b0fec1c' }

If it cannot create room successfully, throw the following exception:

02:22:13 ERR PuppetWebBridge roomCreate(@ac4db07084d16fe10555569c660e5b8526d162bdc0acca2e93dd72d322f22b4a,@35f68307d4ec6b6e0eabead00e446127) exception: undefined
02:22:13 WARN PuppetWeb roomCreate(@ac4db07084d16fe10555569c660e5b8526d162bdc0acca2e93dd72d322f22b4a,@35f68307d4ec6b6e0eabead00e446127, 网球黑龙江1群) rejected: undefined
02:22:13 ERR Room create() exception: [object Object]

The text was updated successfully, but these errors were encountered:

huan · 2017-06-28T06:33:32Z

Could you explain the pros and cons between the two design?

A good purpose should go with good reasons.

lijiarui · 2017-06-28T06:49:49Z

After I thought more, maybe it return Promise<Room | null> will be better.

If it cannot create a room, we want to get the fail status and don't see more error.

Like Contact.find() we will return Promise<Contact | null> instead of throw Error if it cannot find a contact.

And Like Contact.alais(string) we will return Promise<boolean> instead of throw Error if it cannot alias a contact successfully

huan · 2017-06-28T16:19:06Z

Yes, that's the right answer after you think more.

Please think more before post question/suggestions in the future, at least explain your ideas with pros and cons in detail.

lijiarui · 2017-07-01T13:39:10Z

But now it returns and throw error, maybe is should return <Room | null> instead of throw error.

huan · 2017-07-01T15:17:30Z

I agree your purpose.

Please change the title of this issue to better reflect your purpose, and I'll label it as a enhancement

huan · 2017-07-02T15:07:12Z

If we can not find a contact by name, it's not an error because the reason will be there's no contact with that name.
However, if we can not create a room, there must be something wrong.

In CASE 1 we should return null, in CASE 2 we should throw an exception.

So my question is: when will null be returned by Room.create() is necessary, and why?

huan · 2017-07-15T02:15:17Z

Close this issue due to inactivity for almost two weeks.

huan · 2017-07-24T10:14:48Z

We should not return null.

We are planning to throw an Exception instead.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify Function `Room.create` return type, from `Promise<Room>` to `Promise<Room|null>` #616

Modify Function `Room.create` return type, from `Promise<Room>` to `Promise<Room|null>` #616

lijiarui commented Jun 28, 2017 •

edited

Loading

huan commented Jun 28, 2017

lijiarui commented Jun 28, 2017 •

edited

Loading

huan commented Jun 28, 2017 •

edited

Loading

lijiarui commented Jul 1, 2017

huan commented Jul 1, 2017

huan commented Jul 2, 2017

huan commented Jul 15, 2017

huan commented Jul 24, 2017

Modify Function Room.create return type, from Promise<Room> to Promise<Room|null> #616

Modify Function Room.create return type, from Promise<Room> to Promise<Room|null> #616

Comments

lijiarui commented Jun 28, 2017 • edited Loading

If it can create room successfully, return as follows:

If it cannot create room successfully, throw the following exception:

huan commented Jun 28, 2017

lijiarui commented Jun 28, 2017 • edited Loading

huan commented Jun 28, 2017 • edited Loading

lijiarui commented Jul 1, 2017

huan commented Jul 1, 2017

huan commented Jul 2, 2017

huan commented Jul 15, 2017

huan commented Jul 24, 2017

Modify Function `Room.create` return type, from `Promise<Room>` to `Promise<Room|null>` #616

Modify Function `Room.create` return type, from `Promise<Room>` to `Promise<Room|null>` #616

lijiarui commented Jun 28, 2017 •

edited

Loading

lijiarui commented Jun 28, 2017 •

edited

Loading

huan commented Jun 28, 2017 •

edited

Loading