HouseholdNaming class should be overrideable to allow users to define their own schemes

[GoogleCodeExporter]

What steps will reproduce the problem?
1.  Install Households 2.3.7+
2.  Write a class extending HouseholdNaming
3.  Save new contacts and households

What is the expected output? What do you see instead?
Expected: Households are naming according to custom rules
Actual:   Households are named according to default rules

NOTE:  just creating this as a place for discussion of the fix, since it's 
starting to get a bit too technical for the NPSP-Discuss list.

Original issue reported on code.google.com by benj.k...@gmail.com on 18 Aug 2011 at 10:37

[GoogleCodeExporter]

Kevin,
Do you know why the methods Households.insertHousehold and 
Household.evaluateContacts are defined as static?

I'm trying to have them call an overrideable method to delegate Household 
Naming, but since they're static, that method would also have to be static, but 
static methods can't be declared virtual.

From scanning the code, it seems like they wouldn't need to be static, but I 
wanted to check in before making that change.

B

Original comment by benj.k...@gmail.com on 18 Aug 2011 at 10:40

[GoogleCodeExporter]

For earlier discussion of this issue, see 
https://groups.google.com/forum/#!topic/npsp-discuss/Yc1R6SYII8s

Original comment by benj.k...@gmail.com on 18 Aug 2011 at 10:43

[GoogleCodeExporter]

After playing with this a bit more, I think it makes sense to develop in a 
slightly different direction:

Triggers are responsible for instantiating a HouseholdNaming (or subclassed) 
object and passing it in to the Households constructor (if none or null is 
submitted, a default HouseholdNaming object can be created instead).  Whenever 
naming needs to happen, the Households class simply calls or delegates to that 
class.  That way, Households needs to be global (so it can be called from 
outside the npo02 namespace) but *doesn't* have to be virtual, b/c there's no 
need to override it.

There are some other refactorings necessary to make this work, but that's main 
architecture.  Thoughts?

Original comment by benj.k...@gmail.com on 23 Aug 2011 at 1:29

[GoogleCodeExporter]

Sorry for the delay - dreamforce prep has been keeping me a bit more busy than 
I'd like...

To your first post, no, I'm not sure why they're declared static, they were 
written prior to my arrival, so I wasn't part of the architecture discussion 
then. I can't see any compelling reason as to why they need to be static-

I like the idea of calling directly into the Household class and passing the 
naming object, however, making it global still locks the signature in, which 
isn't totally ideal, (of course, not having to virtualize allows us to add 
additional methods/etc. to the class if needed). The issue is the constructor - 
as I believe that will be locked in as well for the global declaration, so 
adding additional objects to it later (say, household address syncing, or the 
like) may be difficult. (OTOH, I could be wrong about the constructor, I'll try 
and check that in a bit, maybe you can overload it in that scenario if you need 
add additional objects to the params)

I think if the trade-off makes sense, I'm fine with that 'lock-in' to open 
things up a bit more for development, just want to make sure we're covering the 
bases

Original comment by kbro...@gmail.com on 23 Aug 2011 at 3:49

[GoogleCodeExporter]

Hey gents- just a quick FYI. There's an up-to-date unmanaged version of the 
NPSP now available, plus some updated developer docs you might be interested 
in. New site here:

http://nonprofitstarterpack.org/DeveloperResources

KB

Original comment by kbro...@gmail.com on 30 Aug 2011 at 9:10

[GoogleCodeExporter]

Love the new website - way to go, Kevin!

Original comment by benj.k...@gmail.com on 31 Aug 2011 at 1:14

[GoogleCodeExporter]

Thanks- wanted to point out the unmanaged version of the NPSP available on 
Github too, should make it easier to install for dev work:

https://github.com/SalesforceFoundation/NPSP-UM

Original comment by kbro...@gmail.com on 31 Aug 2011 at 7:21

[GoogleCodeExporter]

Original comment by kbro...@gmail.com on 8 Apr 2012 at 5:02

• Changed state: Accepted
• Added labels: NonPatch, Package-Households, Type-Enhancement, Usability

[GoogleCodeExporter]

Wow, time totally got away from me on this one.  I may have some bandwidth 
available to help move this forward.  Has there been any other movement in the 
past 8 months?

Original comment by bk...@healthleadsusa.org on 8 Apr 2012 at 3:26

[GoogleCodeExporter]

Funny you should ask, I was literally just playing with this now - 

I think the correct approach would be two fold:
1.  Disable the existing trigger and replace with an identical trigger which 
calls your overriden Householdnaming in place of the packaged naming. 
2.  In your Householdnaming, override any methods you wish to, then call the 
super versions of any you do not.  

Of course, right now because naming is called from within the Households class 
during insert, you could only override the update behavior with the method 
above. I'll need to figure out a better workflow (maybe your suggestion above 
of always having the trigger instantiate the HouseholdNaming class) for making 
that work going forward. 

There are some options around allowing a setting to define the class for 
various use cases as well that I'm going to investigate (uses the new JSON 
parser interestingly enough) - I'm working on putting together a new HH 
release, hopefully before May, would like to have a solid answer sooner rather 
than later. I'll post any updates here. 

Original comment by kbro...@gmail.com on 8 Apr 2012 at 5:15

[GoogleCodeExporter]

I just skimmed the Summer '12 release notes, and it looks like 
Type.newInstance() may be the answer to our challenge:

 * define an interface for the NamingRulesClass
 * define a default implementation of the interface
 * admins can subclass the default class OR define a new implementation
 * add a Custom Setting where admins can define an alternative naming class

then we get:

String namingRulesClass = settings.namingRulesClass__c;
NamingRules nr = Type.forName(namingRulesClass).newInstance();
// use the nr class

What do you think?

Original comment by benj.k...@gmail.com on 7 May 2012 at 1:25