OOP proposal about having two instances of the same class on another

This has caused me great headache, making me delete well-working code to write another and then return to the first and so on.

Code:

---

class X();

class A{
int key;
X *n, *p;
public:(some methods here)
};

---

Now, I want to make a class B, with two ints, and four pointers, this time pointing to n(ext), p(revious), (u)p,d(own). (Note: my real task is making a sparse array). You'll notice this is two sets of class A, and indeed I need each set to have the same method class A has for it. From what I've read, I have the following options:

Code:

---

1)class from scratch containing all data/methods
class B{
int key1,key2;
X *n,*p,*u,*d;
public:(previous methods, and a copy+search->replace of them for key2/u/d)
}

2)class containing classes
class B{
A c1,c2;
}

3)class inheriting one and containg other:
class B:public A{
A c2;
}

4)class inheriting both, using :: in methods to rid ambiguity:
class A1:public A{};
class A2:public A{};
class B:public A1,public A2{};

---

What should I do? 1) sounds silly to me, because isn't OOP meant to avoid copy-pasting? 2) sounds better, but can I not avoid having an additional . operator all the time? 3) seems very asymmetrical to me. 4) seems good, but I have to type classname:: which IS long, plus it invokes fear in me because I see I can manipulate private vars.

What should I choose and why? And if there is another way, please do tell! Thanks!

5) Use a separate base class (possibly virtual) from which class A and B both inherit.

I don't understand how this would solve the problem. Why care about A - it is fine as it is.

Abstract the common subset of A and B into its separate base class that you can inherit from (in this case, you can do protected or private inheritance).
This way, B does not depend on A. So B will get all of As members (which is a common subset).
As for the pointers, it makes more sense to inherit next/prev from A, then define new up and down.

The common subset of A and B is A itself. I don'tt really get why you said that about protected or private inheritance. Actually, as far I'm concerned, I might have used structs everywhere: its strange to me that i make private vars and make a get() and set() public functions for each. I suppose it will make sense in the passing of time as I understand OOP more.

So, Elysia, you propose a hybrid 1) / 3) solution. Still, I'd need to copy the methods of A for the new pointers I'll define. I count your opinion though. Thanks.

Quote:

---

Originally Posted by Lord Asriel

The common subset of A and B is A itself. I don'tt really get why you said that about protected or private inheritance. Actually, as far I'm concerned, I might have used structs everywhere: its strange to me that i make private vars and make a get() and set() public functions for each. I suppose it will make sense in the passing of time as I understand OOP more.

---

Read up a little on inheritance. You'll see there are 3 different types of inheritance.
Getter and setter functions are vital. You will curse yourself for not doing them when you suddenly want to change your implementation in your getting and setting code if you don't make getters and setters.
By putting all the get and set code into functions, you condense an action into a single point into the code. So whenever the code to change that action changes, you only have to change you getter or setter and the rest of the code compiles fine!

Quote:

---

So, Elysia, you propose a hybrid 1) / 3) solution. Still, I'd need to copy the methods of A for the new pointers I'll define. I count your opinion though. Thanks.

---

If the common subset is only A, you could do a direct private/protected inheritance on that.

I think for what you want, containing two A's in B and making A's data public is best. Better names would help people to understand exactly what you're up to.

Why would that be a good idea?
From what I understand of it, B is not an A, hence public inheritance is not right. Furthermore, B does not--or should not--contain A, because B is not compromised of one or more A parts. It doesn't make sense, either. Furthermore, two prev/next instead of prev/next/up/down? Doesn't make sense, either!

While I keep on trying different techniques (4, most notably), I have this to ask:
The only structure I can make so that I have a "reverse" member operator , which will return me the container of the member entered, is by using multiple inheritance and then static_casts, right?

And Elysia, why do you insist on high-level safety inheritance? The whole idea is to be able to use the methods of the base class to each instance of the data sets provided by it to the derived class. You know, laziness and stuff. Am I missing something? Were I not clear?

Quote:

---

Originally Posted by Lord Asriel

1)class from scratch containing all data/methods

1) sounds silly to me, because isn't OOP meant to avoid copy-pasting?

---

In a way, OOP is "meant to avoid copy-pasting". But that is a very crude way to look at it, since most of the tools of abstraction - which permeates computer programming - including things like loop constructs and functions, are also "meant to avoid copy-pasting". Don't repeat yourself is a common rule of thumb.

If you have the opportunity, read Sutter and Alexandrescu's C++ Coding Standards. Item #37 is entitled "Public inheritance is substitutability. Inherit, not to reuse, but to be reused." Granted, the "not to reuse" part is somewhat of an exaggeration, but the point is that when you design for and/or use public inheritance, you should keep in mind the Open-Closed principle and the Liskov substitution principle (PDF documents).

Quote:

---

Originally Posted by Lord Asriel

Note: my real task is making a sparse array

---

Why don't you present this in terms of your "real task" instead of meta classes X, A and B?

Quote:

---

Originally Posted by Lord Asriel

The only structure I can make so that I have a "reverse" member operator , which will return me the container of the member entered, is by using multiple inheritance and then static_casts, right?

---

Probably not.

That sounds good, and thank you for the resources. Since what I'm coding seems to be working, I'd rather learn from the debuggin process and, after all works, I'll present it, the end "product", so you can comment better solutions!

Quote:

---

Originally Posted by Lord Asriel

And Elysia, why do you insist on high-level safety inheritance? The whole idea is to be able to use the methods of the base class to each instance of the data sets provided by it to the derived class. You know, laziness and stuff. Am I missing something? Were I not clear?

---

Design decisions are made to make sure that you separate code into partitions (ie, change A and you don't have to change B), and to make sure you don't repeat code (because then you'd have to change several places to change some code).
To achieve these goals, certain restrictions on how to use tools is often applied in the real world. This is to make sure that we make less mistakes and higher quality code.
Some of these guidelines touch the part of inheritance.

Abstract, encapsulate and object-orientify! But do it properly instead of sloppily and you will be happier in the end!

Alright. I post my complete project now. I have tested insertion and print and they work. I will add deletion later. So, tell me what you think could be designed better and why.

List header file, "DoubleLinkedList.h":

Code:

---

#ifndef INC_DoubleLinkedList_h
#define INC_DoubleLinkedList_h
#include<sstream>
usingnamespace std;
//(D)LL in this file stands for (Double) Linked List
typedefunsignedlong N;
class DLL_member{
DLL_member *p,*n;
N key;
public:
DLL_member(N x):key(x),p(0),n(0){}//Constructor. Sole member that has write access to key.
booloperator <= (N x) {return key<=x;}
booloperator > (N x) {return key>x;}
booloperator == (N x) {return key==x;}
N getkey(void) { return key;}
DLL_member* getn(void) { return n;}
DLL_member* getp(void) { return p;}
void setn(DLL_member* x) { n=x; }
void setp(DLL_member* x) { p=x; }
};
class DLL{//head
DLL_member *h;
public:
DLL(DLL_member *x):h(x){}
DLL_member* get(void) { return h;}
void set(DLL_member* x) { h=x; }
DLL_member* find(N);//if seekand exists, returns its address; else if previous exists, return its address; else return 0
int ins(N,DLL_member* m=0);//2nd parameter is optional: address of (already allocated) new member. If omitted, new allocation occurs.
int del(N,bool deallocate=true);//2nd parameter is optional: do or not deallocation. If omitted, it will.
string print(void);
};
#endif//INC_DoubleLinkedList_h


---

Sparse Array header file, "SparseArray.h"

Code:

---

#ifndef INC_SparseArray_h
#define INC_SparseArray_h
#include"DoubleLinkedList.h"
//SA in this file stands for Sparse Array
class H_DLL_member:public DLL_member{public:H_DLL_member(N x):DLL_member(x){}};//copies of DLL_member, to include in SA_member
class V_DLL_member:public DLL_member{public:V_DLL_member(N x):DLL_member(x){}};
class SA_member:public H_DLL_member, public V_DLL_member{//member of the array, part of two lists. Count: N*N
//DLL_member h,v;
public:
SA_member(N x,N y):H_DLL_member(x),V_DLL_member(y){}
};
class SA_controller: public DLL_member, public DLL{//leader of either a horizontal or a vertical list. Count: 2*N
public:
SA_controller(N x):DLL_member(x),DLL(0){}
};
class SA{//leader of the entire array. Count: 1
DLL h,v;
public:
SA():h(0),v(0){}
int ins(N,N);
DLL* geth(void) { return &h;}
DLL* getv(void) { return &v;}
//void seth(DLL* x) { h.set(x)}
//void setv(DLL* x) { v.set(x)}
string print(void);
};


---

Let me post the source of this last file too, as it contains few lines, but htey are quite complicated, and will probably be good examples to tell me what could be simpler
"SparseArray.cpp"

Code:

---

#include"SparseArray.h"
int SA::ins(N x,N y)
{
SA_controller *temp_ctrl=new SA_controller(y);
if(this->getv()->ins(y,dynamic_cast<DLL_member*>(temp_ctrl))) delete(temp_ctrl);
//make new dll member INSIDE temp_ctrl, IF it doesnt exist. This regards the vertical controller list.
temp_ctrl=new SA_controller(x);
if(this->geth()->ins(x,dynamic_cast<DLL_member*>(temp_ctrl))) delete(temp_ctrl);//Same for horizontal.
//Allocated memory: 2 sa_controllers - unsuccesfull(duplicate) entries.
//Notice how both inserts might have failed, due to already existant members, yet we cannot know the SA_member exists!
//Controller lists are OK now, and surely include v[y] and h[x].
//Possible optimisation: both inserts use find internally. We will use it another time externally here.
//It would make code (much) more complex, but faster to try to use find only twice
SA_member* temp_member=new SA_member(x,y);
if(static_cast<SA_controller*>(this->getv()->find(y))->ins(x,dynamic_cast<DLL_member*>(dynamic_cast<H_DLL_member*>(temp_member))))
{
delete(temp_member);
return 1;
}
//Explanation: find vertical controller, which exists because of previous code. Find returns its DLL_member part, so we
//static_cast it to controller. Then, as controller inherits from dll, we use ins on it, with memory target the horizontal
// dll_member component of the new SA_member we just allocated. If the insert returns error, the member already exists,
//so we exit likewise, after deleting it. Note that if theres no error, the next line won't be too.
static_cast<SA_controller*>(this->geth()->find(x))->ins(y,dynamic_cast<DLL_member*>(dynamic_cast<V_DLL_member*>(temp_member)));
return 0;
}
string SA::print(void)
{
stringstream x;
DLL_member* aux=this->getv()->get();
while(aux)
{
x<<"Now printing list with first node "<<aux->getkey()<<"\n\n";//
x<<static_cast<SA_controller*>(aux)->print();
x<<"\n\n\n";
aux=aux->getn();
}
if(x.str()=="") x<<"List is empty\n";
return x.str();
}


---

Uh, what happened to the identation? Hope you can read this :)

Quote:

---

Originally Posted by Lord Asriel

Uh, what happened to the identation? Hope you can read this

---

Another forum software bug, probably. Let me try...

List header file, "DoubleLinkedList.h":

Code:

---

#ifndef INC_DoubleLinkedList_h
#define INC_DoubleLinkedList_h

#include<sstream>

using namespace std;

//(D)LL in this file stands for (Double) Linked List

typedef unsigned long N;

class DLL_member {
    DLL_member *p, *n;
    N key;
public:
    DLL_member(N x) : key(x), p(0), n(0) {} //Constructor. Sole member that has write access to key.
    bool operator <= (N x) { return key <= x; }
    bool operator > (N x) { return key > x; }
    bool operator == (N x) { return key == x; }
    N getkey(void) { return key; }
    DLL_member* getn(void) { return n; }
    DLL_member* getp(void) { return p; }
    void setn(DLL_member* x) { n=x; }
    void setp(DLL_member* x) { p=x; }
};

class DLL { //head
    DLL_member *h;
public:
    DLL(DLL_member *x) : h(x) {}
    DLL_member* get(void) { return h; }
    void set(DLL_member* x) { h = x; }
    DLL_member* find(N); //if seekand exists, returns its address; else if previous exists, return its address; else return 0
    int ins(N, DLL_member* m = 0); //2nd parameter is optional: address of (already allocated) new member. If omitted, new allocation occurs.
    int del(N, bool deallocate = true); //2nd parameter is optional: do or not deallocation. If omitted, it will.
    string print(void);
};

#endif //INC_DoubleLinkedList_h

---

Sparse Array header file, "SparseArray.h"

Code:

---

#ifndef INC_SparseArray_h
#define INC_SparseArray_h

#include"DoubleLinkedList.h"

//SA in this file stands for Sparse Array

class H_DLL_member : public DLL_member {
public:
    H_DLL_member(N x) : DLL_member(x) {}
}; //copies of DLL_member, to include in SA_member

class V_DLL_member : public DLL_member {
public:
    V_DLL_member(N x) : DLL_member(x) {}
};

class SA_member : public H_DLL_member, public V_DLL_member { //member of the array, part of two lists. Count: N*N
    //DLL_member h,v;
public:
    SA_member(N x, N y) : H_DLL_member(x), V_DLL_member(y) {}
};

class SA_controller : public DLL_member, public DLL{ //leader of either a horizontal or a vertical list. Count: 2*N
public:
    SA_controller(N x):DLL_member(x),DLL(0){}
};

class SA { //leader of the entire array. Count: 1
    DLL h, v;
public:
    SA() : h(0), v(0) {}
    int ins(N, N);
    DLL* geth(void) { return &h; }
    DLL* getv(void) { return &v; }
    //void seth(DLL* x) { h.set(x)}
    //void setv(DLL* x) { v.set(x)}
    string print(void);
};

---

"SparseArray.cpp"

Code:

---

#include"SparseArray.h"
int SA::ins(N x, N y)
{
    SA_controller *temp_ctrl = new SA_controller(y);
    if (this->getv()->ins(y, dynamic_cast<DLL_member*>(temp_ctrl)))
        delete(temp_ctrl);
    //make new dll member INSIDE temp_ctrl, IF it doesnt exist. This regards the vertical controller list.
    temp_ctrl = new SA_controller(x);
    if (this->geth()->ins(x, dynamic_cast<DLL_member*>(temp_ctrl)))
        delete(temp_ctrl);//Same for horizontal.
    //Allocated memory: 2 sa_controllers - unsuccesfull(duplicate) entries.
    //Notice how both inserts might have failed, due to already existant members, yet we cannot know the SA_member exists!
    //Controller lists are OK now, and surely include v[y] and h[x].
    //Possible optimisation: both inserts use find internally. We will use it another time externally here.
    //It would make code (much) more complex, but faster to try to use find only twice
    SA_member* temp_member = new SA_member(x, y);
    if (static_cast<SA_controller*>(this->getv()->find(y))->ins(x, dynamic_cast<DLL_member*>(dynamic_cast<H_DLL_member*>(temp_member))))
    {
        delete(temp_member);
        return 1;
    }
    //Explanation: find vertical controller, which exists because of previous code. Find returns its DLL_member part, so we
    //static_cast it to controller. Then, as controller inherits from dll, we use ins on it, with memory target the horizontal
    // dll_member component of the new SA_member we just allocated. If the insert returns error, the member already exists,
    //so we exit likewise, after deleting it. Note that if theres no error, the next line won't be too.
    static_cast<SA_controller*>(this->geth()->find(x))->ins(y, dynamic_cast<DLL_member*>(dynamic_cast<V_DLL_member*>(temp_member)));
    return 0;
}

string SA::print(void)
{
    stringstream x;
    DLL_member* aux = this->getv()->get();
    while (aux)
    {
        x << "Now printing list with first node " << aux->getkey() << "\n\n";//
        x << static_cast<SA_controller*>(aux)->print();
        x << "\n\n\n";
        aux = aux->getn();
    }
    if (x.str() == "")
        x << "List is empty\n";
    return x.str();
}

---

I think you need to work on your design some more. n and p are not good names. Name them next and prev at least, and don't make N a typedef for unsigned long. That is bound to cause name collision, plus it's just confusing.
I think DLL_member should be named something like DListNode or some such. The "_member" is probably not such a good name. Typically it is said a linked list contains nodes, so it seems like it's a good name for it.

You should absolutely not have a constructor that allows you to set the head in the linked list. This is an implementation details and must not be exposed to the outside.
get should return a pointer to const. Never let a get member return something that can be modified. This goes against get semantics and exposes your implementation to the outside. It should also be a const function.
Again, set should not exist. It's just as dangerous as the constructor.
The semantics of find is weird. Find typically finds an element, and if it exists, returns it. Otherwise, it fails. To return previous upon failure is weird. Do you need that? If so, rename that function.
I am skeptic about insert. You never show any code, so I am going to have to guess. But how I would see it, is that it would insert an element at the end of the list, or if the second parameter is specified, insert it after the node specified. If it does not do this, then it is potentially broken.
delete is also weird, and dangerous. The outside should not know the implementation nor care, and it absolutely not be involved in managing the memory of the linked list. Absolutely not in any circumstance. IF you remove a node, then it's deallocated from the list. The end. If you don't want to delete it, then don't remove it.

Your sparse array seems weird too, like two exactly same looking classes H_DLL_Member and V_DLL_Member, both of which do nothing useful.
And then there is manual allocation of memory with new and dynamic casts. Extremely ugly and prone to errors. You need to redesign your linked list and then use it properly in your sparse array. There should not be any need for casts.