You're part of the way there on creating a module to upload to CPAN. First make it object oriented so that this new module wouldn't need to export anything into peoples' namespaces. Clean up the code and indentation a little. Create some POD. Test it thoroughly. Upload it to CPAN, and you will have contributed to the greater community.

I took the liberty of creating an object oriented interface to your routine. I also changed the name of the "encoding" routine so that it looks more like a verb. I always just feel better when subs names relate to an action being taken. You would have to do some testing to make sure I didn't break anything, and you would have to create the POD markup. Feel free to alter the package name if you think it could be improved upon. Then put it up there for the world to use.

Here's my updated version:

package Google::Charts::Encoding;

# To use this:
# my $enc = Google::Charts::Encoding->new( $string_to_encode );
# my $result = $enc->encode();

# To obtain the last result:
# my $last_result = $enc->encoded();

# You may also pass the encode string to $enc->encode() 
# instead of to $enc->new();
# my $result = enc->encode( $string_to_encode );


use strict;
use warnings;

use POSIX;
use List::Util qw[ min max ];

# Constructor: Pass the string to be encoded, or nothing (in which
# case you must pass the string directly to $obj->encode()

sub new {
    my $self = {};
    $self->{encode_string} = $_[0] || undef;
    $self->{encoded} = undef;
    bless( $self );
    return $self;
}


# If the string to be encoded was passed to $obj->new(), no 
# parameter is needed for calling $obj->encode().  If nothing
# was passed to $obj->new(), you will need to pass the string 
# as a parameter here.

sub encode {
    my $self = shift;
    my $to_encode = $_[0] || $self->{encode_string};
    defined( $to_encode ) 
        or die "Nothing to encode in __PACKAGE__ encode()\n";
    my @characters = ( 
            'A' .. 'Z' , 
            'a' .. 'z', 
            '0' .. '9', 
            '-', 
            '.' 
        );
    my @series = split( /\|/, $to_encode );
    my @seriesmax = split( /\||\,/, $to_encode );
    my $max = max( @seriesmax );
    my $returnstring = '';
        foreach ( @series ){
        my @values = split( /\,/, $_ );
        my $encoding = '';
        foreach my $value ( @values ){
            chomp $value;
            $value = ( $value / $max ) * 4095;
            my $first = floor( $value / 64 );
            my $second = $value % 64;
            $encoding .= $characters[ $first ] . 
                                     $characters[ $second ];
            }
            $returnstring .= $encoding . ',';
        }
    my $encoded = substr( $returnstring, 0, -1 );
    $self->{encoded} = $encoded;
    return $encoded;
}


# Getter for the most recent call to encode().
sub encoded {
    my $self = shift;
    if( defined( $self->{encoded} )  ) {
        return $self->{encoded}
    } else {
        die "Nothing has been encoded yet in __PACKAGE__\n";
    }
}

1;
[download]

OO for the sake of OO? No, thanks!

There is no point in turning this into an object! Look again at the resulting API ... it's awkward. It might make sense to implement class Google::Charts that can load the data in several formats (encoded and not encoded or whatever), do something sensible with them, allow you to look at the details and then export the data in some formats ... one of them being this encoded string, but a class that has exactly one method and nothing you could really call internal state is bad design.

Keep it procedural and export the function on request!

Jenda
Enoch was right!
Enjoy the last years of Rome.

You're right about being almost a CPAN module. But for the minimal case of encoding one string format to another, there is no reason for OOP.

If you provide a way to work with data sets in the abstract, an OOP approach starts to make sense.

• Encoder object keeps an array of series. Series are just arrays of numbers.
• Series can be managed using array-function like methods: push, pop, splice, etc.
• Provide a from_string method that takes decimal comma/pipe delimited decimal strings and makes objects.
• Provide a to_string method that generates comma/pipe delimited decimal strings from the object.
• Provide an encode method that generates an encoded string from the data series in the object.
• Provide a decode method that takes a maximum value and an encoded string and gets an approximation of the raw data.
• Provide a decode_scaled method that takesan encoded string and decodes it to values between 0 and 4095.

Note that you could just as easily expose a few functions and handle everything as native structures and go 100% procedural here, as well. OOP could be a nice win if you have a group of similar encodings that use scaling functions and serialization in similar ways. Then you'll actually be able to benefit from inheritance (at least if you designed your class to support the correct abstractions).

An even better choice would be to check CPAN and see if there are any multi-front-end charting libraries that could be extended to include this output format. What you do then will depend on the charting API's needs.

Oh, BTW, don't do bless( $self );, it silently breaks inheritance. Use 2 argument bless instead. The invoking class name is available as the first argument to your constructor. bless $self, $class;


TGI says moo

Thanks for the insight, and for the suggestion on 2-arg bless. I raised a few eyebrows by suggesting an OOP approach, and I do understand the reluctance for such a trivial situation. My thought was to avoid polluting namespaces with a function that had such a generic name. But the OP would have options: One, don't export, and do use the fully qualified name. Two, do export, but make it optional to do so. Three, offer both an OO interface and an optional export list. That's probably best. To your point, it doesn't make a lot of sense with just one function, but as functionality gets added, it starts making more sense.




Dave

my @characters = ('A' .. 'Z', 'a' .. 'z', 0 .. 9, qw(- .));
[download]

Or you could write that as:

use List::Util qw[ max ];

sub encoding {
    my @characters = ( 'A' .. 'Z', 'a' .. 'z', 0 .. 9, '-', '.' );
    my $max = max( $_[ 0 ] =~ /\d+/g );
    return join ',', map join( '', map $characters[ $_ / 64 ] . $chara
+cters[ $_ % 64 ], map $_ / $max * 4095, split /,/ ), split /\|/, $_[ 
+0 ];
    }
[download]

Udate: or another way to do it:

sub encoding {
    my ( $string ) = @_;
    my @characters = ( 'A' .. 'Z', 'a' .. 'z', 0 .. 9, '-', '.' );
    my $max = max( $string =~ /\d+/g );
    $string =~ s{ ( \d+ ) }{$characters[ ( $1 / $max * 4095 ) / 64 ]$c
+haracters[ ( $1 / $max * 4095 ) % 64 ]}xg;
    $string =~ tr/,//d;
    $string =~ tr/|/,/;
    return $string;
    }
[download]

Wow, thanks for all the feedback and insight guys!