findologic / findologic-api

Library for sending requests to the Findologic API
MIT License
1 stars 3 forks source link

Dev #4

Closed TheKeymaster closed 6 years ago

TheKeymaster commented 6 years ago

@howard It would be great if you could check my changes and maybe you still have some more suggestions for me :). Thanks in advance for your help!

TheKeymaster commented 6 years ago

@howard Thanks for your comments! I will try to fix those as soon as I have time.

Regarding your question about the architecture: The users should handle request objects that handle a client under the hood. Internally the Request::send() (not static) method should return a Response object that is not implemented yet, but should have convenient functions such as Response::getArticles(), Response::getFilters() or Response::getBackendServer(), etc. (all not static of course).

The getFilters method would return an array of Filter and the class might look like this:

// Written in the browser, syntax might be incorrect.

class Filter {
    /** @var string $name */
    public $name;

    /** @var string $display */
    public $display;

    /** @var Select $select */
    public $select;

    /** @var int $selectedItems */
    public $selectedItems;

    /** @var Type $type */
    public $type;

    /** @var Item[] $items */
    public $items;
}

I think you can get it with this.

Please tell me what you think about this idea and if you would go in a different direction. If yes please tell me why.

TheKeymaster commented 6 years ago

After a second thought it might be better to pass parameters directly to a client, without a request object. Let me think about this I will comment again in like 3-5 hours.

TheKeymaster commented 6 years ago

Yesterday I had no time left, but I thought about it a while. I think its better if we pass parameters directly to a client, without a request object. I will try to implement it soon.

TheKeymaster commented 6 years ago

@howard I am refactoring atm and I hope for a quick answer that you might have for me. I am sure you know the issue that you cannot give empty arrays to http_build_query, but FINDOLOGIC requires attrib[Color][]=Black. Do you know a way to still use the function above instead of writing an own function for the parameter building?

EDIT: I have experienced that the API responds with a 500 error when calling attrib[Filter]=value without the second []. I am not sure if this is expected behavior, because the API should not fail when requesting something that is in a wrong format.

EDIT 2: Nevermind I have found a solution for my issue. Sorry for disturbance.

TheKeymaster commented 6 years ago

This PR is closed since the work is continued in #5.