current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

For the below query, show that both summation and product are instances of a more general function, called accumulate, with the following signature:

def accumulate(combiner, start, n, term):
    """Return the result of combining the first n terms in a sequence."""
    "*** YOUR CODE HERE ***"

Accumulate takes as arguments the same arguments term and n as summation and product, together with a combiner function (of two arguments) that specifies how the current term is to be combined with the accumulation of the preceding terms and a start value that specifies what base value to use to start the accumulation. Implement accumulate and show how summation and product can both be defined as simple calls to accumulate:

def summation_using_accumulate(n, term):
    """An implementation of summation using accumulate.

    >>> summation_using_accumulate(4, square)
    30
    """
    "*** YOUR CODE HERE ***"

def product_using_accumulate(n, term):
    """An implementation of product using accumulate.

    >>> product_using_accumulate(4, square)
    576
    """
    "*** YOUR CODE HERE ***"

Below is the solution:

    from operator import mul, add


def accumulate(combiner, start, n, f):
    """Return the result of combining the first n terms in a sequence."""

    total = start           #Result of summation gets stored here
    i = 1                   #Initial value of sequence
    while i <= n:
        total = combiner(total, f(i))
        i = i + 1
    return total


def summation_using_accumulate(n, f):
    """An implementation of summation using accumulate.

    >>> summation_using_accumulate(4, square)
    30
    """
    return accumulate(add, 0, n, f)



def product_using_accumulate(n, f):
    """An implementation of product using accumulate.

    >>> product_using_accumulate(4, square)
    576
    """
    return accumulate(mul, 1, n, f)


def square(x):
    return mul(x, x)

print("product_using_accumulate: ",product_using_accumulate(4, square))
print("summation_using_accumulate: ",summation_using_accumulate(4, square))
print(accumulate(add, 0, 4, square)) 
print(accumulate(mul, 1, 4, square))

I have tested this code and looks good to me.

My questions:

  1. Does the solution look incorrect in any aspect?

  2. Any feedback on naming conventions?

  3. Any feedback on coding style?

share|improve this question
2  
Well for one thing, neither of your ..._using_accumulate functions actually use accumulate! Your calls at the end should be inside those functions. –  jonrsharpe Jul 11 '14 at 11:26
    
@jonrsharpe wrt your point neither of your ..._using_accumulate functions actually use accumulate! , i still did not understand, where is this point asked in the query? Please help me!! –  overexchange Jul 11 '14 at 12:45
1  
Per the specification "show how summation and product can both be defined as simple calls to accumulate" - your implementations do not call accumulate. And in terms of "feedback on Coding style" - see the style guide. –  jonrsharpe Jul 11 '14 at 12:46
    
@jonrsharpe ya you are right!! Now i understood the meaning of 'can both be defined' –  overexchange Jul 11 '14 at 12:47
2  
Why do you think the functions are named ..._using_accumulate?! The purpose is not just to minimise the code (Python has sum and reduce built right in, so why bother with any of this?), it's to understand the processes involved. –  jonrsharpe Jul 11 '14 at 13:00

2 Answers 2

up vote 4 down vote accepted

Here is the implementation I think you were being led towards:

##from functools import reduce # if Python 3.x
from operator import add, mul

def accumulate(combiner, start, n, f):
    """Return the result of combining the first n terms in a sequence."""
##    return reduce(combiner, (f(i+1) for i in range(n)), start) # <- built-in version
    total = start
    for i in range(n):
        total = combiner(total, f(i+1))
    return total 

def summation_using_accumulate(n, f):
    """An implementation of summation using accumulate.

    >>> summation_using_accumulate(4, square)
    30
    """
    return accumulate(add, 0, n, f)

def product_using_accumulate(n, f):
    """An implementation of product using accumulate.

    >>> product_using_accumulate(4, square)
    576
    """
    return accumulate(mul, 1, n, f)

def square(x):
    return mul(x, x)

That's how you can

show how summation and product can both be defined as simple calls to accumulate

i.e. simply by doing it, which gives the results required:

>>> product_using_accumulate(4, square)
576
>>> summation_using_accumulate(4, square)
30

Also, note the use of for and range, which is easier and much less error-prone that manually incrementing values in a while loop.

So the answers to your specific questions:

  1. No; accumulate, product_using_accumulate and summation_using_accumulate were all wrong, but you've fixed that now;
  2. No; not now that you've removed Currentterm (which should have been current_term - per the style guide, variable names are lowercase_with_underscores); and
  3. Yes; you need more spaces, e.g. return accumulate(add,0,n,f) should be return accumulate(add, 0, n, f).
share|improve this answer
1  
I think for i in range(1, n + 1) would be better: less risk of accidentally writing i in the loop body instead of i + 1. –  Gareth Rees Jul 11 '14 at 13:36
1  
@GarethRees that would make sense in a longer loop, for example where i is used more than once, but in such a short one I think it's a moot point. –  jonrsharpe Jul 11 '14 at 13:39
    
@jonrsharpe In my solution, what exactly is reduce technique? calling accumulate() in thos two functions product_using_accumulate() and summation_using_accumulate()? –  overexchange Jul 11 '14 at 14:31
    
@overexchange what? reduce is a built-in Python function that does exactly what your accumulate function is supposed to do, but with slightly different parameters. –  jonrsharpe Jul 11 '14 at 14:34
    
@jonrsharpe what is the advantage of learning/writing such crytic syntax? reduce(combiner, (f(i+1) for i in range(n)), start)? total = combiner(total, f(i+1)) looks more readable –  overexchange Jul 11 '14 at 14:40
up vote 5 down vote

  1. If a homework problem starts with the words "Show that", I would expect the answer to contain a proof (or at least an argument), not just an implementation.

  2. You've misunderstood the problem statement. It says that start "specifies what value to use to start the accumulation", but in your implementation, start is the index of the first item in the sequence to contribute to the accumulation. Both of the test cases have start=1 and f(1)=1 so they are incapable of detecting your mistake.

  3. Your implementations of summation_using_accumulate and product_using_accumulate do not actually use accumulate!

share|improve this answer
    
for your point, Your implementations of summation_using_accumulate and product_using_accumulate do not actually use accumulate! please help me understand, where was this point told in the question? –  overexchange Jul 11 '14 at 12:40
    
for your second point, i edited my query, do u think it make sense now? and i tested with different start values –  overexchange Jul 11 '14 at 12:42
    
@overexchange the start argument to accumulate for adding should be 0. –  jonrsharpe Jul 11 '14 at 12:47
    
@jonrsharpe but question says: a start value that specifies what base value to use to start the accumulation. from this what i understand is, what is the start value of the sequence whose last term in sequence is n. did i mis-understand? so in one example i gave argument as 1 and another example i gave argument as 3 –  overexchange Jul 11 '14 at 12:50
    
@overexchange yes, you misunderstood - that's the value to start the accumulation, not to start the sequence. It should be total = start not total = f(start). The sequence always starts at f(1). –  jonrsharpe Jul 11 '14 at 12:55

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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