C++ファイルストリームについて

[gac]

学校の課題でファイルストリームの課題をやっていたのですが
自分で書いた以下のプログラムを提出したら、
・ファイルオープンのあとにファイルクローズがない
・51行目から65行目の並べ替えのルーチンで、比較対象にムダがある。
と先生から指摘をうけたのですが、どのように直したらいいのかわかりません。
申し訳ありませんが教えてもらえないでしょうか。

コード:

#include<iostream.h>
#include<iomanip.h>
#include<fstream.h>

 const int days_Max = 31 ; //期間
 
 
 struct TemperatureData { 
 	int year ; //年
	int month ; //月
	int day ; //日
	double high ; //最高気温
	double low; //最低気温
 } ;
 
main(){
	
	
	
	cout << "入力した日にちの最高気温と最低気温を、最低気温が高い順に並び替えて表示する\n" << endl ;
	
	TemperatureData tdate[days_Max] ; //構造体の変数宣言
	int result ; //結果の構造体を指定する
	
	
	//ファイルから入力するルーチン
	ifstream fin; //ファイルストリーム
	fin.open("temp.txt", ios::in);
	
	//ファイルのオープンに失敗した場合、警告してプログラムを即、終了する
	if (!fin){
		cerr << "ファイルがオープンできません" << endl;
		exit(1);
	}
	
	int days ; //考える期間
	fin >> days ; //考える期間をファイルから読み込む
	
	//日付、最高気温、最低気温をファイルから読み込む
	for(int i = 0 ; i < days ; i++){
		fin >> tdate[i].year ;
		fin >> tdate[i].month ;
		fin >> tdate[i].day ;
		fin >> tdate[i].high ;
		fin >> tdate[i].low ;
	}
	
	
	
	
	for(int i = 0 ; i < days-1 ; i++){
		for(int t = 0 ; t < days-1 ; t++){
			if(tdate[t].low < tdate[t+1].low){
				TemperatureData swap = tdate[t] ;
				tdate[t] = tdate[t+1] ;
				tdate[t+1] = swap ;
			}else if(tdate[t].low == tdate[t+1].low){
				if(tdate[t].high < tdate[t+1].high){
					TemperatureData temp = tdate[t] ;
					tdate[t] = tdate[t+1] ;
					tdate[t+1] = temp ;
				}
			}
		}
	}
	
	cout << "\n最低気温が同じ場合、最高気温の高い順になっています\n" << endl ;
	
	for(int j=0 ; j < days ; j++){
		cout << tdate[j].year << "年 "
		<< tdate[j].month << "月 "
		<< tdate[j].day << "日 " << endl
		<< "最高気温 " << tdate[j].high << "℃" 
		<< " 最低気温 " << tdate[j].low << "℃" << endl << endl ;
	}
}

[M.R]

ざっとソースを見ただけですが
>ファイルオープンのあとにファイルクローズがない
　これはそのままですね。
　オープンする事ができたならクローズする事は難しくないはず。
　もう少しやってみてはいかがでしょうか。

>51行目から65行目の並べ替えのルーチンで、比較対象にムダがある。
　(tdate[t].low < tdate[t+1].low) と (tdate[t].low == tdate[t+1].low)
　の処理は同じのようです。１つにまとめることで無駄がなくなるという事ではないでしょうか。
　また i のループも特に必要ないと思います。

[usao]

>また i のループも特に必要ないと思います。
line51からのソート処理はバブルソートだと思うので　iのループは必要だと思うのですが．
比較に無駄があるというのは，
・iのループごとに，tのループ範囲を狭めることが可能なのにそうなっていない
・ソート完了判定がない
ということを言っているのでは？

[M.R]

むむ、完成していないものを完成させたいのならば話は変わってきますね

[gac]

>また i のループも特に必要ないと思います。
line51からのソート処理はバブルソートだと思うので　iのループは必要だと思うのですが．
比較に無駄があるというのは，
・iのループごとに，tのループ範囲を狭めることが可能なのにそうなっていない
・ソート完了判定がない
ということを言っているのでは？

おそらくusaoさんの仰っていることで間違いないと思います。
ファイルクローズは調べたらなんとかなりそうですね。
tのループ範囲はどう狭めれば良いのでしょうか…

[usao]

>tのループ範囲はどう狭めれば良いのでしょうか…
line52～64のtのループを一巡したらどうなるのかを考えましょう（あるいは調べましょう）．
tdate[]だと説明しにくいので，簡単にintの配列で考えましょうか．

コード:

int a[5] = { 4, 1, 2, -1, 0 };  //という配列があって小さい順にならべたい

for( t=0; t<4; t++ )  //tのループ
{
    if( a[t] > a[t+1] )
    {    a[t]とa[t+1]の値を入れ替える    }
}

とするとどうなるか．
t=0での処理　→　{ 1, 4, 2, -1, 0 } t[0]とt[0+1]とを入れ替えた
t=1での処理　→　{ 1, 2, 4, -1, 0 } t[1]とt[1+1]とを入れ替えた
t=2での処理　→　{ 1, 2, -1, 4, 0 }
t=3での処理　→　{ 1, 2, -1, 0, 4 } ←これが結果．
結果は完全にソートが終わっていないものの，少なくとも一番大きな要素だけは目的の位置に移動させることができています．
（この動作がバブルソートの名前の由来だったかと）

であれば，次(実際のソート処理では外側でさらにiでループしている）のtのループの処理では最後の要素に触る必要はありません
＝ループ範囲を１つ減らせる


>むむ、完成していないものを完成させたいのならば話は変わってきますね
？？？

[かずま]

クローズはデストラクタで行われるので明示的に書く必要はありません。
読み込み直後にクローズしたければ、ブロックの中に入れればよいでしょう。

コード:

    {
        ifstream fin("temp.txt");  // コンストラクタでオープン
        if (!fin) {
            cerr << "ファイルがオープンできません\n";
            return 1;
        }
        fin >> days ;
        for (int i = 0 ; i < days ; i++) {
            fin >> tdate[i].year ;
            fin >> tdate[i].month ;
            fin >> tdate[i].day ;
            fin >> tdate[i].high ;
            fin >> tdate[i].low ;
        }
        if (fin.fail()) {
            cerr << "ファイルの読み込みに失敗しました\n";
            return 1;
        }
    } // fin はデストラクタによりクローズされる

ifstream なんだから、mode のデフォルト値 ios::in は不要です。
TemperatureData なんだから tdate より tdata のほうがいいと思うんですけど。