patterncppMinor
General advice on a practice linked_list for C++ classes/templates
Viewed 0 times
linked_listpracticegeneraladvicetemplatesforclasses
Problem
Introduction
I'm learning C++ (Coming from Haskell, C, and Assembly - and other languages sparsely) and this was my practice with classes and templates. It's a linked list that you can call in this fashion:
Topic
I am here to ask if some more experienced coders would point out my mistakes (mainly about classes) so I can break the habits. Thanks for your help!
Here is the source code: (Btw, it isn't optimized on purpose, I was only playing with classes/templates)
(I do notice the inefficient and potentially dangerous tail() call, but considering the trivial nature of the project - I decided to leave it for later, simply make it a deque. I also naively checked it with memcheck. No leaks and no errors in my library, seemingly.)
Ideone Link
```
//Compile with "g++ -std=C++11 -O2 -o list main.cpp list.hpp"
#ifndef LIST_H
#define LIST_H
#include
template
class linked_list {
private:
int empty = 1;
struct node {
T data;
struct node * next;
} * head;
struct node * get_add(int index);
public:
linked_list(){head = new node;};
linked_list(T data){head = new node; head = data; empty = 0;}
~linked_list();
int size();
inline int nsize(){return sizeof(struct node);};
inline struct node * lhead() {return head;};
inline struct node * tail() {return get_add(size()-1);};
int search(T key);
void prepend(T data);
void append(T data);
int insert(T data, int index);
int del(int index);
int set(T data, int index);
T get(int index);
T& operator[](int i);
};
template
std::ostream& operator &list);
//////////////IMPLEMENTATION
I'm learning C++ (Coming from Haskell, C, and Assembly - and other languages sparsely) and this was my practice with classes and templates. It's a linked list that you can call in this fashion:
#include "list.hpp"
int main(void){
linked_list test;
char c;
while(c != '\n') //
test.prepend(c = fgetc(stdin));
std::cout << test;
std::cout << std::endl;
return 0;
}Topic
I am here to ask if some more experienced coders would point out my mistakes (mainly about classes) so I can break the habits. Thanks for your help!
Here is the source code: (Btw, it isn't optimized on purpose, I was only playing with classes/templates)
(I do notice the inefficient and potentially dangerous tail() call, but considering the trivial nature of the project - I decided to leave it for later, simply make it a deque. I also naively checked it with memcheck. No leaks and no errors in my library, seemingly.)
Ideone Link
```
//Compile with "g++ -std=C++11 -O2 -o list main.cpp list.hpp"
#ifndef LIST_H
#define LIST_H
#include
template
class linked_list {
private:
int empty = 1;
struct node {
T data;
struct node * next;
} * head;
struct node * get_add(int index);
public:
linked_list(){head = new node;};
linked_list(T data){head = new node; head = data; empty = 0;}
~linked_list();
int size();
inline int nsize(){return sizeof(struct node);};
inline struct node * lhead() {return head;};
inline struct node * tail() {return get_add(size()-1);};
int search(T key);
void prepend(T data);
void append(T data);
int insert(T data, int index);
int del(int index);
int set(T data, int index);
T get(int index);
T& operator[](int i);
};
template
std::ostream& operator &list);
//////////////IMPLEMENTATION
Solution
This won't be as complete an answer as you often see on Code Review, but here are a few pointers to improve your code:
First of all, you should tell the programmer that your index will never be a negative one. In order to do so, you could change your
C++ differentiates between functions that change the class members and functions that do not. If your function does not change the member variables, it should be const-qualified. Moreover, this qualification is a part of the function's signature. That means that you can have both the const-qualified version of the function in your class and the normal one:
You can apply my remarks about
Now, let's talk about this piece of code:
Let's have a look a the function's signature: your parameter is a
About
In C++, you can use declaration statements in the first part of the
And since your list can not have negative indices, the
We will now see this piece of code:
While it is valid, you are passing
template
T& linked_list::operator[](int i){
return (get_add(i)->data);
}First of all, you should tell the programmer that your index will never be a negative one. In order to do so, you could change your
int parameter to be an unsigned int one. Here, since we are talking about sizes, a size_t or std::size_t parameter would be even better:template
T& linked_list::operator[](std::size_t i) {
return get_add(i)->data;
}C++ differentiates between functions that change the class members and functions that do not. If your function does not change the member variables, it should be const-qualified. Moreover, this qualification is a part of the function's signature. That means that you can have both the const-qualified version of the function in your class and the normal one:
template
T& linked_list::operator[](std::size_t i) {
return get_add(i)->data;
}
template
const T& linked_list::operator[](std::size_t i) const {
return get_add(i)->data;
}You can apply my remarks about
const-qualified functions to every function that should not change the class (e.g. size and search).Now, let's talk about this piece of code:
template
std::ostream& operator &list) { ... }Let's have a look a the function's signature: your parameter is a
linked_list&. However, you should specify that this output function will not modify your list by adding a const before the type:template
std::ostream& operator &list) { ... }About
for loops in C++:int i;
for(i = 0; i < list.size(); i++)
out << list.get(i);In C++, you can use declaration statements in the first part of the
for. That means that you don't have to declare your int i before the loop. You can declare it inside the for. And that is what should be done, unless you still need the value of i after you leave the loop. You can write it like this:for(int i = 0; i < list.size(); i++)
out << list.get(i);And since your list can not have negative indices, the
std::size_t is still welcome:for(std::size_t i = 0; i < list.size(); i++)
out << list.get(i);We will now see this piece of code:
int search(T key);While it is valid, you are passing
key by value, which means that if you pass an instance of a big class (if T is a big class), it will be entirely copied before being used. In C++, if we don't need to modify the object passed to a function, it is better to pass it by const reference: it will not change the way you use it, but the compiler will use the address of the instance instead of doing a whole copy. To search, you don't need to modify your key, so it is better to pass by const reference:int search(const T& key);Code Snippets
template <typename T>
T& linked_list<T>::operator[](int i){
return (get_add(i)->data);
}template <typename T>
T& linked_list<T>::operator[](std::size_t i) {
return get_add(i)->data;
}template <typename T>
T& linked_list<T>::operator[](std::size_t i) {
return get_add(i)->data;
}
template <typename T>
const T& linked_list<T>::operator[](std::size_t i) const {
return get_add(i)->data;
}template <typename T>
std::ostream& operator<< (std::ostream &out, linked_list<T> &list) { ... }template <typename T>
std::ostream& operator<< (std::ostream &out, const linked_list<T> &list) { ... }Context
StackExchange Code Review Q#25283, answer score: 8
Revisions (0)
No revisions yet.