-2

I'm trying to create a to-do list program.

I've got a problem removing a task from the txt file, I've tried to read every data from the program and replace that certain data with blank, but in the end, there are still some characters that are left behind.

bool DoList::Remove_Task()
{
    std::ofstream temp_file("temp.txt", std::ios::app);
    std::ifstream file("Tasks_Data.txt");
    if (!temp_file.is_open())
    {
        std::cerr << "temp File cannot be opened to modify";
        return false;
    }
    else if (!file.is_open())
    {
        std::cerr << "File cannot be opened to read";
        return false;
    }
    else
    {
    int ID;
    int search;
    std::string TaskName;
    std::string Description;
    std::string Status;
    char str[26];
    std::string line;

    std::cout << "Enter Your Task ID to remove: ";
    std::cin >> search;

    while (getline(file, line))
    {
        while(file >> ID)

        if (ID == search)
        {

            ID = 0;
            TaskName = "\0";
            Description = "\0";
            Status = "\0";
            strcpy_s(str, "\0");
            temp_file << ID <<  ". " << std::setprecision(5) << TaskName << ", " << std::setprecision(5) << Description << ", " << std::setprecision(5) << Status << ", " << std::setprecision(5) << str << std::endl;
    
        }
        else
        {
            temp_file << line;
        }


    }
    }
    file.close();
    temp_file.close();

    remove("Tasks_Data.txt");
    rename("temp.txt", "Tasks_Data.txt");

    

    return true;
}
Sgt Mahdi
  • 29
  • 6
  • 4
    `while(file >> ID)` ? You already read a whole line via `getline(file, line)` but you are not using `line` – 463035818_is_not_an_ai Aug 21 '23 at 06:04
  • @Someprogrammerdude thats what OP tries to do. They write to `temp.txt` and when done replace the original with the update – 463035818_is_not_an_ai Aug 21 '23 at 06:08
  • @463035818_is_not_an_ai I'm already using it here else { temp_file << line; } – Sgt Mahdi Aug 21 '23 at 06:10
  • ok, but after you read a whole line the next `ID` you read is not from that line, but from the next line – 463035818_is_not_an_ai Aug 21 '23 at 06:11
  • @463035818_is_not_an_ai you are right, it deletes the next line how can I just read the ID? to fix this issue I tried to get the ID only from the file but the result winded up removing whole data from txt file – Sgt Mahdi Aug 21 '23 at 06:20
  • the ID is the first number in `line` you do not have to read it again from the file. – 463035818_is_not_an_ai Aug 21 '23 at 06:22
  • 1
    you should overload `<<` and `>>` for the entry. You should also have a `std::vector` and a function to read/write the whole vector to/from a file. Reading and writing to the file should not be specific to deleting one entry – 463035818_is_not_an_ai Aug 21 '23 at 06:23
  • Does this answer your question? [C++ fstream Erase the file contents from a selected Point](https://stackoverflow.com/questions/4007734/c-fstream-erase-the-file-contents-from-a-selected-point) – Ulrich Eckhardt Aug 21 '23 at 06:31
  • Read entire file into memory. Remove the line to be removed from the in-memory representation. Truncate the file to 0 length. Write the memory representation back to the file. The line has now been removed from the file. Easy as that. – Jesper Juhl Aug 21 '23 at 10:32

2 Answers2

3

Original code:

std::ofstream temp_file("temp.txt", std::ios::app);

File is opened for appending. If previous instance crashed, file will contain data from previous run. Just open it for write.

Original code:

while (getline(file, line))
{
    while(file >> ID)

You already read entire line. file >> ID will read ID from next line. Parse line to determine it's id. You can use, for example, std::stringstream:

int task_id;
std::stringstream(line) >> task_id;

Original code:

temp_file << ID <<  ". " << std::setprecision(5) << TaskName << ", " << std::setprecision(5) << Description << ", " << std::setprecision(5) << Status << ", " << std::setprecision(5) << str << std::endl;

For lines not to remove you don't need to parse them and generate new ones. Just write entire line:

temp_file << line << std::endl;

Original code:

        return false;
    }
    else if (

You don't need else part if previous block return from a function.

Original code:

    remove("Tasks_Data.txt");
    rename("temp.txt", "Tasks_Data.txt");

You don't need to remove old file. rename() will replace it with new one atomically.

Entire function may be like this (id to delete passed as parameters, no format errors checking):

bool Remove_Task(int id)
{
    std::ofstream temp_file("temp.txt");
    if (!temp_file.is_open())  {
        std::cerr << "temp File cannot be opened to modify";
        return false;
    }

    std::ifstream file("Tasks_Data.txt");
    if (!file.is_open()) {
        std::cerr << "File cannot be opened to read";
        return false;
    }

    std::string line;

    while (getline(file, line))
    {
        int task_id;
        std::stringstream(line) >> task_id;

        if (task_id != id) {
            temp_file << line << std::endl;
        }
    }

    file.close();
    temp_file.close();

    rename("temp.txt", "Tasks_Data.txt");

    return true;
}
dimich
  • 1,305
  • 1
  • 5
  • 7
2

Break your problem down into tasks that separate out the user experience (reading and writing to console) with accessing your data (reading and writing to file).

Core of what you need. A function that takes in a list of "lines" (strings) and and removes the string with the designated task id. The magic of this function is that it's only concerned with parsing a "line", and to find the leading number at the start of a line.

bool RemoveTaskFromList(int taskIdToRemove, std::vector<std::string>& lines)
{
    bool found = false;
    for (auto itor = lines.begin(); itor != lines.end();)
    {
        std::string line = *itor;
        std::istringstream ss(line);
        int id;
        ss >> id;
        if (id == taskIdToRemove)
        {
            found = true;
            itor = lines.erase(itor); // returns a new iterator to "one past" what was just erased
        }
        else
        {
            itor++;
        }
    }

    return found;
}

Notice that the above function doesn't have to parse the entire line out into component parts. It just needs to "read" the leading number at the beginning of the line. It's not the most robust. If the line doesn't lead with a number, id will be assigned zero. You can change this however you want.

Another function that does the actual i/o on the file (reading and writing to disk). It doesn't need to know anything about the schema of format of individual lines. It just needs to read all the lines as individual strings into a vector of strings.

bool RemoveTaskFromFile(int taskIdToRemove, const std::string& fileName, bool& ioError)
{
    std::ifstream ifile(fileName);
    ioError = false;
    if (!ifile)
    {
        ioError = true;
        return false;
    }

    // read all the lines
    std::vector<std::string> lines;
    std::string line;
    while(std::getline(ifile, line))
    {
        lines.push_back(line);
    }

    ifile.close();

    // remove the line
    bool found = RemoveTaskFromList(taskIdToRemove, lines);

    if (found == false)
    {
        return false;
    }

    // open the file again for writing
    std::ofstream ofile(fileName);
    if (!ofile)
    {
        ioError = true;
        return false;
    }

    // write all the lines back that were not removed from the vector
    for (const auto& line : lines)
    {
        ofile << line << "\n";
    }

    return true;
}

And a third function that actually engages with the user:

bool DoRemove()
{
    int id;
    std::cout << "task id to remove\n";
    std::cin >> id;
    bool ioError;
    bool wasRemoved = RemoveTaskFromFile(id, "Tasks_Data.txt", ioError);
    if (ioError)
    {
        std::cout << "error reading or writing file\n";
    }
    else if (!wasRemoved)
    {
        std::cout << "can't find task in file\n";
    }
    else
    {
        std::cout << "successfully removed\n";
    }
    return wasRemoved;

}

It's an exercise for you to integrate this back into the DoList class.

selbie
  • 100,020
  • 15
  • 103
  • 173
  • A disadvantage to the code here is that there's a period where the data exist only in memory. If anything catastrophic happens before it's written back (filesystem full, power cut, etc), then we completely lose our valuable data. – Toby Speight Aug 21 '23 at 07:47
  • 1
    @TobySpeight - probably not a requirement for an academic exercise. But you are correct. – selbie Aug 21 '23 at 08:45
  • @TobySpeight One way to get around that is to write to a new temporary file and then subsequently do an atomic rename to overwrite the original. – Jesper Juhl Aug 21 '23 at 10:34
  • Yes @Jesper, that's probably what I would do. Or perhaps `mmap()`, `std::move()`, `truncate()` if targeting POSIX. – Toby Speight Aug 21 '23 at 11:55