Refactor open date range to use std::optional #354

pull promag wants to merge 1 commits into bitcoin-core:master from promag:2021-06-date-range changing 3 files +19 −25

promag commented at 8:59 AM on June 3, 2021: contributor

Use std::nullopt for open date range instead of TransactionFilterProxy::MIN_DATE and TransactionFilterProxy::MAX_DATE.
hebasto added the label Refactoring on Jun 3, 2021
promag force-pushed on Jun 3, 2021
hebasto commented at 12:55 PM on June 9, 2021: member

Concept ACK
hebasto approved
hebasto commented at 6:17 PM on June 11, 2021: member

ACK a7837ba08bc10c9aaca6f28d9c2be9b8430323f5, I have reviewed the code and it looks OK, I agree it can be merged.
hebasto commented at 6:20 PM on June 11, 2021: member

nit: include <optional> to the src/qt/transactionview.cpp as std::nullopt are used there.
qt: Refactor open date range to use std::optional 4830f4912a
promag force-pushed on Jun 11, 2021
hebasto approved
hebasto commented at 10:52 AM on June 12, 2021: member

re-ACK 4830f4912a538600e9e9133442e9f8d929006630, only missed header included since my previous review.

in src/qt/transactionfilterproxy.cpp:42 in 4830f4912a

  38 | @@ -46,8 +39,8 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex &
  39 |          return false;
  40 |  
  41 |      QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime();
  42 | -    if (datetime < dateFrom || datetime > dateTo)
  43 | -        return false;
  44 | +    if (dateFrom && datetime < *dateFrom) return false;

Talkless commented at 1:04 PM on August 11, 2021:

std::optional does support comparison without dereferencing (see (26) in https://en.cppreference.com/w/cpp/utility/optional/operator_cmp) but your variant is better as operator< would repeat if (opt) check you already (rightly) did.

Talkless approved
Talkless commented at 1:05 PM on August 11, 2021: none

tACK 4830f4912a538600e9e9133442e9f8d929006630, tested on Debian Sid, filtering seems to work as expected.
hebasto merged this on Aug 11, 2021
hebasto closed this on Aug 11, 2021
sidhujag referenced this in commit 0428adb84e on Aug 15, 2021
bitcoin-core locked this on Aug 16, 2022

Contributors

Labels