-
Notifications
You must be signed in to change notification settings - Fork 26
Issue: Remove exception throwing from property getter #667
Description
What version of FlowSynx?
1.2.2
Describe the bug
There is a property getter in the codebase that includes a try/catch block which throws an exception. Throwing exceptions from property getters is generally considered a bad practice because property accessors should be simple, side-effect-free operations.
File: src/FlowSynx.Application.Services/ICurrentUserService.cs
Current interface implementation
public interface ICurrentUserService
{
string UserId { get; }
string UserName { get; }
bool IsAuthenticated { get; }
List<string> Roles { get; }
void ValidateAuthentication();
}Problem:
These getters properties in concrete implementation should not perform complex operations or throw exceptions.
Throwing exceptions here may lead to unexpected behavior when the property is accessed, especially during serialization, reflection, or UI data binding.
This makes debugging and maintenance harder and can introduce subtle bugs.
Suggested Fix:
Refactor this property getter into a methods (e.g., UserId(), UserName(), IsAuthenticated(), Roles()), which can safely handle exceptions and communicate errors clearly, or remove exception throwing from the getter altogether.
public interface ICurrentUserService
{
string UserId();
string UserName();
bool IsAuthenticated();
List<string> Roles();
void ValidateAuthentication();
}Change correcponding properties into methods for CurrentUserService.cs
Acceptance Criteria:
- No property getter in CurrentUserService.cs should throw exceptions.
- Replace the property with a method where appropriate.
- Unit tests updated to reflect the change.
- Code reviewed for similar patterns elsewhere.