Проанализируйте код программы (пожалуйста)

Модератор: Модераторы разделов

Ответить
Аватара пользователя
Assuri
Сообщения: 678
Статус: #include <brain.h>
ОС: Fedora 12
Контактная информация:

Проанализируйте код программы

Сообщение Assuri »

Посоветуйте пожалуйста:
Как можно было бы сократить размер кода?
Может быть можно было бы использовать другой подход?

В общем мне нужны конструктивные советы. Кстати, подобные советы, как: используй процедурное программирование для таких прог в топку - я практикую ООП .

Код:

#include <iostream> #include <ctime> #include <cstdlib> #include <string> using namespace std; const unsigned short languages = 2; const unsigned short strings = 12; string texture [ languages ] [ strings ] = { { "Сложность: \n1: 0 - 10, 5 попыток\n2: 0 - 50, 8 попыток\n3: 0 - 100, 10 попыток\n4: 0 - 1000, 20 попыток \n5: свой вариант сложности \n>", "Введите верхнуюю границу случайного числа >", "Введите количество попыток >", "Хотите ли вы использовать данные настройки при следующей игре? ( да/нет ) >", "Количество попыток: ", "Введите число > ", "Меньше \n", "Больше \n", "Ура, вы угадали ;) \n", "Заданное число было равно: ", "Хотите ли вы сыграть еще? ( да/нет ) >" }, { "Complexity: \n1: 0 - 10, 5 attempts\n2: 0 - 50, 8 attempts\n3: 0 - 100, 10 attempts\n4: 0 - 1000, 20 attempts \n5: Personal variant of complexity. \n>", "Enter upper border of random number >", "Enter number of attempts >", "Do you want to use these settings in future game ( yes/no ) >", "Number of attempts: ", "Enter a number > ", "Smaller \n", "Bigger \n", "Oh, you are lucky ;) \n", "Proposed number was: ", "Do you want to play again? ( yes/no ) >" } }; class game { private: unsigned short choice; unsigned short randnum; unsigned short entnum; unsigned short steps; unsigned short upbord; unsigned short MAXsteps; string settings; string langstr; unsigned short langindex; public: string ch; // Конструктор ============================== game () : randnum ( 0 ), entnum ( 0 ), steps ( 0 ), choice ( 0 ), settings ( "нет" ), langstr ( "r" ), langindex ( 0 ) { } // ========================================================== void print_texture ( int ); void set_language () { cout << "\nChoose the language ( e/r or english/russian ) >"; cin >> langstr; if ( langstr == "r" || langstr == "russian" ) { langindex = 0; } else { langindex = 1; } } void set_choice() { if ( settings == "нет" || settings == "no") { print_texture ( 0 ); cin >> choice; switch ( choice ) { case 1: upbord = 10; MAXsteps = 5; break; case 2: upbord = 50; MAXsteps = 8; break; case 3: upbord = 100; MAXsteps = 10; break; case 4: upbord = 1000; MAXsteps = 20; break; case 5: print_texture ( 1 ); cin >> upbord; print_texture ( 2 ); cin >> MAXsteps; print_texture ( 3 ); cin >> settings; break; default: upbord = 100; MAXsteps = 8; } } } void set_rand () { srand ( time ( NULL ) ); randnum = rand () % upbord; } void enter_num () { do { print_texture ( 4 ); cout << MAXsteps - steps++ << '\n'; print_texture ( 5 ); cin >> entnum; if ( entnum > randnum ) print_texture ( 6 ); else if ( entnum < randnum ) print_texture ( 7 ); else if ( entnum == randnum ) print_texture ( 8 ); } while ( entnum != randnum && steps < MAXsteps ); if ( steps == MAXsteps ) { print_texture ( 9 ); cout << randnum << '\n'; } steps = 0; } void quast () { print_texture ( 10 ); cin >> ch; } }; void game::print_texture( int num ) { cout << texture [ langindex ] [ num ]; } int main() { cout << "Игра называется \"Угадай число\". Данная игра написана только для практики программирования на С++. Игра основана на Объектно-ориентированном программировании. © Tikhonov Sergey "; game g; g.set_language(); do { g.set_choice(); g.set_rand(); g.enter_num(); g.quast(); } while ( g.ch != "no" && g.ch != "нет" && g.ch!= "not" ); return 0; }


Помогите мне вырасти ценным и грамотным программистом.
Спасибо сказали:
Аватара пользователя
minoru-kun
Сообщения: 620
ОС: Debian GNU/Linux

Re: Проанализируйте код программы

Сообщение minoru-kun »

На мой не сильно опытный взгляд, код хороший.
Единственное что - в идеологическом плане, мешать все в одном файле - некузяво. Да и неудобно. Объявление класса и реализацию функций класса следовало бы выделить в отдельные файлы, так же в отдельный файл - сам алгоритм, использующий твой класс, а потом чудесно собрать это все с помощью make. ^___^


Да, кто-нибудь, пожалуйста, проанализируйте мой код (снабжен обильными комментариями на русском языке), и дайте критику и рекомендации по изменению. Мну тоже - начинающий кодер GNU, нуждающийся в надлежащем наставлении со сторону гуру ^___^
Вложения
subsanity_pre.tar.bz2
(4.75 КБ) 51 скачивание
Спасибо сказали:
Аватара пользователя
serzh-z
Бывший модератор
Сообщения: 8259
Статус: Маньяк
ОС: Arch, Fedora, Ubuntu
Контактная информация:

Re: Проанализируйте код программы

Сообщение serzh-z »

-DooM- писал(а):
09.07.2007 20:48
Помогите мне вырасти ценным и грамотным программистом.
Для этого нужно книжки и существующий код читать, много книжек и много кода.

Имхо, относительно вышенаписанного кода:

- форматирование лажает
- отсутствует соглашение о именах переменных класса
- часть функций включена в описание класса, часть вынесена за пределы. Я бы вообще не стал включать реализацию в описание класса
- класс game привязан к std::count и std::in, хотя ему без разницы какой это поток - лучше абстрагировать game от std::cout, передавая ссылку на поток (cout/in или любой другой) в конструктор game
- в game понамешан код для взаимодействия с пользователем и код выбора вопроса
- game ссылается на глобальную переменную texture, лучше упаковать texture в отдельный класс, который будет заниматься подборкой вопросов и анализом ответов
- более того - texture глобальная для всего приложения (не static)
- 'out << "Игра называется..."' печатается в одном месте, на одном языке, и потом в другом месте, на другом языке задаётся вопрос
- содержимое области std вынесено в глобальное пространство имён. Это имхо, но лучше так не делать, по крайне мере по отношению к std.
...
Спасибо сказали:
Аватара пользователя
serzh-z
Бывший модератор
Сообщения: 8259
Статус: Маньяк
ОС: Arch, Fedora, Ubuntu
Контактная информация:

Re: Проанализируйте код программы

Сообщение serzh-z »

minoru-kun, ужасно... "Тут читай, тут не читай". Кстати, комментарии на русском - это ещё ужаснее. Я, например, в винде, ничего не смог прочитать. -)
Спасибо сказали:
Аватара пользователя
minoru-kun
Сообщения: 620
ОС: Debian GNU/Linux

Re: Проанализируйте код программы

Сообщение minoru-kun »

serzh-z, а нельзя ли критику как-то пояснее изъяснить? :-} Насчет комментариев - понял, намотаю на ус. ^_^
Спасибо сказали:
Аватара пользователя
serzh-z
Бывший модератор
Сообщения: 8259
Статус: Маньяк
ОС: Arch, Fedora, Ubuntu
Контактная информация:

Re: Проанализируйте код программы

Сообщение serzh-z »

minoru-kun писал(а):
10.07.2007 16:23
нельзя ли критику как-то пояснее изъяснить
Про subsanity_pre.tar.bz2? Да там же код читать невозможно... Если бы это не был русскоязычный форум, то я бы заподозрил в Вас индусского программиста. Чего только стоит 'sub -> text = ""', строки длиною 226 символов и, внимание, глобальные имена, начинающиеся с двойного подчёркивания, что запрещено стандартом C++ (параграф 17.4.3.1.2).
Спасибо сказали:
Аватара пользователя
Assuri
Сообщения: 678
Статус: #include <brain.h>
ОС: Fedora 12
Контактная информация:

Re: Проанализируйте код программы

Сообщение Assuri »

minoru-kun писал(а):
10.07.2007 10:19
Да, кто-нибудь, пожалуйста, проанализируйте мой код (снабжен обильными комментариями на русском языке), и дайте критику и рекомендации по изменению. Мну тоже - начинающий кодер GNU, нуждающийся в надлежащем наставлении со сторону гуру ^___^

Единственное, что я хочу сказать и на это имею право, так это то, что код действительно ОЧЕНЬ сложно "читать". Я так и не нашел главного файла. Я даже отступов не вижу...

