Skip to content

Refactor return value of Jobs API functions #80

Description

@dimitrystd

I noticed that all functions that call BaseApiAbstract->sendRequest() return bool. An example,

/**
 * Returns a job.
 *
 * @param string $jobId
 * @return bool
 */
public function getJob($jobId)
{
    $requestData = $this->getDefaultRequestData('query', []);
    $request = $this->prepareHttpRequest('jobs/' . $jobId, $requestData, self::HTTP_METHOD_GET);
    return $this->sendRequest($request);
}

But if you look at sendRequest() more close that it can return only:

  • Some data (JSON or file body)
  • true
  • Exception

In example above we don't expect data, thus it means our function can return only true.
Such "contract" may confuse end-developer. If i see return bool then i add if () then statement in code. But actually i should add try catch.

So we should review SDK methods and remove return statement if it returns only a single value. Let's refactor only Jobs API because we don't use it yet.

DoD:

  • Update following methods:
    • createJob
    • updateJob
    • cancelJob
    • listJobs
    • getJob
    • authorizeJob
    • addFileToJob
  • Update method documentation. Make it clear that it doesn't return value, but may throw exception
@throws \Smartling\Exceptions\SmartlingApiException

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions