0
\$\begingroup\$

I have some question about the code below. It works correctly, but:

  • Is there a better/another way to solve the exercise (as an expert would have done:) or mine it's perfectly done?
  • Also, I don't see the point of using ArrayList instead of a normal vector like int a[].
  • In the method analysis I wanted to set as parameters (ArrayList a,ArrayList b) but an error (incompatible types List cannot be converted to ArrayList) appears in this int a = Testing.analysis(seq1, seq2, t1); line. I thought I was working with ArrayList since I instantiated my sequences as such.

Exercise:

Perform a program that uses a function that returns a 1 if two very large numbers (digits greater than 5 and where each digit is an element of the arrayList) are equal and a 0 otherwise.

    Example 1:

    Entry

    Number 1: 2 3 4 5 6 7

    Number 2: 1 4 6 8 9 0

    Output "They are not equal numbers"


class Testing {

static int analysis(List a, List b, int t) throws Exception { //using 
//ArrayList instead of List marks an error, why?

    int c = 0;

    if (a.size() == b.size() && a.size() > 5) {

        for (int i = 0; i < t; i++) {
            if (a.get(i) == b.get(i)) {
                c++;
            }
        }
    } else {
        System.out.println("oops! the lengths are not the same or some 
    of the sequence is less than 6");
        throw new Exception(); //to finish the method
    }

    if (c == t) {
        return 1;
    } else {
        return 0;
    }

}
}

public class Tarea24agoENGLISH {

public static void main(String[] args) throws Exception {
    Scanner q = new Scanner(System.in);

    List seq1 = new ArrayList(); //I wanted to write this List <int> seq=new ArrayList<int>(); but it marked an error, why?
    System.out.println("Introduce the length of the sequence1 ");
    int t1 = q.nextInt();

    for (int i = 0; i < t1; i++) {
        System.out.println("Introduce the digits for sequence1");
        int n = q.nextInt();
        seq1.add(n);
    }

    List seq2 = new ArrayList();
    System.out.println("Introduce the length of the sequence2");
    int t2 = q.nextInt();

    for (int i = 0; i < t2; i++) {
        System.out.println("Introdce the digits for sequence2");
        int n = q.nextInt();
        seq2.add(n);
    }

    int a = Testing.analysis(seq1, seq2, t1);
    if (a == 1) {
        System.out.println("same numbers");
    } else {
        System.out.println("\n different numbers");
    }

}

}
\$\endgroup\$
  • 1
    \$\begingroup\$ Note: int[] is not a vector, it is an array. There is no such thing as a vector in Java, except for an old, outdated class called java.util.Vector that should never be used. \$\endgroup\$ – bcsb1001 Aug 27 '17 at 17:12
2
\$\begingroup\$

Declaring a method

static int analysis(List a, List b, int t) throws Exception { //using 

This could be just

    public static int isEqual(List<Integer> a, List<Integer> b) {

This should not be an ArrayList, as that is an implementation. You want to use the interface here because that accepts more things without adding limitations.

You have to say <Integer> rather than <int> because generics have to be objects, not primitives.

If you have more questions about why variants of your code are getting errors, first try searching on Stack Overflow. If you can't find anything, then post a MCVE on Stack Overflow. As was done here.

In general, you want to give methods verb names, like analyze or isEqual. I find isEqual more descriptive.

You don't need to throw a checked Exception. You could throw an unchecked exception like RunTimeException or IllegalArgumentException. If you throw an unchecked exception, you don't need the throws declaration.

You don't need t as it is just a.size(). So just use a.size().

Handling exceptional cases

    if (a.size() == b.size() && a.size() > 5) {

Consider flipping the logic around.

    if (a.size() < 6) {
        throw new IllegalArgumentException("The numbers have to have at least 6 digits.");
    }

Now code and message match. We don't have to figure out that > 5 means 6 or more. And then negate that for the else. We don't need an else because throw ends the execution of the method. Implicitly, the rest of the method is in an else.

    if (a.size() != b.size()) {
        return 0;
    }

You could throw an exception here, but as previously mentioned, this feels more like a false condition. So you can return your false value.

Early return

    for (int i = 0; i < t; i++) {
        if (a.get(i) == b.get(i)) {
            c++;
        }
    }

This could be

    for (int i = 0; i < a.size(); i++) {
        if (a.get(i) != b.get(i)) {
            return 0;
        }
    }

If you find an unequal value, you can return immediately.

    if (c == t) {
        return 1;
    } else {
        return 0;
    }

And this could be just

    return 1;

By returning early, you actually simplify the code.

Input to a list

    System.out.println("Introduce the length of the sequence1 ");
    int t1 = q.nextInt();

This is a weird way to do it. If you do it this way, you can just use an array rather than a List. Normally when you use a List, you would get digits until something happened. For example, something like:

    System.out.println("Introduce the digits for sequence1");
    for (char digit : q.nextLine().toCharArray()) {
        seq1.add(digit - '0');
    }

Subtracting '0' from digit converts digit from a character to a number. You could also do Character.digit(digit, 10) which is arguably more portable and flexible.

You may have to cast to Integer if there is a warning.

Anyway, this allows you to enter the digits all next to each other and terminate the number by hitting return.

\$\endgroup\$
  • \$\begingroup\$ why did you exclude the '0' character ?, in the last for loop \$\endgroup\$ – user178403 Aug 27 '17 at 3:27
  • \$\begingroup\$ why is thisfor (int digit : q.nextInt()) { seq1.add(digit);} wrong? (I was trying to change your code to make it more descriptive) \$\endgroup\$ – user178403 Aug 27 '17 at 18:33
  • \$\begingroup\$ That's more of a Stack Overflow question than a Code Review question. Hints: how many digits are in the number that q.nextInt() returns? What does q.nextInt() return? The for loop is expecting something that implements Iterable<Integer> there. \$\endgroup\$ – mdfst13 Aug 28 '17 at 3:53
  • \$\begingroup\$ Answer: q.nextInt() returns only 1 digit. So there's no way to change char by int? \$\endgroup\$ – user178403 Aug 29 '17 at 2:04
  • \$\begingroup\$ oh and I should let you know that for some reason SO block me :( so I can't post questions there anymore \$\endgroup\$ – user178403 Aug 29 '17 at 2:07
2
\$\begingroup\$

First thing I noticed don't count how many digits are equal, soon as 1 digit is unequal return false, if the loop finishes return true. Also it seems to me if the sizes are unequal the proper response would be false not an error.

\$\endgroup\$
  • \$\begingroup\$ the code doesn't need to count how many digits are equal. If the loop finishes and return true then it's because the numbers are equal \$\endgroup\$ – user178403 Aug 27 '17 at 1:20
  • 1
    \$\begingroup\$ yes exactly as my answer states \$\endgroup\$ – tinstaafl Aug 27 '17 at 1:21
  • 1
    \$\begingroup\$ But the exercise doesn't mention unequal lengths, it would be reasonable to assume that they would elicit the same response as any other set of numbers. \$\endgroup\$ – tinstaafl Aug 27 '17 at 1:26
  • 1
    \$\begingroup\$ Ok, that's not mentioned in the exercise. \$\endgroup\$ – tinstaafl Aug 27 '17 at 1:28
  • 1
    \$\begingroup\$ Since equal lengths isn't a requirement of the input your getting, returning an error is misleading and frustrating to the user. \$\endgroup\$ – tinstaafl Aug 27 '17 at 1:41

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.