Форум программистов, компьютерный форум, киберфорум
Наши страницы

С++ для начинающих

Войти
Регистрация
Восстановить пароль
 
 
Рейтинг: Рейтинг темы: голосов - 46, средняя оценка - 4.80
Gepar
1181 / 537 / 20
Регистрация: 01.07.2009
Сообщений: 3,517
#1

Покритикуйте код - C++

02.10.2011, 23:28. Просмотров 6148. Ответов 116
Метки нет (Все метки)

Есть класс Студенты (реализован через односвязный список), хотел бы услышать критику по поводу его улучшения, если кому не лень разбираться в столь поздний час Сам код естественно полностью рабочий и предупреждений тоже компилятор не выдаёт (если не считать в main в условии while, но там всё ок) так что хотелось бы услышать Ваши замечания если что можно сделать лучше.

Students.h
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
#ifndef STUDENTS_H
#define STUDENTS_H
 
#include <string>
using std::string;
 
#include <iostream>
using std::cout;
using std::cerr;
 
#include <iomanip>
using std::setw;
using std::left;
using std::right;
 
#include <stdexcept>
using std::out_of_range;
 
class Students
{
    struct ListItem
    {
        string fullname;
        string group;
        int year;
        int average;
        ListItem *Next;
        ListItem(string ="",int =0,int =0,string ="",ListItem* =0);
    };
 
    ListItem *Head;
    ListItem *Tail;
    ListItem *Current; //указатель на текущий элемент
    int count; // всего элементов
 
public:
    class iterator
    {
        Students::ListItem* current;
 
        //функция для проверки что current инициализирован
        void correct(){if(!*this) throw Students::Exception("access denied");}
 
        public:
        string first(){correct();return current->fullname;}
        string second(){correct();return current->group;}
        int third(){correct();return current->year;}
        int four(){correct();return current->average;}
 
        iterator() {current=0;}
 
        iterator(const Students &right){*this=right;}
        iterator(Students::ListItem* right){*this=right;}
        iterator& operator=(Students &right);
        iterator operator=(Students::ListItem* right);
 
        iterator& operator++();
        iterator operator++(int);
        bool operator==(const iterator& right) const;
        bool operator!=(const iterator& right) const;
        iterator* operator*(){return this;}
        operator bool(){return current!=0;}
    };
 
    class Exception
    {
        string str;
        public:
        Exception(string data) :str(data){};
        string what(){return str;}
    };
 
 
 
    Students(): Head(0), Tail(0), Current(0), count(0) {}
    Students(string data, int y, int a, string g);
    ~Students(){this->deleteAll();}
 
 
    void addToTail(string data, int y, int a, string g);
    void addToHead(string data, int y, int a, string g);
 
 
    void deleteFromHead(bool mode=1);//1 - с предупреждением(исключением) если список пуст
    void deleteFromTail();
    void deleteAll();
 
    //сортировка, принимает функцию для сравнения элементов
    void sort(bool cmp(string&,string&));
 
    //методы возвращающие итераторы
    iterator begin(){return Head;}
    iterator end(){return iterator(0);}
};
 
 
#endif
Students.cpp
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
#include "Students.h"
 
//////////////////////STUDENTS/////////////////
 
Students::ListItem::ListItem(string name,int y,int aver,string gr,ListItem* next)
{
    fullname=name;
    group=gr;
    average=(aver>=0 && aver<=100 ? aver : -1);
    year=(y>=1950 && y<=2012 ? y : -1);
 
    if(average==-1 || year==-1)
     throw Exception("wrong data");
    Next=next;
}
 
Students::Students(string data, int y, int a, string g)
{
    Head=Tail=new ListItem(data,y,a,g);
    Current=0;
    count=1;
}
 
 
void Students::addToTail(string data, int y, int a, string g)
{
    if (Head)
    {
        Tail->Next=new ListItem(data,y,a,g,0);
        Tail=Tail->Next;
        count++;
    }
    else
        Head=Tail=new ListItem(data,y,a,g);
}
 
 
void Students::addToHead(string data, int y, int a, string g)
{
    if (Head)
    {
        ListItem* temp=Head;
        Head=new ListItem(data,y,a,g);
        Head->Next=temp;
        count++;
    }
    else
        Head=Tail=new ListItem(data,y,a,g);
}
 
