-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor Core Logic and Improve Code Maintainability #5
Conversation
Replaced hardcoded magic numbers, simulation parameters, file paths, and array sizes with named constants (e.g., `TN_PARAM`, `FILE_PATH_ARRAY_T`). This change also simplifies `forming_Uvx` and `forming_Uvix` function signatures, as their configuration parameters are now internal constants. Improves readability, maintainability, and centralizes configuration.
Updated comments and print statements in `calc_leading_edge` and `calculate_with_precision` to correctly refer to the calculation of the "leading edge duration" instead of "trailing edge duration".
Corrected the indentation within the main function's switch block to improve code readability and maintain a consistent style. No functional changes have been made.
Replaced hardcoded string comparisons for 'yes'/'no' answers with a more robust and extensible system.
This change introduces a `response_dictionary` module to centralize the parsing of user responses. The `ask_user_continue` function now calls `get_response_type_by_keyword` to receive a structured `ResponseType` enum.
This approach:
- Eliminates magic strings ("да", "нет") from the input logic.
- Improves readability by using a clean `switch` statement.
- Makes it easy to add new response keywords (e.g., 'y', 'yes') in the future by modifying only the dictionary module.
Moved constants that are private to a single module from the global `constants.h` header into their respective implementation files. These constants were converted from preprocessor `#define` macros to `static const` variables to limit their scope. This change improves encapsulation and reduces global namespace pollution. Key benefits: - **Encapsulation:** Modules no longer expose their internal numerical parameters globally. - **Cohesion:** Constants now live alongside the code that uses them. - **Type Safety:** `static const` provides type checking, which is safer than `#define`. Constants that are genuinely shared across multiple modules, such as `ARRAY_SIZE` and file paths, remain in the global `constants.h` file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the codebase to remove hardcoded “magic numbers” in favor of scoped constants, introduces a new response_dictionary module for cleaner input handling, and standardizes related function signatures and file paths.
- Moved global and module-specific magic numbers into
constants.handstatic constdefinitions. - Added
response_dictionaryfor extensible parsing of user inputs (“да”/“нет”). - Updated function calls, error messages, and file operations to use the new constants and cleaned up styling.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signal_analysis.c | Replaced #define N with ARRAY_SIZE and simplified forming calls. |
| src/response_dictionary.c | New module mapping input keywords to ResponseType. |
| src/parameter.c | Replaced local magic numbers with constants and refactored precision loop. |
| src/output.c | Replaced hardcoded banner path with FILE_PATH_ZAST. |
| src/input.c | Replaced manual strcmp checks with response_dictionary and buffer const. |
| src/forming.c | Moved magic numbers into static const parameters and updated logic. |
| src/file.c | Swapped literal file paths for FILE_PATH_* macros. |
| include/response_dictionary.h | Added header for the new response-mapping API. |
| include/forming.h | Updated prototypes for forming_Uvx and forming_Uvix. |
| include/constants.h | New header defining array size and file-path macros. |
Comments suppressed due to low confidence (2)
src/response_dictionary.c:47
- Add unit tests for
get_response_type_by_keywordto cover affirmative, negative, and unknown inputs, as well as edge cases.
ResponseType get_response_type_by_keyword(const char *input) {
src/parameter.c:66
- Consider adding unit tests for
calculate_with_precisionto verify the iterative precision logic, convergence behavior, and warning path when maximum array size is reached.
void calculate_with_precision() {
|
|
||
| // Расчет погрешности | ||
| if (prev_parameter != 1e10) { | ||
| if (prev_parameter != PREV_PARAMETER_INITIAL) { |
Copilot
AI
Jul 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prev_parameter is never updated to current_parameter, so current_precision is never recalculated and the loop cannot terminate. Add prev_parameter = current_parameter; after computing the precision.
| break; | ||
| case RESPONSE_NEGATIVE: | ||
| return false; | ||
| break; |
Copilot
AI
Jul 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The break; after return true; is unreachable. You can remove this break (and the similar one after return false;) to simplify the code.
|
|
||
| #include "response_dictionary.h" | ||
|
|
||
| const KeywordMapping RESPONSE_DICTIONARY[] = { | ||
| // Утвердительные ответы | ||
| {"да", RESPONSE_AFFIRMATIVE}, | ||
| {"Да", RESPONSE_AFFIRMATIVE}, | ||
| {"дА", RESPONSE_AFFIRMATIVE}, | ||
| {"ДА", RESPONSE_AFFIRMATIVE}, | ||
|
|
||
| // Отрицательные ответы | ||
| {"нет", RESPONSE_NEGATIVE}, | ||
| {"Нет", RESPONSE_NEGATIVE}, | ||
| {"нЕт", RESPONSE_NEGATIVE}, | ||
| {"неТ", RESPONSE_NEGATIVE}, | ||
| {"НЕт", RESPONSE_NEGATIVE}, | ||
| {"нЕТ", RESPONSE_NEGATIVE}, | ||
| {"НЕТ", RESPONSE_NEGATIVE} | ||
| }; | ||
|
|
||
| const int DICTIONARY_SIZE = sizeof(RESPONSE_DICTIONARY) / sizeof(RESPONSE_DICTIONARY[0]); | ||
|
|
||
| ResponseType get_response_type_by_keyword(const char *input) { | ||
| for (int i = 0; i < DICTIONARY_SIZE; i++) { | ||
| if (strcmp(input, RESPONSE_DICTIONARY[i].keyword_text) == 0) { | ||
| return RESPONSE_DICTIONARY[i].type; | ||
| } | ||
| } | ||
| return RESPONSE_UNKNOWN; | ||
| } |
Copilot
AI
Jul 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider normalizing the input (e.g., converting to lowercase) and using a case-insensitive comparison (e.g., strcasecmp) instead of enumerating every casing variant in the dictionary.
| #include "response_dictionary.h" | |
| const KeywordMapping RESPONSE_DICTIONARY[] = { | |
| // Утвердительные ответы | |
| {"да", RESPONSE_AFFIRMATIVE}, | |
| {"Да", RESPONSE_AFFIRMATIVE}, | |
| {"дА", RESPONSE_AFFIRMATIVE}, | |
| {"ДА", RESPONSE_AFFIRMATIVE}, | |
| // Отрицательные ответы | |
| {"нет", RESPONSE_NEGATIVE}, | |
| {"Нет", RESPONSE_NEGATIVE}, | |
| {"нЕт", RESPONSE_NEGATIVE}, | |
| {"неТ", RESPONSE_NEGATIVE}, | |
| {"НЕт", RESPONSE_NEGATIVE}, | |
| {"нЕТ", RESPONSE_NEGATIVE}, | |
| {"НЕТ", RESPONSE_NEGATIVE} | |
| }; | |
| const int DICTIONARY_SIZE = sizeof(RESPONSE_DICTIONARY) / sizeof(RESPONSE_DICTIONARY[0]); | |
| ResponseType get_response_type_by_keyword(const char *input) { | |
| for (int i = 0; i < DICTIONARY_SIZE; i++) { | |
| if (strcmp(input, RESPONSE_DICTIONARY[i].keyword_text) == 0) { | |
| return RESPONSE_DICTIONARY[i].type; | |
| } | |
| } | |
| return RESPONSE_UNKNOWN; | |
| } | |
| #include <ctype.h> | |
| #include "response_dictionary.h" | |
| const KeywordMapping RESPONSE_DICTIONARY[] = { | |
| // Утвердительные ответы | |
| {"да", RESPONSE_AFFIRMATIVE}, | |
| // Отрицательные ответы | |
| {"нет", RESPONSE_NEGATIVE} | |
| }; | |
| const int DICTIONARY_SIZE = sizeof(RESPONSE_DICTIONARY) / sizeof(RESPONSE_DICTIONARY[0]); | |
| ResponseType get_response_type_by_keyword(const char *input) { | |
| char normalized_input[256]; | |
| normalize_to_lowercase(input, normalized_input); | |
| for (int i = 0; i < DICTIONARY_SIZE; i++) { | |
| if (strcmp(normalized_input, RESPONSE_DICTIONARY[i].keyword_text) == 0) { | |
| return RESPONSE_DICTIONARY[i].type; | |
| } | |
| } | |
| return RESPONSE_UNKNOWN; | |
| } | |
| void normalize_to_lowercase(const char *input, char *output) { | |
| for (int i = 0; input[i] != '\0'; i++) { | |
| output[i] = tolower((unsigned char)input[i]); | |
| } | |
| output[strlen(input)] = '\0'; | |
| } |
This pull request introduces a series of significant refactorings and fixes aimed at improving the overall code quality, maintainability, and robustness of the application. The codebase has been evolved from using hardcoded "magic numbers" to a more structured, encapsulated, and type-safe design.
Key Changes:
✨ Centralized and Scoped Configuration:
constants.hfile to eliminate hardcoded values from the source code..cfiles usingstatic const. This improves encapsulation, reduces global namespace pollution, and provides better type safety than preprocessor macros.📚 Robust User Input Handling:
strcmpchecks have been replaced with a new, extensibleresponse_dictionarymodule.🐞 Bug Fixes and Styling:
printfstatements has been corrected to accurately describe the calculated parameter ("leading edge" instead of "trailing edge").switchstatements) to improve readability.Goal:
The primary goal of these changes is to transition the codebase from a functional prototype to a more stable and maintainable foundation. This makes future development easier and reduces the risk of introducing bugs.