New Modules Questions, Critique

jeremyrperry's picture

I am in the process of developing two modules right now, and I'm at the point where I feel comfortable enough to share what I got so far in hopes of improving them. Some of my questions are going to seem pretty basic, but these are my first two Drupal modules so bear with me.

The first one is for a customized Geolocaton module that I'm developing for work. It is able to give the user a variety of geographical information based on IP address, lat/lon, zip code, address, etc. and also acts as a REST web service that outputs in JSON. It uses a combination of Maxmind's Geo Lite IP database and Google Maps API. My biggest questions on this module are coding layout. I want to know if there is any way to call up an external file for the node population, either on the hook_menu or the page callback without canceling out the rest of the site's HTML. This is actually very useful for the web service portion, but a toss up on the user tools. Also, this module was modified from a straight PHP application that I used a fair amount of OO on. I am still able to use the OO methods to some extent, but Drupal doesn't seem to like any object with database items in it to be declared as a global variable. I am wondering if there was any way around this since for right now, I have to declare a new object inside every function call that the object is needed in. This isn't causing performance issues yet since no more than one function with the object is being called at any given time, but I know it will. Also, the database tables are pretty hefty at 100mb total. I haven't yet worked on the install portion, and I'm wondering if its best to have the user go out to Maxmind and get the database themselves, or if I should provide some kind of automation (i.e. downloading from Maxmind's site). I have it set up as a separate database, but wondering if it necessary to do that.

My Table to CSV module, like the Geolocator, is a modification from non-Drupal ways. It works by being able to convert data on a front-end HTML table into a CSV file using AJAX, and just requires that the user add a 'table_to_csv' class to the table they want to export. It seems to be working okay (but Chrome is a hit and miss), but I want to gather some feedback on it if so I can improve and expand its capabilities.

If anybody wants to view them, they are up on my GitHub profile. Once again, these are my first ever Drupal modules and while they do work, not all functionality is there, the code isn't the prettiest, and no documentation yet on the Geolocator.

Geolocation - https://github.com/jeremyrperry/geolocation
Table to CSV - https://github.com/jeremyrperry/table_to_csv_drupal

Comments

New Modules Questions, Critique

gwprod's picture

For loading an external file (I assume through include) and outputting its data, you can use the same pattern drupal uses with templates, which is to use ob_start to include the output of the file into a variable, then simply return the variable as (part of?) the output of your callback function.

If by 'load an external file' you mean content (and not a php script) from some non-local location, you could try retrieving it via curl into a variable. It depends entirely on the format of what you're trying to get.

Using 'model' objects as global variables is definitely not 'the drupal way'. This, however, is a typical way one would do it in Zend or CodeIgniter. If you have some PHP library you want to work with, you could try wrapping it in a drupal class like Entity. Although if it's not too complex, you might consider rewriting it. Global variables are not widely used in Drupal (except for some of the core variables like $user), and it's fairly bad practice to use them in PHP in general. One way to get around this is to create a function using the drupal_static pattern, so that you can retrieve your object in any function which requires it, without having to reload it each time. Although the typical case is simply to pass the object from one function to another.

This is just general information. I'd have to know more to give you better advice.