초보자아아
30
2021-02-24 19:57:01
4
241

초보자입니다. C++코드리뷰 받고싶습니다.


안녕하세요 입문한지 얼마 안된사람입니다. C++문법책 때고 effective C++을 1~17문항을 읽으면서 되짚어 볼겸 문제 랜덤 출력 프로그램을 하나 만들어봤습니다. 고수님들 어떻게 하면 좀 더 좋은 코딩을 할 수 있는지 알고싶습니다. 

NODE.h

#pragma once
#define _CRT_SECURE_NO_WARNINGS
#include<iostream>
#include<string>
#include<tchar.h>
#include<fstream>
#include<random>

using namespace std;

/*
설명 : 
	노드를 관리하는 클래스이다
*/

class NODE
{
	friend class DATA_MANAGER;
private:
	NODE* p_next;	//노드의 끝을 나타내는 변수
protected:
public:

	NODE();

	virtual ~NODE() {}

	NODE(const NODE&) = delete;
	NODE& operator= (const NODE&) = delete;

	virtual const char* const Get_Question() const { return nullptr; }
	virtual const char* const Get_Answer() const { return nullptr; }

};

NODE::NODE()
	: p_next(nullptr)
{}


DATA.h

#pragma once
#include"NODE.h"

/*
설명 : 
	노드 안에 들어가는 데이터클래스이다.

*/

class DATA : public NODE
{
private:
protected:
	char* question;		//문제
	char* answer;		//정답
public:

	DATA();

	virtual ~DATA();

	DATA(const char* p_question, const char* p_answer);			//문제 및 정답 할당

	virtual const char* const Get_Question() const { return question; }	//문제 주소값 리턴
	virtual const char* const Get_Answer() const { return answer; }		//정답 주소값 리턴

	//생성자 삭제 
	DATA(const DATA&) = delete;
	DATA& operator=(const DATA&) = delete;

	//할당해제함수 
	void Release();

};

/*
	DATA 기본생성자
*/
DATA::DATA()
	: question(nullptr), answer(nullptr)
{}

/*
	DATA 소멸자
*/
DATA::~DATA() {

	try
	{
		Release();
	}
	catch (...)
	{
		cout << "~DATA ERROR" << endl;
		abort();
	}

}

/*
설명 : 
	노드의 문제, 정답에 해당하는 문자열을 할당한다.

매개변수 : 
	const char* p_question	: 문제 문자열
	const char* p_answer	: 정답 문자열	 
*/
DATA::DATA(const char* p_question, const char* p_answer)
	: DATA()
{
	int question_size = strlen(p_question) + 1;
	int answer_size = strlen(p_answer) + 1;
	try {

		question = new char[question_size];
		answer = new char[answer_size];

		memset(question, 0, question_size);
		memset(answer, 0, answer_size);

		strcpy(question, p_question);
		strcpy(answer, p_answer);
	}
	catch (...)
	{
		cout << "DATA(cosnt char*,const char*)ERROR" << endl;
		abort();
	}
}

/*
설명 : 
	DATA클래스의 메모리를 해제하는 역할을 한다.

*/

void DATA::Release()
{
	if (question != nullptr)
	{
		delete question;
		question = nullptr;
	}
	if (answer != nullptr)
	{
		delete answer;
		question = nullptr;
	}
}



DATA_MANAGER.h

#pragma once
#include "NODE.h"
#include "DATA.h"



/*
설명 : 
	

*/


class DATA_MANAGER
{
private:
	NODE* header;	//노드의 헤더(dummy)
	int count;		//노드의 갯수
protected:

public:

	DATA_MANAGER();

	virtual ~DATA_MANAGER();

	//노드를 추가한다.
	void Add_Node(NODE* data);

	//노드 안에 있는 데이터 출력
	void Print_Data();

	//노드할당해제
	void Release();

};

DATA_MANAGER::DATA_MANAGER()
	: header(nullptr),
	count(0)
{
	header = new DATA("dummy", "dummy");//첫 헤드 생성
}


DATA_MANAGER::~DATA_MANAGER()
{
	try
	{
		Release();
	}
	catch (...)
	{
		cout << "Release Error" << endl;
		abort();

	}
}

/*
설명 : 
	노드를 추가한다. ITERATOR의 READ_DATA함수에서 값을 받아옴

매개변수 : 
	NODE* data : 문제, 정답에 해당하는 문자열을 할당한다.

*/

void DATA_MANAGER::Add_Node(NODE* data)			//노드 추가 
{
	try
	{
		count++;
		data->p_next = header->p_next;
		header->p_next = data;

	}
	catch (...)
	{
		count--;
		delete data;
		abort();
	}
}

