リスト内の値の比較をして出力したい

Question

前提・実現したいこと

Javaの学習をしており練習に以下のようなプログラムを作成しようとしています
1．入力した値を比較し最もゼロに近く値を出力する
2．正と負の値で絶対値が同じ場合（2とー2など）、正の値を出力する

発生している問題・エラーメッセージ

期待通りの出力ではなく、入力していない[-1]が出力される場合がある

入力した正負の値を一度絶対値にしてから比較してより0に近いものを出力しようとしているのですが
入力した数字によって正しい出力が得られる場合と、そうでない場合が発生しています。
例えば、
入力回数: 3
入力: 3,-2,2,3
とした場合、出力が[-1]となってしまいます
しかし
入力回数: 3
入力: 3,-2,2,1
とした場合であれば、期待通り[1]が出力されます

該当のソースコード

Java

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;

	public class Temperatures {

	    public static void main(String args[]) {
	        Scanner in = new Scanner(System.in);
	        int n = in.nextInt(); // the number of temperatures to analyse
	        List<Integer> Minus = new ArrayList<>();
	        List<Integer> Plus = new ArrayList<>();

	        int answer =0;
	        for (int i = 0; i < n; i++) {
	            int t = in.nextInt(); 
	            if(t<0) {
	            	Minus.add(t);
	            }else if(t>0) {
	            	Plus.add(t);
	            }
	        }

	        Collections.sort(Plus);
	        Collections.sort(Minus);
	            if(Minus.isEmpty()) {
	            	answer=Plus.get(0);
	            }
	            else if(Plus.isEmpty()) {
	            	answer=Minus.get(Minus.size());
	            }    
	            
	            else {
	            if(Math.abs(Plus.get(0))>Math.abs(Minus.size()))
	            	answer =Minus.size()*-1;
	            else if(Math.abs(Plus.get(0))<Math.abs(Minus.size()))
	            	answer =Plus.get(0);
	            else if(Math.abs(Plus.get(0))==Math.abs(Minus.size()))
	            	answer =Plus.get(0);
	            }
	            System.out.println(answer);//出力が-1になる
	            
	    }
	    
	}

この問題の発生理由と解決策を教えていただけませんか？
宜しくお願いします

Comments

[a_saitoh]

入力されたデータをListなどに格納して保持することは必須なのでしょうか？絶対値が最も小さい値を出すだけなら、格納する必要はないとおもうのですが。

Answer 1

別解として記載します。

まず、絶対値を扱う時は負の最大値Integer.MIN_VALUE or Long.MIN_VALUEを意識する必要があります
参考 -> Math#abs

int値の絶対値を返します。引数が負でない場合は引数そのものを返します。負のときは、その正負を逆にした値を返します。
引数がInteger.MIN_VALUEの値(intの最小値)と等しい場合は、結果も同じ値(負の値)になります。

質問文の要求仕様として入力データを出力表示しないので、MinusとPlusで List を分ける必要がないのではと思います。そして、Collections.sort(Minus);の引数に比較関数:Comparatorを渡せます。

List<Integer> sequence = Arrays.asList(3, -2, 2, 3);
Collections.sort(sequence, (x, y) -> {
// 比較処理
});

数字の比較にはInteger.compareが使えます。

Comments

[LouiS0616]

> 絶対値を扱う時は負の最大値Integer.MIN_VALUE or Long.MIN_VALUEを意識する必要があります

あー... 見逃しがちですが、非常に大事な点ですね。
趣味グラマとしてはなかなかこういう観点が持てないので、非常に参考になります。

ただ、クラス名がTemperaturesであることを見るに、Integer.MIN_VALUEの入力はあり得ないと仮定してしまっても良いような気もします。

[umyu]

＞LouiS0616さんへ
クラス名を絶賛無視して、数字の正しい比較方法を熱く語ってしまいました。。。

[LouiS0616]

そゆこともあります。。。

私の書いたgetNearlyZeroメソッドについては、最初に次の処理を追加すれば問題ないでしょうか。
if(a == Integer.MIN_VALUE) return b;
if(b == Integer.MIN_VALUE) return a;

[umyu]

＞LouiS0616さんへ
それで大丈夫ですが、今回の件にいえばMath.min(a,b)を計算して結果がInteger.MIN_VALUEだったら不正データとして例外を投げちゃうのも一つの手かと。

[LouiS0616]

それが良さそうですね。ありがとうございます。

Answer 2

if(Math.abs(Plus.get(0))>Math.abs(Minus.size()))
answer =Minus.size()*-1;

