[RFC] Module code and POD for CPAN

Having needed to implement a simple workflow for taking card payments by Stripe and knowing that I need to do the same again soon for a different product set, I have created a module that I think will be useful to other people. Existing modules to connect with Stripe either do not cater for the latest security measures of 3D card payments in Europe or are a wrapper for the Stripe API. Both of which are, of course useful. But I wanted to create something easy to use that can be used for the typical simple workflow required by many small businesses.

This gives a simple workflow of adding products to a 'Trolley' and then sending the user directly to the Stripe hosted checkout. From there, Stripe returns to either a success URL if payment was taken successfully or a cancel URL if, for any reason, the transaction failed.

Could you please provide me with some feedback regarding both the code and the POD ahead of uploading it to CPAN?

I was thinking Business::Stripe::Simple as the module name - is that sensible?

Note - at the moment, the examples in the documentation have not been tested. They will be before uploading!

package Bod::Stripe;

# TODO - Pre  release
#
# TODO - Post release
#
# Improve obtaining success/cancel URLs from environment
# Add P&P

our $VERSION = '0.1';

use HTTP::Tiny;
use JSON::PP;

use strict;
use warnings;

sub new {
    my $class = shift;
    my %attrs = @_;
    
    my @products;
    $attrs{'trolley'} = \@products;
    
    $attrs{'currency'} //= 'GBP';
    
    $attrs{'error'} = '';

    $attrs{'cancel-url'}  //= "$ENV{'REQUEST_SCHEME'}://$ENV{'HTTP_HOS
+T'}$ENV{'SCRIPT_NAME'}";
    $attrs{'success-url'} //= "$ENV{'REQUEST_SCHEME'}://$ENV{'HTTP_HOS
+T'}$ENV{'SCRIPT_NAME'}";
    $attrs{'error'}         = 'cancel-url and success-url cannot be de
+rived from the environment and need to be provided' unless ($attrs{'c
+ancel-url'} and $attrs{'success-url'});

    $attrs{'error'} = 'Public API key provided is not a valid key' if 
+$attrs{'api-public'} and $attrs{'api-public'} !~ /^pk_/;
    $attrs{'error'} = 'Secret API key provided is not a valid key' unl
+ess $attrs{'api-secret'} =~ /^sk_/;
    $attrs{'error'} = 'Secret API key provided as Public key' if $attr
+s{'api-public'} and $attrs{'api-public'} =~ /^sk_/;
    $attrs{'error'} = 'Public API key provided as Secret key' if $attr
+s{'api-secret'} =~ /^pk_/;
    $attrs{'error'} = 'Secret API key is too short' unless length $att
+rs{'api-secret'} > 100;
    $attrs{'error'} = 'Secret API key is missing' unless $attrs{'api-s
+ecret'};
    
    return bless \%attrs, $class;
}

sub success {
    my $self = shift;
    return !$self->{'error'};
}

sub error {
    my $self = shift;
    return $self->{'error'};
}

sub add_product {
    my ($self, %product) = @_;
    $self->{'error'} = '';
    
    unless ($product{'price'} > 0 and $product{'price'} !~ /\./) {
        $self->{'error'} = 'Invalid price.  Price is an integer of the
+ lowest currency unit';
        return;
    }
    unless ($product{'qty'} > 0 and $product{'qty'} !~ /\./) {
        $self->{'error'} = 'Invalid qty.  Qty is a positive integer';
        return;
    }
    
    unless ($product{'name'}) {
        $self->{'error'} = 'No product name supplied';
        return;
    }
    $self->{'intent'} = undef;
    # Update existing Product by ID
    foreach my $prod(@{$self->{'trolley'}}) {
        if ($prod->{'id'} eq $product{'id'}) {
            foreach my $field('name', 'description', 'qty', 'price') {
                $prod->{$field} = $product{$field};
            }
            return scalar @{$self->{'trolley'}};
        }
    }
    
    my $new_product;
    foreach my $field('id', 'name', 'description', 'qty', 'price') {
        $new_product->{$field} = $product{$field};
    }
    push @{$self->{'trolley'}}, $new_product;
}

sub list_products {
    my $self = shift;
    my @products;
    foreach my $prod(@{$self->{'trolley'}}) {
        push @products, $prod->{'id'};
    }
    return @products;
}

sub get_product {
    my ($self, $id) = @_;
    $self->{'error'} = '';
    
    unless ($id) {
        $self->{'error'} = 'Product ID missing';
        return;
    }

    foreach my $prod(@{$self->{'trolley'}}) {
        if ($prod->{'id'} eq $id) {
            return $prod;
        }
    }
    $self->{'error'} = "Product ID $id not found";
}

sub delete_product {
    my ($self, $id) = @_;
    $self->{'error'} = '';
    
    unless ($id) {
        $self->{'error'} = 'Product ID missing';
        return;
    }
    
    for (my $i = 0; $i < scalar @{$self->{'trolley'}}; $i++) {
        if (${$self->{'trolley'}}[$i]->{'id'} eq $id) {
            $self->{'intent'} = undef;
            splice $self->{'trolley'}, $i, 1;
            return scalar @{$self->{'trolley'}};
        }
    }
    $self->{'error'} = "Product ID $id not found";
}

# Private method called internally by get_intent and get_intent_id
# Attempts to obtain a new session intent from Stripe
# Returns existing session if it exists and Trolley hasn't changed
sub _create_intent {
    my $self = shift;
    
    if ($self->{'intent'}) {
        return $self->{'intent'};
    }
    
    $self->{'reference'} //= __PACKAGE__;
    
    my $http = HTTP::Tiny->new;
    my $headers = {
        'Authorization' => 'Bearer ' . $self->{'api-secret'},
    };
    my $vars = {
        'headers'        => $headers,
    };
    my $payload = {
        'cancel_url'                => $self->{'cancel-url'},
        'success_url'               => $self->{'success-url'},
        'payment_method_types[0]'    => 'card',
        'mode'                        => 'payment',
        'client_reference_id'        => $self->{'reference'},
    };
    $payload->{'customer_email'} = $self->{'email'} if $self->{'email'
+};
    
    my $i = 0;
    foreach my $prod(@{$self->{'trolley'}}) {
        $payload->{"line_items[$i][currency]"}        = $self->{'curre
+ncy'};
        $payload->{"line_items[$i][name]"}            = $prod->{'name'
+};
        $payload->{"line_items[$i][description]"}    = $prod->{'descri
+ption'} if $prod->{'description'};
        $payload->{"line_items[$i][quantity]"}        = $prod->{'qty'}
+;
        $payload->{"line_items[$i][amount]"}        = $prod->{'price'}
+;
        $i++;
    }

    my $response = $http->post_form('https://api.stripe.com/v1/checkou
+t/sessions', $payload, $vars);
    
    $self->{'error'} = '';
    if ($response->{'success'}) {
        $self->{'intent'} = $response->{'content'};
    } else {
        $self->{'error'}  = decode_json($response->{'content'})->{'err
+or'}->{'message'};
    }
}

sub get_intent {
    my ($self, %attrs) = @_;
    
    $self->{'reference'} = $attrs{'reference'} if $attrs{'reference'};
    $self->{'email'} = $attrs{'email'} if $attrs{'email'};

    $self->{'error'} = '';
    return $self->_create_intent;
}

sub get_intent_id {
    my ($self, %attrs) = @_;
    
    $self->{'reference'} = $attrs{'reference'} if $attrs{'reference'};
    $self->{'email'} = $attrs{'email'} if $attrs{'email'};

    $self->{'error'} = '';
    my $intent = $self->_create_intent;
    if ($self->{'error'}) {
        return $intent;
    } else {
        return decode_json($intent)->{'id'};
    }
}

sub get_ids {
    my ($self, %attrs) = @_;
    
    $self->{'public-key'} = $attrs{'public-key'} if $attrs{'public-key
+'};
    
    $self->{'error'} = '';
    unless ($self->{'api-public'}) {
        $self->{'error'} = 'Required Public API Key missing';
        return;
    }
    
    $self->{'reference'} = $attrs{'reference'} if $attrs{'reference'};
    $self->{'email'} = $attrs{'email'} if $attrs{'email'};

    my $intent_id = $self->get_intent_id;
    
    my %result;
    if ($self->{'error'}) {
        $result{'status'}  = 'error';
        $result{'message'} = $self->{'error'};
    } else {
        $result{'status'}  = 'success';
        $result{'api-key'} = $self->{'api-public'};
        $result{'session'} = $intent_id;
    }
    
    return encode_json(\%result) if lc($attrs{'format'}) eq 'json';
    return $result{'message'} || "$result{'api-key'}:$result{'session'
+}";
}

sub checkout {
    my $self = shift;
    
    my $data = $self->get_ids( 'format' => 'text', @_);
    
    return if $self->{'error'};
    
    my ($key, $session) = split /:/, $data;
    
    unless ($key and $session) {
        $self->{'error'} = 'Error getting key and session';
        return;
    }
    
return <<"END_HTML";
Content-type: text/html

<html>
<head>
<script src="https://js.stripe.com/v3/"></script>
<script>
var stripe = Stripe('$key');
var result = stripe.redirectToCheckout({sessionId: '$session'});
if (result.error) {
    alert(result.error.message);
}
</script>
</head>
<body>
END_HTML

}

1;

__END__

=pod
 
=encoding UTF-8
 
=head1 NAME

Bod::Stripe - Simple way to implement payments using Stripe hosted che
+ckout

=head1 SYNOPSIS

  use Bod::Stripe;
  
  my $stripe = Bod::Stripe->new(
      'api-secret'  => 'sk_test_00000000000000000000000000',
  );
  
  # Note price is in lowest currency unit (i.e pence or cents not poun
+ds or dollars)
  $stripe->add_product(
      'id'        => 1,
      'name'    => 'My product',
      'qty'        => 4,
      'price'    => 250,
  );
  
  foreach my $id($stripe->list_products) {
      print "$id is " . $stripe->get_product($id)->{'name'} . "\n";
  }
  
  $stripe->checkout(
      'api-public'    => 'pk_test_00000000000000000000000000',
  );
  
=head1 DESCRIPTION
      
A simple to use interface to the Stripe payment gateway utilising the 
+Stripe hosted checkout.  The only dependencies are the core modules L
+<HTTP::Tiny> and L<JSON::PP>.

L<Bod::Stripe> has a Trolley into which products are loaded.  Once the
+ Trolley is full of the product(s) in the order, this is passed to th
+e Stripe hosted checkout either using Javascript provided by Stripe (
+see L<https://stripe.com/docs/payments/accept-a-payment?integration=c
+heckout>), Javascript provided in this document or the B<checkout> ut
+ility method that allows a server side script to send the user to Str
+ipe invisibly. 

At present L<Bod::Stripe> only handles simple, one-off payments.  Mani
+pulation of customers, handling subscriptions and user hosted checkou
+t is not supported.  However, this implementation makes payment for a
+ single item or group of items simple to implement.

=head2 Keys

Stripe provides four API Keys.  A Secret and a Publishable (called Pub
+lic within L<Bod::Stripe>) for both testing and for live transactions
+.  When calling the B<new> method it is necessary to provide the Secr
+et Key.  Before calling the B<checkout> method to redirect the user t
+o the Stripe hosted checkout, the Public Key is also required so this
+ is usually provided to the B<new> method.

See L<https://stripe.com/docs/keys>

=head2 Workflow

The basic workflow for L<Bod::Stripe> is to initially create an instan
+ce of the module with at minimum the Secret Key. If using a currency 
+other than GBP this should also be set at this time.

  my $stripe = Bod::Stripe->new(
      'api-public' => 'pk_test_00000000000000000000000000',
      'api-secret' => 'sk_test_00000000000000000000000000',
      'currency'   => 'USD',
  );

Next, products are assembled in the Trolley.  There are methods to add
+, update, remove and list the products in the Trolley.

  $stripe->add_product(
      'id'        => 1,
      'name'    => 'My product',
      'qty'        => 4,
      'price'    => 250,
  );
  my @products = $stripe->list_products;

Once the Trolley contains all the products, the user is redirected to 
+the Stripe hosted checkout where they pay for the Trolley.  Once this
+ happens, Stripe returns to your site using one of the URLs provided 
+delending on whether the payment was successful or not.  Where no ret
+urn URLs are provide, the script URL is used although in practice thi
+s is not usually sufficient and return URLs will be needed.

  $stripe->checkout;
  
Examples of other ways of redirecting the user to the Stripe hosted ch
+eckout and listed in the B<Examples> section.

=head1 METHODS

=head2 new

  Bod::Stripe->new('api-secret' => 'sk_test_00000000000000000000000000
+');
  
The constructor method.  The Secret Key is required.

The following parameters may be provided:

=over 4

=item *

C<api-secret> - B<required> The Secret Key.

=item *

C<api-public> - The Public Key.
This would normally be provided to the B<new> method but can be left u
+ntil sending the user to the Stripe hosted checkout.

=item *

C<success-url>

=item *

C<cancel-url> - The callback URL that Stripe returns to once the payme
+nt transaction has completed either successfully or otherwise.  If th
+ese are not explicitly included, the current script URL is used.  Nor
+mally these need setting but can be omitted for testing or if the Str
+ipe payment dashboard is being relied on to confirm successful paymen
+ts.

=item *

C<currency> - The currency to use for the transaction.  The default is
+ British Pounds Stirling (GBP).

This should be a 3 letter currency code supported by Stripe (see L<htt
+ps://stripe.com/docs/currencies>).

=item *

C<reference> - The reference to use for the transaction

Defaults to "Bod::Stripe" as this is required by Stripe.

=item *

C<email> - If provided, this pre-fills the user's email address in the
+ Stripe hosted checkout.  If provided, this is then non editable duri
+ng checkout.

=back

=head2 success

Returns true is the last method call was successful

=head2 error

Returns the last error message or an empty string if B<success> return
+ed true

=head1 Trolley Methods

=head2 add_product

Adds a product to the Trolley.  Or update the product if an existing B
+<id> is provided.

A product consists of the following hash entries

=over 4

=item *

C<id> - B<required>
A unique ID for the product.
This is not passed to Stripe and is only used by L<Bod::Stripe> to ide
+ntify current products.

If B<add_product> is called with an existing ID, that product is updat
+ed.

=item *

C<name> - B<required>
The name of the product as it will be passed to Stripe.

=item *

C<description> - B<optional>
A one line decsription of the product as it will be passed to Stripe. 
+ This is typically used to specify options such as colour.

=item *

C<qty> - B<required>
The number of the product to add to the Trolly.

=item *

C<price> - B<required>
The price of the product in the lowest currency unit.  For example - E
+<pound>2.50 would be 250 as it is 250 pence;  $10 would be 1000 as it
+ is 1000 cents.

Note that special rules apply to Hungarian Forint (HUF) and Ugandan Sh
+illing (UGX) - see L<https://stripe.com/docs/currencies>

=back

On success, returns the number of products in the Trolley

=head2 delete_product(id)

Delete the product with the specified id

On success, returns the number of products in the Trolley

=head2 list_products

Returns an array contining the IDs of the products in the Trolley

=head2 get_product(id)

On success, returns a hash with the product details.  Each key of the 
+hash corresponds to items listed for B<add_product>

=head1 Checkout Methods

=head3 parameters

The following methods all take the following optional parameters.  See
+ B<new> for their descriptions.

=over 4

=item *

C<reference>

=item *

C<email>

=back

=head2 get_intent

This method will not normally need calling.

Returns the full session intent from Stripe if successful or the Strip
+e error otherwise.

=head2 get_intent_id

Returns the intend_id that needs passing to the Stripe hosted checkout
+ if successful or the Stripe error otherwise.

=head2 get_id

In addition to the parameters listed above, this method also accepts t
+he following optional parameters

=over 4

=item *

C<public-api> - See B<new>

=item *

C<format> - The format of the returned information.  Current options a
+re JSON or text.  The default is text.

=back

Provides the Public Key and Intent Session ID as these are the two pie
+ces of information required by the Javascript provided by Stripe and 
+te Javacsript provided here.  If text output is used (the default) th
+e Public Key and Intent Session ID are provided as a colon separated 
+string.

=head2 checkout

A simple implementation of redirecting the user to the Stripe hosted c
+heckout.

Calling this method provides a fully formed HTML document including th
+e Content-Type header that can be sent to the users browser.  The HTM
+L document contains all the Javascript required to sent the user to t
+he Stripe hosted checkout transparently.  Unless you are building a c
+heckout with entirely AJAX calls, you will almost certainly want to u
+se this method.

=head1 EXAMPLES

=head2 1 - Using the Stripe provided Javascript

See L<https://stripe.com/docs/payments/accept-a-payment?integration=ch
+eckout>

=head3 Javascript

  <html>
    <head>
      <title>Buy cool new product</title>
      <script src="https://js.stripe.com/v3/"></script>
    </head>
    <body>
      <button id="checkout-button">Checkout</button>
  
      <script type="text/javascript">
        // Create an instance of the Stripe object with your publishab
+le API key
        var stripe = Stripe('pk_test_00000000000000000000000000');
        var checkoutButton = document.getElementById('checkout-button'
+);
  
        checkoutButton.addEventListener('click', function() {
          // Create a new Checkout Session using the server-side endpo
+int you
          // created in step 3.
          fetch('https://example.com/cgi-bin/trolley.pl?stripe=getInte
+nt', {
            method: 'POST',
          })
          .then(function(response) {
            return response.json();
          })
          .then(function(session) {
            return stripe.redirectToCheckout({ sessionId: session.id }
+);
          })
          .then(function(result) {
            // If `redirectToCheckout` fails due to a browser or netwo
+rk
            // error, you should display the localized error message t
+o your
            // customer using `error.message`.
            if (result.error) {
              alert(result.error.message);
            }
          })
          .catch(function(error) {
            console.error('Error:', error);
          });
        });
      </script>
    </body>
  </html>

=head3 Perl trolley.pl

  use Bod::Stripe;
  use strict;
  use CGI;
  
  my $cgi = CGI->new;
  
  if ($cgi->param('stripe') eq 'getIntent') {
      my $stripe = Bod::Stripe->new(
          'api-public'    => 'pk_test_00000000000000000000000000',
          'api-secret'    => 'sk_test_00000000000000000000000000',
          'success-url'   => 'https://www.example.com/yippee.html',
          'cancel-url'    => 'https://www.example.com/ohdear.html',
        'reference'     => 'My Payment',
      );
      
      $stripe->add_product(
          'id'          => 'test',
          'name'        => 'Expensive Thingy',
          'description' => 'Special edition version',
          'qty'         => 1,
          'price'       => 50000,
      );
      
      print "Content-Type: text/json\n\n";
      print $stripe->get_intent;
  }
  

=head2 2 - Simpler Javascript using XHR

=head3 Javascript

  <html>
  <head>
  <script src="https://js.stripe.com/v3/"></script>
  <script>
  var xhr=new XMLHttpRequest();
  function checkout() {
      xhr.open("POST", "https://www.example.com/cgi-bin/trolley.pl", t
+rue);
      xhr.onreadystatechange=function() {
      if (xhr.readyState == 4 && xhr.status == 200) {
          var keys = xhr.response.split(':');
          var stripe = Stripe(keys[0]);
          var result = stripe.redirectToCheckout({ sessionId: keys[1] 
+});
          if (result.error) {
              alert(result.error.message);
          }
      }
      xhr.send("stripe=getKeys");
  }
  </head>
  <body>
  <input type="button" value="Buy Now!" onClick="checkout();">
  </body>
  </html>
  
=head3 Perl - trolley.pl

  use Bod::Stripe;
  use strict;
  use CGI;
  
  my $cgi = CGI->new;
  
  if ($cgi->param('stripe') eq 'getKeys') {
      my $stripe = Bod::Stripe->new(
          'api-public'    => 'pk_test_00000000000000000000000000',
          'api-secret'    => 'sk_test_00000000000000000000000000',
          'success-url'   => 'https://www.example.com/yippee.html',
          'cancel-url'    => 'https://www.example.com/ohdear.html',
        'reference'     => 'My Payment',
      );
      
      $stripe->add_product(
          'id'          => 'test',
          'name'        => 'Expensive Thingy',
          'description' => 'Special edition version',
          'qty'         => 1,
          'price'       => 50000,
      );
      
      print "Content-Type: text/text\n\n";
      print $stripe->get_ids;
  }
  
=head2 3 - Simpest method (no Javascript required)

=head3 HTML

  <html>
  <head>
  <title>Simple Checkout</title>
  </head>
  <body>
  <form method="post" action="https://www.example.com/cgi-bin/trolley.
+pl">
  <input type="hidden" name="stripe" action="checkout">
  <label for="qty">How many do you want?</label>
  <input type="number" name="qty" min="1" name="qty">
  <input type="submit" value="Buy Now!">
  </form>
  </body>
  </html>
  
=head3 Perl - trolley.pl

  use Bod::Stripe;
  use strict;
  use CGI;
  
  my $cgi = CGI->new;
  
  if ($cgi->param('stripe') eq 'checkout') {
      my $stripe = Bod::Stripe->new(
          'api-public'    => 'pk_test_00000000000000000000000000',
          'api-secret'    => 'sk_test_00000000000000000000000000',
          'success-url'   => 'https://www.example.com/yippee.html',
          'cancel-url'    => 'https://www.example.com/ohdear.html',
        'reference'     => 'My Payment',
      );
      
       $stripe->add_product(
          'id'          => 'test',
          'name'        => 'Expensive Thingy',
          'description' => 'Special edition version',
          'qty'         => $cgi->param('qty'),
          'price'       => 50000,
      );
      
      if ($stripe->success) {
          print $stripe->checkout;
      } else {
          # handle errors...
      }
  }

This last example prints out a fully formed HTML to the browser comple
+te with Content-Type header.  If other headers are required, such as 
+Set-Cookie headers, they can be included immediately before printing 
+calling B<checkout>.

=head1 SEE ALSO
 
L<Net::Stripe>, L<Net::Stripe::Simple>, L<Business::Stripe>

=head1 AUTHOR

=over 4
 
=item *
 
Ian Boddison <ian@boddison.com>
 

=back
 
=head1 COPYRIGHT AND LICENSE
 
This software is copyright (c) 2021 by Ian Boddison.
 
All rights reserved.

This program is free software; you can redistribute it and/or modify i
+t under the same terms as Perl itself.
[download]

Thank you to the Monks who have helped me develop the skills to get the module this far and to the ones who will give useful feedback.
It is very much appreciated.

UPDATE:
I have run the code through Perl::Critic and it passes at 'stern' but generates warnings at 'harsh'. One thing it has found are tab characters instead of spaces despite changing my editor to use spaces after this discussion. I will take out the tabs that have crept in from copying and pasting a few bits of code.

On the suggestion of Critic, I have moved the declaration of $VERSION to after use strict;. Although I thought that had to be first for the CPAN toolchain?

Comment on [RFC] Module code and POD for CPAN Select or Download Code

Replies are listed 'Best First'.
Re: [RFC] Module code and POD for CPAN by eyepopslikeamosquito (Archbishop) on Apr 14, 2021 at 10:49 UTC
Years ago I wrote Writing Solid CPAN Modules and have updated it periodically. It is quite long though and with too many references (which I'm sure you don't have time to read). So I skimmed through it just now to extract an abbreviated "Bod's essential reading list": pause: On the Naming of Modules perlmodstyle Re: What do I use to release a module to CPAN for the first time? by davido Release::Checklist I'm rusty on CPAN authorship nowadays, so if I've overlooked other mandatory reading for new CPAN authors (and any other cool CPAN author references you may know of) please let us know.	[reply]
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 22:36 UTC
OK, so I have followed the instructions and used module-starter to create a package directories. I've them edited `Makefile.PL` and `MANIFEST`. `gmake test` revealed an error under Perl v5.32 which didn't show up in v5.16 where I was testing: `Error: Experimental splice on scalar is now forbidden at C:\Users\use +r\Perl\Business-Stripe-WebCheckout\blib\lib/Business/Stripe/WebChecko +ut.pm line 127, near "1;"` [download] Now corrected changing this line to explicitly dereference the array: `splice $self->{'trolley'}, $i, 1; splice @{$self->{'trolley'}}, $i, 1;` [download] It installs locally with `gmake install` Is this a good test result or do I need to be adding in more tests? C:\Users\user\Perl\Business-Stripe-WebCheckout>gmake test "C:\Strawberry\perl\bin\perl.exe" "-MExtUtils::Command::MM" "-MTest::H +arness" "-e" "undef Test::Harness::Switches; test_harness(0, 'blib\l +ib', 'blib\arch')" t/.t t/00-load.t ....... 1/? # Testing Business::Stripe::WebCheckout 0.11, +Perl 5.032001, C:\Strawberry\perl\bin\perl.exe t/00-load.t ....... ok t/manifest.t ...... skipped: Author tests not required for installatio +n t/pod-coverage.t .. skipped: Author tests not required for installatio +n t/pod.t ........... skipped: Author tests not required for installatio +n All tests successful. Files=4, Tests=1, 0 wallclock secs ( 0.06 usr + 0.01 sys = 0.08 CPU +) Result: PASS [download] But...I am getting an error at the `gmake dist` stage. C:\Users\user\Perl\Business-Stripe-WebCheckout>gmake dist "C:\Strawberry\perl\bin\perl.exe" -MExtUtils::Command -e rm_rf -- Busi +ness-Stripe-WebCheckout-0.1_1 "C:\Strawberry\perl\bin\perl.exe" "-MExtUtils::Manifest=manicopy,manir +ead" \ -e "manicopy(maniread(),'Business-Stripe-WebCheckout-0.1_1', ' +best');" mkdir Business-Stripe-WebCheckout-0.1_1 mkdir Business-Stripe-WebCheckout-0.1_1/t mkdir Business-Stripe-WebCheckout-0.1_1/lib mkdir Business-Stripe-WebCheckout-0.1_1/lib/Business mkdir Business-Stripe-WebCheckout-0.1_1/lib/Business/Stripe Generating META.yml Generating META.json tar cvf Business-Stripe-WebCheckout-0.1_1.tar Business-Stripe-WebCheck +out-0.1_1 a Business-Stripe-WebCheckout-0.1_1 a Business-Stripe-WebCheckout-0.1_1/Changes a Business-Stripe-WebCheckout-0.1_1/lib a Business-Stripe-WebCheckout-0.1_1/LICENSE a Business-Stripe-WebCheckout-0.1_1/Makefile.PL a Business-Stripe-WebCheckout-0.1_1/MANIFEST a Business-Stripe-WebCheckout-0.1_1/META.json a Business-Stripe-WebCheckout-0.1_1/META.yml a Business-Stripe-WebCheckout-0.1_1/README a Business-Stripe-WebCheckout-0.1_1/t a Business-Stripe-WebCheckout-0.1_1/t/00-load.t a Business-Stripe-WebCheckout-0.1_1/t/manifest.t a Business-Stripe-WebCheckout-0.1_1/t/pod-coverage.t a Business-Stripe-WebCheckout-0.1_1/t/pod.t a Business-Stripe-WebCheckout-0.1_1/lib/Business a Business-Stripe-WebCheckout-0.1_1/lib/Business/Stripe a Business-Stripe-WebCheckout-0.1_1/lib/Business/Stripe/WebCheckout.pm "C:\Strawberry\perl\bin\perl.exe" -MExtUtils::Command -e rm_rf -- Busi +ness-Stripe-WebCheckout-0.1_1 gzip -9f Business-Stripe-WebCheckout-0.1_1.tar process_begin: CreateProcess(NULL, gzip -9f Business-Stripe-WebCheckou +t-0.1_1.tar, ...) failed. make (e=2): The system cannot find the file specified. gmake: *** [Makefile:612: Business-Stripe-WebCheckout-0.1_1.tar.gz] Er +ror 2 [download] I've check the `tar` file and everything appears to be in there so I take it the error is packing the `tar` file into a `.gz` archive. But I'm not sure what to try next...	[reply] [d/l] [select]
Re^3: [RFC] Module code and POD for CPAN by hippo (Bishop) on Apr 14, 2021 at 22:53 UTC
Is this a good test result or do I need to be adding in more tests? Both! It is good in that the stock tests pass but they are only testing for very basic things - have a read of `t/00-load.t` to see how little it actually does. What you should do next is write your own tests which actually test your code to show that it does what it is supposed to do and that it handles edge cases as intended, etc. Also, the `skipped: Author tests not required for installation` is great for end users but not for you as author. You want to run those yourself so be sure to set the `AUTHOR_TESTING` environment variable to get them to run for you. 🦛	[reply] [d/l] [select]
Re^4: [RFC] Module code and POD for CPAN - Testing and test file by Bod (Parson) on Apr 15, 2021 at 17:23 UTC
Re^5: [RFC] Module code and POD for CPAN - Testing env vars by hippo (Bishop) on Apr 16, 2021 at 08:51 UTC
Re^5: [RFC] Module code and POD for CPAN - Testing and test file by choroba (Cardinal) on Apr 16, 2021 at 06:55 UTC
Some notes below your chosen depth have not been shown here
Re^5: [RFC] Module code and POD for CPAN - Testing and test file by hippo (Bishop) on Apr 15, 2021 at 22:02 UTC
Some notes below your chosen depth have not been shown here
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 11:18 UTC
Fabulous...thank you 😊 I shall do some more reading later today. As it is Weekend Wednesday and the sun is shining, outside is calling first...	[reply]
Re: [RFC] Module code and POD for CPAN by jwkrahn (Abbot) on Apr 14, 2021 at 03:42 UTC
`for (my $i = 0; $i < scalar @{$self->{'trolley'}}; $i++) { if (${$self->{'trolley'}}[$i]->{'id'} eq $id) { $self->{'intent'} = undef; splice $self->{'trolley'}, $i, 1; return scalar @{$self->{'trolley'}}; } }` [download] That will not work correctly if you have more than one deletion. Also, splice on an array reference will not work with all versions of perl. This should be better: `for ( my $i = $#{ $self->{ trolley } }; $i >= 0; --$i ) { if ( ${ $self->{ trolley } }[ $i ]->{ id } eq $id ) { $self->{ intent } = undef; splice @{ $self->{ trolley } }, $i, 1; return scalar @{ $self->{ trolley } }; } }` [download]	[reply] [d/l] [select]
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 11:20 UTC
Thank you for pointing that out. It shows a gap in my testing...I hadn't tried deleting a second product! I had planned to convert this loop into a more Perl-ish version rather than the current C-ish version: `for my $i(0...@{$self->{'trolley'}})` [download]	[reply] [d/l]
Re^3: [RFC] Module code and POD for CPAN by choroba (Cardinal) on Apr 14, 2021 at 12:26 UTC
This isn't python, the last element in the range is not excluded! `for my $i (0 .. $#{ $self->{trolley} }) {` [download] `map{substr$_->[0],$_->[1]\|\|0,1}[\\|\|{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^ARGV,3]`	[reply] [d/l] [select]
Re^4: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 12:43 UTC
Re^3: [RFC] Module code and POD for CPAN by jwkrahn (Abbot) on Apr 14, 2021 at 16:52 UTC
The problem is not the type of for loop, the problem is starting at the beginning of the array when changing the number of elements in the array. If you are going to remove elements of an array in a for loop you have to start at the end of the array. So this might work UNTESTED: `for my $i ( reverse 0 .. $#{ $self->{ trolley } } )` [download]	[reply] [d/l]
Re^4: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 18:53 UTC
Re: [RFC] Module code and POD for CPAN by hippo (Bishop) on Apr 14, 2021 at 08:47 UTC
First and foremost: nice work! It's good that you are (a) writing this module and (b) looking to release it to CPAN. It is also in pretty good shape for a first go - well done. I was thinking Business::Stripe::Simple as the module name - is that sensible? It isn't crazy :-) However, since Stripe can do a great deal more than you are using it for here (according to the other CPAN Stripe modules) maybe putting something about the purpose in the name would be even better. Business::Stripe::Checkout perhaps? Or ...::WebCheckout to indicate the useragent talks to the Stripe servers directly. Of course if you are planning some later features unrelated to checkout then this becomes less useful as a name. I will have a look through the code later as time permits since this is something I might actually have a use for. 🦛	[reply]
Re^2: [RFC] Module code and POD for CPAN by eyepopslikeamosquito (Archbishop) on Apr 14, 2021 at 09:32 UTC
Also note that pause: On the Naming of Modules exhorts you to avoid `Simple` in module names: The terms Simple, Easy, Reduced, and Tiny are some of the worst parts of the names on CPAN. They all indicate that the module is a variation of another module, but why is that variation interesting? It's usually missing or hiding some features, less flexible than the original, and in most cases, tailored to the task the author needed. What is that task though? Making it easy for you doesn't mean it's easy for the next programmer.	[reply] [d/l]
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 11:15 UTC
Business::Stripe::WebCheckout or ~~Net::Stripe::WebCheckout~~ are the top contenders so far Thanks hippo Update: Having read some of the material suggested by eyepopslikeamosquito, Net::Stripe::WebCheckout has been eliminated.	[reply]
Re: [RFC] Module code and POD for CPAN by Discipulus (Canon) on Apr 14, 2021 at 06:56 UTC
Hello Bod, is long to read it carefully, so just some notes. `$attrs{'currency'} //= 'GBP';` defined or was intruduced in `5.10.0` so you need `use 5.010;` in your code and in your `Makefile.PL` or what you use instead of it. If you want to support older perl versions you need an alternative syntax like: `$attrs{'currency'} = $attrs{'currency'} ? $attrs{'currency'} : 'GBP' ;` `my %attrs = @_;` you put some type checking some line below. I like it splitted into another, dedicated sub: `my %attrs = _check_args(@_);` here you can excercise your fantasy at will. Wrong args can be common for a public used module. Carp is another things I like a lot. Generally a misuse by a module consumer should point to code invoking and provoking the error, not to your module code at line ... Internal errors of your module should be highlighted more carefully, possibly pointing both to your and their code. Searching for `attrs` in your code I noticed something strange: your methods `get_intent get_intent_id get_ids` all starts with `my ($self, %attrs) = @_;` I do not understand: only `get_ids` is called with arguments. Then (but I do not looked to it closer) why these attrs are not incapsulated inside the main object? Internal usage of `HTTP_HOST` and the alike should be documented. I dont recall the whole thing, but document it. They are always used by every server? Only by some? (I'm not a web expert as you can see ;) Finally I like your pod because it explains nicely what the matter is: this is a rare quality! It appears you are using `CGI` as all your example use it: in 2021 one might expect some example at least in Dancer2 Then, but is my lack of understanding, I dont get why your module ends returning some html. I think you try to explain it in the pod but I dont get it :) Do you plan to test your module against a live sandbox api? In author tests or during `make test` aka everytime someone install your module? This can be fun :) L* There are no rules, there are no thumbs.. Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.	[reply] [d/l] [select]
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 11:11 UTC
Thank you so much for your helpful feedback A few question about them if you don't mind... If you want to support older perl versions... This is something I haven't looked at yet. I have looked at what version of the dependencies are required which are v0.014 of HTTP::Tiny and v2.00 of JSON::PP. Do you think I should support Perl versions older than v5.10 given that Perl v5.12's 11th birthday was 2 days ago!? If I decide not to support versions prior to v5.10, does the declaration need to go in the module or just in Makefile.PL? Searching for attrs in your code I noticed something strange: your methods get_intent get_intent_id get_ids all starts with my ($self, %attrs) = @_; I do not understand: only get_ids is called with arguments. Then (but I do not looked to it closer) why these attrs are not incapsulated inside the main object? You have highlighted a typo and a shortcoming in the clarity of the documentation! `get_intent`, `get_intent_id` and `get_ids` (not `get_id` as the documentation says!) all take the optional parameters `reference` and `email` (users may or may not want to use these parameters depending on their use case). Only `get_ids` optionally takes two other parameters: `public-api` and `format</i>. Controlling the output format between JSON and text is only relevant with this method. The <code>public-api` is needed by this method but not by others and needs to be passed in either here or at instantiation. I imagine users will pass both keys to the `new` method but having the option gives users the choice. Internal usage of HTTP_HOST and the alike should be documented. I dont recall the whole thing, but document it. They are always used by every server? Only by some? I'm really glad you picked this up! I had to do quite a bit of research to find what to do here and I still didn't come up with a definitive answer...this is why there is a TODO at the top of the code to make this more robust and why it checks for callback URLs in the constructor. Within my own environment I have historically used `SCRIPT_URI` but every now and then it disappears for reasons I don't understand - CGI environment variables seem to be bit of a dark art so I have deliberately used ones that seem to be standard. I don't have an instance of IIS to test it on. Having said that, I cannot imagine a situation outside of initial testing, where the end user will not want to send the customer to a different page depending on whether they paid successfully or not. So `cancel-url` and `success-url` would be explicitly set. It appears you are using CGI as all your example use it: in 2021 one might expect some example at least in Dancer2 Internally we use a bespoke decoder of query strings and form data so I had to pick something generally available. I picked CGI because I thought it was core (it isn't any more), I know the (basic) syntax and it is simple to understand. Perhaps Dancer2 would be better in the examples but I only put the examples in to show how easy this module is to use and to give people a headstart. I dont get why your module ends returning some html Thanks - I shall improve the documentation to explain this better	[reply] [d/l] [select]
Re^3: [RFC] Module code and POD for CPAN by Discipulus (Canon) on Apr 14, 2021 at 15:31 UTC
Hello Bod > Do you think I should support Perl versions older than v5.10 given that Perl v5.12's 11th birthday was 2 days ago!? If I decide not to support versions prior to v5.10, does the declaration need to go in the module or just in Makefile.PL? I dont think :) This is up to you. Some ancient systems still run perl versions with mustache and beards. Not being my opinion so much relevant, I assume 5.10 or 5.14 as my own personal backward compatibility limit. That said I suppose you must put it in both place, but probably `Makefile.PL` is enough being the first thing run during installation. My last module has it stated twice: `use 5.010; use strict; use warnings; use ExtUtils::MakeMaker; WriteMakefile( ... MIN_PERL_VERSION => '5.010', ... },` [download] I suspect the first one directly aborts the installation (as the file does not compile) and the second can/should generate a report. The redundance does not hurts me. > take the optional parameters reference and email .. Here also I'd put some check from a banal existence of a `@` to the usage of Regexp::Common::Email::Address > I don't have an instance of IIS to test it on The code is meant to run on IIS? I have made something with it and is a pain. Every other solution is better also handling HTTP requests by hand with pen and paper :) Eventually Apache can run on windows, some `plack` servers too. I'm a windows sysadmin: my best wishes. > CGI.. Well.. sincerely seing CGI example can make people fly in horror. Anyway I think no new projects use CGI nowadays. There are valid alternatives and mainly Mojolicious and Dancer2. Being in your cloths I'd propose a CGI example as the last one or ones. Queryin parameters is easy and readable with Dancer2 `get '/hello/:name?' => sub { my $name = route_parameters->get('name') // 'Whoever you are'; return "Hello there, $name"; };` [download] > Having said that, I cannot imagine a situation outside of initial testing, where the end user will not want to send the customer to a different page depending on whether they paid successfully or not. So cancel-url and success-url would be explicitly set. Reasoning a bit I dont like the solution. Your module should only do the operation needed to interact with this Stripe. I'm not a serious web dev but I think you should avoid it. In Dancer2 or Mojolicious you have the default route `/` and is the same of your `SCRIPT_NAME` +/- I suspect will be a pain refactor this part after the module is in production: rethink it if needed. > $attrs{'error'} = ... In the constructor your code returns only the last one error set. Then I prefere `undef $attrs{error}` over setting it to an empty string in the rest of the module. L* There are no rules, there are no thumbs.. Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.	[reply] [d/l] [select]
Re^4: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 22:26 UTC
Re: [RFC] Module code and POD for CPAN by bliako (Monsignor) on Apr 14, 2021 at 13:29 UTC
$attrs{'error'} = 'cancel-url and success-url cannot be de +rived from the environment and need to be provided' unless ($attrs{'c +ancel-url'} and $attrs{'success-url'}); $attrs{'error'} = 'Public API key provided is not a valid key' if +$attrs{'api-public'} and $attrs{'api-public'} !~ /^pk_/; $attrs{'error'} = 'Secret API key provided is not a valid key' unl +ess $attrs{'api-secret'} =~ /^sk_/; $attrs{'error'} = 'Secret API key provided as Public key' if $attr +s{'api-public'} and $attrs{'api-public'} =~ /^sk_/; $attrs{'error'} = 'Public API key provided as Secret key' if $attr +s{'api-secret'} =~ /^pk_/; $attrs{'error'} = 'Secret API key is too short' unless length $att +rs{'api-secret'} > 100; $attrs{'error'} = 'Secret API key is missing' unless $attrs{'api-s +ecret'}; return bless \%attrs, $class; [download] If you must use this style for signaling an error then perhaps you should document that gettting an object back (as opposed to undef) is not a guarantee that object construction succeeded but it should check for the success() `my $obj = Bod::Stripe->new(...); die $obj->error() unless $obj->success();` (I have run through the POD diagonally and I think I did not see any mention). bw, bliako	[reply] [d/l] [select]
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 14:37 UTC
If you must use this style for signaling an error then perhaps you should document that gettting an object back (as opposed to undef) is not a guarantee that object construction succeeded Good point thanks - I shall add that to the documentation. Returning an object came from advice given by GrandFather in Re: [RFC] Review of module code and POD "Consider having new return an instance of Bod::CRM with its error field set..."	[reply]
Re: [RFC] Module code and POD for CPAN by Arunbear (Prior) on Apr 14, 2021 at 11:43 UTC
I agree with hippo's suggestion that 'Checkout' should be part of the name. Also Your class is doing checkout handling and trolley handling (Single responsibility pattern / Cohesion). Consider having a separate Trolley object for trolley handling. The pattern of iterating over the trolley and doing something to just one item suggests the trolley ought to be a hash (keyed by the product ID). You can add extra keys to aid with sorting if you need that. Consider using Moo or something similar to make the OOP more ergonomic. Exceptions are usually preferable to silent returns because they are harder to ignore (see Try::Tiny). 'unless' is tricky for many people (it doesn't scale up, can lead to double negatives etc). 'and' and '&&' are not the same thing (the former is for combining statements)	[reply]
Re^2: [RFC] Module code and POD for CPAN by hippo (Bishop) on Apr 14, 2021 at 12:43 UTC
I'm about to disagree to a greater or lesser extent with a lot of your points here. Most of them are (I hope you will agree) subjective so please treat these responses as just my own opinions. In general, separation of concerns is good and I am a firm proponent of the Unix Philosophy. However, since a trolley without a facility to check out is pretty useless and a checkout without a trolley equally so I humbly suggest that this is at least not as big a deal for this module as it might be elsewhere and may not even be desirable here at all. Bod should indeed consider it, as you have said. I agree here, unless the items in the array can be referenced by their index but that sounds like even more work in this instance. Considering alternatives like Moo is a worthwhile exercise but also remember that TANSTAAFL. Again, TANSTAAFL. Lots of devs swear by exceptions. Other devs swear at exceptions. I'm fairly ambivalent on them. To use them well you need structure and that means a framework and that means more dependencies. Just be aware of the costs of using them or not. This is the one I really disagree with. No doubt some people find `unless` tricky. Pick any feature of the language and some people will find it tricky. But does a module author care what others find tricky? Postfix deref drives me nuts but that doesn't mean that module authors should stop using it. Further, I actively like `unless` because it means that horrible things like `if (!$foo \|\| !$bar \|\| !$baz) { ... }` become `unless ($foo && $bar && $baz) { ... }` which is (subjectivity klaxon) much clearer to read and in intent. The only reason it doesn't fully scale is that there is no `elsunless` to correspond to `elsif` which is an oversight in the Perl language and not an inherent flaw with `unless` itself. Bod should feel entirely free to use `unless` throughout his code in the same manner that you should feel entirely free to eschew it in yours. I completely agree (phew :-) I am neither intending nor expecting to change any of your opinions by posting this but am trying to provide an opposite view so that Bod and others are aware of the contrary viewpoints. 🦛	[reply] [d/l] [select]
Re^2: [RFC] Module code and POD for CPAN by Bod (Parson) on Apr 14, 2021 at 12:38 UTC
Thanks - quick reply to explain ~~two~~ three points. I'll look at the rest in more detail a little later 1. Your class is doing checkout handling and trolley handling (Single responsibility pattern / Cohesion). Consider having a separate Trolley object for trolley handling. I did think of this. However, the Trolley only exists as a repository of the 'stuff' that is needed by the Stripe checkout. It isn't (intended to be) useful by itself. 3. Consider using Moo or something similar to make the OOP more ergonomic. I expect much of the use of this module to be for small businesses using shared hosting. For this reason, I want to keep the dependencies as only core modules. 4. Exceptions are usually preferable to silent returns because they are harder to ignore (see Try::Tiny) As above, Try::Tiny isn't core. Also, having `$stripe->success` is consistent with the Javascript methods provided by Stripe in `stripe.js` I'm not suggesting I made the optimal choices for these, just explaining why I chose to do things the way I did.	[reply] [d/l] [select]
Re^3: [RFC] Module code and POD for CPAN by Arunbear (Prior) on Apr 14, 2021 at 16:23 UTC
Class::Tiny is similar in purpose to Moo, and like Try::Tiny it has no non-core dependencies, which means installing those (on shared hosting) is no more difficult than installing this particular project.	[reply]

Back to Meditations