void Students::deleteFromHead(bool mode)
{
    if(Head->Next)
    {
        ListItem *temp=Head;
        Head=Head->Next;
        count--;
        delete temp;
    }
 
    else if (Head)
    {
        ListItem *temp=Head;
        Head=Tail=Current=0;
        delete temp;
        count=0;
    }
 
    else if(mode)
     throw Exception("list is empty");
}
 
void Students::deleteFromTail()
{
    if(Tail != Head)
    {
        ListItem *temp=Head;
        while(temp->Next!=Tail)
            temp=temp->Next;
 
    ListItem *toDelete=temp->Next;
    Tail=temp;
    temp->Next=0;
 
    delete toDelete;
    count--;
    }
 
    else if(Tail==0)
     throw Exception("list is empty");
 
    else
     Head=Tail=0;
}
 
 
void Students::deleteAll()
{
    while (Head)
     deleteFromHead(0);
 
    count=0;
}
 
void Students:: sort(bool cmp(string&,string&))
{
    ListItem* new_begin=NULL;
    ListItem* new_end=NULL;
    ListItem* sprev=NULL;
 
    for(ListItem *scur=this->Head;scur!=NULL;scur=this->Head)
    {
        ListItem *smin=NULL;
        ListItem *sminprev=scur;
        string min_name=scur->fullname;
        for(ListItem *gp=scur->Next;gp!=NULL;gp=gp->Next)
        {
            if(cmp(gp->fullname,min_name))
            {
                min_name=gp->fullname;
                smin=gp;
                sprev=sminprev;
            }
            sminprev=gp;
        }
        if(smin==NULL)
        {
            smin=scur;
        }
        else if(smin==scur->Next)
        {
            scur->Next=scur->Next->Next;
        }
        else
        {
            sprev->Next=smin->Next;
        }
        if(new_begin!=NULL)
        {
            new_end->Next=smin;
            new_end=smin;
        }
        else
        {
            new_begin=smin;
            new_end=smin;
        }
        if(smin==this->Head)
         this->Head=smin->Next;
    }
    this->Head=new_begin;
    this->Tail=new_end;
 
}
 
 
//////////////////////ITERATOR/////////////////
Students::iterator& Students::iterator::operator++()
{
    current=current->Next;
     return *this;
}
 
Students::iterator Students::iterator::operator++(int)
{
    iterator temp(*this);
    current=current->Next;
     return temp;
}
 
bool Students::iterator::operator==(const iterator& right) const
{
    return (current==right.current);
}
 
bool Students::iterator::operator!=(const iterator& right) const
{
    return !(*this==right);
}
 
Students::iterator Students::iterator::operator=(Students::ListItem* right)
{
    current=right;
    return *this;
}
 
Students::iterator& Students::iterator::operator=(Students &right)
{
    current=right.Current;
    return *this;
}
main.cpp
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
#include <iostream>
using std::cout;
using std::cin;
using std::endl;
using std::string;
#include <windows.h>
#include <string>
#include "Students.h"
#include "StudentsIterator.h"
#include "algorithm"
using std::for_each;
 
 
bool cmp( string &str1, string &str2 )
{
    return strcmp( str1.c_str(), str2.c_str() ) < 0;
}
 
 
void print(Students::iterator *ita)
{
    cout<<setw(15)<<left<<ita->first()
     <<setw(7)<<ita->second()
     <<setw(5)<<ita->third()
     <<setw(7)<<ita->four()<<"\n";
 
}
 
