Skip to content

Conversation

@fs6-kde
Copy link

@fs6-kde fs6-kde commented Jun 22, 2025

요구사항

기본

  • [x]
  • []
  • []

심화

  • [x]
  • []

주요 변경사항

스크린샷

image

멘토에게

  • 상품 댓글 부분은 기능수정을 다시 하느라 ts로 변환시키지 못했습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

createdAt DateTime @default(now())

@@unique([userId, productId])
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 잘 설계된 Prisma 스키마입니다.

다만, “조금 더 정규화”라는 관점에서 보면 실무 확장성 및 데이터 무결성 측면에서 약간의 개선 여지가 있는 지점들이 있습니다. 아래에 제안드릴게요.


✅ 개선 제안 요약

항목 현재 상태 개선 제안 이유
User.nickName @unique만 있음 nullable 아님은 좋음. 단, 실무에서는 닉네임 중복 허용할 수 있도록 soft unique 구조 고려 가능 실무에서는 닉네임 중복 허용 또는 표시이름 구조로 분리하는 경우도 많음
Product.tags String[] 배열 Tag 모델로 분리 후 Many-to-Many 관계 추천 정규화 관점에서 배열은 검색·조인·통계에서 불리
Product.images String[] 배열 별도 ProductImage 모델 추천 이미지 순서, 타입, 썸네일 여부 등 메타데이터 추가 용이
Favorite 괜찮음 👍 잘 설계됨 (복합 unique, cascade, 시간 기록 등)  
ProductComment 괜찮음 댓글에 parentId로 대댓글 구조 추가 고려 가능 실시간 커뮤니티화 고려 시 확장성↑

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔁 예시: Tag, ProductImage 정규화 예

🧩 Tag 모델 추가


model Tag {
  id       Int       @id @default(autoincrement())
  name     String    @unique
  products Product[] @relation("ProductTags", references: [id])
}

model ProductTag {
  product   Product @relation(fields: [productId], references: [id])
  productId Int
  tag       Tag     @relation(fields: [tagId], references: [id])
  tagId     Int

  @@id([productId, tagId])
}

🖼 ProductImage 모델 분리

model ProductImage {
  id        Int     @id @default(autoincrement())
  url       String
  order     Int     @default(0)
  product   Product @relation(fields: [productId], references: [id], onDelete: Cascade)
  productId Int
}


📌 정리하면

  • 지금 모델은 일반 CRUD 백엔드로서는 충분히 잘 구성되어 있음

  • 하지만 검색/통계/필터링이 필요한 실무에서는

    • tags, images → 정규화 필요
    • comments → depth 구조 필요 가능
  • 정규화를 너무 일찍 하면 오버엔지니어링이 될 수 있으므로,

    정규화는 "검색·정렬·확장성" 요구가 생겼을 때 적용하는 것이 가장 효율적입니다

}
}
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 코드가 잘 구조화되어 있고, 명확한 흐름과 에러 처리를 갖추고 있습니다. 다만, 코드의 반복성·가독성·확장성 측면에서 리팩토링 여지가 있습니다. 아래에 중첩 해소 및 역할 분리를 중심으로 리팩토링 가이드를 제시드립니다.


✅ 리팩토링 포인트 요약

