softwareconstruction240 / softwareconstruction

Content for BYU CS 240: Advanced Software Construction
MIT License
45 stars 39 forks source link

Simplify Phase 0 Starter Code & Instruction #145

Open frozenfrank opened 1 month ago

frozenfrank commented 1 month ago

Overview

Currently, the ChessGame spec details are duplicated verbatim on both the Phase 0 and Phase 1 spec pages. The information is not necessary in Phase 0, but it is required in Phase 1.

There is already the following comment in the Phase 0 specs that describes ChessGame as such:

You will not need to implement any code in this class for this phase.

However, some confusion arises from students thinking they need to work in the ChessGame class simply because it is included in the spec. Some students are confused by seeing the ChessGame class in the working directory, but no corresponding ChessGameTests in the test directory.

There is no need to provide detailed ChessGame information until Phase 1.

Current State

The ChessGame class is currently included in the Phase 0 starter code because it contains the TeamColor enum which defines the WHITE and BLACK color constants. There was apparently already a discussion between ChessGame and ChessPiece about where this enum best lives, and it evidently ended up in the ChessGame class.

The Chess Game section in the specs may be provided to give an overview of what is to come, and help students understand how the current code fits in with the upcoming code.

Proposal

Simplify the Phase 0 process by not including the ChessGame code files in the initial working directory, and simplifying the presentation of it in the specs. Move ChessGame into the starter-code folder, and refactor TeamColor to live inside ChessPiece instead.

Argument

The specs can shows the ChessGame's role in the final product without showing the details of how it works. The specs can indicate that ChessGame will exist and hold the completely valid movement rules (whereas the ChessPiece rules ignore some important aspects of chess gameplay) while removing the implementation information about the method calls.

The TeamColor enum can also reasonably be placed inside the ChessPiece class. Even if there is a marginally better reason to place it in the ChessGame class for holistic reasons, for Phase 0 and the Programming Exam together, it streamlines the process to remove a dependency on the ChessGame class.

Next Steps

Review

This proposal has been peer reviewed by @Truthwatcher64.

Approval from the professors would be good before starting down the path @leesjensen @jerodw.

A code organization opinion would be appreciated from @19mdavenport.

leesjensen commented 1 month ago

I agree that moving the duplicated documentation for ChessGame out of Phase 0 would clarify things. However, I would probably stop there. I don't think we want to create the "perfect" chess code. In fact I think it is really valuable to have things that are "wrong" in the code so that we can discuss them as a class.

Should TeamColor be in the game, in the piece, or as a standalone enum? Should it even be called TeamColor? Notice that the we use ChessGame.TeamColor pieceColor does that indicate a smell that one of the two is named wrong? Should the enum be PieceColor? Should it be something more generic such as just Color? What does the domain say about this? What is the most natural fit for the Domain? What violates POLA?

jerodw commented 3 weeks ago

Sorry I'm joining the discussion pretty late. I also support moving the ChessGame documentation out of the phase-0 spec. I support moving TeamColor if that helps separate the phase-0 and phase-1 specs.