Students::iterator findElement(string toFind, Students::iterator begin, Students::iterator end)
{
    for(;begin!=end;++begin)
    {
        if (begin.first().find(toFind)!=(size_t)-1)
         return begin;
    }
    //если ничего не найдено - вернуть итератор указывающий за конец списка ( на NULL)
    return 0;
}
 
 
int main()
{
    SetConsoleCP(1251);
    SetConsoleOutputCP(1251);
 
 
    Students test("Иванов Иван",1992,100,"КС-09");
    test.addToTail("ЛЛЛ ФФ",1991,100,"КС-09");
    test.addToTail("БББ ВВ",1992,100,"КС-09");
    test.addToHead("Иванов Василий",1993,100,"КС-08");
    test.addToHead("Иванов Петр", 1990,100,"КС-07");
    test.addToHead("ЯЯЯ ММ",1993,100,"КС-08");
    test.addToTail("ЮЮЮ ЛЛЛ",1994,100,"КС-07");
    test.sort(cmp);
 
    cout<<"All list(after sort):\n";
    for_each(test.begin(),test.end(),print);
    test.deleteFromHead();
    test.deleteFromTail();
    cout<<endl<<"After delFromHead + delFromTail:\n";
    for_each(test.begin(),test.end(),print);
 
    cout<<endl<<"All Students with name \"Иванов\" (searching from begin to end of list):\n";
    Students::iterator it=test.begin();
    while(it=findElement("Иванов",it,test.end()))
     print(*it++);
}
Комментариев только в коде совсем мало, но функции и переменные несут смысловую нагрузку так что надеюсь код не очень сложный в чтении получился.
0
Надоела реклама? Зарегистрируйтесь и она исчезнет полностью.
Similar
Эксперт
41792 / 34177 / 6122
Регистрация: 12.04.2006
Сообщений: 57,940
02.10.2011, 23:28
Я подобрал для вас темы с готовыми решениями и ответами на вопрос Покритикуйте код (C++):

Покритикуйте код - C++
Покритикуйте код, я точно знаю, что он нубовский но все же. Это моя первая программа на с++ которая делает , что то полезное и типо мой...

Графы. Покритикуйте код - C++
Нужно помощь тех кто работает и пишет хороший и красивый код. У меня построено три матрицы, подскажите как улучшить код. Где можно...

