-
-
Notifications
You must be signed in to change notification settings - Fork 349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Encapsulation is broken #104
Comments
Hi, thanks for stating your views. I do understand your concern but FbBot is designed this way so that you can have any number of bots in a single application by just extending |
Really? And how any number of bots relate to broken encapsulation? |
Looks like you have no idea what is encapsulation and lazy enough to read and understand my point. Well, at least you had a chance to learn something. |
Such hauteur and derisive comments were uncalled for. Was I in anyway rude to you? Anyways, just pontificating won't help anyone here, we both maybe talking two completely different things. If you really want to help you can either raise a PR or at least show some code snippets to expound your point clearly. Only one point I could deduce from your comments, i.e, instead of creating the But do keep in mind that JBot is more of a boilerplate than a library (as per my view) for making bots because of the fact that it is made on spring-boot which is a big dependency. This wasn't the best decision to use spring-boot for JBot but that's another story. |
@rampatra |
So you replied with bullshit, then thought for 2 days, found that what I wrote was correct and fixed that here https://github.com/rampatra/jbot/pull/106/commits |
@laptevn In the PR you mentioned, I have just moved the application properties to a class as I had many similar url with a slight difference. I actually followed the approach in this PR #95 as it seemed logical to me. Users still need to do these in their bot classes: /**
* Set this property in {@code application.properties}.
*/
@Value("${fbBotToken}")
private String fbToken;
/**
* Set this property in {@code application.properties}.
*/
@Value("${fbPageAccessToken}")
private String pageAccessToken; I think you also wanted to get rid of this. Anyways, if my above PR aligns with your suggestion then I am glad. I see you had one more suggestion of taking out |
To create new Facebook bot using JBot I need to do the following:
1 Define fbSubscribeUrl, fbSendUrl and fbMessengerProfileUrl properties.
2 Define RestTemplate bean.
All these are internal implementation details that JBot users don't care at all. Having them complicates workflow and introduces copy paste from bot example.
I would suggest the following:
1 Hide internal properties behind JBot code and use Bot#getPageAccessToken method where you need page access token.
2 Hide RestTemplate bean behind JBot code.
So after the fix JBot users will need just to add it to dependencies, extend Bot class and use proper annotations.
Easy and quick and no strange magic code is required.
Please share your vision on this. If everybody is happy I will introduce a PR with a fix.
The text was updated successfully, but these errors were encountered: