Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor switchlink header files #169

Merged
merged 4 commits into from
Nov 8, 2024
Merged

Refactor switchlink header files #169

merged 4 commits into from
Nov 8, 2024

Conversation

ffoulkes
Copy link
Contributor

@ffoulkes ffoulkes commented Nov 3, 2024

  • Renamed switchlink.h to switchlink_defs.h.
  • Renamed bazel target from switchlink_types to switchlink_defs.
  • Extracted switchlink_types.h from switchlink_defs.h.
  • Extracted switchlink_db_types.h from switchlink_db.h.
  • Defined switchlink_types and switchlink_db_int bazel targets.

The main objective of the refactoring was to separate the data type definitions from the function prototypes. This resulted in cleaner, more precise dependency declarations in the Bazel build system.

It is also a better design, in that it enforces separation of concerns between the data type definitions and the API.

The distinction became apparent when I looked into the possibility of converting switchlink from C to C++. The API is tightly coupled to the class interface, since it defines the methods; the data types are loosely coupled, and could easily go in another header file.


The process separating the definitions was as follows:

  1. Make a copy of switchlink.h and name it switchlink_types.h.
  2. Remove all the type definitions from switchlink.h.
  3. Remove all the non-type defintions from switchlink_types.h.
  4. Rename switchlink.h to switchlink_defs.h. (This allowed me to create separate switchlink and switchlink_defs targets in Bazel.)
  5. Make a copy of switchlink_db.h and name it switchlink_types.h.
  6. Remove all type definitions from switchlink_db.h.
  7. Remove all non-type definitions from switchlink_types.h.
  8. Rename switchlink.h to switchlink_defs.h.
  9. Update #includes as necessary.
  10. Update CMake and Bazel build files as necessary.

It was verified by:

  1. Switching to the main branch.
  2. Making copies of switchlink.h and switchlink_db.h to a temporary directory.
  3. Switching to the header-file-refactoring branch.
  4. Comparing temp/switchlink.h with switchlink_defs.h.
  5. Comparing temp/switchlink.h with switchlink_types.h.
  6. Comparing temp/switchlink_db.h with switchlink_db.h.
  7. Comparing temp/switchlink_db.h with switchlink_db_types.h.

It was obvious from the side-by-side comparison that a block of definitions had been transferred, intact, from switchlink.h to switchlink_types.h, and from switchlink_db.h to switchlink_db_types.h.

- Renamed switchlink.h to switchlink_defs.h.
- Changed Bazel target from switchlink_types to switchlink_defs.

Signed-off-by: Derek Foster <[email protected]>
- Extracted switch_db_types.h from switchlink_db.h.
- Defined switchlink_db_types bazel target.
- Defined switchlink_db_int bazel target.

Signed-off-by: Derek Foster <[email protected]>
Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ffoulkes ffoulkes merged commit 9104a5e into main Nov 8, 2024
4 checks passed
@ffoulkes ffoulkes deleted the header-file-refactoring branch November 8, 2024 00:35
@ffoulkes ffoulkes restored the header-file-refactoring branch November 15, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants