Pussy: Refactoring. Part three or brushing roughness

In the first and second parts of a series of articles, we did a little work on the separation of the code and those unnecessary actions that we wrote in one function. Basically, we dealt with the classes HttpClient and Cache, and their different implementations, to write a test client for api pussies.

Data presentation

Before that, we paid a lot of attention to the behavior and the general structure of the code, but forgot about the data we are dealing with. Now, we all have strings, including the return value of CatApi :: getRandomImage (). That is, by calling this method, we “know” that we will get a string. I say “know,” since PHP can return everything — an object, a resource, an array, etc. Nevertheless, even in the case of RealCatApi :: getRandomImage () we can be sure that a string will come to us, since we explicitly give the value to it, we cannot say for sure that this string will be “useful” (validna) for the one who called this method: it could be an empty string, a string that does not contain a URL (such as "I am not a URL"), and so on.

class RealCatApi implements CatAPi { ... /** * @return string URL of a random image */ public function getRandomImage() { try { $responseXml = ...; } catch (HttpRequestFailed $exception) { ... } $responseElement = new \SimpleXMLElement($responseXml); return (string) $responseElement->data->images[0]->image->url; } } 

To make our code more accurate and reliable, a good solution would be to make sure that we return the correct value.

The first thing we can do is check the post-conditions of our method:
')

 $responseElement = new \SimpleXMLElement($responseXml); $url = (string) $responseElement->data->images[0]->image->url; if (filter_var($url, FILTER_VALIDATE_URL) === false) { throw new \RuntimeException('The Cat Api did not return a valid URL'); } return $url; 

Although correct, it can be hard to read. It will be even worse if there are some more functions that also require such checks. You need to reuse this validation logic somehow. In any case, the return value is still the same useless string and it would be cool if we assure everyone that this is a URL. In this case, already any part of the program that uses the CatApi :: getRandomImage () method will know exactly what the URL is, there are no errors in it, and in general it is not some email.

URL Value Object

Instead of writing post-conditions for CatApi :: getRandomImage () implementations, we can write pre-conditions for the URL of our images. How do we make sure that the URL of the image can only exist if it is valid? That's right, let's create another object and make sure that it is not created using something that is not a valid address:

 class Url { private $url; public function __construct($url) { if (!is_string($url)) { throw new \InvalidArgumentException('URL was expected to be a string'); } if (filter_var($url, FILTER_VALIDATE_URL) === false) { throw new \RuntimeException('The provided URL is invalid'); } $this->url = $url; } } 

This type of object is called a value object .

Such an object cannot be created incorrectly:

 new Url('I am not a valid URL'); // RuntimeException: "The provided URL is invalid" 

Now any object of the URL class exactly represents a valid address; there can be no other. We can change the code of our function again to return a new object:

 $responseElement = new \SimpleXMLElement($responseXml); $url = (string) $responseElement->data->images[0]->image->url; return new Url($url); 

Usually, value objects have methods for creating them from primitive types and converting them into them so that they can be prepared for saving / loading, or creating from other value objects. For this case, we need the methods fromString () and __toString (). Also, these methods lead to the possibility of implementing methods of parallel creation (such as fromParts ($ scheme, $ host, $ path, ...)) and special getters (host (), isSecure () ..). Of course, you should not write these methods before they are really needed.

 class Url { private $url; private function __construct($url) { $this->url = $url; } public static function fromString($url) { if (!is_string($url)) { ... } ... return new self($url); } public function __toString() { return $this->url; } } 

Now we need to change the getRandomImage () method and make sure that the default value for the image is also returned as a URL object:

 class RealCatApi implements CatAPi { ... /** * @return Url URL of a random image */ public function getRandomImage() { try { $responseXml = ...; } catch (HttpRequestFailed $exception) { return Url::fromString('http://cdn.my-cool-website.com/default.jpg'); } $responseElement = new \SimpleXMLElement($responseXml); return Url::fromString((string) $responseElement->data->images[0]->image->url); } } 

Naturally, such changes will be reflected in the Cache interface and any class that implements it (FileCache, for example) - so you need to accept and return URL objects:

 class FileCache implements Cache { ... public function put(Url $url) { file_put_contents($this->cacheFilePath, (string) $url); } public function get() { return Url::fromString(file_get_contents($this->cacheFilePath)); } } 

Parse the XML response

It remains to change this part of the code:

 $responseElement = new \SimpleXMLElement($responseXml); $url = (string) $responseElement->data->images[0]->image->url; 

Honestly, I myself do not like SimpleXML, but the problem is not in it. The problem here is that we always expect to get a valid response containing a root element, containing one element called data, which contains at least one images element, containing one image element, which has one url element, the value of which is it seems to us that the string is the URL address. At any point of this chain, the assumption may be erroneous and this will lead to an error.

Our task at the moment is to handle these errors, instead of PHP throwing exceptions. To do this, we again define our exception, which we will then catch. Again, we need to hide all the details about what element names and hierarchy the XML response exists for. Such an object should also handle any exceptions. The first step is to introduce a simple DTO (data transfer object), representing the essence of the image from apics.

 class Image { private $url; public function __construct($url) { $this->url = $url; } /** * @return string */ public function url() { return $this->url; } } 

It is seen that this DTO hides such concepts as, and other elements of the answer. We are only interested in the URL address, so we make it possible to access it via a simple getter url ()

Now the code in getRandomImage () looks something like this:

 $responseElement = new \SimpleXMLElement($responseXml); $image = new Image((string) $responseElement->data->images[0]->image->url); $url = $image->url(); 

As you can see, this is not very helpful yet, since we are still dependent on this chain of XML elements.

Instead of creating a DTO directly, it is better to do this through a factory, which will know what kind of structure the XML response will have.

 class ImageFromXmlResponseFactory { public function fromResponse($response) { $responseElement = new \SimpleXMLElement($response); $url = (string) $responseElement->data->images[0]->image->url; return new Image($url); } } 

It remains only to embed the ImageFromXmlResponseFactory instance into the RealCatApi class, which reduces the code in the RealCatApi :: getRandomImage () method to the following state:

 $image = $this->imageFactory->fromResponse($responseXml); $url = $image->url(); return Url::fromString($url); 

Delivering code in this way makes it possible to write some things in a better way. Smaller classes are easier to test. As I pointed out earlier, our tests are still testing mostly the “ideal scenario” and most of the boundary cases are not covered by tests. Here, for example, what can happen.

1. Empty response from server
2. Broken XML
3. Valid XML whose structure has changed
4. ...

Carrying out the logic of processing XML in separate classes, you can focus only on those things that are responsible for this task. And this makes it possible to use real TDD, in which you define the situation (as in the list above) and the expected results.

Conclusion

On this, the “Refactoring the Cat API client” series ( Pussies: Refactoring ) comes to an end and I hope you like it. If you have any suggestions for improvement, please post comments. Good luck!