# It is an argument, not a trojan horse!


![image.png](https://cdn.hashnode.com/res/hashnode/image/upload/v1608163500900/g6-OckHFj.png)


Let's talk about something cool: trojan horse parameters.
This is how I call this fancy technique which many of us, developers, might have used (or still doing). Ok, so what is it?

As a daily work, you face a problem, think about a solution and start writing a piece of code for it, then you pack it into a [hopefully] well-named function.

Once you're done, a bright idea comes to your mind telling you: Hey! why not extend your solution so that it also includes** "case B"**? You think a bit and answer: Yes! why not?

Let's illustrate this through an example: You work on a class which is responsible for data persistence, and there are two ways of doing this: either local storage or remote/cloud storage. 

And as you appreciated the pop-up idea that asked you to extend your solution so that it covers both cases, you ended up writing this nice code:

```
<?php

class FileStorage
{
    public function store(File $file, $remote = false): void
    {
        if ($remote) {
            $compressedFile = $this->compress($file);
            $fileParts = $this->splitFile($compressedFile);
            $this->logger->info('Started uploading to AWS/S3 started.');
            foreach ($fileParts as $filePart) {
                $this->awsClient->upload($filePart);
            }
            $this->logger->info('Finished uploading to AWS/S3 started.');
        } else {
            $fp = fopen($this->getStoragePath() . $file->getName(), 'w');
            fwrite($fp, $file->getContent());
            fclose($fp);
            $this->logger->info('File successfully saved on local storage.');
        }
    }
}
``` 

Ok, can you tell me what's wrong with that thing above? I can tell you my point of view:

![image.png](https://cdn.hashnode.com/res/hashnode/image/upload/v1608164726244/RAn832VSJ.png)

If you have a look at it twice, you'll realise the *$remote* parameter is actually acting as a trigger or switch button, responsible for a complete change of the function's behavior. You can conclude that in case of *$remote = true* we have a totally standalone logic pushing data to S3/AWS, and in case *$remote = false* a second different logic is writing data to local disk.

Which rule are we violating here?  

- [Single Responsibility Principle](https://en.wikipedia.org/wiki/Single-responsibility_principle) ?
  
-  [Separation of Concerns](https://en.wikipedia.org/wiki/Separation_of_concerns) ? 

- [Modularity](https://en.wikipedia.org/wiki/Modular_programming) ?  

-  [Rigidity](https://notherdev.blogspot.com/2013/07/code-smells-rigidity.html)  ?

This tiny little optional parameter turned into a trojan horse that changes the logic of the function 180 degrees, in a totally silent way.

So, why not be transparent, modular, flexible and single responsible?
Besides, no big effort must to be taken here, just **split and rename ! **
The only thing we have to do is to get rid of the optional/default parameter and move each block under the if statement into a separated new function, which name should reflect the actual task being achieved; something like this:

```
 <?php

class FileStorage
{
    public function saveToDisk(File $file): void
    {
        $fp = fopen($this->getStoragePath() . $file->getName(), 'w');
        fwrite($fp, $file->getContent());
        fclose($fp);
        $this->logger->info('File successfully saved on local storage.');
    }

    public function upload(File $file): void
    {
        $compressedFile = $this->compress($file);
        $fileParts = $this->splitFile($compressedFile);
        $this->logger->info('Started uploading to AWS/S3.');
        foreach ($fileParts as $filePart) {
            $this->awsClient->upload($filePart);
        }
        $this->logger->info('Finished uploading to AWS/S3.');

    }
}
``` 

You can even go a step further towards clean-code by coding to an interface, not a concrete implementation (class),  and moving each logic to a separated, ensuring even more modular parts and being closer to the  [Open/Closed Principle](https://en.wikipedia.org/wiki/Open%E2%80%93closed_principle) 

```
<?php

interface StorageInterface
{
    public function save(File $file): void;
}

class AwsStorage implements StorageInterface
{
    public function save(File $file): void
    {
        $compressedFile = $this->compress($file);
        $fileParts = $this->splitFile($compressedFile);
        $this->logger->info('Started uploading to AWS/S3 started.');
        foreach ($fileParts as $filePart) {
            $this->awsClient->upload($filePart);
        }
        $this->logger->info('Finished uploading to AWS/S3 started.');
    }
}

class LocalStorage implements StorageInterface
{
    public function save(File $file): void
    {
        $fp = fopen($this->getStoragePath().$file->getName(), 'w');
        fwrite($fp, $file->getContent());
        fclose($fp);
        $this->logger->info('File successfully saved on local storage.');
    }
}

``` 
And that was it!

I would also be glad to know your opinion about the topic if you consider it from  a different point of view.

Happy coding!