Hej!

Jag förstår inte varför min kod crashar vid utskriften av listan. Tacksam för tips och hjälp.

<b>Main:</b>

<code>
#include <iostream>
#include "vertex.h"
#include "ShapeList.h"
#include "Shape.cpp"
using namespace std;
int main() {
ShapeList list;
Vertex varr[] = { Vertex(1, 1), Vertex(10, 0), Vertex(5, 2), Vertex(5, 5) };
list.add(Polygon(1, 4, varr, 4));
list.print();
Vertex nyarr[] = { Vertex(2,2), Vertex(9,2), Vertex(7,4), Vertex(3,3) };
list.add(Polygon(6, 8, nyarr, 2));
return 0;
}
</code>

<b>ShapeList.h:</b>

<code>
#ifndef SHAPELIST_H
#define SHAPELIST_H
#include "vertex.h"
#include "Shape.h"
using namespace std;

class ShapeNode;

class ShapeList {
protected:
ShapeNode* ny;
public:
ShapeList();
ShapeList(const ShapeList& shapes);
~ShapeList();
void add(const Shape& s);
void remove(const Vertex& v);
double area();
void print();
};
#endif
_____

# ShapeList.cpp:

#include <iostream>
#include "ShapeList.h"
using namespace std;

class ShapeNode {
public:
const Shape* s;
ShapeNode* bak;
ShapeNode* fram;
ShapeNode(const Shape* _s, ShapeNode* _bak, ShapeNode* _fram)
: s(_s), bak(_bak), fram(_fram) {}
};

ShapeList::ShapeList() : ny(0) {}
ShapeList::ShapeList(const ShapeList& shapes) {}
ShapeList::~ShapeList() {}
void ShapeList::add(const Shape& s) {
ShapeNode *tmp = ny;
ny = new ShapeNode(&s, tmp, 0);
ny->bak = ny;
}
void ShapeList::print() {
Shape* Shp;
ShapeNode* p;
for (p=ny->fram; p!=ny; p=p->fram)
Shp->printShape(); <--------------- !!!
}
</code>

<b>Shape.h:</b>

<code>
#ifndef SHAPE_H
#define SHAPE_H
#include <iostream>
#include <string>
using namespace std;

class Shape {
public:
Shape() {}
~Shape() {}
virtual Shape* clone() const = 0;
virtual double area() = 0;
virtual void printShape() = 0;
};
#endif
_____

# Shape.cpp:

#include <iostream>
#include <cmath>
#include "Vertex.h"
#include "Shape.h"
using namespace std;

class Polygon : public Shape {
private:
Vertex* Pol;
int storlek, pX, pY;
public:
Polygon::Polygon(int x = 0, int y = 0, Vertex* varr = 0, int num = 0) : pX(x), pY(y), storlek(num) {
if (storlek > 0) {
Pol = new Vertex[storlek];
for (int ix=0; ix<storlek; ix++)
Pol[ix] = varr[ix];
} else Pol = 0;
}
virtual Polygon::~Polygon() { delete[] Pol; }
virtual Shape* clone() const { return new Polygon; }
virtual double Polygon::area() {
double s = 0.0;
double r = 0.0;
for (int ix=1; ix<storlek; ix++)
s += (Pol[ix -1].Xvrd() * Pol[ix].Yvrd()) - (Pol[ix].Xvrd() * Pol[ix -1].Yvrd());
return abs(s / 2);
}
void Polygon::printShape() {
cout << "POLYGON: (" << pX << "," << pY << ") { ";
for (int ix=0; ix<storlek; ix++)
cout << "(" << Pol[ix].Xvrd() << ", " << Pol[ix].Yvrd() << ") ";
cout << "}" << endl;
}
};
</code>

<b>Vertex.h:</b>

<code>
#ifndef VERTEX_H
#define VERTEX_H
#include <iostream>
using namespace std;

