Code smells
1 The word AND
Solrphpclient addDocuments
impossible to render out Solr documents without sending them to Solr
atomic actions
God objects - they do more tha one thing and know too much.
Use composition
2 The word OR
Sometimes do one thing, sometimes do another.
_registry_check_code
type argument is sometimes a type and sometimes a flag
second argument is polymorphic too
what the hell does this function do
write to functions
make object oriented so they can share data
[doesn't solve the staticness of e.g. a REQUEST-persistent cache
3 Complexity of logic
comment node view - insane conditional structure. Cyclomatic complexity: how many different routes through the code. Depends on an unstructured object and a string. Unit test THAT if you can!
"If you need more than three levels of indentation, you're screwed anyway and should fix your program." Linus Torvalds
Run-time type identification - naive identity of some entity types. Instead a registry of entity-type callbacks - that's what core does. But it's slow. Entities should have a label() method: polymorphism.
4 Units and unit testing
DrupalWebTestCase treats 1 unit as 1 Drupal install. THIS IS NOT A UNIT TEST. It's a system test, which is nice, but bugs can cancel each other out.
DrupalUnittestCase - use this instead. 1000 times faster.
More testable
avoid globals and statics
Dependency injection - pass in what all your function needs
Minimize singletons - hard dependencies like function calls
5 Documentation
You can't teach what you don't know, and if you can't document it, neither you nor I know what you're doing.
Lack of comments - laziness, lack of comprehension, indifference, embarrassment
What to document - function, method, class, object property, constant, parameter. No exceptions
6 inappropriate intimacy / tight coupling
Can't change code x without coding y
What are the knock-on effects when implementation details change? Probably a 400k patch.
If you expose implementation details, this breaks the API. People will rely on it
Form API
render API / hook page alter
Field / language API
Node API
Implementation details hidden by query objects
Solution
interfaces - document them - well documented interfaces
[Is there's any way of doing AOP in PHP without strong coupling?]
7 impurity
Pure function - deterministic with no side effects
If you sant impure functions, then do so, but separate out as much pure as possible.
Drupal theme initialise
solution is to refactor as OO
"Can't I do anything right?"
Good smells
single purpose
self contained
predictable
repeatable
unit testable
documented