Огромное спасибо, serzh-z, за ответ, но есть несколько вопросов:
serzh-z писал(а):
10.07.2007 13:01
- форматирование лажает

Не совсем понял, что вы имеете ввиду. Можно на примере объяснить, пожалуйста?
serzh-z писал(а):
10.07.2007 13:01
- отсутствует соглашение о именах переменных класса

Не могли бы вы это соглашение озвучить? Я такого не слышал.
serzh-z писал(а):
10.07.2007 13:01
- часть функций включена в описание класса, часть вынесена за пределы. Я бы вообще не стал включать реализацию в описание класса

То-есть вы бы не оставили внутри класса ни одного метода? Насколько я знаю, все методы, находящиеся в классе - inline функции. А так как все функции у меня очень маленькие, я решил их оставить inline. Я думаю вы и сами знаете какая разница.

serzh-z писал(а):
10.07.2007 13:01
- в game понамешан код для взаимодействия с пользователем и код выбора вопроса

Не понял: что такое "выбор вопроса"?
serzh-z писал(а):
10.07.2007 13:01
- содержимое области std вынесено в глобальное пространство имён. Это имхо, но лучше так не делать, по крайне мере по отношению к std.

А что вы предлогаете? И почему лучше так не делать?
Спасибо сказали:
Аватара пользователя
serzh-z
Бывший модератор
Сообщения: 8259
Статус: Маньяк
ОС: Arch, Fedora, Ubuntu
Контактная информация:

Re: Проанализируйте код программы

Сообщение serzh-z »

-DooM- писал(а):
10.07.2007 18:21
Не совсем понял,
Опять же поясню, что это лишь имхо, но лишние пробелы перед и после скобкой, мягко говоря, выглядят очень непривычно. Список инициализации класса game поражает - слишком длинная строка, без переносов.

Открывающая фиг. скобка после do, if и т.д. - как бы сказать, занимает много лишнего места. Это, конечно, дело стиля, но, например, K&R-стиль использует открывающую скобку на одной строке с оператором do, if и т.д. Для функций (опять же в K&R) всё верно - скобки на отдельной строке.
-DooM- писал(а):
10.07.2007 18:21
Я такого не слышал.
Не заметно общего стиля в именовании переменных класса - часть переменных в нижнем регистре, часть в смешанном. Ну и напоследок - довольно общепринятой практикой является помечать переменные класса - например "_" после имени. Это позволит избежать неоднозначности при ссылках на переменную в функциях, где есть локальные переменные с таким же именем. Т.е. подобные соглашения Вы вырабатываете сами для себя и потом неукоснительно им следуете во всём приложении. Чем более общепринятым это соглашение будет, тем проще будет править и читать чужой код и работать в команде.
-DooM- писал(а):
10.07.2007 18:21
То-есть вы бы не оставили внутри класса ни одного метода?
Да. Современные компиляторы обычно сами решают какие функции встраивать, а какие нет. В случае выноса реализации функции за пределы описания класса и желании сделать её встраиваемой - можно использовать подсказку "inline". Читать описание класса отдельно от его реализации намного проще.
-DooM- писал(а):
10.07.2007 18:21
Не понял: что такое "выбор вопроса"?
Сама логика выборки (print_texture) нужного вопроса из массива. В этом конкретном случая она примитивна, но в более сложном случае лучше разделять взаимодействие с пользователями и основную логику задачи.
-DooM- писал(а):
10.07.2007 18:21
А что вы предлогаете? И почему лучше так не делать?
Писать "std::count", "std::vector" и т.д. Имя области "std" не зря сделано таким коротким - его несложно лишний раз напечатать. Не стоит "загрязнять" область глобальных имён лишними именами. Опять же - в случае мелкого приложения "для себя" это не играет роли, но в других случаях лучше бы этого избегать.
Спасибо сказали:
v04bvs
Сообщения: 636
ОС: Debian GNU/Linux

Re: Проанализируйте код программы

Сообщение v04bvs »

советую просто писать больше кода, от игрушечных программ много толка не будет. мелких придирок можно найти много, но это всё, в конечном счёте ерунда.
Спасибо сказали:
Аватара пользователя
Assuri
Сообщения: 678
Статус: #include <brain.h>
ОС: Fedora 12
Контактная информация:

Re: Проанализируйте код программы

Сообщение Assuri »

-DooM- писал(а):
10.07.2007 18:21
Сама логика выборки (print_texture) нужного вопроса из массива. В этом конкретном случая она примитивна, но в более сложном случае лучше разделять взаимодействие с пользователями и основную логику задачи.

