OrderBook

In the function `getOrderDetailsString(uint256 _orderId)`, there is a redundant and logically inconsistent status assignment block which could lead to confusion and misinterpretation of the order's actual state when debugging, displaying, or logging order details.

​

Problematic Section:

​

```javascript

string memory status = order.isActive

? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")

: "Inactive (Filled/Cancelled)";

if (order.isActive && block.timestamp >= order.deadlineTimestamp) {

status = "Expired (Awaiting Cancellation)";

} else if (!order.isActive) {

status = "Inactive (Filled/Cancelled)";

} else {

status = "Active";

}

```

This block redundantly assigns the status variable:

​

- First with a nested ternary.

​

- Then again using a full if-else block with overlapping logic.

​

Effectively, the first assignment is overwritten entirely by the second block, rendering it useless.

​

​

Although this issue is low severity, it introduces:

​

1. Code Confusion: Makes the function harder to maintain and understand due to logically conflicting branches and duplication.

​

2. Inconsistent Behavior: If the logic in either block is updated in the future and not the other, it may lead to misleading status reporting.

​

3. Developer Misinterpretation: A developer reading or modifying this function might mistakenly assume both parts of the logic are used.

​

This is not a runtime-breaking bug, but it violates clean code principles and may result in misreporting order status if changes are made later.

​

Call the `getOrderDetailsString()` function for an expired but still active order:

​

```javascript

vm.warp(block.timestamp + 4 days); // Order created with 3-day deadline

​

string memory result = orderBook.getOrderDetailsString(orderId);

```

​

Despite this correct output, the original ternary assignment was:

​

```javascript

​

order.isActive

? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")

: "Inactive (Filled/Cancelled)"

```

That ternary result is immediately overwritten, meaning:

​

- The ternary logic is redundant.

​

- Updates in the ternary logic have zero effect on output.

​

​

​

Refactor the logic to a single clean block for status assignment.

​

```diff

if (order.isActive && block.timestamp >= order.deadlineTimestamp) {

status = "Expired (Awaiting Cancellation)";

} else if (!order.isActive) {

status = "Inactive (Filled/Cancelled)";

} else {

status = "Active";

}

​

```

​