Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling

Changing Modules in Bad Code

by Kozz (Friar)
on Jul 09, 2003 at 14:08 UTC ( #272681=perlquestion: print w/replies, xml ) Need Help??
Kozz has asked for the wisdom of the Perl Monks concerning the following question:

Most esteemed monks:

I have been asked to evaluate some CGI code for revisions. The code in question is written *horribly*, without "use strict" in most of them, no modularization or custom packages for easy configuration, lots of HTML inside the CGI scripts.

It also uses the somewhat antiquated Postgres module (which I dislike). However, I've been asked to update these scripts to use the DBI module (which I am familiar with -- I think it works with Postgres?).

Now I'm wrestling with two possible solutions, and would like your opinions.

I've been told that I should not go to any heroic efforts to make all the code "better", because so long as it is working, they don't want to spend the money rewriting it from scratch. Basically, make it use the new DBI module, keep it working, but not much else.

I thought about two different solutions:

  1. The straight-forward editing of every single perl CGI script, replacing "use Postgres" with "use DBI", then fixing up all the code to use DBI methods for connecting, executing, fetching results, error handling
  2. OR, the more challenging (but maybe easier, too??) of writing my own "Postgres" module that actually acts as a *wrapper* to the current DBI module

The first one could be done, but would be exceedingly tedious, and there would surely be a higher chance of missing some code somewhere, or messing up other things as I try to "fix" the code.

The second one sounds more intriguing, so that if I wrote wrapper methods that made all the proper DBI method calls, the "use Postgres" would then call *my* module instead, but everything would go through the DBI module.

The drawback of #2 is that I have never found the need / excuse to create my own package or module, ever. I don't (yet) know how to do it. I've looked at perltoot and perlmod (the latter of which is by no means a "how-to" document IMO).

I'm not sure what sort of problems I would be making for myself if I used solution #2. I would vastly appreciate all monks' advice offered.

Replies are listed 'Best First'.
Re: Changing Modules in Bad Code
by davorg (Chancellor) on Jul 09, 2003 at 14:15 UTC
    the more challenging (but maybe easier, too??) of writing my own "Postgres" module that actually acts as a *wrapper* to the current DBI module

    I don't think this is a good idea. To access a Postgres database using DBI, you'd need the DBD::Pg module. This converts standardised DBI calls to the Postgres API. I assume that the existing Postgres module looks a lot like the Postegres API (that's how most of these older database access modules work). Your suggested module would therefore convert Postgres API calls to DBI calls, only to have DBD::Pg turn them straight back again. That pretty much seems to defeat the object of using DBI to me.


    "The first rule of Perl club is you do not talk about Perl club."
    -- Chip Salzenberg

      Looking at it from an engineering point of view, you're right. But it's possible that development time is more important than making it look right, in which case modifying the existing Postgres module will probably take less time.

      I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
      -- Schemer

      Note: All code is untested, unless otherwise stated

Re: Changing Modules in Bad Code
by Zaxo (Archbishop) on Jul 09, 2003 at 14:17 UTC

    Option 1, hands down.

    You'll find it's not as bad as you fear to translate, and you'll gain in portability of the code - only SQL dialect changes needed to port to another db if you haven't used any DBI methods that are not widely supported.

    It would be a remarkable feat to roll your own tested Pg interface more cheaply than rewriting the application from scratch. I don't believe in it.

    After Compline,

Re: Changing Modules in Bad Code
by dragonchild (Archbishop) on Jul 09, 2003 at 14:18 UTC
    The second solution also has the benefit of not actually touching the existing codebase. But, as someone who's done this kind of work (GL -> OpenGL), it's not as easy as it sounds.

    Problems you will encounter:

    1. There isn't a 1-1 mapping between functions in the old and functions in the new.
    2. You'll probably have to write a bunch of state maintenance.
    3. Testing this will be absolutely imperative.
    You might find it easier to take the existing Postgres module and rewriting its calls to use DBI. That way you know you have every possible call to Postgres covered and you don't have to worry about state maintenance and the like. Remember - all you have to change is the communication with the DB - the external stuff. You do not (and should not) have to change how Postgres works, internally. In fact, keeping it the same will improve your success potential.

    We are the carpenters and bricklayers of the Information Age.

    Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

    Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.

Re: Changing Modules in Bad Code
by l2kashe (Deacon) on Jul 09, 2003 at 14:33 UTC
    I'll also throw out a vote for option 1.

    Option 2 could lead to *very* interesting situations on down the road. Say you do end up wrapping the module, or rewriting it's methods. Someone else comes along later, and upgrades the module. oops!

    MMMMM... Chocolaty Perl Goodness.....
(Kozz) Re: Changing Modules in Bad Code
by Kozz (Friar) on Jul 09, 2003 at 14:37 UTC

    Thank you all very much for your helpful advice and insight. I have once again been reassured of the invaluable experience upon which I can draw here at PerlMonks. Not having written a module or package, I must take heed the many warnings of stumbling blocks in solution #2.

    I believe I will indeed try to recode the scripts using DBD::Pg as suggested.

    To my helpful monks, I've deposited a ++ into your PM account. ;)

Re: Changing Modules in Bad Code
by sgifford (Prior) on Jul 09, 2003 at 21:05 UTC
    I'm sure you already know this, but when you're looking to make sure you've stomped out every last occurence of use Postgres, find and grep are you friends....
      Renaming would probably be a good idea too :-)
Re: Changing Modules in Bad Code
by chunlou (Curate) on Jul 10, 2003 at 08:54 UTC
    For maintainability, a third option, like a combination of your option 1 and 2, is to write a wrapper module (DB::MyDB or something) with methods behaving like the ones being used. That way, if you make backend changes, you modify the implementation of MyDB, not the scripts that use it, as long as the methods' input-output spec stays the same.
Re: Changing Modules in Bad Code
by Jenda (Abbot) on Jul 10, 2003 at 14:31 UTC

    I think it would be better to do the 1. but to make it easy you might install Devel::TraceSubs and add something like:

    use Devel::TraceSubs; open my $_LOG, ">> $0-Postgres.log"; my $_dbg = Devel::TraceSubs->new( verbose => 0, pre => '', post => '<', level => '', params => 0, logger => sub { my ($package, $filename, $line, $subroutine, $hasargs, $wantarray, $evaltext, $is_require, $hints, $bitmask) = call +er(1); return if $_[2] eq '<' or $filename =~ m{$}; print $_LOG "Called $_[5] in $filename at $line\n"; }, ); $_dbg->trace( 'Postgres::' );
    on top of the scripts and run them. It should create a report with all the places where you call the's functions/methods. Of course you may need to run the scripts several times to make sure all branches are covered, but still this should help in preparing the list of places you need you change.

    HTH, Jenda
    Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
       -- Rick Osborne

    Edit by castaway: Closed small tag in signature

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://272681]
Approved by gjb
Front-paged by particle
[Corion]: Ugh - the FSF wants to merge Magit into Emacs (or rewrite Magit). It seems they haven't learned from Perl+CPAN that not having things in core is good.

How do I use this? | Other CB clients
Other Users?
Others perusing the Monastery: (3)
As of 2017-07-21 11:33 GMT
Find Nodes?
    Voting Booth?
    I came, I saw, I ...

    Results (321 votes). Check out past polls.