Customizing SMF > SMF Coding Discussion

Request for code review

(1/2) > >>

pljones10101833:
Hi all.

I'd like to get some feedback on whether I'm doing things right (rather than doing the right things...) in terms of SMF coding practice.

As mentioned in another thread, I'm working on an MSc dissertation.  To aid this, I needed to write a pair of small SMF mods (my first).  These have been tested and demonstrated here (and are in the process of going "live") but I would still like some experienced eyes cast over my work and would greatly appreciate any feedback that can be provided.  Whilst I may well included the comments in my dissertation (with links to this thread as a citation), I'll likely have to wait to apply any suggestions until after I've completed my work as I'm overrunning the software writing stage already!  (Don't all software projects overrun? ;))

The two packages are here:

hxxp:www.drealm.info/smf/pljones10101833_pushInteraction.zip [nonactive]
This is the main feature.  It provides a facility for the site admin or authorised users to "push interactions" onto other site users.  Users can also request an interaction that will appear the same way.  These are stored on a new table and polled for (frequency configurable).  The package introduces jQueryUI to SMF, so may clash with other packages that do the same.  I'm open to ideas on how to avoid such collisions.  The UI is themeable - although the standard jQueryUI folder locations don't follow SMF standards... Initially I tried a version with stuff moved around and the jQueryUI CSS edited to suit.  I've simply dropped that for the time being.  There's an example custom skin included, anyway...  (Ideally, once SMF 2.1 is out, it'll come with jQueryUI-SMF-theme integration...)

The feature is "cut down" from the original set of requirements to focus on those essential to getting my dissertation done as simply as possible, so I don't consider it "complete".

hxxp:www.drealm.info/smf/pljones10101833_pIntSurvey.zip [nonactive]
Of far less general interest, I imagine, this package adds "survey integration".  For my dissertation, I want to get people who've been involved in an interaction to complete a survey -- but I don't want to bug them about it once they've participated.  So this lets me track who's gone off to the survey once prompted.

emanuele:
Posting here to find it again.

pljones10101833:
I'm going to bump this because it's been a while...

Yoshi:
Looks good at first glance, the pushInteraction package I mean.

Sources/PushInteraction:

--- Code: ---unset($tgtsid);
--- End code ---
The variable doesn't exist in that function, it isn't made global, and it isn't a paramter.

Also I'd suggest to move the code for submitting data in functions. Not that it would make a huge improvement, but it can be more clear. I, myself, need to cope with writing more complex functions, making them bigger and more secure, for instance :P

I could only check that file for now. I might check more later, if I have enough time.

pljones10101833:

--- Quote ---Also I'd suggest to move the code for submitting data in functions
--- End quote ---
Ah... yes... I'd sort of naturally picked up on putting the read code into its own space but for some reason hadn't done the same for writes.  Good point!

There's been a lot of code change since it went "live", of course, too... I'm hoping that "unset($tgtsid)" got spotted in one of the iterations.  There were some other odds and ends like that... and at least one missing global declare (didn't spot it during testing either, ugh...).

Thanks and any more input is appreciated, too :).

Hey, and good luck getting into coding professionally - it's a great day job.  And night job. :D

Navigation

[0] Message Index

[#] Next page