/*
설명 : 
	중복 없는 난수를 배열에 넣은 후 그 난수만큼의 노드이동 후 
	데이터를 출력


*/
void DATA_MANAGER::Print_Data()
{
	//난수생성
	random_device rd;
	mt19937 gen(rd());
	uniform_int_distribution<int> dis(0, count - 1);

	//배열생성
	shared_ptr<NODE* []>node_arr(new NODE * [count], default_delete<NODE* []>());
	shared_ptr<int[]>num_arr(new int[count], default_delete<int[]>());

	char answer[256];

	//문제 출력
	for (int i = 0; i < count; i++)
	{
		int t1;
		t1 = num_arr[i] = dis(gen);
		for (int j = 0; j < i; j++)
		{
			if (num_arr[j] == num_arr[i])
				i--;
		}
	}

	for (int i = 0; i < count; i++)
	{
		NODE* node = header->p_next;
		for (int j = 0; j < num_arr[i]; j++)
		{
			node = node->p_next;
		}

		cout << "문제 : " << node->Get_Question() << endl;
		cin >> answer;				//정답쓰는건데 정답인지 아닌지 체크는안함

		cout << "정답 : " << node->Get_Answer() << endl << endl;
	}

}

//노드할당해제
void DATA_MANAGER::Release()
{
	NODE* node = header->p_next;
	NODE* delete_node = nullptr;

	while (node)
	{
		delete_node = node;
		node = node->p_next;
		delete delete_node;
	}
	header->p_next = nullptr;
	delete header;
}



#pragma once
#include "DATA_MANAGER.h"


/*
설명 : 
	UI_MANAGER 가 직접 DATA와 맞닫지않게 하기 위한 클래스

*/
class ITERATOR
{
private:
	DATA_MANAGER data_manager;
	enum { buf_size = 1024 };
protected:
public:
	ITERATOR() {};
	~ITERATOR() {};
	ITERATOR(const ITERATOR&) = delete;
	ITERATOR& operator=(const ITERATOR&) = delete;

	//파일읽는 함수
	void Read_Data();

	//문제 출제
	void Print_Question();

	//프로그램 사용법
	void exp() const;
};

/*
설명 : 
	파일로 부터 문제 및 정답을 받아옴
	문제당 개행 하나로 분류함

*/
void ITERATOR::Read_Data()
{
	ifstream file;
	char buf1[buf_size] = { 0, };
	char buf2[buf_size] = { 0, };
	cout << "파일 이름을 입력해주세요(ex : file.txt) : ";
	cin >> buf1;
	file.open(buf1);


	try
	{
		system("cls");
		if (file.is_open())								//파일이 열렸는가?
		{
			while (!file.eof()) {						//eof검사
				memset(buf1, 0, buf_size);
				memset(buf2, 0, buf_size);
				file.getline(buf1, buf_size);
				file.getline(buf2, buf_size);

				data_manager.Add_Node(new DATA(buf1, buf2));

				if (!file.eof())
					file.getline(buf1, buf_size);		//개행 제거
			}
			file.close();
		}
		else
		{
			cout << "입력하신 파일이 해당 경로에 없거나 여는데 실패하였습니다" << endl;
			return;
		}


	}
	catch (...)
	{
		cout << "Read_Data Error" << endl;
		abort();
	}
}

//랜덤된 데이터를 출력한다.
void ITERATOR::Print_Question()
{
	data_manager.Print_Data();
}


void ITERATOR::exp() const
{
	cout << "문제 랜덤 출력             -> 제작자 : morisummer2" << endl;

	cout << "=======================사용법=============================" << endl;
	cout << "txt파일에 문제와 정답을 각각 한줄에 써 넣은 후 한칸 띄고 또다른 문제 정답 입력 한다(각 문제당 개행으로 구분함)" << endl;
	cout << "예시: " << endl;

	cout << "문제1" << endl;
	cout << "문제1에 대한 정답   " << endl;
	cout << "                         <-개행" << endl;
	cout << "문제2" << endl;
	cout << "문제2에 대한 정답" << endl;
	cout << "                         <-개행" << endl;

}


UI_MANAGER.h

#pragma once
#include"ITERATOR.h"

/*
설명 : 
	사용자가 볼 수 있는 UI를 설정하는 클래스

*/
class UI_MANAGER
{

private:
	ITERATOR* iterator;
protected:
public:

	UI_MANAGER(ITERATOR& p_iterator);
	~UI_MANAGER() {}
	UI_MANAGER(const UI_MANAGER&) = delete;
	UI_MANAGER& operator=(const UI_MANAGER&) = delete;



	void Start() const;
};

UI_MANAGER::UI_MANAGER(ITERATOR& p_iterator)
{
	iterator = &p_iterator;
}

//시작 함수
void UI_MANAGER::Start() const
{
	iterator->exp();

	iterator->Read_Data();

	iterator->Print_Question();
}

main.c

