testingbydev / smart-lencioni-image-resizer

Automatically exported from code.google.com/p/smart-lencioni-image-resizer
GNU General Public License v3.0
0 stars 0 forks source link

DON'T USE CONSTANTS IN CLASSES #23

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This is very bad!

    final public function fullPath()
    {
        return SLIR_DOCUMENT_ROOT . $this->path;
    }

You should not use CONSTANTS inside classes code - this is very BAD style of 
OOP programming. Of course CONSTANTS are very good for making config, but it 
does not mean, that you must use them inside classes. Take a look to 
http://code.google.com/p/lagger/source/browse/trunk/examples/lagger_init.php 
for example of correct using constants for configuring classes objects.

And why all of your methods are "final"?
And why you use "private" but not "protected"?

And why do you write:

final private function isSourceImageDesired() {
        if($this->isWidthDifferent() || $this->isHeightDifferent() || $this->isBackgroundFillOn() || $this->isQualityOn() || $this->isCroppingNeeded())
            return FALSE;
        else
            return TRUE;

    }

but not just:

final private function isSourceImageDesired() {
        return $this->isWidthDifferent() || $this->isHeightDifferent() || $this->isBackgroundFillOn() || $this->isQualityOn() || $this->isCroppingNeeded();
    }

?

Original issue reported on code.google.com by barbushin on 29 Jul 2010 at 4:59

GoogleCodeExporter commented 9 years ago
Thanks for the coding tips.

You say that using constants in classes is bad, but you haven't given any 
reasons. Is there an article somewhere that you could point me to that might 
shed some more light on this notion?

| Any why all of your methods are "final"?

I declared them as final because it allows me to be sure that I can change 
things without worrying about affecting some extending class. Now that I think 
about it, final is actually redundant on private functions. I will probably 
remove the final from private functions soon.

| And why you use "private" but not "protected"?

See above.

| And why do you write... but not just...

I think it is easier to read.

Also, PLEASE DON'T USE ALL CAPS EVER. This is very bad!

Original comment by joe.lencioni on 29 Jul 2010 at 1:32

GoogleCodeExporter commented 9 years ago
Hi Joe!

Thank you for your answer, and sorry for my CAPS :)

> You say that using constants in classes is bad, but you haven't given any 
reasons. Is there an article somewhere that you could point me to that might 
shed some more light on this notion?

1. Because constant names are global, and there may be a problem if some 
project already use constants with names like in your lib. There is one issue 
http://thedeployer.com/2009-04-why-should-we-use-php-constants-inside-classes 

but:

2. Some developers will want to use > 1 instances of library in one PHP-process 
but with different configs. In this case it will be impossible if configuration 
is realized by any constants.

> I declared them as final because it allows me to be sure that I can change 
things without worrying about affecting some extending class. Now that I think 
about it, final is actually redundant on private functions. I will probably 
remove the final from private functions soon.

1. As I see in class SLIR all method are "final". Don't you think that it will 
be much better just to define class SLIR as final and forget about "final" 
methods? Like this:

final class SLIR {
  // ...
}

2. But anyway much better if there will be some public/protected 
interface-methods that users can override like they need.

3. Talking about using "private" but not "protected", just take a look to this 
article 
http://www.leftontheweb.com/message/My_privates_are_not_public_they_are_protecte
d may be you will change your mind.

Original comment by barbushin on 29 Jul 2010 at 1:56

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r123.

Original comment by joe.lencioni on 21 Dec 2010 at 6:58