質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

新規登録して質問してみよう
ただいま回答率
85.50%
C++

C++はC言語をもとにしてつくられた最もよく使われるマルチパラダイムプログラミング言語の1つです。オブジェクト指向、ジェネリック、命令型など広く対応しており、多目的に使用されています。

Q&A

解決済

4回答

1554閲覧

コードの実用性に対する疑問

reotantan

総合スコア295

C++

C++はC言語をもとにしてつくられた最もよく使われるマルチパラダイムプログラミング言語の1つです。オブジェクト指向、ジェネリック、命令型など広く対応しており、多目的に使用されています。

0グッド

0クリップ

投稿2015/11/25 23:17

長いコードですいません、
このコードの実用性に疑問を感じました。
retrieveItemなどのアルゴリズムは工夫してあると思うのですが、5のアイテムが欲しかったら、いったんinitializeしてから、retriveitemするのはなんだか面倒くさいなと感じてしまいました。
このコードでItemtype にそれぞれ別名をつけていくのはありだと思いますか?(Itemtype a,b,c,d,eといったように)
なんだかアルゴリズム自体はしっかりしているだけに
実際使ってみるとがっかりしました。
実際の現場でも同じようなコードが使われているのでしょうか?それとも学生用に単純にしているのでしょうか?

コード #include"Sortedlist.h" #include<iostream> using namespace std; int main() { Sortedlist sl; ItemType i; i.Initialize(3); sl.InsertItem(i); i.Initialize(5); sl.InsertItem(i); i.Initialize(7); sl.InsertItem(i); i.Initialize(11); sl.InsertItem(i); bool found = false; i.Initialize(7); sl.RetrieveItem(i, found); cout << found << endl; cout << sl.LengthIs() << endl; sl.DeleteItem(i); cout << sl.LengthIs() << endl; }
コード /* * ItemType.cpp * * Created on: 2015/11/18 * Author: 礼央 */ #include"ItemType.h" #include<iostream> using namespace std; ItemType::ItemType(){ this->value=0; } RelationType ItemType::ComparedTo(ItemType otheritem){ if(value<otheritem.value) return LESS; else if(value>otheritem.value) return GREATER; else return EQUAL; } void ItemType::Initialize(int value){ this->value=value; } void ItemType::print() const{ cout<<this->value<<endl; } ItemType::~ItemType(){ this->value=0; }
コード /* * Sortedlist.cpp * * Created on: 2015/11/19 * Author: 礼央 */ #include"Sortedlist.h" #include"ItemType.h" using namespace std; Sortedlist::Sortedlist() { length = 0; currentPos = 0; } ; void Sortedlist::makeEmpty() { length = 0; } ; bool Sortedlist::IsFull() { return (length = MAX_ITEMS); } ; int Sortedlist::LengthIs() { return length; } ; void Sortedlist::RetrieveItem(ItemType &item, bool &found) { int midPoint; int first = 0; int last = length - 1; bool moreToSearch = first <= last; found = false; while (moreToSearch && !found) { midPoint = (first + last) / 2; switch (item.ComparedTo(info[midPoint])) { case LESS: last = midPoint - 1; moreToSearch = first <= last; break; case GREATER: first = midPoint + 1; moreToSearch = first <= last; break; case EQUAL: found = true; item = info[midPoint]; break; } } } ; void Sortedlist::InsertItem(ItemType item) { bool moreToSearch; int location = 0; moreToSearch = (location < length); while (moreToSearch) { switch (item.ComparedTo(info[location])) { case LESS: moreToSearch = false; break; case GREATER: location++; moreToSearch = (location < length); break; case EQUAL: break; } } for (int index = length; index > location; index--) info[index] = info[index - 1]; info[location] = item; length++; } ; void Sortedlist::DeleteItem(ItemType item) { int location = 0; while (item.ComparedTo(info[location]) != EQUAL) location++; for (int index = location + 1; index < length; index++) info[index - 1] = info[index]; length--; } ; void Sortedlist::ResetList(){ currentPos=-1; }; void Sortedlist::getNextItem(ItemType &item){ currentPos++; item=info[currentPos]; } ; Sortedlist::~Sortedlist() { length = 0; currentPos = 0; };

気になる質問をクリップする

クリップした質問は、後からいつでもMYページで確認できます。

またクリップした質問に回答があった際、通知やメールを受け取ることができます。

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

guest

回答4

0

こんにちは。

5のアイテムが欲しかったら、いったんinitializeしてから、retriveitemするのはなんだか面倒くさいなと感じてしまいました。

なるほど。気持ちはよく分かります。

これはItemTypeで「何か」を保持し、SortedListでItemTypeを管理するアルゴリズムのようです。
この例では、ItemTypeはint型の値をvalueメンバ変数で保持してます。そして、同じItemTypeとの大きさ比較を行う関数ComparedTo()を提供しています。
これにより、SortedListはItemTypeが「何」を保持しているのか知らないままリスト管理できています。
逆に言うと、適切にコーディングすれば、SortedListはint型に限らず他の型(doubleとかでも)でも管理できるのです。

この目的を優先した結果、いったんItemTypeを設定してから渡す冗長な感じになってしまったのです。

実際の現場でも同じようなコードが使われているのでしょうか?

