Literal values vs class constants in method parameters

[Lemon Juice]

Do you think it's a good idea to use class constants as parameters to a method when the parameter is used to indicate an option out of a few choices? Look at the two classes below:

// class 1
class MyDocument {
    public function getContents($format) {
        switch ($format) {
        case 'txt':
            // ...
            break;
            
        case 'html':
            // ...
            break;
            
        case 'pdf':
            // ...
            break;
        }
        
        return $contents;
    } 
}

// usage:
$document->getContents('html');


// class 2
class MyDocument2 {
    const FORMAT_TXT = 1;
    const FORMAT_HTML = 2;
    const FORMAT_PDF = 3;
    
    public function getContents($format) {
        switch ($format) {
        case self::FORMAT_TXT:
            // ...
            break;
            
        case self::FORMAT_HTML:
            // ...
            break;
            
        case self::FORMAT_PDF:
            // ...
            break;
        }
        
        return $contents;
    } 
}


// usage:
$document->getContents(MyDocument2::FORMAT_HTML); 

I know the second method has become popular recently, even in many built-in php libraries. Some time ago I've read a comment by an experienced programmer that the use of constants for parameters is not a good idea but I can't remember the reasoning. I'm always split between which method to use since neither of them has big enough advantage over the other. Basically, I like literal values because they need less typing but I also like the more structural look of the constants. Performance differences are irrelevant to me in this case. I can annotate both using phpdoc so my IDE will give me appropriate prompts, however with constants it is very slightly more elegant.

Which is better?

[th3mus1cman]

I prefer the use of constants. They can be more verbose, but to me the major benefit is better error reporting for typos. If you do $document->getContents('htmk'); The error that will appear because of this will be further away from the source of the problem than if you do $document->getContents(MyDocument2::FORMAT_HTMK);. The later the error will be reported right where the error is - the misspelling of the constant.

[captainccs]

I read recently that the constants should be defined in the class interface, not in the class itself. Constants make for better error checking as th3mus1cman pointed out.

[Lemon Juice]

I prefer the use of constants. They can be more verbose, but to me the major benefit is better error reporting for typos. If you do $document->getContents('htmk'); The error that will appear because of this will be further away from the source of the problem than if you do $document->getContents(MyDocument2::FORMAT_HTMK);. The later the error will be reported right where the error is - the misspelling of the constant.

Thanks, it makes sense that with constants the error will be reported right where it occurs, which is a clear advantage.

I read recently that the constants should be defined in the class interface, not in the class itself.

Interesting, I've never thought of it this way. And I think I can see why this would be logical. But we would need to always define interfaces - some argue that we should define an interface for every class.

[cpradio]

I agree that constants are the better approach (in my opinion). However, I think a slightly different approach to what you seem to be doing would be the following:

interface IDocument
{
  function getContents();
}

class TextDocument : IDocument
{
  public function getContents()
  {
  }
}

class PdfDocument : IDocument
{
  public function getContents()
  {
  }
}

class HtmlDocument : IDocument
{
  public function getContents()
  {
  }
}

// class 1 
class MyDocument { 
    public function getContents(IDocument $format) { 
        return $format->getContents(); 
    }  
} 

// usage: 
$document->getContents(new HtmlExtension()); 

Why? Because 1) it requires you to only provide something that is implemented (meaning you can't pass an integer to replace a constant, or a string for that matter) nor can you pass a misspelling and 2) It allows you to use DI to separate logic that may be different for each extension, thus making your MyDocument class less crowded.

Interesting, I've never thought of it this way. And I think I can see why this would be logical. But we would need to always define interfaces - some argue that we should define an interface for every class.

I'm a HUGE fan of interfaces, but you need to use them where it makes sense. Most base classes should have an interface, beyond that, when you extend a class you usually lose the purpose of the interface at that level. So in my prior example, I created an IDocument interface that TextDocument, HtrmlDocument, and PdfDocument all utilize. If I ever needed a base class (Document) to provide something common among the three classes then I'd only have Document associated to the interface and TextDocument, HtmlDocument, and PdfDocument would inherit Document (which allows them to utilize the Interface too, I wouldn't build an Interface for each of them unless there was a specific implementation I needed it to conform to) -- I hope that makes sense.

Edit:

I like how oddz kept his named Document, so I've updated my example to be similar

[oddz]

I would argue that the document doesn't need to know anything about how it is converted. Instead pass the document into a separate instance of a class that knows how to convert the document.

pseudo code:

// Create base document
$doc = new Document();

// Create format
$format = new PDFDocumentFormat();

/*
 * Convert document to pdf format 
 * All document formats implement an interface containing the convert() method accepting 
 * a single argument that is the document to be converted
 */
$pdf = $format->convert($doc); 

Is document some type of domain object with fielded data that is stored in some of persistent storage mechanism such as a relational database?

or

Is document big data and the objective is to create strategies that convert it between several different formats?

[Lemon Juice]

I agree that constants are the better approach (in my opinion). However, I think a slightly different approach to what you seem to be doing would be the following:

interface IDocument
{
  function getContents();
}

class TextDocument : IDocument
{
  public function getContents()
  {
  }
}

class PdfDocument : IDocument
{
  public function getContents()
  {
  }
}

class HtmlDocument : IDocument
{
  public function getContents()
  {
  }
}

// class 1 
class MyDocument { 
    public function getContents(IDocument $format) { 
        return $format->getContents(); 
    }  
} 

// usage: 
$document->getContents(new HtmlExtension()); 

Why? Because 1) it requires you to only provide something that is implemented (meaning you can't pass an integer to replace a constant, or a string for that matter) nor can you pass a misspelling and 2) It allows you to use DI to separate logic that may be different for each extension, thus making your MyDocument class less crowded.

I would argue that the document doesn't need to know anything about how it is converted. Instead pass the document into a separate instance of a class that knows how to convert the document.

pseudo code:

// Create base document
$doc = new Document();

// Create format
$format = new PDFDocumentFormat();

/*
 * Convert document to pdf format 
 * All document formats implement an interface containing the convert() method accepting 
 * a single argument that is the document to be converted
 */
$pdf = $format->convert($doc); 

cpradio, oddz, these are good alternative solutions to the problem, I admit. I didn't mean to go as far as this since I was asking about a simplest scenario where just a flag parameter is enough. Maybe my example with the document was not the best one in this case because converting to different formats may well be a task for separate classes. Nevertheless, it was useful to see these implementations.