Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The following article is severly outdated as Drupal 7 brings in PHP 5, and PHP class use for PDO and SimpleTest....
http://api.drupal.org/api/file/developer/topics/oop.html
Comment | File | Size | Author |
---|---|---|---|
#7 | 326881.patch.txt | 12.45 KB | jhodgdon |
#7 | oop.html_.txt | 16.33 KB | jhodgdon |
Comments
Comment #1
jhodgdonThis is API documentation. Moving to correct issue queue.
Comment #2
webchickFixing title.
Comment #3
matt2000 CreditAttribution: matt2000 commentedI think the article still makes the good point that syntax does not OOP make. There is good OOP (conceptual) code that is not OOP syntax, and there is OOP syntax code that is NOT good OOP conceptually.
So, just +1 to update and not retire this article.
Comment #4
Nick Lewis CreditAttribution: Nick Lewis commentedI took a stab at rewriting it. Honestly, the argument holds almost no water now that Drupal 6.x and beyond allow PHP5 objects. Almost every argument made begs the question, "so why not just use proper objects? ". I think the article should be retired, and replaced with an article quickly explaining design decisions in the past (php 4.x), current efforts to bring objects into drupal, as well as big areas we think could benefit from a more object oriented approach.
Or better yet -- maybe just a broad overview/discussion of drupal's architecture at present [separate issue].
Posting this proposal for review. Forgive me if there's a way to make a patch that removes a file from certain versions but not others.
Comment #5
catchTo retire it from HEAD, I'm pretty sure you'd just checkout HEAD, delete all the lines, make the patch, then commit it to HEAD - since it's impossible to delete files from CVS anyway. So marking CNW.
Comment #6
jhodgdonI will take a stab at this.
Comment #7
jhodgdonOK, here's my first pass at editing this document. It had some other minor issues (such as outdated references to hook_node_api() and non-conformance with the Doc Style Standards in a place or two), so I fixed those too (sorry for any confusion this may have induced).
In the process of updating the document (having now read it carefully), I have to agree with matt2000 in comment #3 above: I think it makes a good case, and I think it should stay on api.drupal.org.
The attached patch is for D7. I think it could also be used for D6, after removing the new paragraph that is specific to D7, and changing the hook references for node and taxonomy back to their D6 versions (hook_nodeapi and hook_taxonomy -- these hooks were split out into separate hooks in D7).
I have also attached the full HTML file, as I think it's best to review the whole document rather than just the changes.
I've given both a .txt extension, because apparently .html extension attachments are not allowed, and the patch file is in the contrib repository so it would fail auto testing.
Comment #8
matt2000 CreditAttribution: matt2000 commentedLooks good.
The only potential thing it, since this is more of a conceptual essay than actual documentation, it might be better to leave out references to specific functions, to minimize the need for further changes due to future release, for example,
But as far as Drupal 7 is concerned, this patch looks good to me.
Comment #9
Dries CreditAttribution: Dries commentedThis patch is a good first clean-up, but more work could be done. I'm OK with doing that in follow-up patches.
This isn't a patch against CVS HEAD though -- it doesn't apply to HEAD.
Comment #10
jhodgdonThe patch applies fine for me in the Contrib repository on HEAD... of course, I created it... did I do something wrong?
As there were two "this is better than it was" reviews on the content, I went ahead and committed this patch to the contrib repository, with one change: I realized the D7 doc mentioned node_view() which doesn't exist, so I changed that sentence to say "node_build() followed by drupal_render()".
I also made the D6 changes (same patch applied, and then I went back and changed function references and removed the D7 paragraph saying "we're using PHP5 and some classes now), and committed it to the D6 contrib repository.
So I am setting this back to "Active" to get more comments on future changes that might be needed.
Dries: do you have specific comments on what type of work you would like to see done in follow-up patches?
And what does everyone think about removing all mentions of specific Drupal functions in the document? There are about 8 functions referenced in the doc as it is now, and I personally think that they are quite useful as examples, though it is true they make the doc harder to maintain. Thoughts?
Comment #12
jhodgdonThis document is no longer in CVS -- see #547510: Move OOP document to handbook - it is now in the Handbook. Moving this issue to Documentation queue, accordingly.
I'm not sure what changes Dries has in mind, but leaving it open for now...
Comment #13
add1sun CreditAttribution: add1sun commentedSo many dupes. Closed these others in favor of this one.
#554794: Drupal Programming from an Object-Oriented Perspective
#608174: Update and modernize our stance on OO
EDIT: ugh, wrong issue number there. Corrected.
Comment #14
Crell CreditAttribution: Crell commentedSo that's why that thread dropped off of my "My issues" page. Bah. For reference, here's what I posted in the other thread:
chx responded by pointing out that hooks are a form of pointcut: http://en.wikipedia.org/wiki/Pointcut I'd not heard of that before, but it sounds accurate.
Right now this is on my todo list, but performance patches for D7 are higher on the list. Anyone have further input?
Comment #15
jhodgdonThis issue is pretty old, and we aren't using Documentation issue reports to report problems with Community Documentation pages any more. I added a link to this issue as a comment on the page. Please feel free to edit the page...
http://drupal.org/node/547518
Meanwhile I've marked the page as "needs updating" and added a link to this issue in a comment.