class Vertex {
protected:
int x, y;
public:
Vertex() : x(0), y(0) {}
Vertex(int _X, int _Y) : x(_X), y(_Y) {}
~Vertex() {}
int Xvrd() { return x; }
int Yvrd() { return y; }
};
#endif
</code>


Tack på förhand.

Adalte.

Tips:
1. Använd lite vettigare namn, "ny" borde heta typ "head".
2. Se till att testa av varje del av koden separat, nu är det inte så enkelt att se vad som funkar och inte (istället för Shape *, så borde du istället ha haft en int eller så).
3. Jag antar att det är någon slags skoluppgift? För annars ska du förstås använda std::list, inte skriva en egen.

Hur som helst:

void ShapeList::add(const Shape& s) {
  ShapeNode *tmp = ny;
  ny = new ShapeNode(&s, tmp, 0);
  ny->bak = ny;
}

Detta är inte rätt. Ska du sätta in nytt värde i början ska du göra några saker:
1. Skapa nytt värde som pekar rätt framåt och bakåt.
2. Peka om head-värdet
3. Se till att det föregående head-värdet har rätt bakåtpekare.
Du gör nästan rätt, även om det blir lite rörigt med de namnvalen. Det du borde skriva är:

void ShapeList::add(const Shape& s) {
  ShapeNode *tmp = ny;
  ny = new ShapeNode(&s, tmp, 0);
  ny->fram->bak = ny;
  //eller
  tmp->bak = ny;
}

Sen är print-rutinen felaktig:

void ShapeList::print() {
  Shape* Shp;
  ShapeNode* p;
  for (p=ny->fram; p!=ny; p=p->fram)
      Shp->printShape();
}

Det är inte mycket som är rätt där.
- Shp initieras inte alls men används.
- Loopen börjar ett steg in.
- Loopen kommer aldrig att avslutas (såvida du inte har en cirkulär lista, vilket du inte har).

Blanda inte in shapes, håll dig till enklare grejer tills du har ordning på själva listan.

Hej Niklas!

Tack för tipserna.
Jo, det är en skoluppgift. Valde "ny" för att själv hålla koll på pekarna.

Visst är print-rutinen felaktig och det jag försöker göra är ju att skriva ut listan. Men som du själv påpekar loopen avslutas aldrig. Då undrar jag:

1. Hur ska det ser ut för att loopen ska börja från första element i listan.
2. Vilket ska vara vilkoret i for-loopen för att få _ett_ utskrift på listan.
3. Jag har försökt med följande print-rutin (utan resultat):

<code>
void ShapeList::print() {
const ShapeNode* p;
for (p=ny->fram; p!=ny; p=p->fram)
p->s->printShape();
}
</code>

Sist, jag är osäker om Shape verkligen läggs in i listan. Är inte så att Shapen destrueras efter att "void ShapeList::add(const Shape& s)" har körts klart?

Tack på förhand

Adalte.

<b>>Sist, jag är osäker om Shape verkligen läggs in i listan. Är inte så att Shapen destrueras efter att "void ShapeList::add(const Shape& s)" har körts klart?</b>
Shape & refererar till en shape. Om Shape:n tas bort så kommer det inte fungera, det är därför jag säger att du borde strunta i Shape helt först och fokusera på listan.

<b>>Valde "ny" för att själv hålla koll på pekarna.</b>
Och det gjorde att du villade bort dig... =)
Kalla den "head" istället, det är det man kallar den.

<b>1. Hur ska det ser ut för att loopen ska börja från första element i listan.</b>
for (p=ny; ... )

istället för

for (p=ny->fram; ... )

<b>2. Vilket ska vara vilkoret i for-loopen för att få _ett_ utskrift på listan.</b>
Eh?

Du verkar avsluta listan med en null-pekare?
Då ska villkoret bara vara p!=0 eller kortare: p

Alltså

<code>
void ShapeList::print() {
for (const ShapeNode* p=ny; p; p=p->fram)
p->s->printShape();
}
</code>

eller

<code>
void ShapeList::print() {
for (const ShapeNode* p=ny; p!=0; p=p->fram)
p->s->printShape();
}
</code>


Så förslaget är:
1. Låt den länkade listan ha int istället för shape.
2. Testa att göra en tom lista och skriv ut den.
3. Testa att göra en tom lista, lägg till ett element och skriv ut det. Skriv också gärna ut pekaradresser så att du ser vad det är som händer. Har du tillgång till någon bra debugger kan du kolla det så istället.
4. Testa att lägga till två element och skriv ut dem.
5. Sen kan du börja testa med hundra element eller så, men inte förrän alla tidigare steg funkar.
6. Slutligen byter du till ShapeList och tar hand om de problemen som uppstår.

Hej Niklas!

Tack för värdefulla råd. Jag "tränade" på dina råd och resultatet: listan fungerar nu (med hjälp av andra tips). Men det finns två saker som jag har inte lyckats fixa.

1- I ShapeList har jag en remove-funktion som anropa den virtuella funktionen Tabort() från <b>Shape</b>. Tanken är att Tabort() ska just ta bort en Vertex-array från Polygon genom att genföra parametern (Vertex-arrayen) med varje Polygons Vertex-array med hjälp av en for-loop. Problemet är att Vetex-parametern är const medan Vertex-Polygon inte är det och då klagar kompilatorn. Hur kan man komma runt eller lösa det på något annat sätt, tror du?

2- I ShapeList, i funktionen print, anropar jag den virtuella funktionen <b>objArea()</b> för att addera till en variabel arean från varje objekt. Det anropet leder till program-crash men kompilatorn klagar inte. Vad kan vara för fel?


<b>ShapeList.h</b>:

<code>
#ifndef SHAPELIST_H
#define SHAPELIST_H
#include "vertex.h"
#include "Shape.h"
using namespace std;

class ShapeNode;
class ShapeList {
protected:
ShapeNode* head;
public:
static int Rkn;
static double TotArea;
ShapeList();
ShapeList(const ShapeList& shapes);
~ShapeList();
void add(const Shape& s);
void remove(const Vertex& v);
double area();
void print();
};
#endif

// ShapeList.cpp:

#include <iostream>
#include "Vertex.h"
#include "ShapeList.h"
using namespace std;

class ShapeNode {
public:
const Shape* s;
ShapeNode* bak;
ShapeNode* fram;
ShapeNode(const Shape* _s, ShapeNode* _bak, ShapeNode* _fram)
: s(_s), bak(_bak), fram(_fram) {
}
~ShapeNode() {
delete s;
}
};
int ShapeList::Rkn = 0;
double ShapeList::TotArea = 0.0;
ShapeList::ShapeList() {
head = new ShapeNode(0, 0, 0);
head->fram = head->bak = head;
}
ShapeList::ShapeList(const ShapeList& shapes) {}
ShapeList::~ShapeList() {}
void ShapeList::add(const Shape& s) {
Rkn++;
const Shape* Kopian = &s;
Kopian = Kopian->clone();
ShapeNode *tmp = head;
head = new ShapeNode(Kopian, 0, tmp);
tmp->bak = head;
head->fram = tmp;
}
void ShapeList::remove(const Vertex& v) {
Shape* pek;
int Cnt = 0;
for (const ShapeNode* p=head; Cnt<Rkn; p=p->fram) {
Cnt++;
pek->Tabort(v); // <--- !!!
}
}
double ShapeList::area() { return TotArea; }
void ShapeList::print() {
Shape* par;
int Cnt = 0;
for (const ShapeNode* p=head; Cnt<Rkn; p=p->fram) {
p->s->printShape();
TotArea += par->objArea(); // <--- !!!
Cnt++;
}
cout << "Arean= " << TotArea << endl;
}
</code>


<b>Shape.h</b>:

