Unexpected output zero

Question

I'm new to C++ and currently practicing on a Singly Linked List. Somehow the output of the code below is always zero. I think the problem is the nextPoint Method but however I try to change the reference/dereference, it doesn't work.

Where is the problem? Thank you in advance.

// Singly Linked List

#include <math.h>
#include <iostream>

class Point {

        public:
        double x, y;
        Point* next;

        // constructor
        Point (double x, double y) {
                this->x = x;
                this->y = y;
                this->next = NULL;
        }

        void nextPoint(Point nexti) {
                this->next = &nexti;
        }

        double dist(Point &a, Point &b) {
                double dx = a.x - b.x;
                double dy = a.y - b.y;
                return sqrt(dx*dx - dy*dy);
        }

        double length() {
                Point *iter = this;
                double len = 0.0;
                while (iter->next != NULL) {
                        len += dist(*iter, *iter->next);
                        iter = iter->next;
                }
                return len;
        }

};

int main() {

        Point p1(1,1);
        Point p2(2,2);
        Point p3(5,5);
        p1.nextPoint(p2);
        p2.nextPoint(p3);

        std::cout << p1.length() << std::endl;

        return 1;

}

Answer 1

Please turn on more compiler warnings and you'll probably get a warning that in nextPoint you are storing the address of a temporary variable (nexti) permanently (in this->next).

You must either pass the address of or a reference to the point to add.

void nextPoint(Point *nexti) {
    this->next = nexti;
}

p1.nextPoint(&p2);
p2.nextPoint(&p3);

or

void nextPoint(Point &nexti) {
    this->next = &nexti;
}

p1.nextPoint(p2);
p2.nextPoint(p3);

Side note: please replace NULL with nullptr.

Answer 2

There are two problems with your code:

1. nextPoint takes its parameter by value, which means you're storing the address of that by-value parameter which becomes invalid as soon as the execution of nextPoint ends. Change it to accept Point &nexti.

2. Your distance computation function is wrong. You should be adding the squares, not subtracting them: return sqrt(dx*dx + dy*dy);

---

Unrelated to your question, but there are several ways in which you could improve your code:

• Use the mem-initialiser list in the constructor to initialise members instead of assigning to them. This is a good habit to get into, as it will come useful once you start dealing with things where initialisation and assignment are substantially different (references, classes, ...).

  Point (double x, double y) : x(x), y(y), next(nullptr)
  {}
  

• Use nullptr instead of NULL, since the latter is not type-safe.

• length should be marked const, because it does not modify the object on which it's called. Note that iter has likewise been changed to const Point *:

  double length() const {
          const Point *iter = this;
          double len = 0.0;
          while (iter->next != NULL) {
                  len += dist(*iter, *iter->next);
                  iter = iter->next;
          }
          return len;
  }
  

• dist does not use this at all, and so it could (and should) be made a static member function. Also, it should take its parameters by const &, because it doesn't modify them:

  static double dist(const Point &a, const Point &b) {
          double dx = a.x - b.x;
          double dy = a.y - b.y;
          return sqrt(dx*dx - dy*dy);
  }