From e2010c4a4b8f9da425e936c5617ea17dee61193c Mon Sep 17 00:00:00 2001 From: Brett Waldbaum Date: Fri, 8 Mar 2019 14:36:52 -0800 Subject: [PATCH] PR feedback from Daniel --- src/CalcViewModel/Common/TraceLogger.cpp | 3 +- src/CalcViewModel/Common/TraceLogger.h | 2 +- src/Calculator/Views/DateCalculator.xaml.cpp | 4 +- src/CalculatorUnitTests/HistoryTests.cpp | 60 ++++++++++---------- 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/CalcViewModel/Common/TraceLogger.cpp b/src/CalcViewModel/Common/TraceLogger.cpp index 33e85437..087b42dc 100644 --- a/src/CalcViewModel/Common/TraceLogger.cpp +++ b/src/CalcViewModel/Common/TraceLogger.cpp @@ -867,11 +867,12 @@ namespace CalculatorApp } } - void TraceLogger::LogDateDifferenceModeUsed() + void TraceLogger::LogDateDifferenceModeUsed(int windowId) { if (!m_dateDiffUsageLoggedInSession) { LoggingFields fields{}; + fields.AddUInt32(L"WindowId", windowId); LogTelemetryEvent(EVENT_NAME_DATE_DIFFERENCE_USED, fields); m_dateDiffUsageLoggedInSession = true; diff --git a/src/CalcViewModel/Common/TraceLogger.h b/src/CalcViewModel/Common/TraceLogger.h index e9ad412f..e3b50ef6 100644 --- a/src/CalcViewModel/Common/TraceLogger.h +++ b/src/CalcViewModel/Common/TraceLogger.h @@ -91,7 +91,7 @@ namespace CalculatorApp void LogCoreWindowWasNull() const; // Trace methods for Date Calculator usage - void LogDateDifferenceModeUsed(); + void LogDateDifferenceModeUsed(int windowId); void LogDateAddSubtractModeUsed(int windowId, bool isAddMode); void LogDateClippedTimeDifferenceFound(winrt::Windows::Globalization::Calendar const& today, winrt::Windows::Foundation::DateTime const& clippedTime) const; diff --git a/src/Calculator/Views/DateCalculator.xaml.cpp b/src/Calculator/Views/DateCalculator.xaml.cpp index 42fdf915..0df8cd3e 100644 --- a/src/Calculator/Views/DateCalculator.xaml.cpp +++ b/src/Calculator/Views/DateCalculator.xaml.cpp @@ -102,7 +102,7 @@ void DateCalculator::FromDate_DateChanged(_In_ CalendarDatePicker^ sender, _In_ { auto dateCalcViewModel = safe_cast(this->DataContext); dateCalcViewModel->FromDate = e->NewDate->Value; - TraceLogger::GetInstance().LogDateDifferenceModeUsed(); + TraceLogger::GetInstance().LogDateDifferenceModeUsed(ApplicationView::GetApplicationViewIdForWindow(CoreWindow::GetForCurrentThread())); } else { @@ -116,7 +116,7 @@ void DateCalculator::ToDate_DateChanged(_In_ CalendarDatePicker^ sender, _In_ Ca { auto dateCalcViewModel = safe_cast(this->DataContext); dateCalcViewModel->ToDate = e->NewDate->Value; - TraceLogger::GetInstance().LogDateDifferenceModeUsed(); + TraceLogger::GetInstance().LogDateDifferenceModeUsed(ApplicationView::GetApplicationViewIdForWindow(CoreWindow::GetForCurrentThread())); } else { diff --git a/src/CalculatorUnitTests/HistoryTests.cpp b/src/CalculatorUnitTests/HistoryTests.cpp index a22ae83a..f2edd376 100644 --- a/src/CalculatorUnitTests/HistoryTests.cpp +++ b/src/CalculatorUnitTests/HistoryTests.cpp @@ -42,7 +42,7 @@ namespace CalculatorFunctionalTests HistoryViewModel^ m_historyViewModel; StandardCalculatorViewModel^ m_standardViewModel; - void Initialize(unsigned int /*windowId*/ = 0) + void Initialize() { m_standardViewModel = ref new StandardCalculatorViewModel(); m_standardViewModel->IsStandard = true; @@ -50,7 +50,7 @@ namespace CalculatorFunctionalTests m_historyViewModel->SetCalculatorDisplay(m_standardViewModel->m_calculatorDisplay); } - void Cleanup(unsigned int /*windowId*/ = 0) + void Cleanup() { m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeBasic); m_historyViewModel->OnClearCommand(nullptr); @@ -81,7 +81,7 @@ namespace CalculatorFunctionalTests void AddSingleHistoryItem(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); int initialSize = m_historyViewModel->ItemSize; m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); @@ -94,12 +94,12 @@ namespace CalculatorFunctionalTests VERIFY_ARE_EQUAL(initialSize + 1, sizeAfterItemAdd); VERIFY_ARE_EQUAL(expression, StringReference(historyItem->historyItemVector.expression.c_str())); VERIFY_ARE_EQUAL(result, StringReference(historyItem->historyItemVector.result.c_str())); - Cleanup(windowId); + Cleanup(); } void AddMaxHistoryItems(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); @@ -129,12 +129,12 @@ namespace CalculatorFunctionalTests historyItem = m_standardViewModel->m_standardCalculatorManager.GetHistoryItem(0); VERIFY_ARE_EQUAL(expression, StringReference(historyItem->historyItemVector.expression.c_str())); VERIFY_ARE_EQUAL(result, StringReference(historyItem->historyItemVector.result.c_str())); - Cleanup(windowId); + Cleanup(); } void ReloadHistory(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.Reset(); int scientificItems = 5; @@ -186,12 +186,12 @@ namespace CalculatorFunctionalTests VERIFY_ARE_EQUAL(expr, historyItem->historyItemVector.expression); VERIFY_ARE_EQUAL(result, StringReference(historyItem->historyItemVector.result.c_str())); } - Cleanup(windowId); + Cleanup(); } void ClearHistory(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); @@ -206,12 +206,12 @@ namespace CalculatorFunctionalTests VERIFY_ARE_EQUAL(0, m_historyViewModel->ItemSize); VERIFY_IS_TRUE(IsHistoryContainerEmpty(GetHistoryContainerKeyHelper(CM_STD))); VERIFY_IS_TRUE(IsHistoryContainerEmpty(GetHistoryContainerKeyHelper(CM_SCI))); - Cleanup(windowId); + Cleanup(); } void SerializeDeSerializeHistoryItem(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); @@ -222,12 +222,12 @@ namespace CalculatorFunctionalTests m_historyViewModel->ReloadHistory(ViewMode::Scientific); auto itemAfterSerializeDeserialize = m_standardViewModel->m_standardCalculatorManager.GetHistoryItem(0); VERIFY_IS_TRUE((itemBeforeSerializeDeserialize->historyItemVector.expression == itemAfterSerializeDeserialize->historyItemVector.expression) && (itemBeforeSerializeDeserialize->historyItemVector.result == itemAfterSerializeDeserialize->historyItemVector.result) && (itemBeforeSerializeDeserialize->historyItemVector.spCommands == itemAfterSerializeDeserialize->historyItemVector.spCommands) && (itemBeforeSerializeDeserialize->historyItemVector.spTokens == itemAfterSerializeDeserialize->historyItemVector.spTokens)); - Cleanup(windowId); + Cleanup(); } void SaveAndReloadHistory(unsigned int windowid = 0) { - Initialize(windowid); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); @@ -257,12 +257,12 @@ namespace CalculatorFunctionalTests VERIFY_ARE_EQUAL(result, StringReference(historyItem->historyItemVector.result.c_str())); VERIFY_ARE_NOT_EQUAL(itemsBeforeSaveAndReload, itemsAfterSaveAndReload); VERIFY_ARE_EQUAL(itemsBeforeSaveAndReload, itemsAfterSaveAndReload + 1); - Cleanup(windowid); + Cleanup(); } void HistoryItemWithPrettyExpressions(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); Command commands[] = { Command::CommandSIN, Command::CommandCOS, Command::CommandTAN, Command::CommandASIN, Command::CommandACOS, Command::CommandATAN }; Command mode[] = { Command::CommandDEG, Command::CommandRAD, Command::CommandGRAD }; int modes = sizeof(mode) / sizeof(Command); @@ -289,12 +289,12 @@ namespace CalculatorFunctionalTests itemIndex++; } } - Cleanup(windowId); + Cleanup(); } void HistoryItemWithPrettyExpressionsMixedRadix(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); ResourceLoader^ m_uiResourceLoader = ResourceLoader::GetForViewIndependentUse(L"CEngineStrings"); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandDEG); @@ -320,12 +320,12 @@ namespace CalculatorFunctionalTests expr = UtfUtils::LRO + expr + UtfUtils::PDF; VERIFY_ARE_EQUAL(historyItem->historyItemVector.expression, expr); - Cleanup(windowId); + Cleanup(); } void HistoryItemClicked(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); @@ -347,12 +347,12 @@ namespace CalculatorFunctionalTests VERIFY_ARE_EQUAL(StringReference(L" "), m_standardViewModel->ExpressionTokens->GetAt(5)->Token); VERIFY_ARE_EQUAL(StringReference(L"+"), m_standardViewModel->ExpressionTokens->GetAt(6)->Token); VERIFY_ARE_EQUAL(StringReference(L" "), m_standardViewModel->ExpressionTokens->GetAt(7)->Token); - Cleanup(windowId); + Cleanup(); } void HistoryItemLoadAndContinueCalculation(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeBasic); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); @@ -384,12 +384,12 @@ namespace CalculatorFunctionalTests item = ref new HistoryItemViewModel(expression, result, historyItem->historyItemVector.spTokens, historyItem->historyItemVector.spCommands); MockOnHistoryItemClicked(item); VERIFY_ARE_EQUAL(StringReference(L"14"), m_standardViewModel->DisplayValue); - Cleanup(windowId); + Cleanup(); } void DisplayValueAutomationNames(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::CommandADD); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command8); @@ -413,12 +413,12 @@ namespace CalculatorFunctionalTests expression = StringReference(L"Display is 3"); VERIFY_ARE_EQUAL(expression, m_standardViewModel->CalculationResultAutomationName); - Cleanup(windowId); + Cleanup(); } void RadixAutomationName(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeProgrammer); m_standardViewModel->IsProgrammer = true; m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::Command1); @@ -434,26 +434,26 @@ namespace CalculatorFunctionalTests expression = StringReference(L"Binary 1000"); result = L"Binary " + Utils::GetStringValue(m_standardViewModel->BinaryDisplayValue); VERIFY_ARE_EQUAL(expression, result); - Cleanup(windowId); + Cleanup(); } void HistoryEmpty(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); VERIFY_ARE_EQUAL(0, m_historyViewModel->ItemSize); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); VERIFY_ARE_EQUAL(0, m_historyViewModel->ItemSize); - Cleanup(windowId); + Cleanup(); } void HistoryClearCommandWithEmptyHistory(unsigned int windowId = 0) { - Initialize(windowId); + Initialize(); VERIFY_ARE_EQUAL(0, m_historyViewModel->ItemSize); m_standardViewModel->m_standardCalculatorManager.SendCommand(Command::ModeScientific); m_historyViewModel->OnClearCommand(nullptr); VERIFY_ARE_EQUAL(0, m_historyViewModel->ItemSize); - Cleanup(windowId); + Cleanup(); } };