<code>
#ifndef SHAPE_H
#define SHAPE_H
#include <iostream>
#include <string>
#include "Vertex.h"
using namespace std;

class Shape {
protected:
public:
Shape() {}
~Shape() {}
virtual void Tabort(const Vertex& Vx) = 0;
virtual Shape* clone() const = 0;
virtual double objArea() = 0;
virtual void printShape() const = 0;
};
#endif

// Shape.ccp:

#include <iostream>
#include <cmath>
#include "Vertex.h"
#include "Shape.h"
using namespace std;

class Polygon : public Shape {
private:
Vertex* Pol;
int storlek, pX, pY;
public:
Polygon::Polygon(int x = 0, int y = 0, Vertex* varr = 0, int num = 0) : pX(x), pY(y), storlek(num) {
if (storlek > 0) {
Pol = new Vertex[storlek];
for (int ix=0; ix<storlek; ix++)
Pol[ix] = varr[ix];
} else Pol = 0;
}
virtual Polygon::~Polygon() {
delete[] Pol;
}
virtual Shape* clone() const {
return new Polygon(pX, pY, Pol, storlek);
}
double Polygon::objArea() {
double s = 0.0;
for (int ix=1; ix<storlek; ix++)
s += (Pol[ix -1].X() * Pol[ix].Y()) - (Pol[ix].X() * Pol[ix -1].Y());
return abs(s / 2);
}
void Polygon::Tabort(const Vertex& Vx) {
for (int ix=0; ix<storlek; ix++)
if (Vx.X() == Pol[ix].X() && Vx.Y() == Pol[ix].Y())
delete Pol;
}
void Polygon::printShape() const {
cout << "POLYGON: (" << pX << "," << pY << ") { ";
for (int ix=0; ix<storlek; ix++)
cout << "(" << Pol[ix].X() << ", " << Pol[ix].Y() << ") ";
cout << "}" << endl;
}
};
</code>


<b>Vertex.h</b>:

<code>
#ifndef VERTEX_H
#define VERTEX_H
#include <iostream>
using namespace std;

class Vertex {
protected:
int x, y;
public:
Vertex() : x(0), y(0) {}
Vertex(int _X, int _Y) : x(_X), y(_Y) {}
~Vertex() {}
int X() { return x; }
int Y() { return y; }
};
#endif
</code>


Slutligen <b>main</b>:

<code>
#include <iostream>
#include "vertex.h"
#include "ShapeList.h"
#include "Shape.cpp"
using namespace std;
// g++ Vertex.h ShapeList.cpp Shape.cpp ShapeTest.cpp -o Zhapes.exe
int main()
{
ShapeList list;
Vertex varr[] = { Vertex(1, 1), Vertex(10, 0), Vertex(5, 2), Vertex(5, 5) };
list.add(Polygon(1, 4, varr, 4));
list.print();
Vertex nyarr[] = { Vertex(2,2), Vertex(9,2), Vertex(7,4), Vertex(3,3) };
list.add(Polygon(6, 8, nyarr, 4));
list.print();
return 0;
}
</code>


Tack på förhand.
Adalte.

Några grejer:
1. Länkade listor kan antingen vara cirkulära:
node1 -> node2 -> node3 -> node4 -> node1
då pekar första elementets bak-pekare på sista elementet, och vice versa.

eller 0-terminerade:
0 -> node1 -> ... node4 -> 0

Du verkar göra både och:

head = new ShapeNode(0, 0, 0);
head->fram = head->bak = head;

resp.
head = new ShapeNode(Kopian, 0, tmp);



Om man inte kör med en cirkulär lista så bör en tom lista ha head = NULL, alltså:

ShapeList::ShapeList() {
head = new ShapeNode(0, 0, 0);
head->fram = head->bak = head;
}

borde istället vara

ShapeList::ShapeList() {
head = NULL;
//Eller head = 0; beroende på smak
}


och

