20250114 ‐ threading - cywongg/2025 GitHub Wiki
You're absolutely right that adding a IsLoading
property directly to your business object (or domain model) feels out of place. The IsLoading
property is a UI-related concern, not a business logic concern, and mixing UI concerns with business logic violates the separation of concerns principle.
Here's how you can avoid polluting your business object while still achieving the desired functionality:
Instead of adding IsLoading
to your business object, you can manage it in your ViewModel. A ViewModel is specifically designed to bridge the gap between your UI and business logic, making it the perfect place to handle UI-related state like IsLoading
.
If you're not already using a ViewModel, create one. Add the IsLoading
property to the ViewModel, where it belongs:
public class MyViewModel : INotifyPropertyChanged
{
private bool _isLoading;
public bool IsLoading
{
get => _isLoading;
set
{
_isLoading = value;
OnPropertyChanged(nameof(IsLoading)); // Notify the UI when the value changes
}
}
private ObservableCollection<MyBusinessObject> _dataCollection;
public ObservableCollection<MyBusinessObject> DataCollection
{
get => _dataCollection;
set
{
_dataCollection = value;
OnPropertyChanged(nameof(DataCollection)); // Notify the UI when the collection changes
}
}
// Example: Simulate data loading
public async Task LoadDataAsync()
{
IsLoading = true;
// Simulate data loading (replace with actual logic)
await Task.Delay(2000);
DataCollection = new ObservableCollection<MyBusinessObject>
{
new MyBusinessObject { Name = "Item 1" },
new MyBusinessObject { Name = "Item 2" }
};
IsLoading = false;
}
// Implementation of INotifyPropertyChanged
public event PropertyChangedEventHandler PropertyChanged;
protected void OnPropertyChanged(string propertyName)
{
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
}
}
In this example:
-
IsLoading
is managed in the ViewModel, isolating it from your business logic. -
DataCollection
is the collection bound to yourDataGrid
.
In your XAML, set the DataContext
to an instance of the ViewModel:
<Window.DataContext>
<local:MyViewModel />
</Window.DataContext>
The XAML you provided can remain largely the same, but now the IsLoading
property comes from the ViewModel, not the business object:
<Grid>
<!-- Main Data Grid -->
<DataGrid ItemsSource="{Binding DataCollection}" IsEnabled="{Binding IsLoading, Converter={StaticResource InverseBooleanConverter}}" />
<!-- Loading Indicator -->
<TextBlock Text="Loading..."
HorizontalAlignment="Center"
VerticalAlignment="Center"
FontSize="16"
FontWeight="Bold">
<TextBlock.Style>
<Style TargetType="TextBlock">
<Setter Property="Visibility" Value="Collapsed" />
<Style.Triggers>
<DataTrigger Binding="{Binding IsLoading}" Value="True">
<Setter Property="Visibility" Value="Visible" />
</DataTrigger>
</Style.Triggers>
</Style>
</TextBlock.Style>
</TextBlock>
</Grid>
-
Separation of Concerns:
- UI-related properties like
IsLoading
stay in the ViewModel. - The business object remains focused on business logic.
- UI-related properties like
-
Scalability:
- If you need to add more UI-related state properties in the future (e.g.,
IsError
,IsEmpty
), they can all reside in the ViewModel.
- If you need to add more UI-related state properties in the future (e.g.,
-
Reusability:
- You can reuse the same business object across different applications or scenarios without worrying about UI-related properties.
-
Testability:
- By isolating
IsLoading
in the ViewModel, you make it easier to test the UI state independently of the business logic.
- By isolating
If you frequently toggle Visibility
based on bool
values, you might still consider using a BooleanToVisibilityConverter
. Here's why:
- It simplifies XAML bindings for
Visibility
(no need for triggers). - It's reusable across multiple controls.
Define the converter in your resources:
<Window.Resources>
<BooleanToVisibilityConverter x:Key="BooleanToVisibilityConverter" />
</Window.Resources>
Then use it like this:
<TextBlock Text="Loading..."
HorizontalAlignment="Center"
VerticalAlignment="Center"
Visibility="{Binding IsLoading, Converter={StaticResource BooleanToVisibilityConverter}}" />
However, if you prefer the DataTrigger
approach, that's perfectly fine as well.
Avoid adding IsLoading
(or any other UI-related state) to your business object. Instead, use a ViewModel to manage UI state and bind the View to the ViewModel. This keeps your code clean, maintainable, and adherent to good architectural principles.
——————
Yes, you can avoid using a BooleanToVisibilityConverter
by directly binding the Visibility
property to your IsLoading
property and using data triggers in XAML. This approach eliminates the need for converters and keeps the code simple.
Here’s how you can achieve this:
We will use a Style
with a DataTrigger
to bind the Visibility
property of the loading indicator directly to the IsLoading
property.
No changes are needed in your code-behind or ViewModel. You’ll still use the IsLoading
property as before:
private bool _isLoading;
public bool IsLoading
{
get => _isLoading;
set
{
_isLoading = value;
OnPropertyChanged(nameof(IsLoading)); // Notify the UI when the value changes
}
}
Here’s how you can define the UI:
<Grid>
<!-- Main Data Grid -->
<DataGrid ItemsSource="{Binding YourDataCollection}" IsEnabled="{Binding IsLoading, Mode=OneWay}" />
<!-- Loading Indicator -->
<TextBlock Text="Loading..."
HorizontalAlignment="Center"
VerticalAlignment="Center"
FontSize="16"
FontWeight="Bold">
<!-- Style with DataTrigger to control Visibility -->
<TextBlock.Style>
<Style TargetType="TextBlock">
<Setter Property="Visibility" Value="Collapsed" />
<Style.Triggers>
<DataTrigger Binding="{Binding IsLoading}" Value="True">
<Setter Property="Visibility" Value="Visible" />
</DataTrigger>
</Style.Triggers>
</Style>
</TextBlock.Style>
</TextBlock>
</Grid>
-
DataTrigger
:- The
DataTrigger
binds directly to theIsLoading
property. - When
IsLoading
istrue
, theVisibility
is set toVisible
. - When
IsLoading
isfalse
, theVisibility
is set toCollapsed
(default).
- The
-
No Converters:
- There is no need to define a
BooleanToVisibilityConverter
since theDataTrigger
handles the toggling ofVisibility
.
- There is no need to define a
-
IsEnabled
Binding:- The
DataGrid
is bound toIsLoading
. WhenIsLoading
istrue
, theDataGrid
will be disabled; whenfalse
, it will be enabled.
- The
-
When
IsLoading = true
:- The
TextBlock
(loading indicator) becomes visible. - The
DataGrid
is disabled.
- The
-
When
IsLoading = false
:- The
TextBlock
(loading indicator) becomes collapsed. - The
DataGrid
is enabled and displays the loaded data.
- The
-
No Converters: You avoid introducing additional classes or logic (like
BooleanToVisibilityConverter
). -
Simple and Clean: WPF's
DataTrigger
provides a declarative way to toggleVisibility
based on a property. - Reusable: You can use this style with other controls if needed.
Let me know if you need further assistance!
——— Great questions! Let’s break this down step by step and address each point you raised thoroughly:
Yes, extracting the code inside if (asiaSettings.IsAsia)
to a separate async
function is a good idea if you don’t want it to impact the other parts of your method (like initializing IcomDataUpdateMonitor
and otherDBs
). This way, the IcomDataUpdateMonitor
and otherDBs
initialization can continue to run synchronously, while the long-running operations in the if (asiaSettings.IsAsia)
block can run asynchronously.
Here’s how you can refactor it:
private async Task StartDatafeedsAsync()
{
// Synchronous initialization of IcomDataUpdateMonitor and otherDBs
IcomDataUpdateMonitor = piconnection.streamingAccessor.piGetTickingTree(
EnvironmentAndSettings.icomTree,
EnvironmentAndSettings.someOtherParam
);
IcomDataUpdateMonitor.itemChangeHandlers += DataUpdateMonitor_itemChangeHandlers;
IcomDataUpdateMonitor.init();
otherDBs.newDivAndCaData += otherDBs_newDivAndCaData;
otherDBs.newSnapData += otherDBs_newSnapData;
otherDBs.init();
// Handle the Asia-specific logic asynchronously
if (asiaSettings.IsAsia)
{
await HandleAsiaSpecificLogicAsync();
}
// Refresh the UI after everything is initialized
RefreshUI();
}
private async Task HandleAsiaSpecificLogicAsync()
{
var restrikingTask = Task.Run(() => restrikingValidationService.Init(myInstruments.Values));
var corporateActionTask = Task.Run(() =>
{
ICollection<string> prIndexes = GetPrIndexes();
ICollection<string> prStocks = GetPrStocks();
DateTime latestNextFixingDate = GetLatestNextFixingDate();
corporateActionService.Init(prIndexes, prStocks, from: DateTime.Today, to: latestNextFixingDate);
});
// Wait for both tasks to complete concurrently
await Task.WhenAll(restrikingTask, corporateActionTask);
}
This refactoring ensures that:
- The
IcomDataUpdateMonitor
andotherDBs
initialization logic remains unaffected. - The Asia-specific logic is handled asynchronously and does not block the UI thread.
No, await Task.WhenAll(restrikingTask, corporateActionTask);
does not block the UI thread. Here’s why:
-
What Happens with
Task.WhenAll
?- The two tasks (
restrikingTask
andcorporateActionTask
) run concurrently on background threads (not on the UI thread). - The
await
keyword tells the UI thread, "Let me know when these tasks are done, but in the meantime, you (the UI thread) are free to do other things." - As a result, the UI thread remains free and responsive while the tasks run in the background.
- The two tasks (
-
What Happens While the Tasks Are Running?
- If you have UI elements (like tabs, grids, or lists) that depend on the data being loaded by these tasks, they will appear empty or display a "loading" message until the tasks complete and populate the data.
- Once the tasks are done, the UI can be updated with the loaded data.
Example:
- Before the tasks finish: A tab or grid might appear empty.
-
After the tasks finish: The UI gets updated with the data (e.g., via
RefreshUI
).
If you don’t want the UI to appear empty while the data is loading, you can implement a "loading state" in your UI. For example:
- Display a spinner or "Loading..." message while the data is being fetched.
- Update the UI once the data is fully loaded.
Here’s how you can achieve this in your code:
Add a TextBlock
or ProgressBar
in your XAML to indicate loading.
<Grid>
<!-- Main Content -->
<TabControl x:Name="MainTabs" IsEnabled="{Binding IsLoading, Converter={StaticResource InverseBooleanConverter}}">
<!-- Your tabs go here -->
</TabControl>
<!-- Loading Indicator -->
<TextBlock Text="Loading..." HorizontalAlignment="Center" VerticalAlignment="Center"
Visibility="{Binding IsLoading, Converter={StaticResource BooleanToVisibilityConverter}}" />
</Grid>
This property will track whether the data is still loading.
private bool _isLoading;
public bool IsLoading
{
get => _isLoading;
set
{
_isLoading = value;
OnPropertyChanged(nameof(IsLoading)); // Notify the UI of the change
}
}
Set IsLoading
to true
before starting the long-running tasks and set it to false
after they complete.
private async Task HandleAsiaSpecificLogicAsync()
{
try
{
// Indicate that data is loading
IsLoading = true;
var restrikingTask = Task.Run(() => restrikingValidationService.Init(myInstruments.Values));
var corporateActionTask = Task.Run(() =>
{
ICollection<string> prIndexes = GetPrIndexes();
ICollection<string> prStocks = GetPrStocks();
DateTime latestNextFixingDate = GetLatestNextFixingDate();
corporateActionService.Init(prIndexes, prStocks, from: DateTime.Today, to: latestNextFixingDate);
});
await Task.WhenAll(restrikingTask, corporateActionTask);
}
finally
{
// Loading complete
IsLoading = false;
}
}
Yes, the tabs (or UI elements) will initially appear empty, as the data hasn’t been loaded yet. However:
- The UI thread won’t freeze because the tasks are running on background threads.
- If you implement a "loading state" as described above, the UI can show a "Loading..." message or spinner until the data is ready.
To make the code cleaner and more maintainable:
-
Separate Concerns: Keep long-running logic (like
restrikingValidationService.Init
) in separate async methods. - Use Loading Indicators: Add visual feedback to the UI while data is being loaded.
-
Stick to Async Programming: Avoid synchronous calls (
.Result
or.Wait
) as they block the thread.
By refactoring the if (asiaSettings.IsAsia)
block into an async
method, you ensure that other parts of your initialization logic (like IcomDataUpdateMonitor
and otherDBs
) aren't affected by the long-running operations. Using await Task.WhenAll
ensures the UI thread remains responsive, and implementing a "loading state" provides a better user experience while the data is being fetched.