First plugin, please review
#1
A user on the community forum asked earlier how to completely disable all links on his board.  Turned out he just wanted to prevent his users from sending referrers when they click links.  I've been using a dereferrer plugin that I wrote for myself for almost as long as I've been using MyBB.  I never realized that this might be something of interest to other users.

I decided to turn it into my first real plugin.  I've tested it on my production board and it seems to work perfectly.  But since it's my first plugin, I'm a bit insecure about it and hesitant to make it public without getting some peer review first.

Basically, I know I can count on you guys to tell me absolutely everything I did wrong, lol.  Any advice on how I could improve it or something like "well, this works, but you could've coded it better by doing this..." or even "Why don't you add X feature to it?"

Your criticism is most welcome Smile


Attached Files
.zip   DeRef.zip (Size: 4.48 KB / Downloads: 114)
Reply
#2
in deref() function you dont need to include the settings files

just have something like

global $mybb;

if($mybb->settings['showderefbypass'] == "no")


For some reason i dont' see the image when i choose Show Bypass Link


Use
$_SERVER['HTTP_USER_AGENT'];
instead of
getenv(HTTP_USER_AGENT);

why this like this?
require('inc/settings.php');

Better use

require_once "./inc/settings.php";

these are enough for now as i need to go sleep Toungue
Reply
#3
Thank you, Zaher, that was very helpful.  I've made those changes Smile

Quote:For some reason i dont' see the image when i choose Show Bypass Link
Did you upload the external.gif image? It works for me.
Reply
#4
Oh no i haven't but it should show the little missing image box as im using IE
Reply
#5
Hmmm..  I get the missing image box in IE when the image isn't there.  I dunno, that sounds weird.

Wait...  maybe you did upload it.  The image is blue.  Does your background happen to be blue?  Eh, maybe I need to use a different image. Sorry. I'm completely color blind so things like that just don't occur to me.
Reply
#6
My first post in a while but here goes.
Please note that the following are my own opinions on how to write code and might not be the best/accepted practice for writing code. I'm just being a jerk and nitpicking with some of the things mentioned below so just review each one and


/link.php

Line 13
$url = $_GET['url']; //You should use trim on the URL. Stops people using "?url= " and the script still working.

Line 20
if (!$url) //You should use !empty(). I think it's slightly quicker in execution (couple milliseconds! Toungue) Plus "!$url" is not very descriptive of what it's checking

Line 29 - 31
$forum_url = strpos("$url", "$settings[bburl]"); //You don't need the speech marks around the variable, especially if you are just using this one variable. If you are going to use speech marks, you should enclose the variables in {} like "{$url}" or "{$settings['bburl']}"

Line 44 & 68
$settings[showderefmsg] - Arrays should have quotes around the array key unless using numeric key references. eg $settings['showderefmsg'] or $settings[2]


/inc/plugins/deref.php

function deref_deactivate()
This is a personal prefernce for me so you might not want to bother doing this
You might want to consider deleting settings by gid. When you insert settings, you insert them by gid so deleting them by it would make sense.
If you add a new setting, you might forget to add it to the deactivate function, thus leaving the setting lurking in the db without a group. But it'll still be counted if the user decides to reactivate the function
So if you delete by gid, it gets rid of all settings in that group; much cleaner.
I use something along the lines of (I've not tested the code below because I'm not on home computer, but the theory is the same)


global $db;
$row = $db->fetch_array($db->query("SELECT gid FROM " . TABLE_REPFIX . "settinggroups WHERE name LIKE 'deref'"));
$gid = $row['gid'];
$query("DELETE FROM " . TABLE_PREFIX . "settings WHERE gid = '{$gid}'");

Congrats on the first plugin and lets hope we see many more from you Big Grin.
(Again, the info above is not meant to be condescending. Just passing on my knowledge/experience)
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)