Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

Hotfix/Preetify the AIResponse And Result #10

Closed
wants to merge 6 commits into from
Closed

Hotfix/Preetify the AIResponse And Result #10

wants to merge 6 commits into from

Conversation

anujpradhaan
Copy link

We don't have a way to user generated sessionId's. This change will provide a functionality related to the same.

@anujpradhaan
Copy link
Author

I guess the test case is an OLD issue. Even I had to work after commenting that out.

@folomeev
Copy link
Contributor

Hello @nujpradhaan!

Thank you for your interest to the project!

Would you be so kind to describe what profit new CustomAIServiceContext class gives comparing to calling
ai.api.AIServiceContextBuilder#buildFromSessionId(String sessionId) method?

What do you think, how justified is to add 0,5 MB library into dependency to call single method in a single place? May there be a better way to prettify toString() output?

You added import org.apache.commons.lang3.builder.ToStringBuilder; line into AIResponse.java file but never used it. Is it correct?

@anujpradhaan anujpradhaan changed the title Hotfix/custom service context Hotfix/Preetify the AIResponse And Result Feb 21, 2017
@anujpradhaan
Copy link
Author

@folomeev Didn't see that StringUtils.isEmpty is already implemented in the project. Found it, thus removing the apache commons library.

CustomAIServiceContext looked more intuitive to me because a developer using this project will straight away look a class which can provide custom sessionId functionality. After a handful of thoughts, decided to remove it. Because, AIServiceContextBuilder#buildFromSessionId(String sessionId) provides more abstraction and ultimately both will implement the same interface. A factory kind of method provides better abstraction.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants