Оценка кода C++
Дорогие опытные программисты, если вам не трудно - посмотрите пожалуйста на написанный мною код текстового редактора (самого примитивного). Я старался его написать в стиле ООП. Для меня будет важна любая критика. Напишите, пожалуйста, противно ли вам смотреть на данный код?
Код:
#include <iostream>
#include <conio.h>
#include <string>
#include <fstream>
#include <io.h>
#include <vector>
#include <utility>
#include <Windows.h>
#define CTRLF 6
#define ESC 27
#define s 115
#define ENTER 13
#define BACKSPACE 8
using namespace std;
class TextRedactor {
private:
string path;
string input_file;
ifstream input;
ofstream write;
string data;
void start() {
cout << "Пожалуйста, введите путь до текстового файла (расширение .txt)" << endl;
cin >> this->path;
// Проверяем наличие расширения ".txt" в пути к файлу
while (this->path.find(".txt") == string::npos) {
cout << "Указанный файл не является текстовым файлом (.txt)\n";
cout << "Пожалуйста, введите путь до текстового файла с расширением .txt\n";
cin >> this->path;
}
// Проверяем существование файла
while (_access(this->path.c_str(), 0) != 0) {
cout << "Файл не найден\n";
cout << "Пожалуйста, введите корректный путь до существующего файла\n";
cin >> this->path;
}
// Открываем файл для чтения
this->input.open(this->path, std::ios::in);
if (!this->input.is_open()) {
cout << "Не удалось открыть файл\n";
// Можно выбрать, завершить программу или запросить новый путь к файлу
exit(0);
}
// Считываем данные из файла
string line;
while (getline(this->input, line))
this->data += line + "\n";
this->input.close();
}
// реализуем логику завершения программы
void exitProgram() {
cout << "Вы действительно хотите завершить работу(ввите enter)?\n";
int ans;
ans = _getch();
if (ans == ENTER) {
system("cls");
cout << this->data << endl;
cout << "Сохранить изменения - enter, не сохранять - esc.\n";
ans = _getch();
if (int(ans) == ENTER) {
this->write.open(this->path, std::ios::out);
this->write << this->data;
this->write.close();
cout << "Изменения сохранены...\n";
exit(0);
}
else {
cout << "Изменения не сохранены.\n" << "Завершение работы...\n";
exit(0);
}
}
else
return;
}
// функция вывода "изменений" на экран. Цветом подствечиваем важные данные
void print_changes(const vector<pair<size_t, size_t>>& pos) {
HANDLE hConsole = GetStdHandle(STD_OUTPUT_HANDLE);
CONSOLE_SCREEN_BUFFER_INFO consoleInfo;
WORD savedAttributes;
GetConsoleScreenBufferInfo(hConsole, &consoleInfo);
savedAttributes = consoleInfo.wAttributes;
for (size_t i = 0; i < this->data.size(); ++i) {
bool found = false;
for (const auto& p : pos) {
if (i >= p.first && i <= p.second) {
SetConsoleTextAttribute(hConsole, FOREGROUND_INTENSITY | FOREGROUND_BLUE | FOREGROUND_RED); // изменить цвет текста на фиолетовый
found = true;
break;
}
}
if (!found) {
SetConsoleTextAttribute(hConsole, savedAttributes); // восстановить исходный цвет текста
}
cout << this->data[i];
}
SetConsoleTextAttribute(hConsole, savedAttributes); // восстановить исходный цвет текста
cout << endl;
}
// вставка строки в начало
void start_insert(const string& ch) {
this->data = ch + this->data;
}
// удаление последнего символа
void pop_back() {
if (this->data.length() > 0)
this->data.pop_back();
}
// Дополнительная функция для алгоритма КМП для поиска подстрок в строке - префиксная функция
static vector<int> computePrefixFunction(const string& pattern) {
int m = pattern.length();
vector<int> pi(m, 0);
int k = 0;
for (int q = 1; q < m; ++q) {
while (k > 0 && pattern[k] != pattern[q]) {
k = pi[k - 1];
}
if (pattern[k] == pattern[q]) {
++k;
}
pi[q] = k;
}
return pi;
}
// Основная функция поиска в строке
void findSubstrings(const string& pattern) { // Алгоритм КМП
vector<pair<size_t, size_t>> indices;
int n = this->data.length();
int m = pattern.length();
vector<int> pi = computePrefixFunction(pattern);
int q = 0;
for (int i = 0; i < n; ++i) {
while (q > 0 && pattern[q] != this->data[i]) {
q = pi[q - 1];
}
if (pattern[q] == this->data[i]) {
++q;
}
if (q == m) {
indices.push_back({ i - m + 1, i });
q = pi[q - 1];
}
}
print_changes(indices);
}
public:
// Логика программы
void run() {
setlocale(LC_ALL, "RU");
start();
char ipt;
string line;
cout << this->data << endl;
while (true) {
cout << "Введите ctrl + f для поиск в строке, s для вставки строки в начало, backspace для удаления последнего символа.\n";
ipt = _getch();
switch (int(ipt)) {
case ESC:
exitProgram();
break;
case CTRLF:
cout << "Введите строку для поиска (нажмите esc для выхода)...\n";
ipt = _getch();
while (int(ipt) != ESC) {
line += ipt;
if (int(ipt) == BACKSPACE && line.length() > 1) {
line = line.substr(0, line.length() - 2);
}
system("cls");
cout << "Поиск: " << line << endl;
if (line.length() > 0) {
findSubstrings(line);
ipt = _getch();
}
else if (line.length() == 0) {
while (int(ipt) == BACKSPACE) {
system("cls");
cout << "Введите строку для поиска (нажмите esc для выхода)...\n";
cout << this->data << endl;
ipt = _getch();
}
}
}
line = "";
cout << this->data << endl;
break;
case s:
system("cls");
cout << "Введите строку для вставки в начало..." << endl;
getline(cin, line);
if (line.empty())
getline(cin, line);
start_insert(line);
print_changes({ { 0, line.length() - 1} });
line = "";
break;
case BACKSPACE:
system("cls");
pop_back();
cout << this->data << endl;
break;
}
}
}
};
int main() {
TextRedactor Redactor;
Redactor.run();
return 0;
}
Ответы (1 шт):
Мелкие придирки, сверху вниз:
<Windows.h>
-><windows.h>
(с маленькой буквы). Это важно только в одном случае, при кросс-компиляции (компиляции под одну ОС из под другой). На линуксе имена к файлам регистрозависимые, аwindows.h
в MinGW называется с маленькой буквы.Вроде мелочь, но иногда этим грешат даже серьезные программы.
Макросы как константы - лучше
constexpr
переменные. (#define s
- ужс). Какие-то популярные символы, вроде'\n'
и'\b'
можно было бы писать прямо так, не делая именованные константы.using namespace std;
- не самая лучшая привычка. Сложнее на глаз определить, где символ из стандартной библиотеки а где нет, и иногда вызывает конфликты имен с ней. What's the problem with "using namespace std;"?.Непонятно, зачем везде писать
this->
, но допустим, что это такой стиль.while (this->path.find(".txt") == string::npos)
- неправильно, правильноpath.ends_with(".txt")
.Проверьте, работает ли у вас кириллица в путях к файлам, на винде с этим бывают проблемы.
-
// Проверяем существование файла while (_access(this->path.c_str(), 0) != 0) {
Непонятно зачем. Если файла нет, то
.open()
и так выдаст ошибку. -
// Считываем данные из файла string line; while (getline(this->input, line)) this->data += line + "\n";
Я бы советовал считать одним махом, так:
std::ostringstream ss; ss << input.rdbuf(); std::string data = std::move(ss).str();
system("cls")
- выглядит затратно запускать целое приложение (cls
), только чтобы очистить экран. Я бы попробовал что-то из этого: https://learn.microsoft.com/en-us/windows/console/clearing-the-screen (на первый взгляд (3) лучше всего, или (1)).Поиск подстроки: возможно стоит использовать готовый
std::search
, если это не в учебных целях.В перемешку разные стили наименования функций:
foo_bar
иfooBar
. Лучше выбрать один.
Более существенные замечания:
- Я бы посмотрел в сторону pdcurses, чтобы работало не только на винде.
- В какой кодировке сохраняется и читается кириллица? (Она вообще работает?) Сейчас в моде UTF-8, но тут явно не он.
- Скорее всего один длинный
std::string
будет тормозить на больших файлах. Как можно оптимизировать? Возоможно хранить вектор отдельных строк, но наверное есть способы еще лучше. - Если уж делать свои функции работы со строками, то можно вынести их в отдельный файл и/или неймспейс, чтобы потом можно было использовать повторно.