deniskristov / checkbox-api-client

Java API client
https://checkbox.ua
3 stars 1 forks source link

Fixes and new methods for text/xml reports and text/pdf receipts view #8

Closed dimaloop closed 2 years ago

dimaloop commented 2 years ago

According to the docs - CARD type deprecated, so removed

dimaloop commented 2 years ago

also added related_receipt_id and previous_receipt_id fields to ReceiptSellPayload according docs

dimaloop commented 2 years ago

[+] getReportTextById method for text report view [+] getReportXMLById method for xml report view [+] getForString private service method for getting content without json mapper update version to 1.10.7

dimaloop commented 2 years ago

[+] getReceiptTextById method for text receipt view [+] getReceiptPdfById method for pdf receipt view

dimaloop commented 2 years ago

Fix getReceiptPdfById method to work with byte[] instead String:

There were only methods for working with string body: HttpResponse.BodyHandlers.ofString()

I added a method to work with bytes

Perhaps other committers will have ideas for unification of getForObjectImpl - I will gladly accept ideas!

dimaloop commented 2 years ago

[+] getReceiptPngById method for PNG receipt view with optional paper/content width [+] getReceiptQRCodeById method for QRCode of receipt (png image) [+] added getters with defaults settings

deniskristov commented 2 years ago

Thank you for the new features!!! There is the only recommendation, as mentioned in the Abstract principle:

Each significant piece of functionality in a program should be implemented in just one place in the source code. Where similar functions are carried out by distinct pieces of code, it is generally beneficial to combine them into one by abstracting out the varying parts.

so I'd prefer to have the only one getForObjectImpl() with

HttpResponse<byte[]> response = httpClient.send(request.build(), HttpResponse.BodyHandlers.ofByteArray());

instead of both getForObjectImpl() and getForBytesImpl() as they are pretty the same. To use this HttpResponse<byte[]> instead of HttpResponse<String> you have to change several parent methods, but this changes are simple.

deniskristov commented 2 years ago

And, please, use code formatting. For example it's better to have spaces in such peaces of code for better reading:

id+"/text"+(width>=10 && width<=250?"?width="+width:""

and also I'd prefer to assign optional param to a variable instead of a long string:

public String getReportTextById(String id,int width)
{
    return getForString(URI.create(apiPrefix + REPORTS_PATH + "/" + id+"/text"+(width>=10 && width<=250?"?width="+width:"")));
}

use something like this:

    public String getReportTextById(String id,int width)
    {
        String widthParam = 
            width >= MIN_WIDTH && width <= MAX_WIDTH 
                ? "?width=" + width 
                : "";
        return getForString(URI.create(apiPrefix + REPORTS_PATH + "/" + id + "/text" + widthParam));
    }

and of cause no magic numbers use constants please

deniskristov commented 2 years ago

And one more thing ) Please, pay attention to getReceiptTextById(String id) as it recursively calls itself (looks like it shouldn't).

dimaloop commented 2 years ago

And one more thing ) Please, pay attention to getReceiptTextById(String id) as it recursively calls itself (looks like it shouldn't).

Fixed, thank you

And, please, use code formatting.

changed

instead of both getForObjectImpl() and getForBytesImpl() as they are pretty the same. To use this HttpResponse<byte[]> instead of HttpResponse<String> you have to change several parent methods, but this changes are simple.

Can you provide code example? have no glue how to get responseFunction's type and implement instanceOf generic type

deniskristov commented 2 years ago

CheckboxApiClient.java.zip

Sure, I'm sharing my implementations

dimaloop commented 2 years ago

Tested, works as expected! Thank you!

dimaloop commented 2 years ago

I guess it's just a typo, but should be fixed:

 MAX_CHARS_PNG_RECEIPT <= 100

hard day, sorry. Thank you for your attention