ruyaoyao / LineAPI4J

LineAPI for Java.
MIT License
31 stars 17 forks source link

Fix fetch operations #20

Closed hanshsieh closed 7 years ago

hanshsieh commented 7 years ago

Sorry that this PR looks quite long. This PR is mainly for fixing three problems:

In addition to above changes, I also removed the dependency to Unirest.
I'm afraid you won't be comfortable for this change.
I did that because I don't like its design. The lib's purpose is to simplify the code, but it heavily use global settings, and static instances, which will become a big issue in multi-thread environment. And, it will also be very difficult to write unit test because it will be troublesome to mock static invocations.

By the way, the code uses 2 spaces for indentation. Do you mind to change it to 4 spaces? I think it is more commonly used.

hanshsieh commented 7 years ago

Hi, any suggestions or discussions are welcome.

ruyaoyao commented 7 years ago

Hi @sleepingpig

Thank you so much for putting efforts on LineAPI4J :) Interesting investigations! Just one thing that curious me, why did you remove sendFileWithURL? Is it going to be a problem if we keep this method?

The above amendments are going to improve the quality of this library, I will surly merge this PR to master.

For personal reason, I rarely update this project nor fix bugs. I am really happy someone is taking care it for me.

I don't really care about the indentation, it has 2 spaces indention because I used syntax template from google for my eclipse. That's why :P

Knowing you are also from Taiwan, perhaps you would like to be friend with me on FB for further discussion.

hanshsieh commented 7 years ago

Hi @cslinmiso, Thank you. The reason why I removed sendFileWithURL was because LINE server doesn't seem to support chunked encoding, so we must know the total size to be transferred beforehand. The current interface will store the whole input stream into memory, which may cause problems when the object is extremely large. I think whether it would be better to just keep interfaces for uploading files or images from files, and possibly byte array? If it is still preferred to keep the interface for uploading with URL for convenience, I think we could modify it to first download the file from the URL to a temporary file, then upload the file.

For the indentation, I search the Internet for a while, and it seems that both 2 and 4 spaces have their supporters. There's no "correct" answer. So, just ignore it.