Cardinal sins of Vanilla plugin development

Linc

Major WTFs:

• Not filtering user-generated output (like comments)
• Query that returns all records from a potentially huge table (user, activity, log, discussions, comment)
• Directly editing SQL (best: can you use an existing model method?)
• Implementing permissions incorrectly
• Making up your own coding standards
• Re-implementing core functionality (usually worse or buggy)
• Overriding views needlessly
• No docblocks / inline comments
• Breaks other popular plugins
• Using OnDisable to make destructive changes (like wiping your plugin data)

Minor WTFs:

• Doing SQL queries that won't scale up (borderline major issue depending on severity)
• Monster-length methods
• Registers a slew of new permissions with little gain
• Too many options ("knobs & dials") in its settings page
• Code duplication
• Hooking into Base_Render_Before because you were too lazy to find the right hook(s)
• Adding columns/tables to the database needlessly (e.g.: can you use Attributes instead?)

What else can you think of?

hgtonight

Things I don't like:

• Storing User Specific data in the config
• Using root level Config items (e.g. 'ItemsPerPage')
• Using untranslatable text
• Not using functions in functions.general.php and functions.render.php
• Throwing errors with debug information by default
• Not throwing errors in known error states
• Completely breaking with JS disabled
• Modules that don't allow overriding views
• Mis-aligned code
• TABS
• Duplicating DB queries

Things I like:

• Sensible defaults with 'hidden' configs (optimally the plugin writes the config to config.php on enable so it isn't really hidden)

peregrine

Completely breaking with JS disabled

doesn't vanilla break with js disabled as well. I think this is a dream! or a different semantic for breaking.

vrijvlinder

I hate when plugins break other things... generally it's the js . I just found out that my TinyMCE breaks the options drop down ugh! I was fixing something else about , the fix worked but broke something else ARG!!

peregrine

@Linc said:
Major WTFs:

• Not filtering user-generated output (like comments)
• Query that returns all records from a potentially huge table (user, activity, log, discussions, comment)
• Directly editing SQL (best: can you use an existing model method?)
• Implementing permissions incorrectly
• Making up your own coding standards
• Re-implementing core functionality (usually worse or buggy)
• Overriding views needlessly
• No docblocks / inline comments
• Breaks other popular plugins
• Using OnDisable to make destructive changes (like wiping your plugin data)

Minor WTFs:

• Doing SQL queries that won't scale up (borderline major issue depending on severity)
• Monster-length methods
• Registers a slew of new permissions with little gain
• Too many options ("knobs & dials") in its settings page
• Code duplication
• Hooking into Base_Render_Before because you were too lazy to find the right hook(s)
• Adding columns/tables to the database needlessly (e.g.: can you use Attributes instead?)

What else can you think of?

most of these are obvious to discern.

the one that could be illustrated with a correct and incorrect way would perhaps be useful....

with respect to

Adding columns/tables to the database needlessly (e.g.: can you use Attributes instead?)

Linc

@peregrine The Attributes column exists on the user & discussion & comment tables for storing meta data about the records. Unless you need to filter or count rows based on the data you're adding (i.e. use it directly in a query), you could probably store it in Attributes rather than adding more columns.

hgtonight

@peregrine said:
doesn't vanilla break with js disabled as well. I think this is a dream! or a different semantic for breaking.

I am fine with gracefully degrading the user experience. No JS? You can't sort things. This seems reasonable.

@Linc said:
peregrine The Attributes column exists on the user & discussion & comment tables for storing meta data about the records.

Is the user meta table deprecated?

Linc

@hgtonight said:
Is the user meta table deprecated?

Nope! And explaining when to use one over the other is a finer point than I have the brain or willpower to do right now. Your call, basically.

x00

UserMeta table is great.

Also people forget about Gnd::Set(), Gnd::Get() this uses the UserMeta table, but is for system variables, were you wouldn't want store in the config.php, I have used it for some moderation meta, that changes fairly often.

Linc

One thing to remember about config (and locale.php) is that the whole thing gets loaded into memory on every pageload, so it's definitely not the place to dump your baking recipes collection.

peregrine

Am I to surmise that

-telling people to add definitions to conf/locale.php (what i've been advocating)
- is not a great idea. and adding it to the other definition files is better

Are the other definition files in themes and applications, and locale loaded on every page load.

 1 - application based and dashboard based definitions
    applications/locale/en-CA/definitions.php
    applications/dashboard/locale/en-CA/definitions.php
    2- plugin based - plugins/yourplugin/locale/en-US.php
    plugins/your plugin/locale/en-US/definitions.php
    3 - theme based - themes/yourtheme/locale/en-CA.php
    4 - locale based - /locales/en-US/definitions.php
    5 - locale based locales/en-US/other_definitions.php


loaded into memory one every page load....

  6- conf based - conf/locale.php
    7- conf - specialized conf/locale-en-US.php 

x00

conf/locale.php is fine if it doesn't relay belong elsewhere, in fact it great for little site specific mods.

If you are translating your whole site then not the best idea.

it isn't really to do with plugin development though.

There were some issues with locale in 2.0 so I'm guessing it is a lot easier now.

Linc

@peregrine said:
Am I to surmise that telling people to add definitions to conf/locale.php (what i've been advocating) is not a great idea.

No. I just meant that transcribing a novel would be a bad choice and that, where applicable & appropriate, it wouldn't hurt to put very long custom text directly in your theme.

Tim

@x00 said:
UserMeta table is great.

Also people forget about Gnd::Set(), Gnd::Get() this uses the UserMeta table, but is for system variables, were you wouldn't want store in the config.php, I have used it for some moderation meta, that changes fairly often.

Finally, someone else who uses these

businessdad

@Linc said:
peregrine The Attributes column exists on the user & discussion & comment tables for storing meta data about the records. Unless you need to filter or count rows based on the data you're adding (i.e. use it directly in a query), you could probably store it in Attributes rather than adding more columns.

That's an excellent explanation. In fact, we added two columns to the User table because we use them to establish relationships, while we rely on SaveAttribute() for any non-relational user custom settins.