Покритикуйте код финкции нахождения интеграла - C++
typedef double(*tfunc)(double); double integral(double start, end, func f, double dx) { double left, right, result; for (x=start,...

Покритикуйте и помогите улучшить код моей игры - C++
Здравствуйте, недавно начал писать игру и собственно пишу потихоньку, но не в этом суть, просто я только сегодня понял, что мой код может...

Покритикуйте пожалуйста программу - C++
Student.h#ifndef _STUDENT_H #define _STUDENT_H class Student { public: Student(); void del(); ...

Покритикуйте мою игру - C++
Выкладываю код своей первой игры. Она готова процентов на 90, но уже работает. Интересно узнать мнение людей, что в ней можно улучшить,...

116
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 01:17 #16
Ну и дальше обращение через интерфейс

C++
1
2
3
4
5
6
StudentList lst;
 
//...
 
for( StudentList::iterator it = lst.begin(); it != lst.end(); it++ )
   (*it).setName( "nameless" );
Кстати, итераторы тоже являются частью интерфейса. Они обеспечивают пользователю списка взаимодействие со студентами в этом списке, беря на себя все заботы по обходу списка, проверки на переход к нулевому элементу и так далее.


Цитата Сообщение от Gepar Посмотреть сообщение
talis, не-не, под интерфейсом там именно интерфейс, препод своих примеров понапихивал написанных так вот там есть у него лаба с "мордой", не знаю какая по счёту она будет.
Н-да?.. Ну это сути не меняет. Красивый графический интерфейс не отменяет хороший программный
0
Gepar
1181 / 537 / 20
Регистрация: 01.07.2009
Сообщений: 3,517
03.10.2011, 01:23  [ТС] #17
Странно, что-то не срабатывает моя перегрузка оператора...
итератор:
C++
1
 CStudent& operator*(){return current->student;}
в классе Students.h:
C++
1
friend std::ostream &operator<<(std::ostream &os, const CStudent &st);
в Students.cpp:
C++
1
2
3
4
5
6
7
8
std::ostream &operator<<(std::ostream &os, const CStudent &st)
{
    os<<setw(15)<<left<<st.fullname
     <<setw(7)<<st.group
     <<setw(5)<<st.year
     <<setw(7)<<st.average<<"\n";
 
}
в main:
C++
1
2
3
    cout<<"\nPrint list with ostream:\n";
    for(it=test.begin();it!=test.end();it++)
     cout<<(*it);
И не хочет, пишет что не нашло:
C++
1
C:\c++\Projects\StudentsProject\main.cpp|74|error: no match for 'operator<<' in 'std::cout << it.Students::iterator::operator*()'|
Странно.

Добавлено через 2 минуты
Хотя я вспомнил что перегрузка же должна быть отдельно от объявления, и всё же почему не подходит cpp файл, это разве не отдельно (из main компилируется если в main запихнуть код перегрузки) ?
0
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 01:25 #18
Ну пока бросилось в глаза, что std::ostream &operator<<(std::ostream &os, const CStudent &st) должна ещё вернуть os. Вы return забыли. А так - надо помедитировать... it какого типа?
0
Gepar
1181 / 537 / 20
Регистрация: 01.07.2009
Сообщений: 3,517
03.10.2011, 01:33  [ТС] #19
talis, об этом компилятор мне напомнил и это я добавил уже когда смог скомпилировать проэкт.
it - Students::iterator конечно, я сейчас попробую ещё в vs скомпилировать когда перегрузка находится в cpp файле, а не в main.

Добавлено через 2 минуты
А VS компилирует нормально, это только minigw чего-то обижается ... ну да ладно, так как уже поздно то я буду спать, думаю класс уже не столь страшен после Вашей критики так что можно и на глаза преподавателю его показать ) Спасибо за помощь в поиске bug'ов и недочётов класса
0
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 01:33 #20
Цитата Сообщение от Gepar Посмотреть сообщение
CStudent& operator*(){return current->student;}
а должно быть

C++
1
CStudent & Students::iterator::operator*(){ return current->student; }
Если, конечно, это не в объявлении класса итератора было.
0
Gepar
1181 / 537 / 20
Регистрация: 01.07.2009
Сообщений: 3,517
03.10.2011, 01:34  [ТС] #21
Класс правда ещё нужно будет усовершенствовать во второй лабораторной так что я думаю я его ещё здесь покажу после того как внесу указанные в задании изменения )
0
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 01:34 #22
Цитата Сообщение от Gepar Посмотреть сообщение
Спасибо за помощь
Да бога ради, обращайтесь
0
Gepar
1181 / 537 / 20
Регистрация: 01.07.2009
Сообщений: 3,517
03.10.2011, 01:35  [ТС] #23
talis,
Цитата Сообщение от talis Посмотреть сообщение
Если, конечно, это не в объявлении класса итератора было.
В объявлении это было, там какие-то специфические minigw-тараканы, в vs компилируется как надо (сдавать в vs 6.0).
0
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 01:37 #24
Gepar, ну выложите хоть что получилось. У меня mingw, теперь я репу почешу
0
Gepar
1181 / 537 / 20
Регистрация: 01.07.2009
Сообщений: 3,517
03.10.2011, 08:43  [ТС] #25
Вот так оно компилируется в VS, но не компилируется с minigw.
0
Вложения
Тип файла: zip wrong ostream operator.zip (3.1 Кб, 7 просмотров)
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 12:23 #26
Gepar, operator<< у вас объявлен как друг класса Students, то есть класса списка (чтобы не путаться, переименовали бы вы его в CStudentsList или как-то так). А выводите-то вы студента, CStudent.

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
class Students
{
    // ...
public:
    
        // ...
 
    /* эти функции НЕ должны быть в классе списка.*/
    /*
    friend std::ostream &operator<<(std::ostream &os, const CStudent &st);
    friend std::istream &operator>>(std::istream &is, CStudent &st);
    */
 