void ShapeList::add(const Shape& s) {
Rkn++;
const Shape* Kopian = &s;
Kopian = Kopian->clone();
ShapeNode *tmp = head;
head = new ShapeNode(Kopian, 0, tmp);
tmp->bak = head;
head->fram = tmp;
}

borde vara

void ShapeList::add(const Shape& s) {
Rkn++;
const Shape* Kopian = &s;
Kopian = Kopian->clone();
ShapeNode *tmp = head;
head = new ShapeNode(Kopian, 0, tmp);
tmp->bak = head;
}

2. Räknaren bör du inte ha eller använda. Det behövs inte och är inte det snygga sättet att lösa det.

3. Removefunktionen tilldelar aldrig någonting till "pek"
4. Dela upp beräkningen och utskriften. Samma sak där, oinitierade variabler. Jag skriver en loop för att räkna ut arean så ser du vad du gör för fel:

double ShapeList::area() {
double tot_area=0;
for (const ShapeNode* p=head; p!=0; p=p->fram) {
tot_area += p->objArea();
}
return tot_area;
}

5. Slutligen så kommer detta inte funka:
if (Vx.X() == Pol[ix].X() && Vx.Y() == Pol[ix].Y())
delete Pol;

"delete Pol" kommer ta bort första elementet i arrayen.
mer korrekt är "delete Pol[ix]", men det duger inte heller. Vad du ska göra är att ta bort det elementet, vilket innebär att alla element efter måste flyttas ett steg bak.

Hej Niklas!

Jag trodde jag hade en cirkulär-lista. Anledning till att jag hade

<b>head = new ShapeNode(0, 0, 0);</b>

var att jag ville ha "ett speciellt första listelement" där "det första element, vars värde är oväsentligt, inte tillhör själva listan." - detta enl. boken. Men det är lite förvirrande det där med pekarna.


I <b>Area</b>-funktionen i ShapeList slarvade jag och den ska se ut:

<code>
double ShapeList::area() {
double TotArea;
for (const ShapeNode* p=ny; p!=0; p=p->fram)
TotArea += p->s->objArea();
return TotArea;
}
</code>
Tack för påpekande.


Men däremot <b>remove</b>-funktionen (i ShapeList) är svårare att lösa. Det går inte med "p->s->Tabort(v);" kompilator klagar:
"ShapeList.cpp: In member function `void ShapeList::remove(const Vertex&)':
ShapeList.cpp:62: error: no matching function for call to `Shape::Tabort(const Vertex&) const'
Shape.h:18: note: candidates are: virtual void Shape::Tabort(Vertex&)"

Så jag löste det genom att skapa en metod i Shape<b>Node</b>:

<code>
void radera(const Vertex& v) const {
Vertex vx = v;
Shape* pk;
pk->Tabort(vx);
}
</code>
som jag anropar från Shape<b>List</b>ans remove-funktionen:

<code>
void ShapeList::remove(const Vertex& v) {
for (const ShapeNode* p=head; p!=0; p=p->fram) {
p->radera(v);
// p->s->Tabort(v);
}
}
</code>
Det 'verkar' som om jag kommer runt problemet: kompilatorn klagar inte och programmet crashar inte <b>men</b> objektet inte tas bort alls!


"Vad du ska göra är att ta bort det elementet" skriver du: det är precis vad jag försöker göra och det kommando jag känner för det är ju delete. Hur annars kan man ta bort elementet då, undrar jag.

Adalte.

Tyvärr för upptagen för mer ingående svar, men återigen: Du _måste_ tilldela värden till variabler.

Shape* pk;
pk->Tabort(vx);

kan aldrig någonsin fungera!


Det riktiga problemet du hade innan var att måste använda rätt constness;'
virtual void Shape::Tabort(Vertex&)
borde vara
virtual void Shape::Tabort(const Vertex&)

Stor tack för dina tips och hjälp, Niklas.

Adalte.