Комментарии 68
Под кат срочно, пока не заминусовали!
Где обработка ошибок? Что за магические числа в коде?
какие именно числа?
Вообще C++ не самый лучший…
1) Почему-то используются простые указатели на виждеты, хотя вроде бы можно обернуть;
2) Члены класса можно инстанциировать списком инициализации (это тот, который с двоеточием);
3) Есть переменные, которым можно поставить const;
4) стиль растановки фигурных скобочек не везде одинаковый
5) зачем-то «using namespace std»
6) почему-то используется то itoa, то QString::number
Сам-то считаю, что плохо пишу…
1) Почему-то используются простые указатели на виждеты, хотя вроде бы можно обернуть;
2) Члены класса можно инстанциировать списком инициализации (это тот, который с двоеточием);
3) Есть переменные, которым можно поставить const;
4) стиль растановки фигурных скобочек не везде одинаковый
5) зачем-то «using namespace std»
6) почему-то используется то itoa, то QString::number
Сам-то считаю, что плохо пишу…
Ещё вот такие штуки бесят:
То есть когда переменная не инициализируется при объявлении (что желательно), а где-то далеко после.
passwd *pw;
uid_t uid;
uid = geteuid();
pw = getpwuid(uid);
То есть когда переменная не инициализируется при объявлении (что желательно), а где-то далеко после.
1) Я уже привык, да и в книге по которой я учусь тоже используются простые указатели на виджеты.
2) Это у меня осталось от Java, там списка инициализации нету.
3) Проект маленький, так что я считаю здесь много мер защиты не надо, он бы работал даже если бы переменные были публичными а не закрытыми.
4) Вот такие вот у меня странности, да и в принципе, компилятору всё равно без разницы как раставлены скобки.
5) Не люблю усложнять код используя много using или часто использую оператор разрешения области видимости.
6) Пересмотрел весь код, метода itoa я не нашёл, нашёл только atoi, да и разница между itoa и QString::number есть, itoa возвращает char*, а QString::number — QString
2) Это у меня осталось от Java, там списка инициализации нету.
3) Проект маленький, так что я считаю здесь много мер защиты не надо, он бы работал даже если бы переменные были публичными а не закрытыми.
4) Вот такие вот у меня странности, да и в принципе, компилятору всё равно без разницы как раставлены скобки.
5) Не люблю усложнять код используя много using или часто использую оператор разрешения области видимости.
6) Пересмотрел весь код, метода itoa я не нашёл, нашёл только atoi, да и разница между itoa и QString::number есть, itoa возвращает char*, а QString::number — QString
Суть в том, что раз заявили, что пишете на C++ и Qt, так и пишите на C++ и используйте Qt, не примешивая кусочки C.
Конечно, можно писать по привычке, но привычки бывают как полезные, так и вредные.
Да, компилятору вообще пфк на то, что в коде происходит, если он синтаксически верен, но код для людей пишут.
Если что, то это не наезд, а попытка поделиться опытом.
Конечно, можно писать по привычке, но привычки бывают как полезные, так и вредные.
Да, компилятору вообще пфк на то, что в коде происходит, если он синтаксически верен, но код для людей пишут.
Если что, то это не наезд, а попытка поделиться опытом.
я понимаю что это не наезд, и ваши советы мотаю на ус, но всё же C++ полностью включает в себя C, и я думаю что в коде C++ старые функции никто не отменял
но всё же C++ полностью включает в себя C, и я думаю что в коде C++ старые функции никто не отменял
Это плохой подход, правильно вам говорят. Относительно некоторых других пунктов, можно смело поспорить, но тут претензия по делу.
Ну и относительно скобок и прочего. Придерживайтесь единого стиля во всем проекте. Не относитесь шапкозакидательски к таким «мелочам».
но всё же C++ полностью включает в себя C
Лучше так не говорить, могут придраться. В С++ действительно есть доступ к стандартной библиотеке С, но нет полной поддержки его синтаксиса. Например, этот код не компилируется в С++, хотя верен в С:
void f() {}
int main()
{
f(1, 2, 3);
}
Есть и другие примеры.
он верен потому что если в C оставить скобки пустыми, то компилятор принимает это за неопределённое число параметров, да?
В С++ надо обязательно декларировать функцию перед использованием, а в С необязательно. Поэтому С++ будет ругаться, так как не найдет функции с 3 параметрами. С же будет верить, что где-то в другом месте она найдется.
Нет, не поэтому. В С описание функции
f() говорит о том, что в нее можно передать произвольное количество параметров. Если надо сказать, что параметров действительно не должно быть, пишут f(void). В С++ это эквивалентные описания.Почему-то мне всегда казалось, что там должно быть троеточие в случае произвольного количества параметров. Хотя я могу быть не прав, не спорю.
А можно ссылочку на то, что f() предполагает переменное число аргументов? Почему-то гугл упорно не хочет такое показывать, а показывает только f(int a, ...), т.е. как раз вариант с одним обязательным аргументом и тем самым троеточием. MSDN тоже так говорит. Просто интересно, а то я уже чистый си забывать начал:)
Многие пункты — это общий стиль Qt. После «чистого» С++ очень сложно смотреть на Qt-код. Буквально на прошлой неделе начал участие в Qt-проекте на работе, первые негативные впечатления, судя по справке в интернете и работе коллег:
— namespace — нигде не встретите
— константность — никто не заботиться, практически везде CopyOnWrite, но в некоторых случаях объекты ведут себя как shared_ptr
— указатели — везде сырые, умные использовать довольно сложно, интерфейс qt умные указатели практически не использует
— за памятью вообще никто не следит
— несмотря на довольно хорошую документацию и несложные исходники, если хотите чтобы ваша программа была надежной, приходится много дебажить qt-код и смотреть, когда и что вызывается.
— namespace — нигде не встретите
— константность — никто не заботиться, практически везде CopyOnWrite, но в некоторых случаях объекты ведут себя как shared_ptr
— указатели — везде сырые, умные использовать довольно сложно, интерфейс qt умные указатели практически не использует
— за памятью вообще никто не следит
— несмотря на довольно хорошую документацию и несложные исходники, если хотите чтобы ваша программа была надежной, приходится много дебажить qt-код и смотреть, когда и что вызывается.
1) Потому что виджеты регистрируются у родителя и он их автоматически удаляет. Это поведение Qt. Оборачивать в (shared/auto/unique)_ptr — приведёт к двойному удалению объекта.
3) Уточните, не увидел, где имеет смысл писать const.
4) Qt coding style — после функций скобки на новой строке, после if/for/etc — на той же. А ту функцию автор похоже скпировал и не менял, поэтому и остались с новой строки.
2, 5, 6 — согласен.
Насчёт atoi добавлю для DenSoLo777
7) имхо, если Qt, то стоит использовать std::string → QString, std::vector → QList/QVector, std::iostream → QTextStream.
8) Использовать русский текст в строках — зло, правильно — tr(«Text») и делать файлы с переводом.
3) Уточните, не увидел, где имеет смысл писать const.
4) Qt coding style — после функций скобки на новой строке, после if/for/etc — на той же. А ту функцию автор похоже скпировал и не менял, поэтому и остались с новой строки.
2, 5, 6 — согласен.
Насчёт atoi добавлю для DenSoLo777
7) имхо, если Qt, то стоит использовать std::string → QString, std::vector → QList/QVector, std::iostream → QTextStream.
8) Использовать русский текст в строках — зло, правильно — tr(«Text») и делать файлы с переводом.
Отвечу не конкретно Вам, а в том числе советующим выше тоже — приятная ситуация, когда читаешь статью, набираются замечания по коду, доходишь до комментов — а там уже все написано, добрые люди уже дали с десяток советов джуниору) Светлая сторона Хабра =)
Спасибо за ответ про указатели, я довольно давно работал с Qt и забыл про этот аспект.
Про const могу сказать только то, что существует такая практика «если переменную/объект/метод можно сделать константным, то лучше сделать это». В таком случае компилятор гарантированно сможет сделать constant propagation и сгенерировать более эффективный код. Кроме того, это может помочь:
Поэтому я стараюсь пихать const как можно больше и чаще, хуже от этого точно не становится. Пожалуйста, дополните/возразите, если можно.
Ещё маленькая хитрость из той же серии — ограничивать scope переменных, вводя дополнительные группы операторов (фигурные скобки {… }) — тоже облегчает чтение программы, т.к. становится видно, что переменная больше не нужна, а значит читающему программисту придётся удерживать в голове меньше контекста.
Про const могу сказать только то, что существует такая практика «если переменную/объект/метод можно сделать константным, то лучше сделать это». В таком случае компилятор гарантированно сможет сделать constant propagation и сгенерировать более эффективный код. Кроме того, это может помочь:
- избежать ошибок при переиспользовании переменных и ошибок использования неинициализированных переменных
- помочь человеку, который читает код — увидели const, и можем мысленно прекратить следить за содержимым переменной.
- покажет, что параметры функции точно не изменятся (даже при передаче по значению). По-моему это не очень хорошо, когда мы передали в функцию A, а оно через какое-то время внутри неё превратилось в B.
Поэтому я стараюсь пихать const как можно больше и чаще, хуже от этого точно не становится. Пожалуйста, дополните/возразите, если можно.
Ещё маленькая хитрость из той же серии — ограничивать scope переменных, вводя дополнительные группы операторов (фигурные скобки {… }) — тоже облегчает чтение программы, т.к. становится видно, что переменная больше не нужна, а значит читающему программисту придётся удерживать в голове меньше контекста.
Что-то мне подсказывает, что под Windows и OS X это работать не будет. Хотя в статье об этом ни слова.
Теперь его основные возможности определены, и я могу приступать к описанию кода, предупреждаю сразу что код будет работать только в линуксе, так как почти все его возможности основаны на парсинге папки /proc.
спасибо, сам забыл про этот кусочек текста
Погорячился я. Но этот текст не видно в списке постов, а значит все равно нельзя понять, нужна ли статья до посещения её полной версии.
в будущем я это учту
Добавьте тег linux и повешайте дисклаймер (можно в спойлере) до хабраката.
а можно по подробнее, что такое дисклаймер?
Дисклаймер: ru.wikipedia.org/wiki/Disclaimer
Попросту — указание границ, за которыми автор не гарантирует работоспособность и отсутствие проблем.
Попросту — указание границ, за которыми автор не гарантирует работоспособность и отсутствие проблем.
А что мешает добавить тег Linux или уточнить в названии?
«Пишем свой системный монитор» => «Пишем свой системный монитор (Linux)»
«Пишем свой системный монитор» => «Пишем свой системный монитор (Linux)»
знаю, но думаю догадаться об этом не трудно, даже если человек никогда не пользовался линуксом, он всё равно заметит что файловая система не такая как в его ОС, и значит у него программа работать не будет.
Ужасный стиль кодирования. Излишне длинная и плоская простыня
PS стиль кодирования — такая штука, что сильно облегчает понимание кода. С опытом приходит понимание этого, так что рекомендую даже в «небольших проектах» обращать на это много внимания (кстати, современные IDE вполне способны помогать поддерживать хотя бы форматирование кода).
void Info::update(), к тому же получение данных вперемешку, объявление переменных и инициализация зачем-то разведены в разные строки, используется два разных способа получения данных в строку в соседних строках (
stream >> strgetline(stream,str) stream >> str;
for(int i = 0; i < 15;i++) stream >> str;Мне кажется так выглядеть будет намного лучше, частично код сохранён и в местах, где надо бы тоже поправить ради комментариев к этим местам
void Info::update() {
ifstream stream("/proc/sys/kernel/hostname");
string str;
getline(stream, str);
hostname->setText("Hostname: " + QString::fromStdString(str));
stream.close();
uid_t uid = geteuid();
passwd *pw = getpwuid(uid);
user->setText("Пользователь: " + QString::fromAscii(pw->pw_name));
struct sysinfo o;
sysinfo(&o);
long up = o.uptime;
int hour = up / 60 / 60;
int min = (up - hour * 60 * 60) / 60;
int sec = ((up - hour * 60 * 60) - min * 60);//в константы, не?
QString e = QString::number(hour) + QString(" h. ") + QString::number(min) + QString(" m. ")
+ QString::number(sec) + QString(" s.");
uptime->setText("Uptime: " + e);
stream.open("/proc/cpuinfo");
for(int i = 0 ; i < 16 ; i++)
stream >> str;
getline(stream, str);
proc->setText("Процессор: " + QString::fromStdString(str));
for(int i = 0 ; i < 7 ; i++)
stream >> str;
freq->setText("Частота: " + QString::fromStdString(str) + " MHz");
stream.close();
cpubar->setValue(getCpuLoad(0.3));
stream.open("/proc/meminfo");
stream >> str; stream >> str;
int totalMemory = atoi(str.c_str());
int gb = (totalMemory / 1024) / 1024;
int mb = (totalMemory - gb * 1024 * 1024) / 1024;
int kb = (totalMemory - (gb * 1024 * 1024 + mb * 1024));//1024 давно в константы просится
if (gb > 0) //с высокой вероятностью будет целое число, в крайнем случае с точностью до мегабайт, поэтому отображаем Mb и Kb только если надо
e = QString::number(gb) + QString(" Gb ");
else
e = QString("");
if (mb > 0)
e += QString::number(mb) + QString(" Mb ");
if (kb > 0)
e += QString::number(kb) + QString(" Kb ");
mem->setText("Оперативная память: " + e);
//выносим в функцию или в цикл, я, для простоты покажу цикл:
int freeMemory = 0;
for (i = 0 ; i < 3 ; i++) {
stream >> str; stream >> str; stream >> str;
freeMemory += atoi(str.c_str());
}
int usedMemory = totalMemory - freeMemory;
gb = usedMemory / 1024 / 1024;
mb = (usedMemory - gb*1024*1024) / 1024;
kb = (usedMemory - ((mb*1024) + (gb * 1024 * 1024)));
if (gb != 0 ) {
e = QString::number(gb) + " Gb " + QString::number(mb) + QString(" Mb ") + QString::number(kb)
+ QString(" Kb");
} else { // а что, если Mb пустое? проще было сделать так же как я с total memory, кстати totalMemory тоже может иметь < Gb.
e = QString::number(mb) + QString(" Mb ") + QString::number(kb) + QString(" Kb");
}
memload->setText("Используемая оперативная память: " + e);
int usage = 100 * usedMemory / totalMemory;
membar->setValue(usage);
}
PS стиль кодирования — такая штука, что сильно облегчает понимание кода. С опытом приходит понимание этого, так что рекомендую даже в «небольших проектах» обращать на это много внимания (кстати, современные IDE вполне способны помогать поддерживать хотя бы форматирование кода).
getline я использовал там где могут быть пробелы, а конструкция
И да, а что не так с
for(int i = 0; i < 15;i++) stream >> str;
И да, а что не так с
void Info::update()
И всё-таки нехорошо смотрятся разные способы получения данных…
Про цикл — у вас ровно перед циклом ещё одно это же действие.
Я же написал, что не так с этой функцией (все претензии к её телу и описаны в комментарии… можете сравнить приведённый мной код и ваш в плане читабельности)
Про цикл — у вас ровно перед циклом ещё одно это же действие.
Я же написал, что не так с этой функцией (все претензии к её телу и описаны в комментарии… можете сравнить приведённый мной код и ваш в плане читабельности)
претензии понял, увеличу число итераций в цикле, а действие над ним уберу, а вот с getline я ничего поделать не могу, имя модели процессора с пробелами, а другого способа чтения из файла без учёта разделителей я не знаю
Отлично, часть рекомендаций по оформлению вы поняли, постараюсь донести остальное:
— желательно не делать слитный блок кода занимающегося разными задачами. Нужно или выносить в функции (если оправдано) или, хотя бы, разделать блоки пустыми линиями. В данном случае в теле этой функции вычисляются разные блоки информации, получение которых мало зависит друг от друга, именно поэтому вы можете наблюдать в моём примере пустые строки, разрывающие код. Читать так легче.
— никогда не стоит в одну строку писать два разных действия:
В данном случае объявление переменной не оправдано абсолютно ничем, а искать его будет сложно.
— нежелательно писать в одну строку объявление цикла и тело цикла. Опять же визуально не воспринимается и можно не заметить кусок кода.
— настройте в своей IDE форматтер кода и пользуйтесь им. Это значительно облегчит восприятие кода.
Есть ещё рекомендации по именованию и использованию переменных (по невнимательности могут уйти не те данные, которые хотелось бы), но это вопрос сложный, его я касаться сейчас не буду.
— желательно не делать слитный блок кода занимающегося разными задачами. Нужно или выносить в функции (если оправдано) или, хотя бы, разделать блоки пустыми линиями. В данном случае в теле этой функции вычисляются разные блоки информации, получение которых мало зависит друг от друга, именно поэтому вы можете наблюдать в моём примере пустые строки, разрывающие код. Читать так легче.
— никогда не стоит в одну строку писать два разных действия:
ifstream stream("/proc/sys/kernel/hostname"); string str;В данном случае объявление переменной не оправдано абсолютно ничем, а искать его будет сложно.
— нежелательно писать в одну строку объявление цикла и тело цикла. Опять же визуально не воспринимается и можно не заметить кусок кода.
— настройте в своей IDE форматтер кода и пользуйтесь им. Это значительно облегчит восприятие кода.
Есть ещё рекомендации по именованию и использованию переменных (по невнимательности могут уйти не те данные, которые хотелось бы), но это вопрос сложный, его я касаться сейчас не буду.
Искал аналог seek для файловых потоков С++, наткнулся на такой код (источник) для перехода на конкретную строку в файле:
#include
#include
std::fstream& GotoLine(std::fstream& file, unsigned int num){
file.seekg(std::ios::beg);
for(int i=0; i < num — 1; ++i){
file.ignore(std::numeric_limits::max(),'\n');
}
return file;
}
Вопрос в том: будет ли он быстрее простого считывания файла построчно?
#include
#include
std::fstream& GotoLine(std::fstream& file, unsigned int num){
file.seekg(std::ios::beg);
for(int i=0; i < num — 1; ++i){
file.ignore(std::numeric_limits::max(),'\n');
}
return file;
}
Вопрос в том: будет ли он быстрее простого считывания файла построчно?
Надо же, мой код кому-то пригодился. Я уже сам с трудом помню, как он работает, да и смотреть на него спустя два года страшно :)
спасибо за ваш код :) наверное единственный нормальный источник по получению загрузки процессора который я нашёл :)
man 5 proc ;-)
Кошмарное сочетание stl & qt
Выглядит куда лучше в виде:
А такое:
на
atoi на QString::toUint заменяется.
e = QString::number(gb) + QString(" Gb ") + QString::number(mb) + QString(" Mb ") + QString::number(kb) + QString(" Kb");Выглядит куда лучше в виде:
e = QString("%1 Gb %2 Mb %3 Kb").arg(gb).arg(mb).arg(kb);А такое:
int hour = up / 60 / 60;
int min = (up - hour * 60 * 60) / 60;
int sec = ((up - hour * 60 * 60) - min * 60);//в константы, не?
QString e = QString::number(hour) + QString(" h. ") + QString::number(min) + QString(" m. ") + QString::number(sec) + QString(" s.");
на
QTime time_n(0,0,0,0);
QTime uptime = time_n.addSecs(up);
QString e = uptime.toString("hh' h. 'mm' m. 'ss' s.'");
atoi на QString::toUint заменяется.
QProcess::execute(«sleep»,QStringList() << QString::number(dt));man 3 sleep
QProcess::execute(«kill», QStringList() << str);man 2 signal
Вообще, если вы в плюсах запускаете другую программу — почти наверняка вы что-то делаете не так.
man signal, indeed, но и вот ещё:
Когда оно
Для завершения процесса я использую программу которая есть во всех линуксах(если в вашем линуксе её нету, то я даже не представляю что это за линукс такой)
Когда оно
$ type kill kill is a shell builtin
QProcess::execute(«kill», QStringList() << str);
man 2 signal
Тогда уж лучше man 7 signal.
Заодно и про типы сигналов человек узнает.
Лучше QThread::sleep/msleep/usleep.
А не подскажет уважаемое сообщество, как подобная задача в виндовс решается? Передо мной стоит задача собрать статистику по загрузке процессора и памяти за сутки. Вернее, меня интересуют пиковые значения и интервал времени в котором было превышение определенного уровня. Буду благодарен за любые наводки.
Там всё уже написано, вам надо будет только отчёт правильно создать. technet.microsoft.com/en-us/library/cc749249.aspx
есть такая штука — библиотека hyperic/sigar, она кроссплатформенная, и используется как раз для получения таких значений как загрузка cpu и потребление оперативки, погугли, может найдёшь примеры её применения в C++, я гуглил, но нашёл только про то как её в Java использовать
Все ругают код, а я хочу сказать спасибо за описание того, КАК это всё можно сделать. Сама реализация в этой статье для меня уходит на второй план.
PS: сам года два назад хотел такую штуку написать (и даже начал), но времени не хватило.
PS: сам года два назад хотел такую штуку написать (и даже начал), но времени не хватило.
Знающие люди, напишите, пожалуйста: как можно узнать реальный размер оперативной памяти, занимаемый конкретным процессом?
Вроде-как для этого подходит утилита top, но все пишут, что там данные о памяти не совсем правильные, т.к. каждый процесс использует два типа памяти: совместную и свою личную. И типа эта совместная память делится между всеми процессами одной программы. А как все это свести воедино, чтобы узнать, к примеру, сколько в итоге потребляет Apache или Mysql?
Вроде-как для этого подходит утилита top, но все пишут, что там данные о памяти не совсем правильные, т.к. каждый процесс использует два типа памяти: совместную и свою личную. И типа эта совместная память делится между всеми процессами одной программы. А как все это свести воедино, чтобы узнать, к примеру, сколько в итоге потребляет Apache или Mysql?
Несколько замечаний по коду (по логике, а не по оформлению):
1. Возможно, вместо ifstream имеет смысл использовать кьютовские классы QDataStream и QTextStream
2. vector можно заменить на QVector, а string на QString.
Собственно, я к тому, что если уж Вы используете библиотеку Qt, то надо использовать ее по максимуму, чтобы не мешать в одном коде разные библиотеки.
1. Возможно, вместо ifstream имеет смысл использовать кьютовские классы QDataStream и QTextStream
2. vector можно заменить на QVector, а string на QString.
Собственно, я к тому, что если уж Вы используете библиотеку Qt, то надо использовать ее по максимуму, чтобы не мешать в одном коде разные библиотеки.
В догонку.
Если логику получения данных вынести из классов с виджетами, то с помощью макросов
#if defined Q_WS_WIN
//…
#elif defined Q_WS_X11
//…
#endif
можно будет в дальнейшем обеспечить кроссплатформенность приложения, дописав виндо-специфочную часть.
Если логику получения данных вынести из классов с виджетами, то с помощью макросов
#if defined Q_WS_WIN
//…
#elif defined Q_WS_X11
//…
#endif
можно будет в дальнейшем обеспечить кроссплатформенность приложения, дописав виндо-специфочную часть.
А чем плох ifstream?
Я понимаю нежелание мешать библиотеки, но для стандартной библиотеки языка разве не имеет смысла сделать исключение?
Я понимаю нежелание мешать библиотеки, но для стандартной библиотеки языка разве не имеет смысла сделать исключение?
Сходу минус (не так давно столкнулся): как открыть файл в Windows, имя которого содержит символы не из текущей 8-битной кодировки? Добавлю, что я знаю, что метод open в VC++ отклоняется от стандарта и позволяет это сделать. Но при использовании MinGW, который следует стандарту, нас ждёт облом. С clang ситуация будет аналогичная (по крайней мере при использовании библиотек gcc).
Хотя это ещё вопрос минус это библиотеки или Windows, но тем не менее, с точки зрения использования комплексно — это минус.
Хотя это ещё вопрос минус это библиотеки или Windows, но тем не менее, с точки зрения использования комплексно — это минус.
ifstream, как таковой, не плох, но я не люблю, когда в коде появляется логика перевода типов данных одной библиотеки в типы данных другой. Эту логику ведь придется отлаживать, а потом еще и поддерживать. Я стараюсь этого по возможности избежать.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Пишем свой системный монитор (Linux)