2
\$\begingroup\$

This function is going to be part of a class which can determine the efficiency and max/average power that can be produced by specific wind turbines based on the location given by the user. Is the documentation OK?

def WindEnergy_calc(density, surfaceArea, windspeed, powerCoefficient):


    """
    This function returns the value of wind energy given density of the air,

    surface area of blades, and windspeed.

    Args:

    * density (float or int) - the value represents the density of the air

    * surface area (float or int) - the value represents the total surface area of turbine

    * windspeed (float or int) - the value represents the windspeed in the area

    *Power Coeffecient of turbine (float) - value cannot exceed

    Returns:

    * total possible kinetic energy created by the wind on the turbine

    * P = 1/2*row*A*V^3*C

    """
    try:
        if powerCoefficient > 0.59:
            powerCoefficient = "Not Possible" #to break code
    except:
        print "Power Coefficient must be less than 0.59 due to Betz limit"
        return()

    half = 0.5
    windEnergy = float(half*density*surfaceArea*windspeed**3)
    return(windEnergy)
\$\endgroup\$
0
2
\$\begingroup\$
  • The param names in the function header are different than the param names in the docstring. surfaceArea vs surface area and powerCoefficient vs Power Coeffecient of turbine. In addition you have some spelling errors and an unfinished phrase. Did you proof this before submitting it?
  • What is the purpose of stating that some of the params are "float or int" and one is "float"? Shouldn't you list the actual allowed ranges, and then test for them?
  • You don't explain what the params actually mean, you just restate what the parameter name is.
  • Your documentation doesn't line up with itself. Please make sure to align it.
  • The formula listed in the "Returns" section uses different parameters entirely. There's no way to map row A V C onto the parameters listed. And the function actually calculated is different still!

There are also some problems with the code. Your function should raise an exception if there's a problem. It does not need a try block. The caller of the function should catch the exception, not you. As it stands the except block will never be taken. Also, your function should not ever print anything. It should either do its calculation if it can, or raise if it can't.

I don't see any reason to stick 0.5 into a variable called half.

\$\endgroup\$
3
  • 4
    \$\begingroup\$ There's no such thing as throw in Python, it's called raise. \$\endgroup\$ – Daniel Dec 2 '17 at 11:16
  • \$\begingroup\$ Thank you for the input about the descriptions of the params. I am a little confused by your bullet about there is no way to map row AVC onto the parameters listed, so I will look into this! \$\endgroup\$ – Thomas Byrne Dec 3 '17 at 15:09
  • \$\begingroup\$ Your code needs to be clear enough so that someone who does not know the formula and did not write the code can understand what's going on and be able to maintain it. when I say "No mapping" I mean that the mapping between those single-character shorthand and the parameter names is completely unclear. I mean maybe "V" is speed, and maybe "C" is coefficient, but they're in a different order and I can't figure anything else out. Someone looking over your code should not have to guess and should not need special knowledge. \$\endgroup\$ – Snowbody Dec 4 '17 at 15:56