대상 제안
  1. 타입 중복 | Request body 관련 type ProductInput, type ProductUpdateInput 등 명확한 DTO 정의
  2. 태그/이미지 파싱 반복 | `parseArrayField(input: string[]
  3. 인증 체크 중복 | getUserIdOrThrow(req) 유틸 함수로 압축
  4. 서비스 호출 직전 객체 조립 중복 | buildProductData(req) 형태로 공통 로직 분리

}
);

export default productController;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📦 utils 예시

  • 검증은 이렇게 따로 분리하는걸 추천드려요.
export function parseArrayField(
  input: string[] | string | undefined,
  label: string
): string[] {
  if (Array.isArray(input)) return input;
  if (typeof input === "string") {
    try {
      const parsed = JSON.parse(input);
      if (!Array.isArray(parsed)) throw new Error();
      return parsed;
    } catch {
      throw new ValidationError(`${label} 형식 오류`);
    }
  }
  if (input !== undefined) throw new ValidationError(`${label} 형식 오류`);
  return [];
}

export function getUserIdOrThrow(req: { auth?: { userId?: number } }) {
  const userId = req.auth?.userId;
  if (!userId) throw new AuthenticationError("인증 정보가 없습니다.");
  return userId;
}

✨ productController 리팩토링 예시 (등록/수정 일부만)

productController.post("/", auth.verifyAccessToken, async (req, res, next) => {
  try {
    const userId = getUserIdOrThrow(req);

    const { name, description } = req.body;
    const price = Number(req.body.price);
    if (isNaN(price)) throw new ValidationError("가격은 숫자여야 합니다.");

    const tags = parseArrayField(req.body.tags, "태그");
    const images = parseArrayField(req.body.images, "이미지");

    const newProduct = await productService.createProduct(
      { name, description, price, tags, images },
      userId
    );

    res.status(201).json(newProduct);
  } catch (err) {
    next(err);
  }
});

productController.patch("/:id", auth.verifyAccessToken, async (req, res, next) => {
  try {
    const userId = getUserIdOrThrow(req);
    const id = Number(req.params.id);

    const existing = parseArrayField(req.body.existingImages, "기존 이미지");
    const newImages = parseArrayField(req.body.newImagePaths, "새 이미지");
    const tags = parseArrayField(req.body.tags, "태그");

    const updated = await productService.updateProduct(id, {
      name: req.body.name,
      description: req.body.description,
      price: Number(req.body.price),
      tags,
      images: [...existing, ...newImages],
      ownerId: userId,
    });

    res.json(updated);
  } catch (err) {
    next(err);
  }
});

🧠 추가적으로 고려 가능한 구조 개선

  • middlewares/validate.ts: express-validator 또는 zod 기반 스키마 유효성 검증
  • dto/ProductDTO.ts: 타입 정의와 파서 함수 묶기
  • middleware/auth.ts 내부에서 userId를 req에 강제 주입하면 타입 좁히기 가능

📌 결론

  • 지금 구조도 좋지만, 파싱/유효성 검사/에러핸들링 중복 제거만 해도 코드 가독성이 크게 향상됩니다.
  • 이 코드는 NestJS 전환 시 DTO 구조화 및 Pipe 변환에 자연스럽게 이어지는 방향입니다.

console.error("[Unknown Error]", err);
res.status(500).json({ message: "서버 내부 오류가 발생했습니다." });
});

Copy link
Contributor

@loquemedalagana loquemedalagana Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err: any를 없애고 싶을 때, 타입 세분화 없이도 안전하게 핸들링하는 방법은 있습니다.

다만 Express의 error-handling middleware는 기본적으로 err 타입을 알 수 없도록 설계되어 있기 때문에,

타입을 명시하려면 범용 오류 타입을 활용하거나 커스텀 타입 단언이 필요합니다.


✅ 현실적인 대안 1: unknown 타입 + 타입 가드 사용

app.use((err: unknown, req: Request, res: Response, next: NextFunction): void => {
  if (err instanceof UnauthorizedError) {
    console.error("JWT 인증 오류:", err.message);
    res.status(401).json({ message: "인증이 유효하지 않습니다." });
    return;
  }

  next(err);
});

app.use((err: unknown, req: Request, res: Response, next: NextFunction): void => {
  if (err instanceof AppError) {
    console.error(`[AppError] ${err.name}:`, err.message);
    res.status(err.code || 500).json({ message: err.message, data: err.data });
    return;
  }

  console.error("[Unknown Error]", err);
  res.status(500).json({ message: "서버 내부 오류가 발생했습니다." });
});

unknown은 타입 안정성이 높고, instanceof, typeof, 커스텀 isAppError() 등의 타입 가드와 함께 쓰기 좋습니다.


✅ 대안 2: 커스텀 타입 좁히기 (AppError 등 구조화된 에러만 받는 경우)

type ExpressError = Error & Partial<AppError> & Partial<UnauthorizedError>;

app.use((err: ExpressError, req: Request, res: Response, next: NextFunction): void => {
  if ("status" in err && err.name === "UnauthorizedError") {
    res.status(401).json({ message: "인증이 유효하지 않습니다." });
    return;
  }

  if (err instanceof AppError) {
    res.status(err.code || 500).json({ message: err.message });
    return;
  }

  res.status(500).json({ message: "서버 내부 오류가 발생했습니다." });
});

단점: ExpressError는 모든 가능한 에러 필드를 union으로 뭉치기 때문에 타입 안정성이 떨어질 수 있습니다.

→ 그래도 any보단 낫고, 실무에서 커버할 범위가 명확할 경우 유용합니다.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 결론 및 추천

방법 추천도 이유
err: any ❌ 지양 타입 안전성 없음
err: unknown + instanceof ✅ 권장 타입스크립트의 정석적인 오류 핸들링 방식
ExpressError 타입 좁히기 ⚠️ 보완용 특정 프레임워크에 의존적인 구조라면 유효

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📌 보너스: 타입 가드 함수 예시


function isAppError(err: unknown): err is AppError {
  return typeof err === "object" && err !== null && "code" in err;
}

이걸 활용하면 아래처럼 작성도 가능:


if (isAppError(err)) {
  res.status(err.code).json({ message: err. Message });
}

@loquemedalagana
Copy link
Contributor

Generic 부분 조금 더 연구해보세요!

@loquemedalagana loquemedalagana merged commit 7c0a667 into codeit-sprint-fullstack:express-김다은 Jun 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants