Skip to main content
Fix some typos
Source Link

I completely agree agree with what the others said, but here are some additional remarks.

I completely agree with what the others said, but here are some additional remarks.

I agree with what the others said, but here are some additional remarks.

I completely aggreeagree with what the others said, but here are some additional remarks.

  1. Use default initialization when possible

     class node {
     public:
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    
  2. Provide a constructor for your node class that passes key and type

     class node {
     public:
         node(int Key, char Type)
             : key(Key)
             , type(Type)
         {}
    
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    

    Lets have a look at how that affects you helper functions addFront firtfirst

     void dList::addFront(int k, char t) { // creates new node at front of list
         node *newNode = new node(k, t);
         newNode->next = head;          
         if (head != nullptr) {
             head->prev = newNode;
         }
         head = newNode;
    
         // If youyour list was constructed with empty arrays you need topto set tail too
         if (tail ==nullptr== nullptr) {
             tail = newNode;
         }
     }
    

    Is there anyreasonany reason you traverse the list when you have tail for it in addBack when you have a pointer to tail?

     void dList::addBack(int k, char t) { // creates new node at front of list
         // If there is no Headhead makecreate a shortcut to AddFrontaddFront
         if (head == nullptr) {
             AddFrontaddFront(k,t);
         }
    
         node *newNode = new node(k, t);
         tail->next = newNode
         newNode->prev = tail;
         tail = newNode;
     }
    
  3. More a personal preference, but i wouldratherwould rather use a strutstruct that is private to the list thatn afullythan a fully opaque class but whatever.

  4. You question is tagged C++ however you use plain C-arrays in the constructor dList(int arrayNums[], char arrayChars[], int size);. Here std::vector would be better. Also think about what happens when one of the arrays is sortershorter than size?

     dList::dList(const std::vector<int>& arrayNums, const std::vector<char>& arrayChars) {
         if (arrayNums.size() != arrayChars.size()) {
             //Do something here, e.g. throw an exception whator everwhatever you want
         }
    
         for (std::size_t i = 0; i < arrayNums.size(); ++i) {
             addBack(arrayNums[i], arrayChars[i]);
         }
      }
    

I completely aggree with what the others said, but here some additional remarks.

  1. Use default initialization when possible

     class node {
     public:
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    
  2. Provide a constructor for your node class that passes key and type

     class node {
     public:
         node(int Key, char Type)
             : key(Key)
             , type(Type)
         {}
    
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    

    Lets have a look how that affects you helper functions addFront firt

     void dList::addFront(int k, char t) { // creates new node at front of list
         node *newNode = new node(k, t);
         newNode->next = head;          
         if (head != nullptr) {
             head->prev = newNode;
         }
         head = newNode;
    
         // If you list was constructed with empty arrays you need top set tail too
         if (tail ==nullptr) {
             tail = newNode;
         }
     }
    

    Is there anyreason you traverse the list when you have tail for it in addBack?

     void dList::addBack(int k, char t) { // creates new node at front of list
         // If there is no Head make a shortcut to AddFront
         if (head == nullptr) {
             AddFront(k,t);
         }
    
         node *newNode = new node(k, t);
         tail->next = newNode
         newNode->prev = tail;
         tail = newNode;
     }
    
  3. More a personal preference, but i wouldrather use a strut that is private to the list thatn afully opaque class but whatever.

  4. You question is tagged C++ however you use plain C-arrays in the constructor dList(int arrayNums[], char arrayChars[], int size);. Here std::vector would be better. Also think about what happens when one of the arrays is sorter than size?

     dList::dList(const std::vector<int>& arrayNums, const std::vector<char>& arrayChars) {
         if (arrayNums.size() != arrayChars.size()) {
             //Do something here, e.g. throw an exception what ever you want
         }
    
         for (std::size_t i = 0; i < arrayNums.size(); ++i) {
             addBack(arrayNums[i], arrayChars[i]);
         }
      }
    

I completely agree with what the others said, but here are some additional remarks.

  1. Use default initialization when possible

     class node {
     public:
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    
  2. Provide a constructor for your node class that passes key and type

     class node {
     public:
         node(int Key, char Type)
             : key(Key)
             , type(Type)
         {}
    
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    

    Lets have a look at how that affects you helper functions addFront first

     void dList::addFront(int k, char t) { // creates new node at front of list
         node *newNode = new node(k, t);
         newNode->next = head;          
         if (head != nullptr) {
             head->prev = newNode;
         }
         head = newNode;
    
         // If your list was constructed with empty arrays you need to set tail too
         if (tail == nullptr) {
             tail = newNode;
         }
     }
    

    Is there any reason you traverse the list in addBack when you have a pointer to tail?

     void dList::addBack(int k, char t) { // creates new node at front of list
         // If there is no head create a shortcut to addFront
         if (head == nullptr) {
             addFront(k,t);
         }
    
         node *newNode = new node(k, t);
         tail->next = newNode
         newNode->prev = tail;
         tail = newNode;
     }
    
  3. More a personal preference, but i would rather use a struct that is private to the list than a fully opaque class but whatever.

  4. You question is tagged C++ however you use plain C-arrays in the constructor dList(int arrayNums[], char arrayChars[], int size);. Here std::vector would be better. Also think about what happens when one of the arrays is shorter than size?

     dList::dList(const std::vector<int>& arrayNums, const std::vector<char>& arrayChars) {
         if (arrayNums.size() != arrayChars.size()) {
             //Do something here, e.g. throw an exception or whatever you want
         }
    
         for (std::size_t i = 0; i < arrayNums.size(); ++i) {
             addBack(arrayNums[i], arrayChars[i]);
         }
      }
    
Source Link
miscco
  • 4.4k
  • 12
  • 17

I completely aggree with what the others said, but here some additional remarks.

  1. Use default initialization when possible

     class node {
     public:
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    
  2. Provide a constructor for your node class that passes key and type

     class node {
     public:
         node(int Key, char Type)
             : key(Key)
             , type(Type)
         {}
    
         node *prev = nullptr;
         node *next = nullptr;
         int key;
         char type;
     };
    

    Lets have a look how that affects you helper functions addFront firt

     void dList::addFront(int k, char t) { // creates new node at front of list
         node *newNode = new node(k, t);
         newNode->next = head;          
         if (head != nullptr) {
             head->prev = newNode;
         }
         head = newNode;
    
         // If you list was constructed with empty arrays you need top set tail too
         if (tail ==nullptr) {
             tail = newNode;
         }
     }
    

    Is there anyreason you traverse the list when you have tail for it in addBack?

     void dList::addBack(int k, char t) { // creates new node at front of list
         // If there is no Head make a shortcut to AddFront
         if (head == nullptr) {
             AddFront(k,t);
         }
    
         node *newNode = new node(k, t);
         tail->next = newNode
         newNode->prev = tail;
         tail = newNode;
     }
    
  3. More a personal preference, but i wouldrather use a strut that is private to the list thatn afully opaque class but whatever.

  4. You question is tagged C++ however you use plain C-arrays in the constructor dList(int arrayNums[], char arrayChars[], int size);. Here std::vector would be better. Also think about what happens when one of the arrays is sorter than size?

     dList::dList(const std::vector<int>& arrayNums, const std::vector<char>& arrayChars) {
         if (arrayNums.size() != arrayChars.size()) {
             //Do something here, e.g. throw an exception what ever you want
         }
    
         for (std::size_t i = 0; i < arrayNums.size(); ++i) {
             addBack(arrayNums[i], arrayChars[i]);
         }
      }