#include"NODE.h"
#include"ITERATOR.h"
#include"UI_MANAGER.h"


int _tmain()
{

	ITERATOR iterator;
	UI_MANAGER ui(iterator);
	ui.Start();
	

}

혹시 몰라서 깃허브도 같이 올려놓겠습니다.

https://github.com/morisummer2/Random-question-program

0
  • 답변 4

  • Hide_D
    676
    2021-02-24 23:23:42 작성 2021-02-24 23:38:16 수정됨

    꼭 effective C++와는 관계 없어도 되겠지요?


    그러니 전 숙제를 잔뜩 쌓아두고 가겠습니다 ㅋㅋㅋ

    문법적인것, 로직적인것 섞여있으니 적당히 구분해서 보세요.

    (1) 스마트 포인터를 씁시다.

    unique_ptr, shared_ptr, weak_ptr 3종 세트!

    new만 보이고, delete는 없는 삶을 사는 것도 괜찮습니다. 요즘 언어는 다 그러니까요.

    (2) 크게 상관은 없지만 C++의 header는 hpp로 많이 쓰죠?

    (3-1) txt 파일 한줄에 최대 1024자까지로 정해져 있는데 만약 더 들어오면 어떻게 될까요?

    에러를 낸다면 어떻게 에러를 내야할까요? 1024로 안된다면 2048을 주면 될까요? 더 길어지면?

    (3-2) 보통 문제/답 구성에서 문제는 여러줄에 걸쳐있을 수도 있는데, 여러줄 입력을 받으려면 어떻게 해야할까요?

    (4) 아직 신경쓸 단계는 아닌데, 겸사겸사 대소문자 구분도 필요할 것 같네요. snake_case, camelCase, PascalCase 같은 것들을 어떤 경우에 쓰는가 같은것들요.

    (5) DATA 클래스에서 Release()를 호출하고, Add_Node()를 호출하면 버그가 발생하지 않을까요~

    Release후 Add_Node를 부르지 못하게 해야 할까요? 아니면 Release와 소멸자의 동작이 약간 달라야할까요?

    (6) if, loop의 깊이를 가능하면 얕게가는게 좋습니다.

    가령 Read_Data()함수에서 if(!file.is_open()) 으로 시작해서 빠르게 return;을 한다면, 파일 읽기 코드의 들여쓰기를 한 단계 낮출 수 있겠죠

    (7) List를 직접 만들어보셔서 잘 동작한다면 std::list를 써볼 차례입니다.

    코드를 보아선 std::list보단 std::vector가 더 어울릴것 같네요.

    아니면 std::list, std::vector를 쓰기 전에, '길이가 계속해서 늘어나는 배열'을 만들어보시는것도 좋겠네요.

    배열이 가득차면, 새 배열을 만들어서 복사하고, 이전 배열을 지우는거죠.

    (8) string, string&을 씁시다. 생각보다 훨씬 더 효율적이에요

  • 초보자아아
    30
    2021-02-25 09:47:32 작성 2021-02-25 15:36:14 수정됨

    Hide_D

    HIDE님 정말로 감사합니다.

    https://github.com/morisummer2/Random-question-program/blob/main/Hide%EB%8B%98%20%EC%88%99%EC%A0%9C.zip

    1. 전부 스마트 포인터로 변경하였습니다.

    2. 헤더파일을 전부다 .h에서 .hpp로 변경하였습니다.

    3-1. getline(cin,string)으로 \n을 받을때까지 받도록 했습니다.

    3-2. 이건 나중에 처리방법을 생각해서 적용시켜보도록하겠습니다.

    4. 어디에 적용해야 할 지 모르겠어요

    5. Release를 사용하지 않도록 전부다 스마트포인터로 변경했습니다.

    6. 죄송해요 잘 모르겠습니다.

    7. 죄송합니다 제가 list와 vector를 배우지 않아서 어떤건지 잘 모르겠습니다. 그래도 "길이가 계속해서 늘어나는 배열"은 방금 만들었습니다.

    8. 전부다 문자열과 관련된 것들은 string으로 바꿔보았습니다.


  • Hide_D
    676
    2021-02-25 23:33:43

    오.... 생각보다 많이 하셨네요.


    4. ㅎ..

    클래스 이름은 PascalCase,

    함수 이름/변수 이름은 snake_case나 camelCase 중 하나로 고정해서 쓰죠.

    함수쪽은 camelCase를 조금 더 많이 본것 같긴 하네요~

    UI_MANAGER 같은 이름이 보기에 살짝 불편했다는 이야기죠 ㅋㅋㅋㅋ....

    보통 UIManager 로 쓰지 않나? 하고..


    5. 무난하네요.

    지금 LinkedList가 단방향이라서 그나마 괜찮은데,

    head와 tail이 둘다 있는 구조라면 문제가 생길거에요.


    6. zip 파일의 ITERATOR.hpp 파일을 예로 들면 이런거죠

    system("cls");
    if (!file.is_open())								//파일이 열렸는가?
    {
    	cout << "입력하신 파일이 해당 경로에 없거나 여는데 실패하였습니다" << endl;
    	return;
    }
    while (!file.eof()) {						//eof검사
    	buf1.clear();
    	buf2.clear();
    
    	getline(file, buf1,'\n');
    	getline(file, buf2,'\n');
    
    	data_manager.Add_Node(make_shared<DATA>(buf1.c_str(), buf2.c_str()));
    
    	if (!file.eof()){
    		getline(file, buf1); //개행 제거
    	}
    }
    file.close();
    return;

    while (!file.eof()) { 로 시작하는 부분의 들여쓰기가 한 단 내려갔죠?

    에러 관련 처리를 위로 몰아줄 수 있고, 아래쪽 본문에 실제 작업이 적혀있으니까 읽기 편해져요.


    7.

    (1) 길이가 계속해서 늘어나는 배열에다가 두가지 더 넣어보세요.

    [1] operator []

    https://docs.microsoft.com/ko-kr/cpp/cpp/subscripting?view=msvc-160

    [2] 현재 배열의 크기

    이것까지 하면 진짜로 배열처럼 다룰 수 있게 됩니다.


    (2) 길이가 늘어나는 이 배열을 퀴즈 문제를 받는데 사용해보세요.

    LinkedList 대신 넣으면 나중에 랜덤으로 문제를 뽑을 때 변환을 거칠 필요가 없어지겠죠?


  • 초보자아아
    30
    2021-02-26 23:41:29 작성 2021-02-26 23:42:15 수정됨

    Hide_D

    이거 맞나요?

    #define _CRT_SECURE_NO_WARNINGS
    #include<iostream>
    #include<tchar.h>
    using namespace std;
    
    class MyString 
    {
    private:
    	shared_ptr<char>my_str;
    	int my_size;
    
    
    	
    public:
    	MyString() : my_str(nullptr),my_size(0){}
    	MyString(const char* param_str_size) { Set_String(param_str_size); }
    	
    	char& operator[](int index)
    	{
    		if (my_size < index)
    		{
    			int sub_size = my_size;
    			Re_Size(index);
    			shared_ptr<char>sub_str(new char[index+2]);
    			memset(sub_str.get(), ' ', index+2);
    			
    			for (int i = 0; i < sub_size; i++)
    				sub_str.get()[i] = my_str.get()[i];
    		
    			sub_str.get()[index+1] = '\0';
    		
    			
    			my_str.swap(sub_str);
    		}
    		
    		return my_str.get()[index];
    	}
    
    	const char* const Print_String() { return my_str.get(); }//문자열 반환
    	
    	void Set_String(const char* param_string)				//문자열 셋팅
    	{
    		int param_str_size = strlen(param_string);
    
    		Re_Size(param_str_size);
    
    		shared_ptr<char>sub_str(new char[param_str_size + 1]);
    		memset(sub_str.get(), 0,param_str_size+1);
    		strcpy(sub_str.get(), param_string);
    
    
    		my_str.swap(sub_str);
    
    	}
    	
    	void Append_String(const char* param_string)			//문자열 더하기
    	{
    		int param_str_size = strlen(param_string);
    		int sub_my_size = my_size;
    
    		Re_Size(param_str_size + my_size);
    
    		shared_ptr<char>sub_str(new char[my_size + 1]);
    		memset(sub_str.get(), 0, my_size + 1);
    		
    		strcpy(sub_str.get(), my_str.get());
    
    	
    		for (int i = sub_my_size; i < my_size; i++)
    			sub_str.get()[i] = param_string[i - sub_my_size ];
    
    		my_str.swap(sub_str);
    
    	}
    
    private:
    
    	void Re_Size(int index)								//문자열 크기 재 설정
    	{
    
    		shared_ptr<char>backup_str;
    		if (!my_size)
    		{
    			backup_str.reset(new char);
    			*backup_str.get() = '\0';
    		}
    		else
    		{
    			backup_str = my_str;
    		}
    
    		shared_ptr<char>sub_str(new char[index+1]);
    		memset(sub_str.get(), 0, index+1);
    		strcpy(sub_str.get(), backup_str.get());
    
    
    		my_str.swap(sub_str);
    		my_size = index;
    
    	}
    
    	
    
    };
    
    int _tmain() 
    {
    
    	MyString my_str;
    
    	my_str.Set_String("hello");
    
    	my_str.Append_String(" world");
    	my_str[40] = 'a';
    	cout << my_str.Print_String() << endl;
    	
    
    }


  • 로그인을 하시면 답변을 등록할 수 있습니다.