5
\$\begingroup\$
  1. First, get rid of the double-spacing. I don't know if that's an artifact of posting on this site, or if you really coded it that way, but fix it!

    def WindEnergy_calc(density, surfaceArea, windspeed, powerCoefficient):
        """
        This function returns the value of wind energy given density of the air,
        surface area of blades, and windspeed.
    
        Args:
        * density (float or int) - the value represents the density of the air
        * surface area (float or int) - the value represents the total surface area of turbine
        * windspeed (float or int) - the value represents the windspeed in the area
        *Power Coeffecient of turbine (float) - value cannot exceed
    
        Returns:
        * total possible kinetic energy created by the wind on the turbine
        * P = 1/2*row*A*V^3*C
        """
        try:
            if powerCoefficient > 0.59:
                powerCoefficient = "Not Possible" #to break code
        except:
            print "Power Coefficient must be less than 0.59 due to Betz limit"
            return()
    
        half = 0.5
        windEnergy = float(half*density*surfaceArea*windspeed**3)
        return(windEnergy)
    
  2. Next, fix the name of your function. This is a function not a procedure. You aren't performing some "helper" operation and leaving your results in global variables or class members. You are performing a computation and returning the result! Pick an appropriate name:

    def wind_energy():
    def calculate_wind_energy():
    def wind_energy_of():
    

    In general, function names should include a verb and some supporting words - adverbs or nouns - to flesh out what is being done. For a "pure" function, it's possible that the verb is implicit: the word "compute" or "calculate". For example sqrt() and cos() have no verb, because the verb form would implicitly be compute_sqrt() or calculate_cosine(), which doesn't add much value. (On the other hand, we fopen() and fread() because the verb isn't quite so obvious.)

  3. For a function taking lots of inputs, you want to have a really obvious ordering of the inputs. That is, open('foo.txt', 'r') should make it obvious that the filename is first, and the mode is second. (It doesn't, btw. But it should!)

    In your case, you have four incoming parameters. None of them seem obvious to me. I'd suggest that you take advantage of python's named-only parameters to put a name on each one of them:

    def wind_energy(*, density, area, velocity, power):
    

    Above, the * marks the end of positional parameters - meaning there are none. Every parameter after the * is required to be named. So you call this like:

    we = wind_energy(density=1.0, area=4.6, velocity=20, power=0.1)
    
  4. Cite your sources! I found a website that I think contains your source equations:

    http://formulas.tutorvista.com/physics/wind-energy-formula.html

    You need to include this, and the name(s) of your equations, in your doc comment. Don't write this:

    # No!
    """...returns the value of wind energy ... """
    

    Instead, write this:

    # Yes!
    """... return the wind energy computed with Farnsworth's Formula
    (http://www.farnsworth.com/wind-energy-formula.html) ... """ 
    
  5. Command your minions. This is purely a "social convention." But the standard convention for commit comments and function documentation is to use the Imperative Mood rather than passive voice. It's almost as if your program is telling the computer what to do. (Wait, what?)

    Change this:

    This function returns the value of wind energy...
    

    Into this:

    Return the value of wind energy ...
    

    Likewise, when you commit your changes to Mercurial, don't write this:

    I updated the code based on suggestions from codereview...
    

    Instead, write this:

    Make changes from codereview ...
    
  6. Validate and rename!

    You have two constraints, one explicit (the Betz limit) and the other implicit (all the parameters are numeric). Validate them!

    I'll suggest you have a look at Python's numbers module, which provides abstract base classes for numeric types, and applies a "hierarchy" to them. The most appropriate type, since you use "float or int" a lot, seems to be Real. The Real type adds "conversion to float" which is right up our alley:

    rho = float(density)
    V = float(velocity)
    

    (Also worth noting: rho is the name of a Greek letter. row is something you do in a boat.)

    You might even consider the fact that Python supports unicode identifiers, like so:

    𝜌 = float(density)
    

    (Sadly, there's no good way to spell except for V ** 3.)

    For your explicit validation, you have totally bungled your code:

        try:
            if powerCoefficient > 0.59:
                powerCoefficient = "Not Possible" #to break code
        except:
            print "Power Coefficient must be less than 0.59 due to Betz limit"
            return()
    

    First, try: / except: is used when you believe that the code inside the try: block will raise an exception. You have an if-statement inside the try block that will not raise.

    Second, this is not a function that should ever call print. I understand you might insert print statements for debugging, but printing anything is outside the scope of this function. If your caller has passed you an invalid value, the correct behavior is to raise an exception - typically a ValueError - and let the caller deal with their own stupidity. (But make sure you document the limit in your doc comment!)

    Try this, instead:

    # Somewhere global, like outside the function or top of module:
    BETZS_COEFFICIENT = 0.593
    """Betz's coefficient- see https://en.wikipedia.org/wiki/Betz's_law"""
    
    # Inside your function:
    if power > BETZS_COEFFICIENT:
        raise ValueError("Power coefficient must be <= Betz's coeff. It's the law!")
    

    Also worth noting: when you invoke return() with empty parens, it doesn't mean "return nothing". It means "return a tuple with zero items". The () expression (note: expression) is an empty-tuple constructor. This is consistent with calling a function with no args, since positional args are a tuple:

    >>> def f():
    >>>     return()
    ...
    ...
    >>> x = f()
    >>> print(x)
    ()
    >>> type(x)
    <class 'tuple'>
    

    What you meant to do was just a bare return. Except that was wrong, and what you really meant to do was raise, as above.

\$\endgroup\$
1
  • \$\begingroup\$ I had not known about 'raise' and the proper way to design and cite a function. This will be really helpful in the future!! \$\endgroup\$ – Thomas Byrne Dec 3 '17 at 15:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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