ここでMinus.size() * -1をanswerに放り込んでいるのが直接的原因では。

そもそも

難しく考えすぎな気もします。

int n = 入力;
int result = 入力;  // 暫定的に『ゼロに一番近い値』とみなす。

for(n回繰り返す) {
    int num = 入力;

    numとresultの絶対値をそれぞれ計算;
    if(numの絶対値 < resultの絶対値) {
        result = num;
    }
    else if(numの絶対値 > resultの絶対値) {
        // 更新は不要
    }
    else {
        result = numとresultのうち大きい方;
    }
}

変数名が適当ですが、悪しからず。

さらに

メソッドを利用するともっと簡潔です。

// 絶対値がゼロに近い方を返す。絶対値が同じ場合正の数を返す。
static int getNearlyZero(int a, int b) {
     aの絶対値とbの絶対値をそれぞれ計算;

     if(aの絶対値 < bの絶対値) return a;     
     if(aの絶対値 > bの絶対値) return b;

     return aとbのうち大きい方;
}

public static void main(String[] args) {
     ...

     int result = ...;
     for(...) {
         int num = ...;
         result = getNearlyZero(result, num);
     }
}

Comments

[Kokekokko1]

解決法及びアドバイスをいただき有難うございました。他の方々もたくさんのアドバイスを下さり現在調べながら消化吸収させて頂いておりますが、一先ず、リストを使わない、メソッド使うなど、私のコードをもとに分かりやすい回答とブラッシュアップを提示していただいたLouiS0616さんをベストアンサーとさせて頂きます。
あらためて有難うございました。

Answer 3

import static java.util.Comparator.*;

import java.util.Scanner;
import java.util.stream.IntStream;

public class Q125097 {

	public static void main(String[] args) {
		try (Scanner s = new Scanner(System.in)) {
			int n = s.nextInt();
			int x = IntStream.generate(() -> s.nextInt()).limit(n).boxed()
						.min(comparing((Integer i) -> Math.abs(i)).thenComparing(reverseOrder())).get();
			System.out.println(x);
		}

	}

}

Comments

[Kokekokko1]

なるほど。このような書き方もあったのですね。非常に参考になります。ラムダ式、StreamAPIを用いたコーディングは、これまで挑戦したことがなかったのですが、ここまで端的に書けるものなんですね。ぜひとも今後の課題として理解を深めていこうと思います。

Answer 4

質問文のコードの
answer=Minus.get(Minus.size());
や
if(Math.abs(Plus.get(0))>Math.abs(Minus.size()))

の Minus.size() は Munus.get(Minus.size() -1) の間違いとおもわれます。

Integer をつかって、　入力値がない場合を null であつかうようにする、
0 以外の入力をすべて List に保持せずに、それまでの入力の正の最小値と負の最大値だけを保持するようにする
という方針で書き換えてみました。
もっと短く書けるとは思いますが、とりあえず素直に条件判定を書いていきました。

import java.util.Scanner;

public class Temperatures {

    public static void main(String args[]) {
        Scanner in = new Scanner(System.in);
        int n = in.nextInt(); // the number of temperatures to analyse
        Integer minus_max = null; // その時点での 正値の最小値
        Integer plus_min = null; // その時点での 負値の最大値

        for (int i = 0; i < n; i++) {
            int t = in.nextInt();
            if (0 < t) {
                if (plus_min == null || t < plus_min) {
                    plus_min = t;
                }
            } else if (t < 0) {
                if (minus_max == null || minus_max < t) {
                    minus_max = t;
                }
            }
        }
        // System.out.println("" + plus_min + "," + minus_max);

        Integer answer = null;
        if (minus_max == null && plus_min == null) {
            answer = null;
        } else if (minus_max == null) {
            answer = plus_min;
        } else if (plus_min == null) {
            answer = minus_max;
        } else {
            if ((minus_max * (-1)) < plus_min) {
                answer = minus_max;
            } else {
                answer = plus_min;
            }
        }

        if (answer != null) {
            System.out.println(answer);
        }
    }
}

実行例

Answer 5

ソートして最初の要素を出力

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

public class Temperatures {
    public static void main(String args[]) {
        List<Integer> numbers = new ArrayList<>();

        Scanner in = new Scanner(System.in);
        int n = in.nextInt();
        for (int i = 0; i < n; i++) {
            int t = in.nextInt();
            numbers.add(t);
        }

        numbers.sort((x, y) -> {
            int d = Math.abs(x) - Math.abs(y);
            if (d != 0) return d;
            return Integer.compare(y, x);
        });

        System.out.println(numbers.get(0));
    }
}

Comments

[umyu]

>hichonさんへ
d = x - y;とすると、xとyの差がInteger.MAX_VALUEを超える時に出力結果が正しくありません。
これは符号付きの数字の差はその数字型の整数値で表せないために発生します。
Integer.compareがあるので、そちらを使って頂くほうがよいかと思いました。

[umyu]

以下のテストデータで再現できるかと。
List<Integer> numbers= Arrays.asList(Integer.MIN_VALUE+3, Integer.MAX_VALUE-2);
for(int x:numbers) {
System.out.println(x);
}

[nullpon]

なるほど、オーバーフローの可能性がありますね。修正しました。

[umyu]

＞hichonさんへ
お手数をおかけしました。修正ありがとうございました。

[nullpon]

良く考えると、Math.abs(Integer.MIN_VALUE)もオーバーフローしますね。これは、入力値を制限した方が良いかも。

[umyu]

＞hichonさんへ
hichonさんへコメントした後にLouiS0616さんから指摘を受けたのですが、クラス名がTemperatures (気温）だからそもそも論としてInteger.MIN_VALUEの値が入らないのではないかという話が。。
修正対応していただいて、すごく申し訳ないのです。。

[nullpon]

なんとクラス名は見落としていました。しかし、Integer.compareのおかげでさらにコードが短くなりました。

Answer 6

入力回数がわかっているのなら配列を使うこともできます。
(i)入力回数をnとしサイズ2 * nの配列を準備します。(名前をdataにしておきます。)

(ii)0行目には入力されたデータを格納し、1行目にはその絶対値を格納します。

(例)入力回数 n = 3
入力された整数 3 1 -5の時　[][]dataは　

3 1 -5
3 1  5

となっています。

(iii)絶対値の中で最小値をとります。(今回は1です。)

(iv)絶対値の中の最小値の配列の番号を使って求めたいものを表示します。

今回は最小値が1でしたがそれはdata[1][1]です。
ということは、求めたい「最も0に近い整数」は**data[0][1]**と　なっているはずです。

私が、書いたソースコードを載せておきます。

import java.util.*;

public class Main {
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        int n = sc.nextInt();
        int [][]data = new int[2][n];
        int min;
        int []min2 = {0,0};
        int []count = {0,0};
        for(int i = 0; i < n; i++){
            data[0][i] = sc.nextInt();
            data[1][i] = Math.abs(data[0][i]);
            min = data[1][0];
            if(min >= data[1][i]){
                if(data[1][i] >= 0){
                    min2[0] = data[0][i];
                    count[0]++;
                }
                else{
                    min2[1] = data[0][i];
                    count[1]++;
                }
            }
        }
       
       int flag = 0;
        for(int i = 0; i < 2; i++){//0に最も近い整数が1つの時(正、負のどちらか片方だけある)
           if(count[i] == 0){
                flag = 1;
                System.out.println(min2[1 - i]);
            }
        }
        if(flag == 0){//0に最も近い整数が2つの時(正、負両方あり)
            System.out.println(min2[0]);
        }
    }
}

<入力例>
7
-1
4
-2
3
6
5
1
<出力例>
1

Comments

[umyu]

1点目,入力数値データを2乗するとIntger.MAX_VALUE /2 以上のデータでオーバーフローしてしまいます。そのため、不正な出力結果になります。以下の入力データでテストしてみてくださいな。
3
1073741824
1073741
1
今回の件に関しては、Math#absとを使った方がよいのではーと思いますー。
2点目、入力された数字が全部100以上だとmin =100の初期値によって0が2個出力されるのではと。
3
101
120
956
自作すると不具合を入れやすいので、テスト済みの標準ライブラリを使うか、データの境界値のテストを意識されたほうが良いのではと個人的に思いました。
Stars1024さんの真摯な回答の点は学ばないと行けないと思っているので、その点だけ。

[退会済みユーザー]

＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊＊

[退会済みユーザー]

入力される整数がどれくらいなのかわからなかったのでとりあえずこのような形で書きました。

[退会済みユーザー]

訂正しました。２乗のところを絶対値に変えました。あと最小値のところは初期値を入力した
値を使いました。

[umyu]

＞Stars1024さんへ
ソースコードを変更して頂き、ありがとうございました。

[退会済みユーザー]

>umyuさんへ
こちらこそご指摘ありがとうございました。オーバーフローについて考えてなかったので
勉強になりました。