A method to get a document's path

Question

I have a model called Document, in this I have a method that I use throughout my website to get the full path to the specific document:

public function getDocument($documentName = true)
{
    $documentName = $documentName ? $this->name : null;

    return storage_path('app/' . $this->getPath() . $documentName);
}

public function getPath()
{
    return "{$this->stream->token}/{$this->unique_id}/";
}

I use it like this (example):

$documentPath = $this->document->getDocument();

However, as you can see, the document path is made up of:

$this->stream->token/$this->unique_id/

Where $this->stream->token is calling a relationship in my Document model.

public function stream()
{
    return $this->belongsTo(Stream::class);
}

Now the method works - I can use the getDocument() to get the full path to the actual file (or just the folder, if I pass in false).

However, this code is called from multiple places at different times - and because the folder structure is dependent on the stream_token, it makes a query to get the associated stream on each request.

Solution(s)?

I have tried adding a $with = ['stream'] property to my Document model, so the associated stream is always loaded when fetching the model - but this I don't believe is a good option either, since then all the other queries I have for the document model, that don't care about the stream property make an extra query.

What else can I do to improve the method and keep the # of queries to the database to a minimum?

Answer 1

I'm sorry, this is not an answer to your question, but it is a partial review of your code.

It seems you've got a serious problem with choosing your names. They don't actually tell me what the purpose of the methods is, or the content of the variables.

Your method getDocument() can do two things: Return the path or the folder of the document. The name doesn't tell me that. It should be something like getDocumentPathOrFolder(). The argument $documentName doesn't contain a document name. It's a boolean. The only way to find out, what really is contained in this argument, is to read the code in the method. It seems to decide whether a path or folder should be returned. A better name would have been $addNameToFolder. Still not ideal.

To make matters worse you have a method called getPath() which, you've guessed it, doesn't return a path. By now I no idea what is actually returned by this method.

If I'm allowed to completely rewrite your code, with a better structure and better names, it would look something like this:

class Document 
{

    public function getDocumentName()
    {
        return $this->documentName;
    }

    public function getDocumentFolder()
    {
        return storage_path("app/{$this->stream->token}/{$this->unique_id}/"); 
    }

    public function getDocumentPath()
    { 
        return $this->getDocumentFolder() . $this->getDocumentName();
    }

}

I assume that storage_path() is part of Laravel, and cannot be changed. The long argument to storage_path() should probably be split into its seperate parts, especially if you also use it elsewhere, but please, spent some time to choose good names.

References:

Ten tips to help you choose good names

Writing Good Method & Variable Names

The importance of naming in programming