I don't see much to improve from algorithmic point of view. So I will focus on Object Oriented design.
How about doing it similarly to Sort method? I mean instead of making abstract method, try just injecting Comparator into generic Heap implementation.
People are generally scary of long and complex code. It would be nicer to try to split it into smaller parts:
- First (most general) concept is to extract each huge operation (like
Find
,Downheap
etc) into separate class with appropriate Single Responsibility. They usually accept in constructor original method parameters, hold its internal state as fields instead of local variables.
They can also accept in constructor reference to original object but it would be even better design practice to extract internal Heap state into even smaller object and avoid circular dependency.
Another approach is to make those classes stateless (just moving methods into separate classes and injecting Heap internal state) - Second idea would be to separate defensive programming from actual algorithms to make them cleaner and more readable (I would suggest to move them into a class mentioned in point 3. or into separate
Facade
responsible for validating user input) - You can also extract single steps of
DownHeap
andUpHeap
routines into separate methods.
- You mentioned, that you struggled a little bit with
collections.generic
interfaces. My proposed solution would be not to pollute actual implementation but instead useAdapter
Design Pattern
4. Another thing that comes into mind (beside OO) is to calculate numberOfLevels
from either Count
or capacity
(using ceiled logarithm)
As #harold mentioned in comment it is probably unused
You can simplify main body of the find routine:
var rightResult = FindInternal(item, IndexToRightChildNode(index)); var leftResult = FindInternal(item, IndexToLeftChildNode(index)); return rightResult >= 0 ? rightResult : leftResult;
Please notice that in your case when neither rightResult
nor leftResult
has value greather than 0, the function will return -1 anyway.
(Optionally, as applied in example of 5) You can add a an arrow function
IndexToLeftChildNode
for completeness or to increase readability when you decide to inline some variables.You can extract common (utility) subroutine from
DownHeap
:private int UpdateLargestIndex(int currentLargestIndex, int childIndex) { if (childIndex < Count && CloserToRoot(innerT[childIndex].CompareTo(innerT[currentLargestIndex]))) { return childIndex; } return currentLargestIndex; }