I'm trying to extend the PayflowProRecurring class in order to add functionality that I'm not able to wait to be added to a release. However, methods I need to class from the class are private, such as profileWrap(). It would be easier to create submodules if methods such as this were protected.

(Unless you absolutely don't want anyone to do that and that's why you set them to private.)

CommentFileSizeAuthor
#6 uc_payflowpro-d6-1024934.patch4.39 KBcwithout
#6 uc_payflowpro-d6-dev-1024934.patch4.01 KBcwithout

Comments

cwithout’s picture

cwithout commented
Title: Suggestion making class methods Protected instead of Private scope » Class methods Protected instead of Private scope
kwinters’s picture

kwinters commented

What exactly are you trying to do?

Drupal module customizations are generally done through hooks, not inheritance. That allows you to put all your customizations in a separate module, and allows for clean updating when a new module version is released.

The other thing I'd suggest is simply making your modifications directly for now and submitting a patch. You can just make do with what you have for now, and by the next release your changes could be incorporated in the official version.

kwinters’s picture

kwinters commented
Status: Active » Postponed (maintainer needs more info)
cwithout’s picture

cwithout commented

I know things are usually done in Drupal with hooks, but I'm not really interested in using hooks. I'd prefer the ability to extend the class to add the ability to interact with the Payflow API in ways that are not already defined.

Unfortunately, there's not usually time to request and wait for official changes to a module when clients unrealistically expect things done yesterday. Also functionality that is needed for one site shouldn't always be added to the module.

What I did was to add the ability to for coupons to apply to the recurring payments. I'm using getStatus(), setAmt() then updating the profile with the new payment amount. I'm using functionality that's not currently defined in your class, but I want the connection functionality so I don't have to recreate something that's already done. Since the PayflowProRecurring class methods are private, I had to copy the class to my custom module. It would have been more efficient to extend the original class.

The functionality I added for that site isn't something that should be patched into the module, because the method I used requires that only one product is allowed to be purchased (which wouldn't apply to all sites).

Unless I'm missing something, I don't really see the down side to making the methods protected and allowing developers to extend the class when they need to add functionality that applies to only their site.

kwinters’s picture

kwinters commented

If you are going to actually use the change from private to protected, it's unlikely to hurt anything and I'm perfectly happy to make it. However, changes that nobody will use can still add bugs (law of unintended consequences) so I avoid that when possible.

Please make the modifications in your local copy and make sure they actually help you. If they do, submit a patch containing just the parts you want pushed upstream (maybe just protected) and I'll make it a part of the next release. This way you can still have your changes "yesterday" without creating a maintenance problem for yourself in the next uc_pfp release.

cwithout’s picture

cwithout commented
StatusFileSize
new uc_payflowpro-d6-dev-1024934.patch4.01 KB
new uc_payflowpro-d6-1024934.patch4.39 KB

I didn't know whether to make the patch for head or dev. So here's both I believe.

cwithout’s picture

cwithout commented
Status: Postponed (maintainer needs more info) » Needs review
kwinters’s picture

kwinters commented

DRUPAL-6--1 is probably the branch you want. --2 is the in-development branch with uc_recurring.

Everything working out OK on your end re: actually using the class?

cwithout’s picture

cwithout commented

Cool. The patch uc_payflowpro-d6-dev-1024934.patch is the DRUPAL-6--1 branch. Yes, it accomplishes the task.