Identified Untypical Design Decisions
We identified eight untypical design decisions in the system, each highlighting areas that vio- late best practices, introduce code smells, or needlessly increase the system’s complexity and maintenance effort.
Some DELETE methods, such as those in the ts-route-service, ts-admin-route-service, ts-assurance-service, and the ts-contact-service invoke repository.delete() before validating whether the object exists in the database. For example:
As a result, the service will consistently return a successful response, regardless of whether the specified object existed in the database. It is unclear whether this implementation was intentionally designed to guarantee the success of deletion. Nonetheless,
consistently returning a successful response for DELETE methods violates REST API best practices, while the unreachable code in the else-branch introduces a code smell that should be avoided.
A GET method within the ts-station-service appends the string "Not Exist" to the result list whenever no entity is found in the database:
As a result, the response may include a mix of valid station IDs and the string "Not Exist", which could lead to confusion and misinterpretation of the results. Furthermore, when no entities are found, the returned list could consist entirely of "Not Exist" elements. Representing the absence of data with a non-empty list is unintuitive and would be more clearly indicated by returning an empty list instead.
In the ts-user-service, users are uniquely identified by their usernames rather than their user IDs. This design allows multiple users to share the same user ID as long as their usernames differ, rendering the user ID non-unique and potentially redundant. Although usernames can serve as identifiers, relying solely on them could be problematic as usernames may change over time. Since user IDs are intended to be persistent, this violates best practices for managing user identifiers.
Some path variables for endpoints in the ts-inside-payment-service and ts-assurance-service are not utilized. Unused parameters are considered dead code, a code smell that should be avoided.
The implementation of some services in the system is duplicated. Specifically, the ts-travel-service and ts-travel2-service, the ts-order-service and ts-order-other-service, as well as the ts-preserve-service and ts-preserve-other-service share the same code and behavior. Such duplication does not affect the system’s functionality but introduces interdependencies between services, which increases the effort required for maintenance.
In the ts-contact-service, a method verifies whether a contact ID exists before creating and saving a new contact object to the database. However, the controller method responsible for invoking this check generates a new random ID beforehand. As a result, the code intended to return an unsuccessful response indicating that the contact object already exists becomes unreachable. Like D1, the system may have been designed to consistently return a successful response upon invoking the create method. However, the unreachable code represents a code smell that should be addressed.
The ts-admin-basic-info-service, ts-price-service, and the ts-station-service implement DELETE methods that take a request body rather than a path variable as input. However, including a request body in a DELETE request should be avoided, as the HTTP/1.1 specification does not define semantics for payloads in DELETE methods, potentially leading to unexpected behavior.
The ts-ticketinfo-service forwards its requests to the ts-basic-service without implementing any additional business logic. Designing such services should be avoided, as the ts-ticketinfo-service introduces complexity to the system’s architecture without providing apparent benefits.