6
\$\begingroup\$

In this exercise, I'm supposed to sum up elements from 2 arrays, then display the answer on one line with spaces in between. I did it and my answer got accepted as correct, but I'm wondering if there is a better way to do this since I feel my way was more complicated than it needed to be.

import java.util.ArrayList;
import java.util.List;

public class Main {

public static void main(String[] args) {
    int n = 11;

    int[] firstArray = {811594, 993574, 299729,559604,161945,969851,588210,
            692459, 28350,43017,797855};
    int[] secondArray = {725888, 233750,191700,944750,380402,319860,766872,764921,
            330218,906679, 65309};

    List<Object> total = new ArrayList<>();

    for(int i = 0; i < n; i++){
        int sum = firstArray[i] + secondArray[i];
        total.add(sum);
    }

    Integer[] sumArray = total.toArray(new Integer[total.size()]);
    for(int i = 0; i < n; i++) {
        System.out.print(sumArray[i] + " ");
    }


}
}
\$\endgroup\$
3
  • \$\begingroup\$ Yousef, you need to mention, what should happen if both arrays have different sizes? And handle that condition. \$\endgroup\$
    – Rao
    Jan 14 '16 at 0:12
  • \$\begingroup\$ Rao, the exercise always gives two arrays that are identical in size. It doesn't even give array it gives 2 groups of numbers and asks you to add the numbers from both groups. \$\endgroup\$
    – Yousef
    Jan 14 '16 at 0:14
  • \$\begingroup\$ If you want to store the intermediate result, you don't need to use a List, you can create an array immediately: int[] total = new int[n];, then total[i] = sum; \$\endgroup\$
    – njzk2
    Jan 14 '16 at 18:53
11
\$\begingroup\$

You don't need to store the intermediate result. You don't need to give the length of the arrays. This assumes that the arrays are allways of equal length.

for(int i = 0; i < firstArray.length; i++){
    System.out.print((firstArray[i] + secondArray[i]) + " ");
}
\$\endgroup\$
1
  • \$\begingroup\$ It's important to use .length over a hard coded "magic" number, as you then don't have to update it if your values change, and it becomes very flexible if you want dynamic content in your arrays. \$\endgroup\$
    – Tom Bowen
    Jan 14 '16 at 10:52
7
\$\begingroup\$

@Orden is right that you don't need an intermediary array. On top of that, I would point out a few violations of good practices.

Use the right types. If you want to add integers in a list, then use a List<Integer> instead of a List<Object>.

If you know the final size of the list in advance, and you intend to convert it to an array, then consider skipping the list completely, and use just an array from the start. Like this:

int[] sumArray = new int[n];

for(int i = 0; i < n; i++){
    sumArray[i] = firstArray[i] + secondArray[i];
}
\$\endgroup\$
2
  • \$\begingroup\$ IntelliJ IDEA made me change it to convert it to an array but it was List<Integer> before. \$\endgroup\$
    – Yousef
    Jan 14 '16 at 0:20
  • 1
    \$\begingroup\$ @Yousef lesson 1 of using an advanced IDE, learn how it works. They don't make you do anything, they provide suggestions that speed up the process. You should not use the advanced functionality of your IDE at all until you have some knowledge of the language. \$\endgroup\$ Jan 14 '16 at 14:56
5
\$\begingroup\$

The basic idea is right, but I don't like this line and its consequences:

List<Object> total = new ArrayList<>();
  • If you just need to print the result, you don't need to store the sums at all; you can just print as you go.
  • If you want to store the sums, and you know the lengths of the inputs, then you can just make an array instead of an ArrayList to help you do it.
  • If you do use an ArrayList, then providing a capacity estimate would be good practice.
  • List<Object> defeats type safety. List<Integer> would be better.

Also, instead of hard-coding n = 11;, I suggest

assert firstArray.length == secondArray.length;
int n = firstArray.length;

So, one good solution would be

for (int i = 0; i < n; i++) {
    System.out.print((firstArray[i] + secondArray[i]) + " ");
}
System.out.println();     // It's customary to finish with a newline

Another good solution would be

int[] sums = new int[n];
for (int i = 0; i < n; i++) {
    sums[i] = firstArray[i] + secondArray[i];
}
for (int i = 0; i < n; i++) {
    System.out.print(sums[i] + " ");
}
System.out.println();
\$\endgroup\$
3
\$\begingroup\$

Others have covered bad practices here, I am just going to demonstrate a Java 8 solution to the problem - as Java 8 is the current version of Java; I believe this is what should be written in modern Java:

assert firstArray.length == secondArray.length;
IntStream.range(0, firstArray.length)
    .map(i -> firstArray[i] + secondArray[i])
    .forEach(System.out::println);

This simply creates an IntStream of [0, n) and then uses those indices to map to the sum of the variables.

\$\endgroup\$
4
  • \$\begingroup\$ That's look so weird from a old java coder point of view but at least I have learnt something new! :-) \$\endgroup\$
    – Orden
    Jan 14 '16 at 14:52
  • \$\begingroup\$ @Orden certainly very un-Java like. But C# has had lambdas and LINQ for donkey's years and Scala changed the JVM language game somewhat. I find the syntax really readable and I find it enforces SRP as each distinct map operation has it's own, simple, function. \$\endgroup\$ Jan 14 '16 at 14:54
  • \$\begingroup\$ not the best way to use stream I would say. too bad java lacks a zip stream. \$\endgroup\$
    – njzk2
    Jan 14 '16 at 18:54
  • \$\begingroup\$ @njzk2 Java 8 decided not to include tuple types so that meant that zip and friends could not be added. I have sorely missed zipWithIndex numerous times in recent months... \$\endgroup\$ Jan 15 '16 at 8:33
-2
\$\begingroup\$

The "modern" java solution puts newlines, not spaces between the elements, of course. Adding the trailing space, and using print instead of println will fix that. Inserting a join (Java has join, right?) might also work.

And, whats' wrong with APL's ⎕←firstArray+secondArray

\$\endgroup\$
5
  • \$\begingroup\$ It's really hard to understand what you're trying to say. "Inserting a join (Java has join, right?) might also work." sounds like guesswork, and doesn't make a good argument. \$\endgroup\$
    – janos
    Jan 14 '16 at 16:16
  • \$\begingroup\$ Join may not be the proper term. What I'm thinking of inserts a delimiter between a list of strings, creating one bigger string:/ 'one&two&three' == join('&',['one','two','three']) \$\endgroup\$ Jan 21 '16 at 14:47
  • \$\begingroup\$ To be honest, I thought my use of 'might' clearly implied some guesswork.... \$\endgroup\$ Jan 21 '16 at 14:50
  • \$\begingroup\$ Your answer as it is doesn't seem to make a clear, good point. I suggest to either rewrite it or delete it. \$\endgroup\$
    – janos
    Jan 21 '16 at 15:03
  • \$\begingroup\$ Having the elements separated by newlines seems incorrect. The Java solution also seems verbose, to this APL person at least. \$\endgroup\$ Feb 12 '16 at 15:46

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.