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

chore: Update AVL Tree #2789

Closed
wants to merge 6 commits into from
Closed

chore: Update AVL Tree #2789

wants to merge 6 commits into from

Conversation

ritk20
Copy link
Contributor

@ritk20 ritk20 commented Oct 8, 2024

Description of Change

Updated AVL Tree data structure using modern C++ STL library features like std::unique_ptr and implemented Object Oriented structure. Also added additional tests and updated existing tests to use assert.

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
  • Relevant documentation/comments is changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

Notes:
Completed the following TODO

/**
 * \todo update code to use C++ STL library features and OO structure
 * \warning This program is a poor implementation and does not utilize any of
 * the C++ STL features.
 * /

data_structures/avltree.cpp Outdated Show resolved Hide resolved
data_structures/avltree.cpp Show resolved Hide resolved
data_structures/avltree.cpp Show resolved Hide resolved
data_structures/avltree.cpp Outdated Show resolved Hide resolved
data_structures/avltree.cpp Show resolved Hide resolved
return 0;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did not understand this issue... Is it resolved?

@realstealthninja realstealthninja added automated tests are failing Do not merge until tests pass awaiting modification Do not merge until modifications are made labels Oct 8, 2024
@realstealthninja
Copy link
Collaborator

realstealthninja commented Oct 8, 2024

this pr should be put on hold until #2746 merges

@realstealthninja realstealthninja added the on hold Pull request is temporarily not reviewed label Oct 8, 2024
@ritk20
Copy link
Contributor Author

ritk20 commented Oct 8, 2024

Is it necessary to add class names and variables in snake_case as well?

@realstealthninja
Copy link
Collaborator

Is it necessary to add class names and variables in snake_case as well?

No, the contributing.md file prefers pascal or camel case last i checked please do check and confirm

@ritk20
Copy link
Contributor Author

ritk20 commented Oct 9, 2024

Is it necessary to add class names and variables in snake_case as well?

No, the contributing.md file prefers pascal or camel case last i checked please do check and confirm

For classes, variables, and functions, there is no explicit mention of a case preference, although the description below led me to believe that snake_case is preferred.

/**
 * @brief Class documentation
 */
class class_name {
 private:
    int variable;   ///< short info of this variable
    char *message;  ///< short info

 public:
    // other members should be also documented as below
}

Also, this makes me believe that snake_case is preferred for functions

template <typename T>
bool is_number_on_array(const std::vector<T> &arr, const int &number) {
    for (int i = 0; i < sizeof(arr) / sizeof(int); i++) {
        if (arr[i] == number) {
            return true;
        }
        else {
            // Number not in the current index, keep searching.
        }
    }

    return false;
}

@realstealthninja
Copy link
Collaborator

realstealthninja commented Oct 9, 2024

feel free to use snake case instead
#2488
is a discussion worth checking out if you'd like to further make your point

Copy link
Contributor

github-actions bot commented Nov 9, 2024

This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Author has not responded to the comments for over 2 weeks label Nov 9, 2024
Copy link
Contributor

Please ping one of the maintainers once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to ask for help in our Gitter channel or our Discord server. Thank you for your contributions!

@github-actions github-actions bot closed this Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automated tests are failing Do not merge until tests pass awaiting modification Do not merge until modifications are made on hold Pull request is temporarily not reviewed stale Author has not responded to the comments for over 2 weeks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants