0

Hi everyone I'm a newbie to programming and self-taught C++ by a website. I'm writing a program to calculate the distance between the beginning and the end point. Here's my code

File Point.cpp:

#include<iostream>
#pragma once

using namespace std;

class Point {
private:
    int x;
    int y;
public:
    Point() {
        
    }
    Point(int x, int y) {
        this->x = x;
        this->y = y;
    }
    void setX(int x) {
        this->x = x;
    }
    int getX() {
        return x;
    }
    void setY(int y) {
        this->y = y;
    }
    int getY() {
        return y;
    }
};

File Line.cpp :

#include<iostream>
#include<math.h>
#include "Point.cpp"

class Line {
private:
    Point begin;
    Point end;
public:
    Line(Point begin, Point end) {
        this->begin = begin;
        this->end = end;
    }
    Line(int x1, int y1, int x2, int y2) {
        x1 = begin.getX();
        y1 = begin.getY();
        x2 = end.getX();
        y2 = end.getY();
    }
    void setBegin(Point begin) {
        begin.setX(int x1);
        begin.setY(int y1);
    }
    Point getBegin() {
        return begin;
    }
    void setEnd(Point end) {
        end.setX(int x2);
        end.setY(int y2);
    }
    Point getEnd() {
        return end;
    }
    double getLength() {
        return sqrt( (begin.getX() - end.getY())*(begin.getX() - end.getY()) + (begin.getX() - end.getY())*(begin.getX() - end.getY()) );
    }
    
};

In file Line.cpp, the setEnd and setBegin can't run and the error said "type name is not allowed" for the parameters of setters although I've made pointers for them.

Hope anyone can have a good explanation and fix my code. Thank you.

Ngô Sơn
  • 5
  • 2
  • Don't use websites to learn C++, use [good books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). [Why is `using namespace std;` considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – Evg Jun 17 '21 at 16:45
  • 2
    It looks like you want `this->begin.setX(begin.x)`. Naming your parameters the same as the members creates all sorts of problems and confusion. Name the parameters something different and you can drop all the `this->` stuff. – 1201ProgramAlarm Jun 17 '21 at 16:45
  • 1
    Take a closer look at lines like `begin.setX(int x1);` or `end.setX(int x2);` – Ruzihm Jun 17 '21 at 16:46
  • @1201ProgramAlarm Or simply `this->begin = begin;` – MikeCAT Jun 17 '21 at 16:50
  • @Evg Can you recommend me any good books? – Ngô Sơn Jun 17 '21 at 16:51
  • 1
    Also note that using `.cpp` file for `#include` is a bad havit that may lead to muitiple-definition error. In this case it looks like just the name is wrong and they should named as `.h` or `.hpp`. – MikeCAT Jun 17 '21 at 16:51
  • @Ruzihm those are where my error comes and I'm asking abt that :))) – Ngô Sơn Jun 17 '21 at 16:53
  • @NgôSơn Evg gave two links. This first is to Stack Overflow's volunteer-curated list of well-regarded C++ programming texts. – user4581301 Jun 17 '21 at 16:56
  • 1
    `Point() {}` -- You failed to initialize the members of `Point` in the default constructor. Thus trying to get the distance will lead to all sorts of weird results. – PaulMcKenzie Jun 17 '21 at 16:57
  • 1
    `Line`'s second constructor doesn't make any sense. What do you expect it to accomplish? – Sam Varshavchik Jun 17 '21 at 16:57
  • 1
    @NgôSơn In `setBegin` what is the `int x1` meant to do? what happens to `begin` when that function ends? – Ruzihm Jun 17 '21 at 16:58
  • @SamVarshavchik I want to give x1, y1 as coordinates of begin point and x2,y2 the coordinates of another point – Ngô Sơn Jun 17 '21 at 17:00
  • 1
    @NgôSơn -- `double length = Line(Point(), Point()).getLength();` -- what do you think `length` will be equal to? No one knows since `Point` has a default constructor that doesn't initialize the `x` or `y` members. The `length` could be `0`, `1.0`, `432.324382,` you don't know. – PaulMcKenzie Jun 17 '21 at 17:08
  • 2
    But why do you "give x1, y1 as coordinates of begin point and x2,y2 the coordinates of another point" in a constructor of a completely unrelated object? This is a constructor for the object that ***contains these points*** and ***receives x1, y1, x2, and y2 as parameters***. As Mr. Spock would say: "this is highly illogical". – Sam Varshavchik Jun 17 '21 at 18:08
  • "Setters" really get my goat. To me they are the quintessential antipattern, making your code tightly-coupled peek-poke style, puncturing any encapsulation you have provided. "Getters" are OK. Get into the habit of thinking of "operations to perform on an object" instead of poking attributes like an assembly programmer :) – Den-Jason Jun 17 '21 at 21:35
  • With what you have here, I would be writing "xDiff" and "yDiff" methods in Point, so you can get the x,y deltas between two points. Then your getLength uses them and does the Pythagoras to calculate the result. – Den-Jason Jun 17 '21 at 21:41

2 Answers2

2

Like @Ruzihm perfectly noticed, a call to a function shall not specify the type of the variable passed to the function...

In your code:

void setBegin(Point begin) {
    begin.setX(int x1);
    begin.setY(int y1);
}

Remove the int where you call the functions, and it should already go a little better... :P

Note
After a second reading, I would rather say that it should at least compile maybe...
But it seems that there are other problems in your code...

Most Obvious One
The constructor Line(int x1, int y1, int x2, int y2) is assigning the values of the coordinate stored by the members begin and end to the parameters of the constructor itself...

This is clearly something that will never do what you expect... You are changing the values of the argument of the constructor. It is allowed and causes no errors, but it will have absolutely not side effect (it won't change the state of the constructed object)...

(keep trying – things cannot be learned without doing mistakes)

Tenphase
  • 101
  • 5
0

@Tenphase has already mentioned that you should not put int in the line:

begin.setX(int x1)
           ^^^

However, there seems to have a lot more problem with your code here. Your Point class seems to work fine, however you seems to have a lot of problems with your Line class.

For instance, in your setBegin function, even after removing the ints in there:

void setBegin(Point begin) {
    begin.setX(x1);
    begin.setY(y1);
}
  • First, you don't actually have x1 or y1. So attempting to call x1 would fail right away.
  • Second, let's say you do have x1 and y1 as the x and y for your beginning point, and imagining you already have a Line line defined, and you will be calling setBegin like:
Point p1(0, 5);
line.setBegin(p1);

What this does is not actually setting the beginning point of your line to (0,5). Instead, you are first creating a copy of p1 and pass that to the setBegin function. Then you perform setX and setY on that copied p1, and set it to the location of your line's x1 and y1.

However, this doesn't actually affect anything. Your line remains unchanged, and the p1 that was modified was actually a copy of your original p1, which will be deleted right after the function.

What you actually want might be:

void setBegin(Point new_point) {
    begin.setX(new_point.getX());
    begin.setY(new_point.getY());
}

Here I changed the parameter begin to new_point, so it's easier to differentiate it with this->begin. Then it sets x of begin to x of new_point, and sets y of begin to y of new_point.

Alternatively, you can also use:

void setBegin(Point new_point) {
    begin = new_point;
}

Now you just set begin as the same as new_point.

The exact same problem happens to your setEnd

While a little bit different, but your Line(int x1, int y1, int x2, int y2) also suffers similar problems.

x1 = begin.getX();

It first creates a copy of x1, then modify the value of x1 to the x of begin. Instead, you are likely looking for:

begin.setX(x1);
Ranoiaetep
  • 5,872
  • 1
  • 14
  • 39