C++の標準ライブラリのstd::vector<>とstd::lower_bound()を使えば、様々な型を管理できるリストを簡単に実装できます。
なので、実際の現場では、標準ライブラリを使うケースが多いだろうと思います。

ただ、その場合でも「5のアイテムが欲しかったら、いったんinitializeしてから、retriveitemする」必要があるコードを書くケースも少なくないと思います。
C++を使いこなせば、暗黙の型変換を使ってsl.RetrieveItem(5, found);と書けるようにもできますが、C++に詳しい人が少ない環境なら使わない方が安全です。

それとも学生用に単純にしているのでしょうか?

その通りと思います。
ただ、問題点が多く、いまいちな教材のように思います。

①DeleteItem()でサーチ範囲を限定していないので、不正アクセスする可能性があります。
②RetrieveItem(), InsertItem(), DeleteItem()で目標のアイテムを検索していますが、検索アルゴリズムを別々に実装する意味が不明です。分ける必要が無いのに分けるってかなりやばいです。無駄に3回デバッグしないといけないし、テストも3倍必要になるし(しかもちゃんとできていないし)。
③RetrieveItem()関数のcase EQUAL:の中で、item = info[midPoint];してますが、意味が無いだけでなく様々な場面で悪さすると思います。サーチはサーチだけとし下手に要素をいじるべきではないです。
④変数名iは通常ループ変数にしか用いません。このような用い方をするなら、tempみたいな一時的なものであることが明確に分かる名前を付けるべきです。
⑤ItemTypeのデストラクタでthis->value=0;してますが、これは完全に無意味です。

精査するともっとあるかも。

投稿2015/11/26 02:11

編集2015/11/26 02:15
Chironian

総合スコア23272

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

reotantan

2015/11/29 01:36

なるほど、丁寧な解説ありがとうございました。 改善点はたくさんありそうですね;;
Chironian

2015/11/29 04:41

教材に色々ケチつけてすいません。 可能であれば教官にお願いしてテキストを変更して貰うのが良いのですが、痛いところをひっぱたいてしまうとまずいですし、難しいかも知れませんね。 問題点をreotantanさんが把握できていれば、鵜呑みにしないで済むと思いますので、教材の問題点に気がついた時には同じように指摘させて頂こうと思っています。 reotantanへの批判ではありませんので、へこまないで下さいね。 また指摘させて頂いた点について良く分からない時は質問返して頂いてOKですよ。
guest

0

実際の現場でも同じようなコードが使われているのでしょうか?それとも学生用に単純にしているのでしょうか?

おそらくアルゴリズムの勉強用で実用性のことは考慮していないと思われます。実際問題、実務ではそのようなコードを実装することはほとんどありません。

面倒くさいという問題に関しては、オブジェクトの初期化はコンストラクタで行うのが定石ですので、そのように実装していればわざわざ初期化関数を呼び出すという手間がなくなり、面倒くささは感じなくなるかもしれません。
適切にコンストラクタを実装していれば、このように書けます。

C++

1sl.InsertItem(ItemType(3)); 2sl.InsertItem(ItemType(5)); 3sl.InsertItem(ItemType(7));

C++ではSTLとの親和性を考慮することで実装効率が格段に向上しますので、実際の開発現場でもそれを意識して実装します。
この件を例にとれば、ItemTypeクラスにコンストラクタやいくつかの演算子を適切に実装することでSTLの各種コンテナが利用できるようになり、Sortedlistをわざわざ自分で実装する必要はなくなります。setまたはmultisetを使います。

投稿2015/11/26 00:09

catsforepaw

総合スコア5938

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

catsforepaw

2015/11/26 00:23

なんだか質問の意図を誤解していたようですね。失礼しました。
reotantan

2015/11/26 00:54

いえいえ、有効なコンストラクタで書いたコードのほうが例題としては良かったのにと思いました。なにしろ教科書が古いもので;; 回答ありがとうございました
guest

0

ベストアンサー

5のアイテムが欲しかったら、いったんinitializeしてから、retriveitemするのはなんだか面倒くさいなと感じてしまいました。

5のアイテムがただ欲しければinitializeしてそのまま使えばいいです。当たり前です。
が、そういうことじゃなくて、
このコード(main)はSortedlistのデモです。
「5」を入れてretriveitemすれば「5」が見つかるよちゃんと動いているね。
というコードです。

投稿2015/11/25 23:55

ozwk

総合スコア13512

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

reotantan

2015/11/26 00:07

というと、retriveitemは確認するだけのメンバで、実際プロの方も同じようなコードを書き、デモしてからretrieveitemを消すという事になりますか?
guest

0

このプログラムの核となる部分はItemTypeの方で。main()関数の方は使い方のサンプル的な位置づけではないでしょうか? main()がないとプログラムは動きませんから。

投稿2015/11/25 23:45

退会済みユーザー

退会済みユーザー

総合スコア0

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

reotantan

2015/11/26 00:05

そうです、もっと効率の良いデータ構造を期待していたのですが、 結局1,3,5,7,9とデータを入れていき、7があるか調べたいならa.initialize(7)して retrieveitem(a)を入れなければならないという事にすこしガッカリしました。 ItemTypeをいれるわけだから、数字を入れるときと違って、手間がかかるのはわかるんですが、なんかほかに良い方法ないのかなと考えてしまいました。
guest

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

15分調べてもわからないことは
teratailで質問しよう!

ただいまの回答率
85.50%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問