        // ...
 
};
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
struct CStudent
{
    string fullname;
    string group;
    int year;
    int average;
 
    /* если бы это был класс, и его поля были бы закрытыми, они должны были бы быть здесь */
    /*
    friend std::ostream &operator<<(std::ostream &os, const CStudent &st);
    friend std::istream &operator>>(std::istream &is, CStudent &st);
    */
};
 
/* а так вот так вот */
std::ostream &operator<<(std::ostream &os, const CStudent &st);
std::istream &operator>>(std::istream &is, CStudent &st);
1
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 12:26 #27
К рассуждениям выше прикладываю архив с исправленным вариантом.
0
Вложения
Тип файла: zip correct_ostream_operator.zip (3.2 Кб, 6 просмотров)
Deviaphan
Делаю внезапно и красиво
Эксперт С++
1306 / 1221 / 50
Регистрация: 22.03.2011
Сообщений: 3,744
03.10.2011, 12:36 #28
Цитата Сообщение от Gepar Посмотреть сообщение
C++
1
2
3
4
5
6
7
8
9
10
Students::iterator findElement(string toFind, Students::iterator begin, Students::iterator end)
{
    for(;begin!=end;++begin)
    {
        if (begin.first().find(toFind)!=(size_t)-1)
         return begin;
    }
    //если ничего не найдено - вернуть итератор указывающий за конец списка ( на NULL)
    return 0;
}
Т.е. тебя не смущает, что "Вася Иванов" и "Изя Иванович" это один и тот же человек?
0
talis
793 / 545 / 37
Регистрация: 11.05.2010
Сообщений: 1,298
Записей в блоге: 1
03.10.2011, 13:45 #29
М-да, поморгал... Gepar, а operator== ещё никто не отменял Да и вообще. В новом варианте с более-менее человеческими итераторами надо бы так:

C++
1
2
3
4
5
6
7
8
9
10
11
Students::iterator findElement( string toFind, Students::iterator begin, Students::iterator end )
{
    for( ; begin != end; ++begin )
    {
        if( ( *begin ).fullname != toFind )
            return begin;
    }
 
    //если ничего не найдено - вернуть итератор указывающий за конец списка ( на NULL)
    return 0;
};
Добавлено через 1 минуту
Ну или если там нужен поиск по фамилии без имени и по имени без фамилии, то их бы в разные поля определить бы...
0
Deviaphan
Делаю внезапно и красиво
Эксперт С++
1306 / 1221 / 50
Регистрация: 22.03.2011
Сообщений: 3,744
03.10.2011, 13:59 #30
Цитата Сообщение от talis Посмотреть сообщение
( *begin ).fullname
begin->fullname
0
03.10.2011, 13:59
MoreAnswers
Эксперт
37091 / 29110 / 5898
Регистрация: 17.06.2006
Сообщений: 43,301
03.10.2011, 13:59
Привет! Вот еще темы с ответами:

Шифратор пароля. Покритикуйте пожалуйста. - C++
Это моя первая программка на С++, если кому не лень, натычте меня носом в ляпы. Чтобы не топтаться по граблям. :) #include...

Код написан в Dev C. Не работает в Visual. Как нужно переделать код? - C++
Здравствуйте. Столкнулась с такой проблемой: код был написан в Dev C, но в Visual он выдаёт ошибку. ...

Не получается запустить длинный код Алгоритма Гомори, код правильный. - C++
Собственно как запустить код через С++Builder 6 #include&lt;ctype.h&gt; #include&lt;string.h&gt; #include&lt;conio.h&gt; #include&lt;stdio.h&gt; ...

Как получить исходный код *.exe или отредактировать его исполняемый код? - C++
Собственно возможно ли декомпилить его так, чтобы можно было потом обратно скомпилировать? Или хотя-бы отредактировать код, только не...


Искать еще темы с ответами

Или воспользуйтесь поиском по форуму:
30
Ответ Создать тему
Опции темы

КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin® Version 3.8.9
Copyright ©2000 - 2018, vBulletin Solutions, Inc.
Рейтинг@Mail.ru