zrwusa / data-structure-typed

Javascript Data Structure & TypeScript Data Structure. Heap, Binary Tree, Red Black Tree, Linked List, Deque, Trie, HashMap, Directed Graph, Undirected Graph, Binary Search Tree, AVL Tree, Priority Queue, Graph, Queue, Tree Multiset, Singly Linked List, Doubly Linked List, Max Heap, Max Priority Queue, Min Heap, Min Priority Queue, Stack.
https://data-structure-typed-docs.vercel.app
MIT License
114 stars 8 forks source link

hashFn should be optional but option type makes it a required parameter #64

Closed simoami closed 8 months ago

simoami commented 8 months ago

Describe the bug

The hashFn of the HashMap class and potentially also other classes is defined as a required parameter. However the code implies that it could be optional:

if (options) {
      const { hashFn } = options;
      if (hashFn) { <--- this implies that it may be optional
        this._hashFn = hashFn;
      }
    }

To Reproduce Steps to reproduce the behavior:

According to the constructor signature: https://github.com/zrwusa/data-structure-typed/blob/main/src/data-structures/hash/hash-map.ts#L30

  constructor(
    elements: Iterable<[K, V]> = [],
    options?: {
      hashFn: (key: K) => string;  <--- the handler function is required
    }
  ) {

This is not helpful if one wants to extend / inherit the class with additional features and need to introduce additional options. In this case you'd typically see:

class ExtendedHashMap<K, V> extends HashMap<K, V> {
  constructor(
    elements: Iterable<[K, V]> = [],
    options?: {
      hashFn?: (key: K) => string;
      someOtherParam: string;
    }
  ) {
    const { someOtherParam, ...restOptions } = options || {};
   // do something with someOtherParam
   super(elements, restOptions) <--- fails here because object exists but not hashFn
}

Expected behavior A clear and concise description of what you expected to happen. hashFn should be declared as optional even if the options parameter is declared optional

zrwusa commented 8 months ago

Thank you for your suggestion; your input is highly valuable. We have incorporated the changes to the design and addressed the issue in version 1.49.7. All items in the 'options' are now optional parameters, enhancing the inheritability of HashMap. Additionally, we have reviewed and standardized the constructor parameters for all data structures, adopting a uniform approach.

simoami commented 8 months ago

Hi, it's very rare that I create an issue and wake up the next day to find it all resolved and published. Thank you for the quick turnaround!