add chainable method for returning flash object

[sbatson5]

What's in this PR

This should address the issue raised by #154. A fluent API allows for chaining methods, but does not give access to the flash object. For users who want to manipulate the object directly, they can access it with the getFlashObject method, which will return the last object added to the queue.

• Add chainable method that returns the flash object
• Add test coverage in service test
• Update Readme to include documentation on getFlashObject

[sbatson5]

@poteto Thoughts on if this is acceptable based on the existing pattern? Addresses #154 but it isn't quite as fluent. Also, thoughts on renaming to getLastFlashObject since it is a public method and could be called anywhere?

[panthony]

IMO getLastFlashObject holds more meaning than getFlashObject.

Or since we are talking about a queue of message, why not provide accessors such as peekFirst and peekLast ?

They would return undefined for an empty queue

[sbatson5]

@panthony trying to wrap this up today. I like peekLast and peekFirst but what about a specific index? Or should we not worry about that?

I kind of like getFlashObject because it implies that it is chainable and returns the flash object we just added. If someone wants to get a specific index, they can access the queue from the service directly.

[panthony]

@sbatson5 well I first I though of peekAt(idx) that would be called by peekFirstand peekLast but it was maybe too much as the requirement was only to retrieve the last.

[panthony]

Here's my current implem:

cogniteev@21727b0

And as I reviewed again I just noticed that I have an unnecessary call to get(this, 'queue') ...

[sbatson5]

Awesome!

I'm thinking of maybe having all 3 methods. I like the readability of:
flash.add({ message: 'foo' }).getFlashObject(); as it returns the object we just added. peekLast doesn't make sense to chain.

Then we can still have peekLast and peekFirst but encourage that they aren't chainable but are public calls to our service.

Thoughts?

[homu]

☔ The latest upstream changes (presumably 29351d4) made this pull request unmergeable. Please resolve the merge conflicts.

[sbatson5]

@panthony let me know what you think of a395f2c

[panthony]

@sbatson5 wow I was not aware of the firstObject and lastObject accessor. That's pretty convenient. Never too late to learn something.

I'd say that maybe getFlashObject is redundant but it's a matter of taste I guess. Not sure why peekXXX would be 'less chainable'.

And didn't you say that getLastFlashObject was better ?

Or maybe rename peekXXX by get{First|Last}FlashObject so that you have the context of what you get no matter where you call the method ?

[sbatson5]

@panthony I'm just thinking of the naming convention: flashMessageService.add(object).getFlashObject seems readable to me as it returns the object I just added.

flashMessageService.add(object).peekLast seems like I am adding an object and then looking at the last one in the queue for some reason. We understand the context of how the queue works, but if you were new to this addon and read that, it wouldn't be clear why you would do it that way. The name doesn't imply that we are returning the result of our add. So in essence, getFlashObject is just an alias of peekLast but I think it is a bit more idiomatic when we are chaining.

[panthony]

@sbatson5 Ok ! So this is looking great in that case :)