5
\$\begingroup\$

I'm new to coding and want to pick up some good habits with regards to coding conventions and general readability (I am using PEP8 as a guideline). Below I added a very simple function. Feel free to give me some pointers on what to improve/change about the commenting. Should I be catching any other errors?

def reynolds(den, vel, dia, vis):
    """Calculate the Reynolds number for a given pipe flow.

    Arguments:
    den -- density of the fluid in [kg/m3]
    vel -- the mean velocity of the fluid in [m/s]
    vis -- dynamic viscosity of the fluid in [Pa*s]
    dia -- inside diameter of the pipe in [m]

    The Reynolds number itself is dimentionless. 
    It is used to charakterize flow patterns.
    """
    try:
        return den*vel*dia/vis       # regular execution of the function
    except ZeroDivisionError:
        print ('divide by zero')     # catch devision by zero
\$\endgroup\$
1
  • \$\begingroup\$ Minor nitpick: In my experience, something like [m] already means "in meters", so you would either write diameter (in m) or diameter [m]. \$\endgroup\$
    – mkrieger1
    Commented Dec 5, 2017 at 14:01

2 Answers 2

6
\$\begingroup\$

There is no reason to abbreviate parameter names to three letters. That just makes the code unnecessarily cryptic. (Using names like rho instead of density could be an acceptable alternative, if brevity is essential.) Furthermore, consider naming the function more specifically, so that it is obvious that it applies to cylindrical pipes.

As per PEP 8, binary operators should have have spaces for readability.

There is no good reason to catch ZeroDivisionError. If the viscosity is 0, then this function would return None, and would likely cause other problems elsewhere in your calling code. Just let the ZeroDivisionError propagate naturally, and your code would be easier to debug. (Furthermore, your two comments are pointless and redundant.)

Watch your spelling ("dimentionless" → "dimensionless", "charakterize" → "characterize").

def reynolds_pipe(density, velocity, diameter, viscosity):
    """
    Calculate the Reynolds number for a given pipe flow.
    The resulting Reynolds number is dimensionless. 
    It is used to characterize flow patterns.

    Arguments:
    density -- density of the fluid in [kg/m3]
    velocity -- the mean velocity of the fluid in [m/s]
    viscosity -- dynamic viscosity of the fluid in [Pa*s]
    diameter -- inside diameter of the pipe in [m]
    """
    return density * velocity * diameter / viscosity
\$\endgroup\$
1
  • \$\begingroup\$ it could also return math.inf instead of ZeroDivisionError \$\endgroup\$
    – jfs
    Commented Dec 2, 2017 at 6:34
1
\$\begingroup\$

The code itself is OK.

Several lines of comment could make the source file harder to navigate. Imagine having 10 of similar simple functions.

I would definitely leave out the last two comments (“Reynolds number is dimensionless”, and “it characterizes the flow”). They can be useful to show your understanding of the phenomenon, if this is a student project. Besides that, it doesn’t help much to yourself in the future.

On the other hand, the characterization comment could hint the need for another function to display characteristics of the flow to the user. If you like to have that feature, you can write a “TODO:” comment to track this as an action item inside the code.

Lastly, as another user has written, function names are better if they explicitly define what they do. ReynoldsPipe(rho, V, D, mu) can be an option.

If you’ll use this function a lot, Re_pipe can be handy. And, if you’ll never have another type of Flow (airfoil, wind tunnel, etc) you could go ahead with Re as well, to make the code more pleasing to the engineer.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.