[development] LinksDB vs. Links

[Morbus Iff]

> You must be looking at a very early release then.  I _AM_ using db_query 
> properly (I have been as soon as I discovered that it could be used like 
> printf), and I _AM_ using database prefixes.
 >
 > At least get your facts straight before you start slating me.

Actually, the only reason you're using database prefixes is because you 
went through and added them all shortly after I sent that message. 
(?r1=1.1.2.11&r2=1.1.2.12, Wed Aug 2 08:00:47 2006 UTC).

  * You've got an empty @file declaration.

  * You're passing an unused array of string
    replacements to t() in linksdb_help().

  * You're using HTML in the help of admin/settings/linksdb, which
    means it has to be translated separately even though it's the
    same string as the one preceding it. '<p>'.t().'</p>' instead.

  * Your code doesn't follow the Drupal style guidelines at all.
    http://drupal.org/node/318. Not mentioned there is an in-bred
    "spaces after commas", which you love to not do.

  * You global $user in linksdb_menu() and you don't have to.

  * Your use of weights in linksdb_menu(), while they certainly
    accomplish something, is abnormal. You never define weight 0,
    default local tasks are set to -10, and the tabs usually are
    ordered by declaration order, not manually defined weights.

  * MENU_NORMAL_ITEM is implied, and doesn't need to be specified.

  * You use "links/category/edit/4" when it really should be
    "links/category/4/edit". Even with that said, you use arg(3)
    in your linksdb_categories_edit() when you should be using
    the naturally passed callback arguments of the menu (where
    you'd use linksdb_categories_edit($cid)). You use arg()
    throughout the module when you really shouldn't be.

  * You don't predeclare a bunch of things, such as $form in
    linksdb_report() - predeclare it as $form = array(); first.

  * You don't do sanity checking - you assume
    that arg(2) exists and is a number.

  * You spelt "concise" wrong, and your sentence has no period.

  * You don't know how to use drupal_get_form(). If you give it
    an ID of "reportform", then it'll happily use "reportform_submit".
    However, instead of naming it "linksdb_report", you override this
    automatic/free feature by forcing a callback function of "linksdb_
    report_support", which is unnecessary.

  * No standardization in your code. You use $fid, $v for linksdb_
    report_submit, $formid, $v in _moderate_submit, and $form_id,
    $values in your _edit_submit. They're all the same thing.

  * Your SQL isn't capitalized.

  * You rarely need to use drupal_goto anymore, and
    almost never in the _submit hook for FormsAPI.

  * Your SQL queries will not work on PostgreSQL. It needs
    db_query('word "%s" word') not db_query("word '%s' word").

  * You use double spaces after sentences in _report_submit.

  * Listen, I know I said you shouldn't use drupal_goto a few bullets
    ago. That doesn't mean you go and use drupal_set_header with a
    Location: instead. Man alive.

  * Oh, and you're passing ; at the end of most of
    your SQL queries. You don't need to do that either.

  * You use "'&lt;Root&gt;'" in linksdb_deadlinks, and '<Root>'
    in linksdb_suggest. Which is it? (For what it's worth, it should
    be '<'. t('root') .'>', per Drupal core).

  * To prevent things like "select * from {links_links} where dead=1
    order by {links_links}.name", use table abbreviations, like:
    "select * from {links_links} ll where dead=1 order by ll.name".

  * if(variable_get('linksdb_show_report',1)==1) - you don't need ==1.

  * linksdb_deadlinks doesn't translate the report messages.

  * You use "links/report/$link->id" and then 'links/refer/' .
    $link->id. Which is it? Concatenation or interpolation?

  * Oooh, you do know how to predeclare $form = array() in _suggest().
    I guess you must have made sloppy mistakes all the other times.
    Story of your life, eh?

  * "({name},{url},{description},{active},{category})" - I have no clue
    what you're even attempting there. Column names aren't prefixed.

  * Is it site staff or site administrators? You use both.

  * theme_link() - Gah, you don't use theme('image') here.

  * Is it "Link" or "link"? Previously you have "Report
    this link", but then you have "Edit Link".

  * $cat->shown=99999999;. Right. Heh.

  * You seem to use the "Report this link as dead"
    thing a lot. That should be made a function.

  * # for comments? What is this, Perl?

And so on. But I'm getting bored.

-- 
Morbus Iff ( drowning in data, bereft of knowledge. )
Technical: http://www.oreillynet.com/pub/au/779
Culture: http://www.disobey.com/ and http://www.gamegrene.com/
icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus