I have a ToDo List project using linked list as its data structure. I encounter problem when in my addTask() in my header file in the line:
if (head != NULL)
{
current = head;
while (current->link != NULL)
{
current = current->link; //error happens here
}
current->link = n; //another error here
}
- '=' cannot convert from 'list*' to 'list::taskPTR'
- '=' cannot convert from 'list::task*' to 'list*'
^^^are the error that occurs. I don't understand my error and how to fix it.
here is the full header file:
#ifndef linkedList
#define linkedList
#include <iostream>
using namespace std;
class list
{
private:
struct task
{
string taskDue;
string taskDesc;
string taskCourse;
string day;
string mon;
string year;
list* link;
};
typedef struct task* taskPtr;
taskPtr head;
taskPtr current;
taskPtr temp;
public:
list();
~list();
void printByDue();
void printByCourse();
void printByStatus();
void addTask(string addTaskDue, string addTaskDesc, string addTaskCourse, string addDay, string addMon, string addYear);
void delTask(string delTaskDue, string delTaskDesc, string delTaskCourse, string delDay, string delMon, string delYear);
};
list::list() //constructor
{
head = NULL;
current = NULL;
temp = NULL;
}
void list::addTask(string addTaskDue, string addTaskDesc, string addTaskCourse, string addDay, string addMon, string addYear)
{
struct task* n = new task;
n->link = NULL;
n->taskDue = addTaskDue;
n->taskDesc = addTaskDesc;
n->taskCourse = addTaskCourse;
n->day = addDay;
n->mon = addMon;
n->year = addYear;
if (head != NULL)
{
current = head;
while (current->link != NULL)
{
current = current->link;
}
current->link = n;
}
else
{
head = n;
}
}
void menu()
{
char option;
cout << " Welcom to the ToDo List Program by ACGR";
cout << "\n" << endl;
cout << "What would you like to do? \n" << endl;
cout << "\t[a]dd a new task" << endl;
cout << "\t[c]omplete a task" << endl;
cout << "\t[d]isplay tasks" << endl;
cout << "\t[q]uit" << endl;
cout << "\nEnter choice here: ";
}
#endif // !linkedList.h
CodePudding user response：
There's a lot to unpack, but the likely simple fix is to change the following line:
list* link;
// Should be
task* link;
Your tasks shouldn't be pointing to lists (they are in a list), they are supposed to be pointing to the next task in the list.
I will say that all this might do is reveal more errors, both logical and syntactical.
However, I would feel bad sending you on your way with that code. What you posted is not great, and for a great many reasons. I would have expected almost everything I'm going to demonstrate to have been taught by the time a linked list is assigned; it is in my curriculum.
Here's a mini-code review of what was posted:
#include <iostream>
// Where is <string> ? Include what you use.
using namespace std; // Bad practice, **especially** in a header
class list
{
private:
struct task // A task has nothing to do with a list
{
string taskDue;
string taskDesc;
string taskCourse;
string day;
string mon;
string year;
list* link; // This has nothing to do with a task.
// And now you've forced yourself to include Rule of 5 in your task.
};
typedef struct task* taskPtr; // Subjective; I prefer `using taskPtr = task*;`
// More subjective; I wouldn't do this at all.
// taskPtr is longer than task*, I fail to see the point.
taskPtr head; // Prefer default member initialization.
taskPtr current; // Likely part of the todo list, which has no bearing on a list data structure
taskPtr temp; // ??? Why should a temp get permenant residence as class data?
public:
list();
~list();
void printByDue(); // Should be getters. Let the caller print if and how they want.
void printByCourse();
void printByStatus();
// If the task and list were separate from each other, this could be simplified quite a bit.
void addTask(string addTaskDue, string addTaskDesc, string addTaskCourse, string addDay, string addMon, string addYear);
void delTask(string delTaskDue, string delTaskDesc, string delTaskCourse, string delDay, string delMon, string delYear);
};
// Use the initializaton section of constructors, or better yet default member initialization.
list::list() //constructor
{
head = NULL;
current = NULL;
temp = NULL;
}
//
// Where is the destructor? This shouldn't be compiling at all
//
void list::addTask(string addTaskDue, string addTaskDesc, string addTaskCourse, string addDay, string addMon, string addYear)
{
struct task* n = new task;
n->link = NULL;
n->taskDue = addTaskDue;
n->taskDesc = addTaskDesc;
n->taskCourse = addTaskCourse;
n->day = addDay;
n->mon = addMon;
n->year = addYear;
if (head != NULL)
{
current = head;
while (current->link != NULL)
{
current = current->link;
}
current->link = n;
}
else
{
head = n;
}
}
// This function has no business being here at all.
void menu()
{
char option;
cout << " Welcom to the ToDo List Program by ACGR";
cout << "\n" << endl;
cout << "What would you like to do? \n" << endl;
cout << "\t[a]dd a new task" << endl;
cout << "\t[c]omplete a task" << endl;
cout << "\t[d]isplay tasks" << endl;
cout << "\t[q]uit" << endl;
cout << "\nEnter choice here: ";
}
The thing that will help you the most is to separate your concerns. A todo list and a linked list are separate entities and should be treated as such. A linked list uses nodes to link data together in a list-type way. A todo list is a list of tasks, and while I did use the word list there, I could write a todo list using an array, a stack, a queue, or even a hash-map. The underlying data structure can be pretty flexible depending on the desired functionality of the list.
So write the linked list, and then incorporate it into your todo list. This makes testing easier, focuses your code, and is just better practice.
For this assignment, the hard part is the list. Here's a quick and dirty one:
#ifndef SINGLY_LINKED_LIST
#define SINGLY_LINKED_LIST
#include <utility> // std::swap
template <typename T>
class SList {
public:
SList() = default;
SList(const SList& other);
SList(SList&& other) noexcept;
~SList();
void add(T val);
void erase(T val);
// Employing copy-swap idiom; that's why the parameter is taken by value
SList& operator=(SList rhs);
friend void swap<>(SList<T>& lhs, SList<T>& rhs);
// This function is for convenience and demonstration; I don't want to be
// bothered implementing the minimal iterator required for a range-based
// for loop.
void print() const;
private:
struct Node {
T data;
Node* next = nullptr;
Node(T d) : data(d) {}
};
Node* head = nullptr;
Node* tail = nullptr;
// Helper
std::pair<Node*, Node*> find_node_and_before(T val);
};
// Implementation
template <typename T>
SList<T>::SList(const SList& other) {
Node* walker = other.head;
while (walker) {
add(walker->data);
walker = walker->next;
}
}
template <typename T>
SList<T>::SList(SList&& other) noexcept : SList() {
swap(*this, other);
}
template <typename T>
SList<T>::~SList() {
while (head) {
Node* tmp = head;
head = head->next;
delete tmp;
}
tail = nullptr;
}
template <typename T>
void SList<T>::add(T val) {
if (!head) {
head = new Node(val);
tail = head;
return;
}
tail->next = new Node(val);
tail = tail->next;
}
template <typename T>
void SList<T>::erase(T val) {
auto toBeErased = find_node_and_before(val);
if (toBeErased.second == head) {
Node* tmp = head;
head = head->next;
delete tmp;
return;
}
(toBeErased.first)->next = (toBeErased.second)->next;
if (toBeErased.second == tail) {
tail = toBeErased.first;
}
delete toBeErased.second;
}
template <typename T>
SList<T>& SList<T>::operator=(SList rhs) {
swap(*this, rhs);
return *this;
}
template <typename T>
void swap(SList<T>& lhs, SList<T>& rhs) {
using std::swap;
swap(lhs.head, rhs.head);
swap(lhs.tail, rhs.tail);
}
template <typename T>
void SList<T>::print() const {
Node* walker = head;
while (walker) {
std::cout << walker->data << ' ';
walker = walker->next;
}
std::cout << '\n';
}
template <typename T>
std::pair<typename SList<T>::Node*, typename SList<T>::Node*>
SList<T>::find_node_and_before(T val) {
if (head->data == val) {
return {nullptr, head};
}
Node* walker = head;
while (walker->next) {
if (walker->next->data == val) {
return {walker, walker->next};
}
walker = walker->next;
}
return {nullptr, nullptr};
}
#endif
Note that there is nothing to do with a todo list or tasks in it. It is just a linked list, as it should be. It can be a SList<int>, SList<std::string>, or any other type.
There are dependencies on that other type, like having an operator<<(), but that's mostly due to the print() function that normally wouldn't be there at all. It's included as a hack to demonstrate the list working.
I put together a super simple "task list" as well.
#include <iostream>
#include <string>
#include "slist.hpp"
struct task {
std::string name;
task() = default;
task(const char* t) : name(t) {}
};
bool operator==(const task& lhs, const task& rhs) {
return lhs.name == rhs.name;
}
std::ostream& operator<<(std::ostream& sout, const task& t) {
return sout << "\"" << t.name << "\"";
}
int main() {
SList<task> one;
one.add("trash");
one.add("dishes");
one.add("walk the dog");
one.add("tidy up");
one.print();
one.erase("dishes");
one.print();
one.erase("tidy up");
one.print();
one.erase("trash");
one.print();
}
Output:
"trash" "dishes" "walk the dog" "tidy up"
"trash" "walk the dog" "tidy up"
"trash" "walk the dog"
"walk the dog"
By the time anyone is ready to write a linked list, nothing in my code should be new. Linked lists are a "dense" assignments that require the learner to be able to properly utilize almost every technique and principle that they've learned up to this point. I consider them the graduation out of "beginner"-level C .
Two major principles that are missing and can be added include iterators and perfect forwarding (to write emplace functions).
CodePudding user response：
Just a glimps on what code would look like without your home brew list :
#include <chrono>
#include <list>
#include <string>
// using namespace std; <== NO
// a task is just that, it isn't the linked list so should not have any list info in it, like a next pointer
struct task final
{
task() = default;
task(const std::string& new_due, const std::string& new_desc, const& std::string new_course, const std::chrono::system_clock::time_point& new_time) :
due{ new_due },
desc{ new_desc },
course{ new_course },
time{ new_time }
{
}
~task() = default;
task(const task&) = default; // default copyable
task& operator=(const task&) = default; // default copy assignable
task(task&& task) = default; // default movable
task& operator=(task&&) = default; // default move assignable
std::string due; // no need to repeat name of struct in members
std::string desc;
std::string course;
std::chrono::system_clock::time_point time; // use chrono for times
};
// print by due etc will need to be free functions, it is not the task of a task
// to know how to print itself. And if you have a list, you can use std::sort(tasks.begin(),task.end(),sort_function) to sort in any order you like
// before doing a print.
int main()
{
std::list<task> tasks;
// this is how easy it can be to add a task to a list
tasks.emplace_back("new due", "new desc", "new course", std::chrono::system_clock::now()); // will call constructor for task.
}
CodePudding user response：
current has type taskPtr, and current->link has type list*. You can't assign one to the other. A possible fix would be to change the type of the member variable link to task*, as mentioned by @Fareanor.
Unless this project's goal is to learn how to implement your own linked lists, I strongly recommend that you avoid doing exactly that, and use std::list instead:
struct task {
string taskDue;
...
string year;
};
std::list<task> tasks;
Then adding a task is simply:
tasks.emplace_back(addTaskDue, addTaskDesc, ...);
Also note that it's not very efficient to store dates as strings. Consider using a suitable type from std::chrono instead.
There are a lot of helpful things in the C standard library; it is worthwhile to spend some time to get familiar with it so you don't reinvent the wheel all the time, and so you can focus on the higher level details of implementing a todo list.