0

Here is my class

#include <fstream>
#include <cstdlib>
#include <math.h>
#include <iomanip>
#include <iostream>
using namespace std;

class Point {
  protected:
    int x, y;
    double operator-(const Point &def){ 
        return sqrt(pow((x-def.x),2.0)+ 
                  pow((y-def.y),2.0));
    }

};

class Circle: public Point {
  private:
    int radius;

  public:
  Circle(){     
    this->x=x;
    this->y=y;
    this->radius=radius;
  }

  Circle(int x, int y, int radius){
this->x=x;
this->y=y;
this->radius=radius;
}
    void printCircleInfo() {
      cout << x << " " << y << " " << radius << " " ;
    }
bool operator==(const Circle &def){ 
  return (x==def.x) & (y==def.y) & (radius==def.radius);
}
    bool doIBumpIntoAnotherCircle(Circle anotherCircle){
      if (anotherCircle.radius + radius >=   *this - anotherCircle    )
    return true;
      return false;
    }

};

Here is Main

int main(){
  int x,y,radius;
  const int SIZE = 13;
  Circle myCircleArry[SIZE];

Here I load 5 9 3 into position 13 of the array

  myCircleArry[13] = Circle(5,9,3);
   cout << endl;
   myCircleArry[13].printCircleInfo(); cout << " : ";
  ifstream Lab6DataFileHandle;

  Lab6DataFileHandle.open("Lab6Data.txt");

This where I load the text file into the array it contains

7 2 3
2 6 8
1 5 10
5 2 2
5 9 3
5 10 5
3 2 3
2 5 9
5 9 3
3 5 1
1 5 3
5 8 3
  while (!Lab6DataFileHandle.eof( )) {
 for (int i = 0; i < SIZE; i++) {
Lab6DataFileHandle>>x;
Lab6DataFileHandle>>y;
Lab6DataFileHandle>>radius;
 myCircleArry[i] = Circle(x,y,radius);
}
for (int i = 0; i < SIZE; i++) {
 if (myCircleArry[13].doIBumpIntoAnotherCircle(myCircleArry[i])) {
      myCircleArry[i].printCircleInfo();

When it reaches here it outputs 5 9 3 : 7 2 3 ; 2 6 8 ; 1 5 10 ; 5 2 2 ; 5 9 3 ; 5 10 5 ; 3 2 3 ; 2 5 9 ; 5 9 3 ; 3 5 1 ; 1 5 3 ; 5 8 3 ; 5 8 3

 if ( myCircleArry[13]==myCircleArry[i])
 {cout <<"*";}

cout << " ; ";
  }

}    
}
  Lab6DataFileHandle.close();

When I try to print position 13 of the array again 5 3 8 is printed

 myCircleArry[13].printCircleInfo();

}

Why is there a extra output and why does position 13 change? Please include an example in you answer. Thank You for your time.

MikeCAT
  • 73,922
  • 11
  • 45
  • 70
MR Hones
  • 61
  • 9
  • 1
    Your `myCircleArry` has only 13 elements, so `myCircleArry[13]` is out-of-range and you must not use its value. Also your usage of `while (!Lab6DataFileHandle.eof( ))` may be [wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). – MikeCAT Aug 03 '20 at 16:07
  • Counting starts from `0`. position 13 of the array is actually 12 – Waqar Aug 03 '20 at 16:08
  • Does this answer your question? [Why is iostream::eof inside a loop condition (i.e. \`while (!stream.eof())\`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) – Asteroids With Wings Aug 03 '20 at 16:27

2 Answers2

0

when you define an array of 13 elements.

You really only have 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 (0-12) indices.

An index accesses a specific element in an array.

This line here myCircleArry[13].printCircleInfo(); actually should say,

 myCircleArry[0].printCircleInfo();

0 is where the array actually starts.

Geno C
  • 1,401
  • 3
  • 11
  • 26
0

Minimal fix

As others already pointed out one of the main problems is that you allocate an inbuilt array of 13 circles and you try to access position 13 which is right outside the memory you allocated.

From your code I understand that your aim was to read your file, which actually has 12 elements, so all following loops try to read one Circle too much and use one Circle too much. Probably it would be better to use a separate variable for your "reference" circle.

Moreover you are using a "while not eof()" loop, which is typically(always) wrong, but in this case it makes no sense, since you already know and decided that the file will contain 12 elements. So the minimal fix for your main could be:

int main() {
    int x, y, radius;
    const int SIZE = 12;
    Circle myCircleArry[SIZE + 1];

    myCircleArry[SIZE] = Circle(5, 9, 3);
    cout << endl;
    myCircleArry[SIZE].printCircleInfo(); cout << " : ";
    ifstream Lab6DataFileHandle;

    Lab6DataFileHandle.open("Lab6Data.txt");
    for (int i = 0; i < SIZE; i++) {
        Lab6DataFileHandle >> x;
        Lab6DataFileHandle >> y;
        Lab6DataFileHandle >> radius;
        myCircleArry[i] = Circle(x, y, radius);
    }
    for (int i = 0; i < SIZE; i++) {
        if (myCircleArry[SIZE].doIBumpIntoAnotherCircle(myCircleArry[i])) {
            myCircleArry[i].printCircleInfo();

            if (myCircleArry[SIZE] == myCircleArry[i])
            {
                cout << "*";
            }

            cout << " ; ";
        }

    }
    Lab6DataFileHandle.close();

    myCircleArry[SIZE].printCircleInfo();
}

Notice that we know that SIZE elements are in the file, we allocate SIZE+1 slots and use the one in position SIZE as the reference one. Loops go from 0 to SIZE (excluded).

Major restructuring

This should probably go to Code Review, but since we are here...

Old includes

These

#include <cstdlib>
#include <math.h>

could become

#include <cmath>

You don't need the old C standard library.

Avoid using namespace std;

Plenty of anwsers here explain why.

My take on your Point

This is more my opinion than actual problems. Your class

class Point {
protected:
    int x, y;
    double operator-(const Point &def) {
        return sqrt(pow((x - def.x), 2.0) +
            pow((y - def.y), 2.0));
    }
};

uses protected attributes, which appears usless overdesign. Moreover it disables the ability of initializing with braces.

operator- between points shouldn't give the distance. It's really counter intuitive with the math everybody studies at school.

Later you will compare circles centers, so an operator== is useful.

Finally why not using double coordinates?

Result:

struct Point {
    double x, y;

    friend double distance(const Point &a, const Point &b) {
        return sqrt(pow(a.x - b.x, 2.0) + pow(a.y - b.y, 2.0));
    }

    bool operator==(const Point &rhs) const {
        return x == rhs.x && y == rhs.y;
    }
};

My take on your Circle

This is your Circle:

class Circle : public Point {

Honestly, a circle is not a Point, so I don't think hineritance is the right choice.

private:
    int radius;

This is really personal: I'd avoid protecting the radius attribute, allowing users to access it.

public:
    Circle() {
        this->x = x;
        this->y = y;
        this->radius = radius;
    }

This is plain wrong. It tells the compiler that the default initialization must take the class (uninitialized) attributes and use them to initialize the same things, because here this->x is the same as x (the same holds for y and radius). Drop this useless thing.

    Circle(int x, int y, int radius) {
        this->x = x;
        this->y = y;
        this->radius = radius;
    }

This makes sense, but my personal choice is to follow Google Style and designate object attributes with an underscore after the name. Others prefer m_ in front of the name. This allows me to avoid the this-> mess and the accidental use of those members.

    void printCircleInfo() {
        cout << x << " " << y << " " << radius << " ";
    }

This is fine, but I'd go for a more standard inserter/extractor couple.

    bool operator==(const Circle &def) {
        return (x == def.x) & (y == def.y) & (radius == def.radius);
    }

Wrong use of &. You definitely want &&.

    bool doIBumpIntoAnotherCircle(Circle anotherCircle) {
        if (anotherCircle.radius + radius >= *this - anotherCircle)
            return true;
        return false;
    }
};

Terrible name (again: personal opinion). Methods should talk about the object they are applied to.

anotherCircle is copied for no reason.

Classic if (something) return true; else return false; which is easily substituted by return something;.

So I'd go with:

struct Circle {
    Point center_;
    double radius_;

    friend std::ostream& operator<<(std::ostream& os, const Circle& c) {
        return os << c.center_.x << " " << c.center_.y << " " << c.radius_;
    }

    friend std::istream& operator>>(std::istream& is, Circle& c) {
        return is >> c.center_.x >> c.center_.y >> c.radius_;
    }

    bool operator==(const Circle &rhs) {
        return center_ == rhs.center_ && radius_ == rhs.radius_;
    }

    bool touches(const Circle& other) {
        return radius_ + other.radius_ >= distance(center_, other.center_);
    }
};

Now the main function

Let's make the program able to read an arbitrary number of circles and compare them to your reference. Stop using inbuilt arrays and jump to std::vector<>.

int main() 
{
    Circle ref = { 5, 9, 3 };
    std::cout << ref << " : ";

    std::vector<Circle> circles;

    std::ifstream is("Lab6Data.txt");
    Circle cur;
    while (is >> cur) {
        circles.push_back(cur);
    }

    for (const auto& c : circles) {
        if (ref.touches(c)) {
            std::cout << c;
            if (ref == c) {
                std::cout << " *";
            }
            std::cout << " ; ";
        }
    }
}

Notice that we can initialize Circle with braces. You can also drop the = and go for uniform initialization.

Opening a stream is just calling the constructor. Closing will be automatically done at object destruction.

The loop now uses the extractor to read a Circle. Honestly this is compact, but the version that I think is the most didactic is this:

while (true) {
    // Read
    Circle cur;
    is >> cur;
    // Check
    if (!is) {
        break;
    }
    // Use
    circles.push_back(cur);
}

Notice the pattern: infinite loop with Read/Check/Use. Check is where we can exit the loop. First you read, than you check if the reading operation was successful or failed, than you can use the data or exit based on that. Believe me: write your loops like that, than, if you really must, change them to other forms.

Ok, we read all circles in the file. Now for all circles (notice the use of a range-based for loop with a const auto reference) if the reference touches the current circle we print it (with a * if it is equal to the reference).

Hope this long runt has some use...

Full code for easier copy and paste

#include <fstream>
#include <cmath>
#include <iomanip>
#include <iostream>
#include <vector>

struct Point {
    double x, y;

    friend double distance(const Point &a, const Point &b) {
        return sqrt(pow(a.x - b.x, 2.0) + pow(a.y - b.y, 2.0));
    }

    bool operator==(const Point &rhs) const {
        return x == rhs.x && y == rhs.y;
    }
};

struct Circle {
    Point center_;
    double radius_;

    friend std::ostream& operator<<(std::ostream& os, const Circle& c) {
        return os << c.center_.x << " " << c.center_.y << " " << c.radius_;
    }

    friend std::istream& operator>>(std::istream& is, Circle& c) {
        return is >> c.center_.x >> c.center_.y >> c.radius_;
    }

    bool operator==(const Circle &rhs) const {
        return center_ == rhs.center_ && radius_ == rhs.radius_;
    }

    bool touches(const Circle& other) const {
        return radius_ + other.radius_ >= distance(center_, other.center_);
    }
};

int main() 
{
    Circle ref = { 5, 9, 3 };
    std::cout << ref << " : ";

    std::vector<Circle> circles;

    std::ifstream is("Lab6Data.txt");
    Circle cur;
    while (is >> cur) {
        circles.push_back(cur);
    }

    for (const auto& c : circles) {
        if (ref.touches(c)) {
            std::cout << c;
            if (ref == c) {
                std::cout << " *";
            }
            std::cout << " ; ";
        }
    }
}
Costantino Grana
  • 3,132
  • 1
  • 15
  • 35