Talk:Kaprekar numbers: Difference between revisions

Content added Content deleted
(Responded to Paddy3118 about code change criteria)
(Responded to Nigel)
Line 167: Line 167:


Hi Larry, what implementation of common lisp are you using? I only replaced a function number of digits, which calculated the number of digits by dividing by 10 until less than 10 with floor (log N / log Base) which is a reasonable way to determine the number of digits in a number. This should be faster unless your implementation has no reasonable log function. Note that BASE and BASE-1 are not variables they are constants. I didn't change the logic. A mute point now as ledrug has done tha task properly.--[[User:Nigel Galloway|Nigel Galloway]] 18:44, 19 September 2012 (UTC)
Hi Larry, what implementation of common lisp are you using? I only replaced a function number of digits, which calculated the number of digits by dividing by 10 until less than 10 with floor (log N / log Base) which is a reasonable way to determine the number of digits in a number. This should be faster unless your implementation has no reasonable log function. Note that BASE and BASE-1 are not variables they are constants. I didn't change the logic. A mute point now as ledrug has done tha task properly.--[[User:Nigel Galloway|Nigel Galloway]] 18:44, 19 September 2012 (UTC)
: Hi Nigel, I did the initial testing using CLISP. I've since tested both versions using LispWorks, which improved your times substantially, but it is still noticeably slower. I'll separate out your other points so that you can respond to each comment directly. --[[User:Lhignight|Larry Hignight]] 22:42, 23 September 2012 (UTC)

"I didn't change the logic."<br>Perhaps you didn't intend to change the logic, but your version fails to identify 9,999,999 and 99,999,999 as Kaprekar numbers whereas mine correctly identifies them, so yes, you did change the logic. --[[User:Lhignight|Larry Hignight]] 22:42, 23 September 2012 (UTC)

"Note that BASE and BASE-1 are not variables they are constants."<br>While I applaud the additional mathematical insight that you provided, I'm concerned that you don't have very much experience as a Lisp developer and should probably refrain from modifying existing Lisp solutions. For example, the CLHS explicitly states that [http://www.lispworks.com/documentation/HyperSpec/Body/s_setq.htm setq] is used with variables "other than a constant variable." In addition to BASE and BASE-1 not being constants, which isn't what you didn't want in the first place, there are a number of other issues:
*In CL, the standard method for defining a constant is the [http://www.lispworks.com/documentation/HyperSpec/Body/m_defcon.htm defconstant] form. It is also the convention in CL to name constants with leading and trailing + characters (eg +base+).
*The unnecessary use of dynamically scoped variables is discouraged. The preferred method for incorporating base would have been to define it as an optional function parameter with a default value of 10.
*The manner in which you replaced the num-digits macro resulted in code that was less readable. In my version, it was very clear when the number of digits were being calculated because I used an abstraction to separate this calculation from the function. You eliminated a useful abstraction.
*For some reason, you replaced the let* form with the lh and rh values with a multiple-value-bind form. I really have no idea why you would do this.
*I'm not sure why you feel that two calls to log followed by floor() and adding 1 would be faster than a loop that runs in theta(log n) time. --[[User:Lhignight|Larry Hignight]] 22:42, 23 September 2012 (UTC)

"A mute point now as ledrug has done tha task properly."<br>Yeah, it's kind of a shame that someone broke it though. --[[User:Lhignight|Larry Hignight]] 22:42, 23 September 2012 (UTC)