Skip to content

[redcap] Delete IRedcapRecord #9913

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MaximeBICMTL
Copy link
Contributor

@MaximeBICMTL MaximeBICMTL commented Jul 16, 2025

Description

The current class hierarchy is RedcapRepeatingRecord extends RedcapRecord implements IRedcapRecord. I believe this is overcomplicated and we should only be a single RedcapRecord class, this PR is a step in that direction.

Changelog

  • Delete the IRedcapRecord interface (which is currently kind of useless since it is directly inherited by a single class).
  • Rename the method RedcapRecord->getInstrumentName() to RedcapRecord->getLorisInstrumentName() (as it is the name of the instrument in LORIS, which may be different from the name in REDCap for repeating instruments).

@github-actions github-actions bot added the Language: PHP PR or issue that update PHP code label Jul 16, 2025
@MaximeBICMTL MaximeBICMTL added Difficulty: Simple PR or issue that should be easy to implement, review, or test Category: Refactor PR or issue that aims to improve the existing code Module: redcap PR or issue related to redcap module labels Jul 16, 2025
@adamdaudrich
Copy link
Collaborator

imu (which stands for 'in my understanding'). You have:

  1. deleted IRedcapRecord because the entire code is already present ("directly inherited") in RedcapRecord class.
  2. renamed things consequently for clarity

correct? or...

because if correct, then I approve.

Copy link
Contributor

@regisoc regisoc left a comment

Choose a reason for hiding this comment

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

Yes, that was a legacy code.
Thanks for this nice and simple PR, that simplifies a bit the record part. LGTM!

@MaximeBICMTL
Copy link
Contributor Author

@adamdaudrich

  • deleted IRedcapRecord because the entire code is already present ("directly inherited") in RedcapRecord class.
  • renamed things consequently for clarity

The renaming is rather unrelated to the deletion but that's the idea yes 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Refactor PR or issue that aims to improve the existing code Difficulty: Simple PR or issue that should be easy to implement, review, or test Language: PHP PR or issue that update PHP code Module: redcap PR or issue related to redcap module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants