締切済み

java Calendarクラス

2008/05/24 22:14

javaで月、日を入力してカレンダーを作成したのですが年と月のsetでmonth-1はマジックナンバーなので直したいのですがどなたかわかる方教えてください。 package sample; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; import java.util.*; public class Test { private final static int firstday = 1; public static void main(String[] args) { //カレンダーのインスタンスを取得します Calendar cal = Calendar.getInstance(); //文字入力 BufferedReader input = new BufferedReader(new InputStreamReader(System.in)); //年を取得 int year =0; //月を取得 int month =0; //最後の日付 int lastDay =0; //月初めの曜日を取得 int week =0; //年妥当性チェック boolean CheckYear = true; //月妥当性チェック boolean CheckMonth = true; try { //年妥当性チェック while(CheckYear){ System.out.println("年を入力してください"); //年を入力します year = Integer.parseInt(input.readLine()); //年が４桁の場合 if(String.valueOf(year).length()==4){ CheckYear = false; }else{ System.out.println("年は4桁で入力してください"); } } //月妥当性チェック while(CheckMonth){ System.out.println("月を入力してください"); //月を入力します month = Integer.parseInt(input.readLine()); //月が１～１２の場合 if(month>=1&&month<=12){ CheckMonth = false; }else{ System.out.println("月１～１２を入力してください"); } } }catch(IOException e){ System.out.println("数字以外は入力しないでください"); System.out.println("処理を中断します"); return; }catch (Exception a) { System.out.println("数字以外は入力しないでください"); System.out.println("処理を中断します"); return; } //年、月をセットします cal.set(year,month-1,cal.getActualMinimum(Calendar.DATE)); //月初めの曜日を取得 week = cal.get(Calendar.DAY_OF_WEEK); //年月を出力する System.out.println(String.valueOf(year)+"年"+String.valueOf(month)+"月"); //曜日を出力する System.out.println("日　月　火　水　木　金　土"); //最後の日付を取得する lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); /* * 最後の日付を取得する */ if(month==1||month==3||month==7||month==8||month==10||month==12) { lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); }else if(month==4||month==6||month==9||month==11){ lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); }else if(year%4==0&&month==2){ lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); }else if(year%4!=0&&month==2){ lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); } /* * カレンダーを出力する */ //最後の日付まで繰り返す // 最後の日付まで繰り返す for (int i = 1; i <= lastDay; i++) { // 1日とそれ以外で分岐する if (i == 1) { // 1日の曜日位置まで移動する for (int j = 1; j < cal.get(Calendar.DAY_OF_WEEK); j++) { System.out.print(" "); } } else { // 日付を増やす cal.add(Calendar.DAY_OF_MONTH, firstday); } // 1～9と10～で表示を変える if (i < 10) { System.out.print(" " + i); } else { System.out.print(" " + i); } // 土曜日になったら改行する if (cal.get(Calendar.DAY_OF_WEEK) == Calendar.SATURDAY) { System.out.println(""); } } } }

tendor
お礼率33% (3/9)

Java
回答数5
ありがとう数4

みんなの回答 （5）
専門家の回答

みんなの回答

ssr-y6
ベストアンサー率71% (5/7)

2008/08/02 19:19 回答No.5

　Calendarクラスの定数フィールドに、JANUARYやSUNDAYなどの定数が定義されているので、直接数値を使わずにそれらを使ってCalendarクラスへはアクセスしたほうがよいと思います。　以下のように対応付けをすれば見栄えもよくなると思います。 import java.util.*; public class calen { private static final int WeekValue[] = { Calendar.SUNDAY, Calendar.MONDAY, Calendar.TUESDAY, Calendar.WEDNESDAY, Calendar.THURSDAY, Calendar.FRIDAY, Calendar.SATURDAY}; private static final String WeekName[] = {" 日", " 月", " 火", " 水", " 木", " 金", " 土"}; public static void main(String args[]) { int year, month, week, lastDay, Int1, Int2; Calendar cal = Calendar.getInstance(); int MonthValue[] = new int[13]; String MonthName[] = new String[13]; MonthValue[1] = Calendar.JANUARY; MonthName[1] = new String("一月");//入力された1をJUNUARYと識別して一月と表示する MonthValue[2] = Calendar.FEBRUARY; MonthName[2] = new String("二月");//以下同じ MonthValue[3] = Calendar.MARCH; MonthName[3] = new String("三月"); MonthValue[4] = Calendar.APRIL; MonthName[4] = new String("四月"); MonthValue[5] = Calendar.MAY; MonthName[5] = new String("五月"); MonthValue[6] = Calendar.JUNE; MonthName[6] = new String("六月"); MonthValue[7] = Calendar.JULY; MonthName[7] = new String("七月"); MonthValue[8] = Calendar.AUGUST; MonthName[8] = new String("八月"); MonthValue[9] = Calendar.SEPTEMBER; MonthName[9] = new String("九月"); MonthValue[10] = Calendar.OCTOBER; MonthName[10] = new String("十月"); MonthValue[11] = Calendar.NOVEMBER; MonthName[11] = new String("十一月"); MonthValue[12] = Calendar.DECEMBER; MonthName[12] = new String("十二月"); try { year = Integer.parseInt(args[0]); if(String.valueOf(year).length() != 4) throw new Exception("年が4桁でない"); month = Integer.parseInt(args[1]); if((month < 1) || (month > 12)) throw new Exception("月が1～12でない"); } catch (NumberFormatException ex) { System.out.println("入力した数値が異常"); return; } catch (Exception ex) { System.out.println(ex.toString()); return; }; cal.set(year, MonthValue[month], cal.getActualMinimum(Calendar.DATE)); week = cal.get(Calendar.DAY_OF_WEEK); System.out.println(String.valueOf(year) + "年" + MonthName[month]); for (Int1 = 0; Int1 < 7; Int1 ++) System.out.print(WeekName[Int1]); System.out.println(""); lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); for (Int1 = 1; Int1 <= lastDay; Int1 ++) { if (Int1 == 1) { for (Int2 = 0; WeekValue[Int2] != cal.get(Calendar.DAY_OF_WEEK); Int2 ++) System.out.print(" "); }; if (Int1 < 10) System.out.print(" " + Integer.toString(Int1)); else System.out.print(" " + Integer.toString(Int1)); if (cal.get(Calendar.DAY_OF_WEEK) == Calendar.SATURDAY) System.out.println(""); cal.add(Calendar.DAY_OF_MONTH, 1); }; }; } 　MonthValueとMonthNameは、入力された数値と月を表す定数と表示する文字を結び付けています。　WeekValueとWeekNameは、週を表す定数と表示する文字とその順番を結び付けています。

ログインすると、全ての回答が全文表示されます。

hirusagari
ベストアンサー率64% (20/31)

2008/05/27 09:36 回答No.4

>hirusagariさんが提示されたメソッドだと結局「-1」を使っているので意味がないと思うので >この程度の簡単なプログラムなので無理に分ける必要もないかな、と思います。結局「-1」を使っているのに、わざわざあのようなメソッドを定義するのは、 Calender#set()の呼び出しが一箇所とは限らないので「-1」をソースコード中にバラまかないようにするためです。それは、マジックナンバーが避けられるべき理由のひとつが、そのようなコーディングは仕様変更に耐えられないから、ということだからです。 >switch構文を使ってCalendar.JANUARYを呼び出すどのようなコードが可読性が高いか、ということは人によってことなるとは思いますが、「-1」を避けるためにswitchを使って十数行増やすのは自分には明らかに可読性が低下しているように見えます。なぜなら、既存のコードは、1月→0、2月→1……という対応が「-1」のたった２文字に表されているのに対して、 switchを使うとそれを確かめるのに12行すべてを確認しなければならず、あきらかに読み解くための手間が増えているからです。また、テストをするときもすべての分岐に対してテストしなければならず、書くコードを増やすことはバグが入る恐れも大きくします(たとえば8月→8と誤って書いてしまうなど)。「-1」を使わない代償としては損な取引だと思います。しかも、 case　1:cal.set(year,Calendar.【違いはここだけ】,cal.getActualMinimum(Calendar.DATE));break という似たようなコードが12行増えるわけです。どう見ても冗長です。やはり「-1」を無理やり避けようとするのが余計にコードをわかりにくくしています。「//Month 値は 0 から始まるため -1 する」と書くのが一番わかりやすいと思いますよ。

ログインすると、全ての回答が全文表示されます。

foggi47
ベストアンサー率87% (7/8)

2008/05/26 22:45 回答No.3

十数行という数字は決して長いものではないと思いますよ。「-1」を使わない代償、くらいに思っておくべきです。メソッドに分けると確かに分かりやすくなりますが、結局全体としての行数は長くなりますし、この程度の簡単なプログラムなので無理に分ける必要もないかな、と思います。 hirusagariさんが提示されたメソッドだと結局「-1」を使っているので意味がないと思うので前に自分が示した方法のメソッドを提示しておきます。ただ、見れば分かりますが結局同じことをしてるんですよね… これ以上ソースを短くする方法がちょっと思いつかないので。(そもそもあるのかな？) static void setMonth(Calender calender, int year, int month){ switch(month){ case 1:cal.set(year,Calendar.JANUARY,cal.getActualMinimum(Calendar.DATE));break; case 2:　　　　……　} }

ログインすると、全ての回答が全文表示されます。

foggi47
ベストアンサー率87% (7/8)

2008/05/25 23:52 回答No.2

前の質問で先ほど回答させていただいたものです。窓表示だと思っていたら、DOS窓だったんですね　すいません。見当違いな答えを提示してしまいました。回答ですが、もしMonthを-1するのが嫌でしたら、switch構文を使ってCalendar.JANUARYを呼び出す、位のことしかできないでしょう。例えば以下のようです switch(month){ case 1:cal.set(year,Calendar.JANUARY,cal.getActualMinimum(Calendar.DATE));;break; case 2:　　　　……　} このようにすればたとえJANUARYが1になったとしても(ならないと思いますが)、対応できると思います。また、hirusagariさんの指摘にもあるように、 >if(month==1||month==3||month==7||month==8||month==10||month==12) { >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(month==4||month==6||month==9||month==11){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(year%4==0&&month==2){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(year%4!=0&&month==2){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >} の部分はどの分岐でも同じことをしていますので、分岐の必要はないでしょう。カレンダークラスはセットされた月の日数を返してくれるので月ごとに分岐する必要はありません。

質問者

お礼 2008/05/26 15:54

色々ありがとうございます。確かにこのようなやり方もあるいますが、 switch(month){ case 1:cal.set(year,Calendar.JANUARY,cal.getActualMinimum(Calendar.DATE));;break; case 2:　　　　……　} ちょっとソースが長くなりすぎてしまうので、メソッドを使ったやり方を教えてください。

ログインすると、全ての回答が全文表示されます。

hirusagari
ベストアンサー率64% (20/31)

2008/05/25 00:32 回答No.1

Calender#set()の仕様が「Month 値は 0 から始まる (1 月は 0 になる)」である以上、 -1して値を渡すのは避けられません。自分はこのような「-1」が「マジックナンバー」であるとは考えていません。どうしても気になるのであれば、つねに static void setMonth(Calender calender, int year, int month){ calender.set(year,month-1,cal.getActualMinimum(Calendar.DATE)); } のようなメソッドを介してCalender#set()を呼び出すことになろうかと思いますが、これが必ずしも直接Calender#set()を呼び出すより可読性が高いとまでは言えないでしょう。また、将来的にCalender#set()の仕様が「Month 値は 1 から始まる (1 月は 1 になる)」に変更される恐れはまずありませんし、必要なら「Month 値は 0 から始まるため」というコメントを書いておけば十分でしょう。従って「month-1」として呼び出すことはとくに問題ないと思います。そもそもそのような些細なことを気に病むより、 >if(month==1||month==3||month==7||month==8||month==10||month==12) { >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(month==4||month==6||month==9||month==11){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(year%4==0&&month==2){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(year%4!=0&&month==2){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >} の部分で、どの分岐でもまったく同じ処理を行うという冗長さのほうがよほど気になります。「マジックナンバー」とは何なのか、もう一度考えてみてください。

参考URL：: http://www.geocities.jp/bleis_tift/cpp/badmagicnum.html

質問者

お礼 2008/05/25 20:29

マジックナンバーではないと解釈するかメッソドを作ってみます。参考になりました。ありがとうございます。それと下のソースですが、 >if(month==1||month==3||month==7||month==8||month==10||month==12) { >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(month==4||month==6||month==9||month==11){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(year%4==0&&month==2){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >}else if(year%4!=0&&month==2){ >lastDay = cal.getActualMaximum(Calendar.DAY_OF_MONTH); >} このソースではいけないのでしょうか？初心者なのでいまいちよくわからなくてこうしたのですが？これでも一応うまくいったのですが他に良い方法があるのでしょうか？

ログインすると、全ての回答が全文表示されます。