-
Notifications
You must be signed in to change notification settings - Fork 1
feat: record post form 추가 #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d0422
commented
Aug 31, 2024
- Record post 폼추가
- mock mutation함수 추가
- TextField multiline props추가
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
const validate = () => { | ||
return title.length > 0 && images.length > 0; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const lengthValidate = (...args: string[]) => {
for (const arg of args) {
if (arg.length === 0) return false;
}
return true;
}
이런 validate 함수들은 이런 식으로 별도의 함수로 만들어도 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array 여러개를 받아서 확인한다는게, 유효성 확인
이라는 관점에서 봤을때, 범용성이 있다고 보기는 조금 어려울 것 같아요(유효성 검증은 array만 하는 것이 아닐 수 있으므로)
같은 부분을 제거하려면 실제 구현부를 아래와 같이 바꾸는 게 더 좋을 것 같다는 생각입니다...!
[title, images].every((arr) => arr.length > 0);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array를 여러 개 받는다는게 무슨 뜻인지는 모르겠지만(?) 위 함수의 사용 예제는 아래와 같습니다.
lengthValidate('arg1', 'arg2', ''); // false
내부적으로는 for...of
보다는 언급해주신 every
가 더 나을 것 같습니다.
const lengthValidate = (...args: string[]) => {
return args.every((arg) => arg.length > 0);
}
또한, 특정 타입(string)의 유효성 확인에 관한 내용이므로 이름을 좀 더 명확하게 한다면 validateStringLength
도 가능할 것 같네요.
const validateStringLength = (length: number, ...args: string[]) => {
return args.every((arg) => arg.length > length);
}
validateStringLength(3, 'test', 'test2'); // true
validateStringLength(5, 'test', 'test2'); // false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 lengthValidate의 역할을 이터러블 객체의 길이 유효성을 검사하는 것으로 이해하였는데요,
그러나 실제 함수의 역할은 다수의 이터러블 객체를 받아 해당 객체의 length를 검사하는 것으로 보입니다.
그렇다면 단일한 객체의 유효성 검사가 아니기에 제안해주신 함수가 공통 util이 되려면 아래와 같은 부분이 조금 더 반영되어야 한다고 생각합니다. 아니라면 해당 함수를 사용하기 애매한 상황이 올 수 있고, 함수 변경으로 이어질 수 있을 것 같아요.
- iterableListLengthValidate로 이름을 변경
- lengthValidate 보다는 isEmptyArray와 같은 더 명확한 역할을 가진 함수로 변경하여 validate 함수에서 해당 함수를 호출하여 검사
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래와 같은 예외가 있을 수 있다는 것을 제가 놓쳤네요.
validateStringLength(3, [1, 2, 3, 4]); // true