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.

I often find myself writing methods in languages such as Java or C++ that are only meant to loop over an array or something similar, accumulate the values, and then return the total. The problem is, it usually takes at least 4 lines (1 for init, 2 for loop, 1 for return), which feels much too bloated for such a common and simple operation.

How can I generically condense accumulation code in a way that's similar to the extremely compact for loop?


Bloated Code Examples (in java):

    int totalDays = 0;
    for (int month= ymd[1] -1; month >0; month--) {
        totalDays += daysInMonth(month);
    } 
    return totalDays; 

    for (int month= ymd[1] -1, totalDays= 0;; month--) {
        totalDays += daysInMonth(month);
        if (month == 1) return totalDays;
    }
share|improve this question
    
To refer to Java's standard writing : put totalDays in the for(..) like in the second sample. The second sample add an if which looks like a doublon with the ;; (if nested and parsed in the for, even if omitted) –  cl-r Dec 3 '12 at 13:40
    
C++/Java/C# are verbose that way. It does not make it a bad thing. Readability of code that will be read by many thousands of person hours is important. If this is just a piece of throw away code then readability is not such an issue then a more compact language may by appropriate. –  Loki Astari Dec 3 '12 at 16:47
    
it doesn't matter whether it takes 3 or 10 lines as long as it is optimized by the compiler. prefer clarity before compactness. –  Anders K Dec 3 '12 at 19:11
    
@cl-r I'm not sure what you're saying in your second sentence; could you elaborate? –  Griffin Dec 3 '12 at 23:05
    
@AndersK I'd say I'm going more for conciseness than compactness, which definitely enhance readability ^_^ –  Griffin Dec 3 '12 at 23:06
show 2 more comments

2 Answers 2

In the case of C++, use the standard library and boost. You want a range from ymd[1]-1 to 0, and you want to accumulate over it. Seeing as the order doesn't matter, it's easier to reverse it.

boost::accumulate(boost::irange(1, ymd[1]), 0, [](int i, int j) { return i + daysInMonth(j); });

As you can see, this isn't much better than a for loop. If you find yourself writing these kinds of algorithms often, then perhaps defining some helper range classes to get rid of some of the boilerplate would be nice. (I suspect there's a transform algorithm that has the new range as its return value, but I'm not familiar with it.

share|improve this answer
add comment

To answer the comment : between ?: (not optimized) and if {.. ;} else {.. ;} (in Java with FizzBuzz sample), what is the good practice ?

public class IfTest {
    static final int nbIter = 30;

    static long ifNested() {
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
        sb.append(//
                i % 15 == 0 //
                ? "FizzBuzz" //
                        : (i % 3 == 0 //
                        ? "Fizz"//
                                : (i % 5 == 0//
                                ? "Buzz" //
                                        : i)));
    }
    final long totT = System.nanoTime() - t;
    System.out.format("ifNested\t%20d\n", totT);
    // sb.append("\n"); System.out.println(sb.toString());
    return totT;
}

static long withIf() {
    final StringBuilder sb = new StringBuilder();
    final long t = System.nanoTime();
    for (int i = 0; i < nbIter; i++) {
        if (i % 3 == 0) {
            sb.append("Fizz");
            if (i % 5 == 0) {
                sb.append("Buzz");
            }
        } else
            if (i % 5 == 0) {
                sb.append("Buzz");
            } else {
                sb.append(i);
            }
    }
    final long totT = System.nanoTime() - t;
    System.out.format("withIf\t\t%20d\n", totT);
    return totT;
}
/*** @param args */
public static void main(final String[] args) {
    for (int i = 0; i < 5; i++) {
        System.out.println("------ " + i + " -----");
        final long totIn = ifNested();
        final long totWi = withIf();
        System.out.format("...   ifNested/withIf :%5d\n", (totIn * 100)
                / totWi);
    }
}
}

output :

------ 0 -----
ifNested                  796750
withIf                     28216
...   ifNested/withIf : 2823
------ 1 -----
ifNested                   89396
withIf                     26260
...   ifNested/withIf :  340
------ 2 -----
ifNested                   46933
withIf                     26540
...   ifNested/withIf :  176
------ 3 -----
ifNested                   46654
withIf                     26260
...   ifNested/withIf :  177
------ 4 -----
ifNested                   47771
withIf                     26260
...   ifNested/withIf :  181
share|improve this answer
    
I still can't tell what you meant by "doublon with the ;; (if nested and parsed in the for, even if omitted)." Are you saying 'for loops' expand to recursive byte code? –  Griffin Dec 6 '12 at 6:40
    
@Griffin Between the two ; an if is programemd to test the limit on each iteration, and the code is optimized to do this operation. So if you put a test on the limit between the {..} you add a second if tested also on each iteration. I call that 'doublon', because I'm not mathematician and I do not know if their is another word. –  cl-r Dec 6 '12 at 7:34
    
@cl-r this not an answer but a tangential, but anyways... the main reason ifNested() is slower than withIf is it has a superfluous type cast to java.lang.Object and a call to toString if you replace i with String.valueOf(i) and make the if statements similar in withIf, i.e. add a check for % 15 == 0 etc they then cost the same in time. –  abuzittin gillifirca Dec 31 '12 at 11:37
add comment

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.