"Khorn (WMF)" posted a comment on Wikimedia.r1338. URL: http://www.mediawiki.org/wiki/Special:Code/Wikimedia/1338#c31184
Commit summary for Wikimedia.r1338: Added _civicrm_recurring_globalcollect_get_payment_by_id(), _civicrm_recurring_globalcollect_get_next_sched_contribution_date(). Updated documentation. Khorn (WMF)'s comment: But perhaps the biggest issue here, is that _civicrm_recurring_globalcollect_get_next_sched_contribution_date has an awful lot going on in it to be recursive. <br>Generally speaking, recursive functions are terrible every time you don't do these things: <ul> <li>Keep the logic in your recursive functions as simple as possible. Do not calculate what is going to behave like a constant on every iteration.</li> <li>Keep it concise. Later editors need to be able to identify it as a recursive function at a glance, even if you didn't comment it as such.</li> <li>Comment it as such</li> <li>Have a very well-defined base case that is immediately identifiable. </li> </ul> If you absolutely must use recursion here, please turn the main function into a wrapper that does all the things that should be one-off error handling and setup stuff first. That wrapper can then call a very lean and easy to humanly parse recursive function, once you make sure the recursive function isn't doing any logic that doesn't have to be repeated. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
