User:Jeroen De Dauw/Static: The S in STUPID
From semantic-mediawiki.org
< Jeroen De Dauw
Jeroen De DauwUser:Jeroen De Dauw/Static: The S in STUPID
Jump to:navigation, search
Loading...
The S in STUPID
YESCaturday will not be stopped!
The S in STUPID
YESpublic function doStuff() { // ... Logger::log( 'doing stuff during caturday is madness' ); // ... DataStore::saveObject( $object ); // ... }
- What are the problems with this code?
The S in STUPID
YESTight coupling!
Problem #1:
- This class is now dependent on Logger and DataStore.
public function doStuff() { // ... Logger::log( 'doing stuff during caturday is madness' ); // ... DataStore::saveObject( $object ); // ... }
The S in STUPID
YESTight coupling!
Problem #2:
- What if the caller wants to use a different logger?
public function doStuff() { // ... Logger::log( 'doing stuff during caturday is madness' ); // ... DataStore::saveObject( $object ); // ... }
- Logger::log and DataStore::saveObject are ALWAYS called
The S in STUPID
YESTight coupling!
public function testDoStuff() { // Cannot prevent disk and database writes // State might be leaked, order might become relevant $object->doStuff(); }
public function doStuff() { // ... Logger::log( 'doing stuff during caturday is madness' ); // ... DataStore::saveObject( $object ); // ... }
- Not just tests, applies to all code making use of doStuff.
The S in STUPID
YESTight coupling!
public function doStuff() { // ... Logger::log( 'doing stuff during caturday is madness' ); // ... DataStore::saveObject( $object ); // ... }
class DataStore { public function saveObject( $object ) { $loadBalancer = LoadBalancerFactory::getInstance(); // ... $database->update( /* ... */ ); // ... Logger::log( 'done saving object' ); } }
- You cannot use a part of the system without initializing whole chuncks of it
- Hard to fix once pervailent through the codebase
The S in STUPID
YESFixing the problem
class Logger { public static function log() { // ... } public static function resetForTesting() { // ... } }
The S in STUPID
YESFixing the problem
public function doStuff() { // ... Logger::getInstance()->log( 'doing stuff during caturday is madness' ); // ... }
The S in STUPID
YESFixing the problem
public function doStuff( Logger $logger, DataStore $dataStore ) { // ... $logger->log( 'doing stuff during caturday is madness' ); // ... $dataStore->saveObject( $object ); // ... }
- Where Logger and DataStore are properly seggeragted interfaces
public function testDoStuff() { // ... // Create simple mocks $logger = new NullLogger(); $dataStore = new MockDataStore(); $object->doStuff( $logger, $dataStore ); // ... }
The S in STUPID
YESWhat about this static call?
class Awesome { public function calculateAwesomeness() { // ... // Clearly this can never be negative return Math.Abs( $awesomeness ); } }
The S in STUPID
YES- Static calls to leaves in the call graph are less evil
- Leaves can stop being leaves
class Awesome { public function calculateAwesomeness() { Utils::getAwesomeFactor(); // ... } }
class Utils { public static function getAwesomeFactor() { return 42; } }
- What if the awesome factor is now stored in a config file or db?
The S in STUPID
YES- Static code is procedural, not OOP
- Static is almost always bad, causing tight coupling, thus avoid it
- Both static and non-static code in a class suggests violation of single responsibility
The S in STUPID
YESFurther reading
- SOLID (article)
- STUPID (blog post)
- 10 things that make your code hard to test (blog post)
- Testable code (screencast)