Skip to content

Commit

Permalink
Fix some clang-tidy warnings - bugprone-unchecked-optional-access & p…
Browse files Browse the repository at this point in the history
…erformance-avoid-endl
  • Loading branch information
sjanel committed Oct 11, 2023
1 parent ca08a43 commit 4a120ff
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/api/common/src/exchangepublicapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ std::optional<Market> ExchangePublic::determineMarketFromMarketStr(std::string_v
break;
}
}
if (ret->quote().size() < kMinimalCryptoAcronymLen) {
if (!ret || ret->quote().size() < kMinimalCryptoAcronymLen) {
log::error("Cannot determine market for {}, skipping", marketStr);
ret = std::nullopt;
}
Expand Down
10 changes: 5 additions & 5 deletions src/api/common/test/exchangeprivateapi_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ TEST_F(ExchangePrivateTest, TakerTradeQuoteToBase) {
tradeBaseExpectCalls();

MonetaryAmount from(5000, market.quote());
MonetaryAmount pri(*marketOrderBook1.computeAvgPriceForTakerAmount(from));
MonetaryAmount pri(marketOrderBook1.computeAvgPriceForTakerAmount(from).value_or(MonetaryAmount{-1}));

MonetaryAmount vol(from / pri, market.base());
PriceOptions priceOptions(PriceStrategy::kTaker);
Expand All @@ -127,7 +127,7 @@ TEST_F(ExchangePrivateTest, TradeAsyncPolicyTaker) {
tradeBaseExpectCalls();

MonetaryAmount from(5000, market.quote());
MonetaryAmount pri(*marketOrderBook1.computeAvgPriceForTakerAmount(from));
MonetaryAmount pri(marketOrderBook1.computeAvgPriceForTakerAmount(from).value_or(MonetaryAmount{-1}));

MonetaryAmount vol(from / pri, market.base());
PriceOptions priceOptions(PriceStrategy::kTaker);
Expand Down Expand Up @@ -350,7 +350,7 @@ TEST_F(ExchangePrivateTest, MakerTradeQuoteToBaseEmergencyTakerTrade) {
// Place taker order
tradeInfo.options.switchToTakerStrategy();

MonetaryAmount pri2 = *marketOrderBook1.computeAvgPriceForTakerAmount(from);
MonetaryAmount pri2 = marketOrderBook1.computeAvgPriceForTakerAmount(from).value_or(MonetaryAmount{-1});
MonetaryAmount vol2(from / pri2, market.base());

PlaceOrderInfo matchedPlacedOrderInfo2(OrderInfo(TradedAmounts(from, vol2), true), OrderId("Order # 1"));
Expand Down Expand Up @@ -675,7 +675,7 @@ TEST_F(ExchangePrivateDustSweeperTest, DustSweeper2StepsSameMarket) {

expectTakerSell(from, pri, 0); // no selling possible

MonetaryAmount xrpDustThreshold = *dustThreshold(dustCur);
MonetaryAmount xrpDustThreshold = dustThreshold(dustCur).value_or(MonetaryAmount{-1});
TradedAmounts tradedAmounts1 = expectTakerBuy(xrpDustThreshold, xrpbtcAskPri, xrpbtcBidPri, xrpbtcMarket);

TradedAmounts tradedAmounts2 = expectTakerSell(from + tradedAmounts1.tradedTo, pri);
Expand Down Expand Up @@ -728,7 +728,7 @@ TEST_F(ExchangePrivateDustSweeperTest, DustSweeper5Steps) {
expectTakerSell(from, priBtc, 0); // no selling possible
expectTakerSell(from, priEur, 0); // no selling possible

MonetaryAmount xrpDustThreshold = *dustThreshold(dustCur);
MonetaryAmount xrpDustThreshold = dustThreshold(dustCur).value_or(MonetaryAmount{-1});
TradedAmounts tradedAmounts1 = expectTakerBuy(xrpDustThreshold, xrpbtcAskPri, xrpbtcBidPri, xrpbtcMarket);
from += tradedAmounts1.tradedTo;

Expand Down
18 changes: 8 additions & 10 deletions src/api/common/test/exchangepublicapi_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,7 @@ TEST_F(ExchangePublicConvertTest, ConvertImpossible) {
CurrencyCode toCurrency{"BTC"};
MarketsPath conversionPath;

std::optional<MonetaryAmount> ret =
exchangePublic.convert(from, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions);
ASSERT_FALSE(ret.has_value());
ASSERT_FALSE(exchangePublic.convert(from, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions));
}

TEST_F(ExchangePublicConvertTest, ConvertSimple) {
Expand All @@ -103,9 +101,9 @@ TEST_F(ExchangePublicConvertTest, ConvertSimple) {
std::optional<MonetaryAmount> ret =
exchangePublic.convert(from, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions);
ASSERT_TRUE(ret.has_value());
MonetaryAmount res = exchangePublic.exchangeInfo().applyFee(*marketOrderBook1.convert(from, priceOptions),
ExchangeInfo::FeeType::kMaker);
EXPECT_EQ(*ret, res);
MonetaryAmount res = exchangePublic.exchangeInfo().applyFee(
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeInfo::FeeType::kMaker);
EXPECT_EQ(ret, std::optional<MonetaryAmount>(res));
}

TEST_F(ExchangePublicConvertTest, ConvertDouble) {
Expand All @@ -115,11 +113,11 @@ TEST_F(ExchangePublicConvertTest, ConvertDouble) {
std::optional<MonetaryAmount> ret =
exchangePublic.convert(from, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions);
ASSERT_TRUE(ret.has_value());
MonetaryAmount res = exchangePublic.exchangeInfo().applyFee(*marketOrderBook1.convert(from, priceOptions),
ExchangeInfo::FeeType::kMaker);
res = exchangePublic.exchangeInfo().applyFee(*marketOrderBook2.convert(res, priceOptions),
MonetaryAmount res = exchangePublic.exchangeInfo().applyFee(
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeInfo::FeeType::kMaker);
res = exchangePublic.exchangeInfo().applyFee(marketOrderBook2.convert(res, priceOptions).value_or(MonetaryAmount{-1}),
ExchangeInfo::FeeType::kMaker);
EXPECT_EQ(*ret, res);
EXPECT_EQ(ret, std::optional<MonetaryAmount>(res));
}

} // namespace cct::api
2 changes: 1 addition & 1 deletion src/api/exchanges/src/huobipublicapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ std::pair<MarketSet, HuobiPublic::MarketsFunc::MarketInfoMap> HuobiPublic::Marke
// value = vol * pri in quote currency
MarketInfo marketInfo;

marketInfo.volAndPriNbDecimals = VolAndPriNbDecimals(volNbDec, priNbDec);
marketInfo.volAndPriNbDecimals = {volNbDec, priNbDec};

marketInfo.minOrderValue = MonetaryAmount(marketDetails["min-order-value"].get<double>(), quote);
auto maxOrderValueIt = marketDetails.find("max-order-value");
Expand Down
3 changes: 1 addition & 2 deletions src/api/exchanges/src/krakenpublicapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,7 @@ std::pair<MarketSet, KrakenPublic::MarketsFunc::MarketInfoMap> KrakenPublic::Mar
auto mkIt = ret.first.emplace(base, quote).first;
log::debug("Retrieved Kraken market {}", *mkIt);
MonetaryAmount orderMin(value["ordermin"].get<std::string_view>(), base);
ret.second.insert_or_assign(
*mkIt, MarketInfo{VolAndPriNbDecimals(value["lot_decimals"], value["pair_decimals"]), orderMin});
ret.second.insert_or_assign(*mkIt, MarketInfo{{value["lot_decimals"], value["pair_decimals"]}, orderMin});
}
log::info("Retrieved {} markets from Kraken", ret.first.size());
return ret;
Expand Down
2 changes: 1 addition & 1 deletion src/api/exchanges/test/commonapi_test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class TestAPI {
// for (const auto &c : currencies) {
// d[string(c.standardStr())] = exchangePrivateOpt->queryWithdrawalFee(c.standardCode()).amountStr();
// }
// std::cout << d.dump(2) << std::endl;
// std::cout << d.dump(2) << '\n';
// }
}

Expand Down
14 changes: 7 additions & 7 deletions src/engine/include/commandlineoptionsparser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,22 @@ class CommandLineOptionsParser {
}

void displayHelp(std::string_view programName, std::ostream& stream) const {
stream << "usage: " << programName << " <general options> <command(s)>" << std::endl;
stream << "usage: " << programName << " <general options> <command(s)>\n";
if (_opts.empty()) {
return;
}
stream << "Options:" << std::endl;
stream << "Options:\n";

const int lenTabRow = computeLenTabRow();
std::string_view previousGroup;
for (const auto& [opt, pm] : _opts) {
std::string_view currentGroup = opt.commandHeader().groupName();
if (currentGroup != previousGroup) {
stream << std::endl << ' ' << currentGroup << std::endl;
stream << '\n' << ' ' << currentGroup << '\n';
previousGroup = currentGroup;
}
if (opt.fullName()[0] != '-') {
stream << std::endl;
stream << '\n';
}

RowPrefix(opt, lenTabRow, stream);
Expand All @@ -109,14 +109,14 @@ class CommandLineOptionsParser {
auto breakPos = descr.find_first_of(kSpaceOrNewLine);
if (breakPos == std::string_view::npos) {
if (linePos + descr.size() > kMaxCharLine) {
stream << std::endl;
stream << '\n';
Spaces(lenTabRow, stream);
}
stream << descr << std::endl;
stream << descr << '\n';
break;
}
if (linePos + breakPos > kMaxCharLine) {
stream << std::endl;
stream << '\n';
Spaces(lenTabRow, stream);
linePos = lenTabRow;
}
Expand Down
8 changes: 4 additions & 4 deletions src/engine/src/coincenteroptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ bool CoincenterCmdLineOptions::isSmartTrade() const noexcept {
}

void CoincenterCmdLineOptions::PrintVersion(std::string_view programName) noexcept {
std::cout << programName << " version " << CCT_VERSION << std::endl;
std::cout << "compiled with " << CCT_COMPILER_VERSION << " on " << __DATE__ << " at " << __TIME__ << std::endl;
std::cout << " " << GetCurlVersionInfo() << std::endl;
std::cout << " " << ssl::GetOpenSSLVersion() << std::endl;
std::cout << programName << " version " << CCT_VERSION << '\n';
std::cout << "compiled with " << CCT_COMPILER_VERSION << " on " << __DATE__ << " at " << __TIME__ << '\n';
std::cout << " " << GetCurlVersionInfo() << '\n';
std::cout << " " << ssl::GetOpenSSLVersion() << '\n';
}

void CoincenterCmdLineOptions::mergeGlobalWith(const CoincenterCmdLineOptions& other) {
Expand Down
4 changes: 2 additions & 2 deletions src/engine/src/queryresultprinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,7 @@ void QueryResultPrinter::printTable(const SimpleTable &simpleTable) const {
os << simpleTable;

if (_pOs != nullptr) {
*_pOs << std::endl;
*_pOs << '\n';
} else {
// logger library automatically adds a newline as suffix
_outputLogger->info(ss.view());
Expand All @@ -1085,7 +1085,7 @@ void QueryResultPrinter::printTable(const SimpleTable &simpleTable) const {
void QueryResultPrinter::printJson(const json &jsonData) const {
string jsonStr = jsonData.dump();
if (_pOs != nullptr) {
*_pOs << jsonStr << std::endl;
*_pOs << jsonStr << '\n';
} else {
_outputLogger->info(jsonStr);
}
Expand Down
11 changes: 6 additions & 5 deletions src/engine/test/commandlineoptionsparser_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ TEST_F(CommandLineOptionsParserTest, OptStringViewEmpty) {
}

TEST_F(CommandLineOptionsParserTest, OptStringViewNotEmpty) {
EXPECT_EQ(*createOptions({"--optSV2", "I need to save the world"}).optSV,
std::string_view("I need to save the world"));
EXPECT_EQ(createOptions({"--optSV2", "I need to save the world"}).optSV,
std::optional<std::string_view>("I need to save the world"));
}

TEST_F(CommandLineOptionsParserTest, AlternativeOptionName) {
Expand All @@ -130,15 +130,16 @@ TEST_F(CommandLineOptionsParserTest, String) {
}

TEST_F(CommandLineOptionsParserTest, OptStringNotEmpty) {
EXPECT_EQ(*createOptions({"--opt4", "2000 EUR, kraken"}).optStr, "2000 EUR, kraken");
EXPECT_EQ(createOptions({"--opt4", "2000 EUR, kraken"}).optStr, std::optional<std::string_view>("2000 EUR, kraken"));
}

TEST_F(CommandLineOptionsParserTest, OptStringEmpty1) {
EXPECT_EQ(*createOptions({"--opt4", "--opt1", "Opt1 value"}).optStr, std::string_view());
EXPECT_EQ(createOptions({"--opt4", "--opt1", "Opt1 value"}).optStr,
std::optional<std::string_view>(std::string_view{}));
}

TEST_F(CommandLineOptionsParserTest, OptStringEmpty2) {
EXPECT_EQ(*createOptions({"--opt4"}).optStr, std::string_view());
EXPECT_EQ(createOptions({"--opt4"}).optStr, std::optional<std::string_view>(std::string_view{}));
}

TEST_F(CommandLineOptionsParserTest, OptStringEmpty3) { EXPECT_EQ(createOptions({"--help"}).optStr, std::nullopt); }
Expand Down
4 changes: 2 additions & 2 deletions src/objects/src/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ int File::write(const json& data, Writer::Mode mode) const {
try {
if (data.empty()) {
static constexpr std::string_view kEmptyJsonStr = "{}";
fileOfStream << kEmptyJsonStr << std::endl;
fileOfStream << kEmptyJsonStr << '\n';
return kEmptyJsonStr.length() + 1;
}
const int indent = mode == Writer::Mode::FromStart ? 2 : -1;
string outStr = data.dump(indent);
fileOfStream << outStr << std::endl;
fileOfStream << outStr << '\n';
return outStr.length() + 1;
} catch (const std::exception& e) {
if (_ifError == IfError::kThrow) {
Expand Down
10 changes: 7 additions & 3 deletions src/objects/src/marketorderbook.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ MarketOrderBook::MarketOrderBook(Market market, OrderBookLineSpan orderLines, Vo
// Just ignore empty lines
continue;
}
AmountPrice::AmountType amountIntegral = *orderBookLine._amount.amount(_volAndPriNbDecimals.volNbDecimals);
AmountPrice::AmountType priceIntegral = *orderBookLine._price.amount(_volAndPriNbDecimals.priNbDecimals);

_orders.emplace_back(amountIntegral, priceIntegral);
const auto amountIntegral = orderBookLine._amount.amount(_volAndPriNbDecimals.volNbDecimals);
const auto priceIntegral = orderBookLine._price.amount(_volAndPriNbDecimals.priNbDecimals);

assert(amountIntegral.has_value());
assert(priceIntegral.has_value());

_orders.emplace_back(*amountIntegral, *priceIntegral);
}

std::ranges::sort(_orders, [](AmountPrice lhs, AmountPrice rhs) { return lhs.price < rhs.price; });
Expand Down
11 changes: 6 additions & 5 deletions src/objects/test/monetaryamount_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ TEST(MonetaryAmountTest, TenDecimals) {
mamount1 = MonetaryAmount("0.0620089", btcCode);
EXPECT_NE(mamount1, 0);
EXPECT_EQ(mamount1.nbDecimals(), 7);
EXPECT_EQ(*MonetaryAmount("-314.451436574563", btcCode).amount(nbDecimals), -3144514365745);
EXPECT_EQ(MonetaryAmount("-314.451436574563", btcCode).amount(nbDecimals).value_or(-1), -3144514365745);
mamount1 = MonetaryAmount("-314.451436574563", btcCode);
EXPECT_EQ(mamount1.nbDecimals(), 12);

mamount1 = MonetaryAmount("2.0036500", btcCode);
EXPECT_EQ(*mamount1.amount(2), 200);
EXPECT_EQ(mamount1.amount(2).value_or(-1), 200);
EXPECT_EQ(mamount1.integerPart(), 2);
EXPECT_EQ(mamount1.nbDecimals(), 5);
}
Expand All @@ -73,9 +73,10 @@ TEST(MonetaryAmountTest, NoDecimals) {
MonetaryAmount mamount3(0, krwCode, nbDecimals);
EXPECT_EQ(mamount3.str(), "0 KRW");

EXPECT_EQ(*MonetaryAmount("0.620089", krwCode).amount(nbDecimals), 0);
EXPECT_EQ(*MonetaryAmount("-31415.0", krwCode).amount(nbDecimals), -31415);
EXPECT_EQ(*MonetaryAmount(3, krwCode).amount(nbDecimals), 3);
EXPECT_EQ(MonetaryAmount("0.620089", krwCode).amount(nbDecimals).value_or(-1), 0);
EXPECT_EQ(MonetaryAmount("-31415.0", krwCode).amount(nbDecimals).value_or(-1), -31415);

EXPECT_EQ(MonetaryAmount(3, krwCode).amount(nbDecimals).value_or(-1), 3);

EXPECT_EQ(MonetaryAmount("35.620089", krwCode).amount(18), std::nullopt);
}
Expand Down
8 changes: 4 additions & 4 deletions src/tech/src/simpletable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void SimpleTable::Row::print(std::ostream &os, std::span<const uint16_t> maxWidt
for (const Cell &cell : _cells) {
cell.print(os, maxWidthPerColumn[columnPos++]);
}
os << std::endl;
os << '\n';
}

SimpleTable::MaxWidthPerColumnVector SimpleTable::computeMaxWidthPerColumn() const {
Expand Down Expand Up @@ -112,17 +112,17 @@ std::ostream &operator<<(std::ostream &os, const SimpleTable &table) {
const auto maxWidthPerColumnVector = table.computeMaxWidthPerColumn();
const auto lineSep = SimpleTable::ComputeLineSep(maxWidthPerColumnVector);

os << lineSep << std::endl;
os << lineSep << '\n';

bool printHeader = table._rows.size() > 1U;
for (const auto &row : table._rows) {
if (row.isDivider()) {
os << lineSep << std::endl;
os << lineSep << '\n';
} else {
row.print(os, maxWidthPerColumnVector);
}
if (printHeader) {
os << lineSep << std::endl;
os << lineSep << '\n';
printHeader = false;
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/tech/test/flatkeyvaluestring_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ TEST(FlatKeyValueStringTest, WithNullTerminatingCharAsSeparator) {
case 2:
ASSERT_STREQ(kvPairPtr, "&newField:&&newValue&&");
break;
default:
ASSERT_TRUE(false);
}
}
EXPECT_EQ(kvPairPos, 3);
Expand All @@ -125,9 +127,6 @@ TEST(FlatKeyValueStringTest, EmptyConvertToJson) { EXPECT_EQ(KvPairs().toJson(),

class CurlOptionsCase1 : public ::testing::Test {
protected:
void SetUp() override {}
void TearDown() override {}

KvPairs kvPairs{{"units", "0.11176"}, {"price", "357.78"}, {"777", "encoredutravail?"},
{"hola", "quetal"}, {"array1", "val1,,"}, {"array2", ",val1,val2,value,"},
{"emptyArray", ","}};
Expand Down Expand Up @@ -177,6 +176,8 @@ TEST_F(CurlOptionsCase1, Iterator) {
EXPECT_EQ(key, "emptyArray");
EXPECT_EQ(val, ",");
break;
default:
EXPECT_TRUE(false);
}
}
EXPECT_EQ(itPos, 7);
Expand Down

0 comments on commit 4a120ff

Please sign in to comment.