Refactoring : JOIN, TOPTIC, PART

alsksssass commented 1 month ago

기존에 사용하던 전체 메시지 보내는 sendJoinmsg 를 SendMessageToChannel 메소드를 이용하여 전송하는 방법으로 수정계획

코드일원화 및 간결화 작업.

nicknamemohaji commented 1 month ago

다음은 제가 생각중인 리팩토링 대상들입니다. 중복되는 메소드를 분리하는 작업도 필요하지만, 일단은 제외하였습니다.

유틸리티 함수들을 분리

인스턴스와 관계없이 작동하는 메소드들이 몇 개 있습니다. 이들을 클래스에서 빼고 별도의 네임스페이스에서 관리하면 클래스를 간소화할 수 있습니다. 다음은 제가 생각중인 분리 대상 메소드입니다.

class IRCServer -> namespace IRCRequestParser
- std::vector<std::string> ParserSep -> SeperateParam
- bool RequestParse -> ParseMessage
- bool AddNewLineToBuffer -> AddNewLineToMessage
- std::string AddPrefixToChannelName -> AddPrefixToMessage
- std::string DelPrefixToChannelName -> DelPrefixFromMessage
class IRCContext -> namespace IRCRequestParser
- std::string ConvertCommandToStr -> ConvertComToStr
- enum IRCCommand ConvertStrToCommand -> ConvertStrToCom
- enum IRCCommand: 선언 위치를 IRCContext.hpp에서 IRCRequestParser.hpp로 변경
StringMatrix IRCServer::parseStringMatrix -> 삭제. 후술하겠습니다.
class IRCServer -> namespace IRCResponseCreator
- void ErrorSender -> SendError
- void RPL_... -> Send... (TOPIC, NINVITING 등)
- std::string MakeResponse -> MakeResponse

`StringMatrix` (`std::vector<std::vector<std::string>>`) 제거

RequestParser 메소드를 구현할 때 하나의 파라미터가 쪼개질 수 있다는 것은 알고 있었지만 벡터 안에 벡터를 담는 것은 좋지 않다고 생각해 std::vector<string>까지만 파싱했습니다. #22 에서 상훈님이 언급하신 것과 같이 각 파라미터를 쪼개도 길이가 다를 수 있으므로, 개별 파라미터를 보면서 길이를 확인하는게 맞다고 생각합니다.

컨테이너를 typedef하여 사용

std::find를 이용하여 특정 값을 찾거나 컨테이너 전체를 순회하는 경우가 많은데, 이 때 마다 std::map<std::string, IRCClient*>::iterator와 같이 쓰는 것은 읽기가 어렵습니다. 컨테이너 자료형은 모두 헤더에 typedef하면 가독성을 높일 수 있다고 생각합니다.

IRCContext 간소화

IRCContext는 원래 서버-채널-클라이언트로 순차적으로 처리될 때를 가정하여 설계하였습니다. 서버 콘텍스트 안에서 모든 동작을 처리하는 지금 구조에서는 작동중인 서버 - 대상 채널 - 요청한 사용자의 포인터를 모두 담고 있는 자료구조가 필요하지 않습니다. 클라이언트/채널의 정보를 담을 필요는 있지만, 이름만 들고 있어도 응답/요청 생성에는 충분합니다.

std::string rawMessage 삭제
IRCServer* server 삭제
IRCChannel* channel -> std::string channel로 변경
IRCClient* client -> std::string clientNick, std::string clientUser로 변경
bool createSource -> bool useClientSource로 변경

의견 부탁드립니다.

alsksssass commented 1 month ago

IRCServer::ActionAcceptClient(IRCContext& context) 메소드도 최적화가 필요해 보입니다 반복적인 부분은 함수나 메소드로 빼는 식으로 진행하면 좋을것 같습니다.

alsksssass commented 1 month ago

void IRCServer::ActionMOTD(IRCContext& context) 도 동일

nicknamemohaji commented 1 month ago

IRCServer::ActionAcceptClient(IRCContext& context) 메소드도 최적화가 필요해 보입니다 반복적인 부분은 함수나 메소드로 빼는 식으로 진행하면 좋을것 같습니다.

CAP, PASS, USER의 경우 한 번만 제공 가능하지만 NICK은 여러 번 변경할 수 있어서 코드가 좀 복잡합니다. 이부분도 설계 미스네요...

일단 IRCRequestParser, IRCResponseCreator 리팩토링이 끝나면 커맨드도 전체적으로 검토하겠습니다. ActionAccept에서는 NICK을 우선 분리하는 쪽으로 생각하고 있습니다.

void IRCServer::ActionMOTD(IRCContext& context) 도 동일

이 코드를 어떻게 고쳤으면 좋겠다고 생각하시나요?

nicknamemohaji commented 1 month ago

@alsksssass @somilee0202 전에 말씀드렸던 대로 에러 메시지를 MakeResponse쪽에서 작성하는 것은 문제가 많아서, 예외를 만들 필요가 있는 곳에서 직접 작성하는 쪽으로 변경중입니다.

추가적으로 Google C++ 스타일에 맞춰 에러 사용을 지양하기 위해 IRCError 클래스 자체를 삭제하는 쪽도 생각중입니다. 다만 IRCError 클래스 삭제는 실행흐름을 수정해야 되는 곳이 많아 나머지 코드들의 리뷰 + 리팩토링이 끝나면 할 생각입니다.

의견 부탁드립니다.

nicknamemohaji / ft_irc