|
-
May 1st, 2013, 06:13 PM
#19
Re: [RESOLVED] catenary problem
Here's a Python translation of my previous code. It gives correct output for a(10, 5, 3). Just to be clear, I modified more than just the fx line. Does your version calculate a correctly while still producing incorrect output later?
Code:
def a(s, h, d):
i=0
tempResult = 1
result = math.sqrt(s**2 - h**2)
while i <= 500:
fx = 2 * tempResult * math.sinh(d/(2 * tempResult)) - result
dFx = 2 * math.sinh(d/(2 * tempResult)) - (d * math.cosh(d/(2 * tempResult)) / tempResult)
if abs(fx) <= 0.0001: break
tempResult -= fx/dFx
i += 1
return tempResult, fx, result
Edit: As for stylistic issues, some that come to mind immediately...
(Note: You didn't post your full code, and it seems you didn't just copy and paste it, so some of this may not be relevant.)
* Bad variable names--tempResult is annoyingly long to type yet not terribly informative; "a" would probably be better. "result" is probably more intuitively named "target" or simiar. "dFx" capitalizes the F for no apparent reason, and the x isn't helpful. "df" and "f" are probably better. "epsilon" is probably alright, though "errorMargin" is more informative and fits with the iterationSteps naming convention without being much longer.
* Bad comments: what you did comment wasn't very efficient/helpful, and you didn't write the important things. You're using Newton's method for root finding, which should probably be at the top of the routine; let someone Google it for an explanation. "basically just a derivative of fx" is incorrect on two counts--it's not just "basically" a derivative, it is the derivative. The long comment on the "if" line is just plain unhelpful--anyone who can read code will understand what that line is doing (well, if it were written correctly). In writing comments, you want to give design details that can't be instantly gleaned from just reading the code.
* Not very clean: your Newton's Method code could be made cleaner in a few ways. Switching from "while" to "for" would get rid of the "i += 1" at the end, the "i=0" at the beginning, and be more intuitive. You repeatedly use (d/(2 * tempResult)) in your formulas, so it might be nice to put this in a variable by itself, just for readability. (A compiler should optimize out the inefficiency of recomputing it anyway, so that shouldn't be an issue.)
* Potentially bad design: there are a zillion implementations of Newton's Method or more powerful numerical solvers out there. Depending on your language, it might be best to just import a numerical library and have it do the work. The main benefit of this general approach IMO is that someone else has done the debugging work for you. Also, depending on the language, there are more intuitive implementations of Newton's Method: you could perhaps create a general routine that takes in a pair of functions as an argument, the target function and the derivative, then runs the algorithm on it.
* Whitespace use: indentation is very helpful, always indent properly, it's worth the time. Your long comment spans multiple lines and should instead be broken up. A lot of programmers use ~80 characters per line, though this varies. There's also a lot of blank lines in your code which may or may not belong there--I'd have to see how the rest of the code looks to have a solid opinion.
Last edited by jemidiah; May 1st, 2013 at 09:28 PM.
The time you enjoy wasting is not wasted time.
Bertrand Russell
<- Remember to rate posts you find helpful.
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|