Кстати, меня очень сильно интересует вопрос: каким еще образом можно сделать выборку текстуры в соответствие с выбранным языком?
Советую просто писать больше кода, от игрушечных программ много толка не будет. мелких придирок можно найти много, но это всё, в конечном счёте ерунда.

Не согласен. Такие вот мелочи помогут мнк в будущем. Тем более как я буду писать большие программы, не зная, как надо правильно кодить маленькие?

Serzh-z, Вы предлагали сделать класс "texture" вместо большого массива, но ведь инициализация невозможна в классе таким образом, каким я сейчас инициализирую. Вот отсюда до конца посмотрите: Несколько вопросов по C++
Спасибо сказали:
Аватара пользователя
minoru-kun
Сообщения: 620
ОС: Debian GNU/Linux

Re: Проанализируйте код программы

Сообщение minoru-kun »

Код: Выделить всё

sub -> text = ""

Хм... и как тогда это делать? :-}
Спасибо сказали:
Аватара пользователя
Aectann
Бывший модератор
Сообщения: 3491
Статус: ...
ОС: OS X, GNU_и_не_только/Linux

Re: Проанализируйте код программы

Сообщение Aectann »

serzh-z писал(а):
10.07.2007 13:01
и существующий код читать,

Не посоветуете какой-нибудь ресурс с исходниками либо конкретные проекты для начала? Очень большая часть софта под Линух написана на C, программы на С++ небольшого масштаба найти бывает довольно проблематично...
My god... it's full of stars!...
Спасибо сказали:
Abaddon
Сообщения: 81
ОС: Gentoo 2006.1.x86_64

Re: Проанализируйте код программы

Сообщение Abaddon »

-DooM- писал(а):
09.07.2007 20:48
Посоветуйте пожалуйста:
Как можно было бы сократить размер кода?
Может быть можно было бы использовать другой подход?

В общем мне нужны конструктивные советы. Кстати, подобные советы, как: используй процедурное программирование для таких прог в топку - я практикую ООП .

В таком случае и код не стоит мешать... половина - ООП, а вторая половина - процедурка.

В плане оптимизаций, если нет причин использовать short (четко определенная 16 битность переменной, например, с целью циклического сдвига), то стоит использовать просто int, работать будет быстрее, хотя и памяти скушает больше:)
Base: Gentoo 2006.1.x86_64 on AMD64_X2-5200+/1024Mb/7300GS-256Mb/250Gb
Serv: Gentoo 2006.1.x86_32 on iCeleron-2.4/1024Mb/Geforce2MX400-64Mb/250Gb+60Gb
Note: Gentoo 2006.1.x86_32 on Transmeta-8800(Efficeon)/512Mb/(Trident-???)/40Gb
Gate: Gentoo 2005.1.x86_32 on AMD-K6.2-500/64Mb/forgot.../3.2Gb+6.4Gb+40Gb
Спасибо сказали:
Аватара пользователя
serzh-z
Бывший модератор
Сообщения: 8259
Статус: Маньяк
ОС: Arch, Fedora, Ubuntu
Контактная информация:

Re: Проанализируйте код программы

Сообщение serzh-z »

-DooM- писал(а):
10.07.2007 21:09
выборку текстуры в соответствие с выбранным языком?
gettext(), как сказано в соседнем топике.

-DooM- писал(а):
10.07.2007 21:09
предлагали сделать класс "texture" вместо большого массива
Я предлагаю инкапсулировать весь код (как и сами данные массива), по работе с данными texture в отдельный класс.

-DooM- писал(а):
10.07.2007 21:09
но ведь инициализация невозможна в классе таким образо
Поскольку уж это C++, то стоит использовать std::vector. У него достаточно скромные накладные расходы, по сравнению с массивом C.

minoru-kun писал(а):
10.07.2007 21:16
и как тогда это делать?
sub->text = "" :)

Aectann писал(а):
11.07.2007 13:39
ресурс с исходниками
http://www.koders.com/
http://www.google.com/codesearch
http://codeguru.com/ (но там большая часть кода для Windows и MFC - по-крайне мере так было раньше, так же к этому сайту очень внимательно присматривается Microsoft)

Abaddon писал(а):
11.07.2007 13:42
код не стоит мешать... половина - ООП, а вторая половина - процедурка
Хотел бы я посмотреть на программу C++ без main(). Так же как на программу C++ без STL (точнее на такую бы не хотел). :)
Спасибо сказали:
Ответить