Zikula Community

• Topiatic
  
  

  • Rank: Expert
  • Registered: Nov 23, 2003
  • Last visit: Dec 13, 2009
  • Posts: 1487

  Posted: Jun 08, 2005 - 04:19 AM

  Hail PostNuke community,
  
  I have just ported a module to PostNuke (Roster Master Everquest guild roster). And being inexperienced in coding, PHP, and postnuke, I am assuming I missed something (ok a few things) glaringly obvios that more experienced eyes might roll at.
  
  So I am requesting that if anyone has time (yes I know that's asking for a lot) if the could quickly peruse the code for glaring omissions and/or non-API compliancy (before I submit it to wasting the developers time spotting them for me).
  
  Any and all feedback appreciated thanks.
  
  Please note, when asking for peoples "free" time I do not expect a response.
  
  --
  Under Construction!

  • 
  •  
• **unknown user**
  
  

  • Rank: Senior
  • Registered: Mar 16, 2002
  • Last visit: Oct 21, 2009
  • Posts: 2330

  Posted: Jun 08, 2005 - 04:23 AM

  ...might help to specify how much code it is... 25k would probably be a cinch for most to work in...whereas 200k might not be so easy.

  • 
  •  
• Topiatic
  
  

  • Rank: Expert
  • Registered: Nov 23, 2003
  • Last visit: Dec 13, 2009
  • Posts: 1487

  Posted: Jun 08, 2005 - 10:12 AM

  Thanks for the suggestion Allar, I was thinking it might be cool to post a link to it too or something 8].
  
  It can be found here: http://mods.postnuke…download-sid-4.html (beta module downloads) and is about 72kb in size.
  
  Edit: I should note, I know of these flaws.
  - HTML in the dataparse function (already removed in next version)
  - usort($cmpi) is not called via pnModFunc() (nor is it's name standardised... not sure I can do that here)
  - a few function names need standardisation, but I need this feedback before I can finalise the API end.
  
  Which brings me to a more specific question, my biggest concern is is that the add and delete functions are mostly user functions, but all I've read says those particular functions belong in the adminapi.
  
  So, do I sort API functions seperatly from the display functions (as in add delete get called from adminapi into user display), or do I keep user and admin functions sepperated by were the call is coming from?
  
  Proly seems silly, but I just never saw any alternate examples.
  
  --
  Under Construction!

  • 
  •  
• **unknown user**
  
  

  • Rank: Softmore
  • Registered: Mar 16, 2002
  • Last visit: Oct 21, 2009
  • Posts: 379

  Posted: Jun 09, 2005 - 04:33 AM

  If I understand your question, you have some functions that typically are "admin" type things, but you need users to be able to use them, too, right? If that's the case, then they can go in the userapi file. Having the API functions is more important to good coding than having the functions separated among user and admin functions.
  
  I ended up separating one of my modules by the different kinds of information it handled (projects, people, and organizations.) So I have pnprojects, which handles the display stuff, and pnprojectsapi, which handles the database stuff. Then there's pnpeople and pnpeopleapi, and pnorgs and pnorgsapi. (And a whole bunch of other things besides.)
  
  If you wanna take a look, you can find it at here

  • 
  •  
• markwest
  
  

  • Rank: Team Member
  • Registered: Mar 18, 2002
  • Last visit: Oct 21, 2009
  • Posts: 6606

  Posted: Jun 09, 2005 - 05:48 AM

  I've taken a quick look at the module and from a brief glance the code looks solid.
  
  The only things i've spotted are
  
  1) return exit; used in several places. This doesn't seem right. The exit statement halts the script so i'm not sure what the result of the that line would be.... It's best to return true if API has completed it's job properly andl then let the rest of the PN script complete.
  2) In RosterMaster_admin_fupdate() you get 'errid' from the input (get/post) but don't use it anywhere but you do use a variable direct that doesn't seem to defined anywhere
  
  Those are the only code errors i've spotted.
  
  The biggest issue I see with the module isn't the code but the output. There are a good number of HTML issues in your templates. Anyone using your module with a theme that's anything other than HTML 4.01 transitional will find that the module output won't render compliant to the doctype. So far things i've spotted.
  
  1) use of the font tag in pnadminapi.php - the font tag is not recommed for use anymore.
  2) keep all tags and attribute names lower case e.g.

  not

  
  3) make sure that all attribute/value pairs are properly quoted e.g. not
  4) don't use deprecated tags or attributes e.g. <center>

  
  5) use the full non-minimised form for boolean attributes e..g. selected="selected" not just selected
  
  These guidelines will allow you to produce markup that will validate under all doctypes.
  
  hope this helps...
  
  -Mark
  
  --
  Visit My homepage and Zikula themes.

  </center>

  • 
  •  
• Slugger
  
  

  • Rank: Expert
  • Registered: Mar 11, 2003
  • Last visit: Oct 21, 2009
  • Posts: 1104

  Posted: Jun 09, 2005 - 06:34 AM

  In the process of cleaning up the HTML, you can't help but clean up everything. In one "Past-Nuke" module I've been upgrading, I've been able to cut my pageload times to a fraction of what they were just by cleaning up the code....mostly PHP outputted HTML. Clean up the queries too and you can fly!
  
  Slugger

  • 
  •  
• Topiatic
  
  

  • Rank: Expert
  • Registered: Nov 23, 2003
  • Last visit: Dec 13, 2009
  • Posts: 1487

  Posted: Jun 11, 2005 - 04:16 AM

  @ jediping - That's exactly what I meant, thanks.
  
  @ -Mark - Direct and errid are leftovers, thanks for the spot. I think I was using return exit for redirects that were going to run same function that was making the origional call... there was a reason I wasn't using return redirect but i don't recall it, I'll work that one out.
  
  Also I knew the HTML was bad in many places... sadly a some of it was introduced by HTML tidy. That I most certainly intend to get to XHTML compliancy before submission.
  
  Again, thanks for the feedback folks.
  
  --
  Under Construction!

  • 
  •  
• markwest
  
  

  • Rank: Team Member
  • Registered: Mar 18, 2002
  • Last visit: Oct 21, 2009
  • Posts: 6606

  Posted: Jun 11, 2005 - 08:31 AM

  No problem - I hope the feedback is useful. Where I can i'm happy to look through people's modules and advise.
  
  -Mark
  
  --
  Visit My homepage and Zikula themes.

  • 
  •  
•  
•  

Users on-line

• bronto,
• designerdogs

This list is based on users active over the last 60 minutes.