openfl icon indicating copy to clipboard operation
openfl copied to clipboard

Migrate defaults to a separate file

Open ishaileshpant opened this issue 11 months ago • 1 comments

This PR refactors the aggregator and collaborator modules by replacing hard-coded “magic” numbers and strings with centralized constant definitions. A new file, constants.py, has been introduced in the openfl/component directory to store these default values. This change improves maintainability and consistency across the codebase.

Motivation

  • Avoiding Magic Numbers/Strings:

Hard-coded values (e.g., 256, "tensor.db", "RESET", etc.) were scattered in the code. Extracting them into a constants file helps prevent errors and makes future updates easier.

  • Enhanced Readability & Maintainability:

Using descriptive constant names (like ROUNDS_TO_TRAIN and CERT_COMMON_NAME) clarifies the purpose of each value and allows for a single point of modification if defaults need to change.

ishaileshpant avatar Feb 09 '25 17:02 ishaileshpant

Thanks for picking this up @ishaileshpant.

Two suggestions:

  • It is recommended to keep constants at a module-level.
  • Can remove DEFAULT_ string for all constants. Upper case obviates the need to be explicit.

The only case that it won't address is some constants span across modules and hence i move to global package level else duplication of constants will be there

agree on the "default" prefix, in fact we can even name the file as defaults, that would further simplify the names and purpose becomes explicit

ishaileshpant avatar Feb 10 '25 17:02 ishaileshpant