3
\$\begingroup\$

I am looking forward to an answer to improve this code?

Thanks.

Test class

package test;
import main.algorithms.LargestSumContiguousSubarray;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
public class LargestSumContiguousSubarrayTest {
 LargestSumContiguousSubarray largestSumContiguousSubarray;
 @Before
 public void setUp(){
 largestSumContiguousSubarray = new LargestSumContiguousSubarray();
 }
 @Test
 public void testSumContiguousSubArray(){
 int[] a = {-2, -3, 4 - 1, -2, 1, 5, -3};
 int sum = 7;
 Assert.assertEquals(sum, largestSumContiguousSubarray.kadenesAlgo(a));
 }
}

LargestSumContiguousSubarray.java class

package main.algorithms;
public class LargestSumContiguousSubarray {
 // O(n)
 // Kadene's algorithm
 public int kadenesAlgo(int[] a) {
 // This is also works for negative numbers
 int max_so_far = a[0];
 int curr_max = a[0];
 for(int i=0;i<a.length; i++){
 curr_max = Math.max(a[i], curr_max+a[i]);
 max_so_far = Math.max(max_so_far, curr_max);
 }
 return max_so_far;
 }
}
asked Aug 1, 2020 at 16:02
\$\endgroup\$
2
  • 2
    \$\begingroup\$ First thing I'd improve is to fix Kadane's name. \$\endgroup\$ Commented Aug 1, 2020 at 16:15
  • 1
    \$\begingroup\$ This looks like a typo to me... , 4 - 1, \$\endgroup\$ Commented Aug 1, 2020 at 17:30

2 Answers 2

2
\$\begingroup\$

@Doi9t 's answer already covered the ways how to improve your code. The first iteration of your for loop will return these assignments:

curr_max = a[0]
//inside your loop 
curr_max = Math.max(a[0], curr_max + a[0]);
max_so_far = Math.max(a[0], curr_max);

So the first iteration will return a value of max_so_far equal to 2 * a[0] if a[0] >= 0 or a[0] if a[0] < 0 and this is a bug, for example your method for array {1} will return the value 2. I have seen from wikipedia that the algorithm set your variables max_so_far and curr_max to 0, this would solve the bug of your code.

answered Aug 2, 2020 at 15:00
\$\endgroup\$
4
\$\begingroup\$

I have some suggestions for your code.

Replace the for loop with an enhanced 'for' loop

In your code, you don’t actually need the index provided by the loop, you can use the enhanced version.

before

for (int i = 0; i < a.length; i++) {
//[...]
}

after

for (int current : a) {
//[...]
}

Variable and parameter name should be in camelCase style

The variables should be in camelCase.

answered Aug 2, 2020 at 13:11
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.