CamelCase, avoid statics, code style
sgilits - August 3, 2009 - 11:00
| Project: | Pageroute |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | sgilits |
| Status: | closed |
Description
Hi, I made a little patch that improves the code. I avoided static functions as far as I could and I changed all internal functions and variables to CamelCase.
| Attachment | Size |
|---|---|
| routepage_camelcase001.diff | 47.65 KB |
#1
Oh, you also replaced usual function names by CamelCase names - they should stay in the drupal naming convention. Only class names, methods and variables should be converted, but not code outside of classes.
* pageroute_page_edit isn't camelcased yet.
* I think those static getDefaultSubmitHandler() and validateHandler should go. Let's move that into the info() definition using "key-value" pairs for them. Perhaps we should also add a simple function to get such info properties.
* I think we should start commenting the classes, just add a usual comment on top of them. Also comments look like
<?php/**
*
..
?>
not
<?php/*
*
..
?>
* configureForm() Method. I think this is tied to node forms and should only be used for node-forms, thus the page types should invoke that function. Also the name really says nothing about what it does... ;)
* Naming conventions. I don't know why the page has a "setForm" method, I'd call that a get() operation. Anyway, perhaps just refactor it to be just form(). But we also have ui(), which is also just a form.
* Those UI forms() sitting directly in the page classes are not ideal, as they are always included even when not necessary. Also it's confusing whether the class code deals about the ui-form or the view-form. Probably best we just move that out of the page-classes, but we can deal with that once we have the first basic patch and tests in.
#2
I corrected all the stuff you told me and modified the code so it matches the drupal coding standards.
#3
ok, I've gone ahead and committed it. I noted that you had a type in it: pageroute_dode_delete_confirm .. ;)
So I suppose the user_edit page type is still broken? If so please open an issue for that, so users are aware of it.
#4
The user_edit page displays everything correct, but it doesn't saves anything I change. So I'm not sure if this is a problem of the user_edit page type or if it is a problem with my local drupal configuration :/
#5
follow up: #544736: user_delete renamed to node_delete
closed.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.