Random line remover from file This asks the user for the file name and the number of lines they want to remove. It then stores each line into a vector. A dynamic array is used to hold the random line numbers (generated by random generator) and the corresponding element in the vector is deleted. This vector is then copied to a new file named "temp.txt" where each element is its own line.

Questions:


What can I improve to make this code for efficient? i.e. Better methods to perform this task? 
What should I not use and should change (for example any bad habits)?
What did I use that is good and should keep it as a habit?
Are there any exceptions I can use to ensure code?


Warnings I received:


  
  Warning    C26495  Variable 'LineEditor::array' is uninitialized. Always initialize a member variable (type.6). for LineEditor().
  Avoid unnamed objects with custom construction and destruction (es.84).    for getFileName() in void removeLines().
  


#include &lt;iostream&gt;
#include &lt;string&gt;
#include &lt;fstream&gt;
#include &lt;random&gt;
#include &lt;ctime&gt;
#include &lt;cstring&gt;
#include &lt;vector&gt;
#include &lt;algorithm&gt;
#include &lt;iterator&gt;

template&lt;typename T&gt;
void writeTo(const std::string&amp; filename, std::vector&lt;T&gt;&amp; v)
{
    std::ofstream fstream(filename);
    if (!fstream)
        exit(1);
    std::copy(v.begin(), v.end(), std::ostream_iterator&lt;T&gt;(fstream, "\n"));
    fstream.close();
}

class LineEditor
{
    std::vector&lt;std::string&gt; buf;
    std::string fileName;
    int noOfLines{ 0 };
    int* array;
public:
    LineEditor()
    {
    }

    ~LineEditor()
    {
        std::cout &lt;&lt; "\"" &lt;&lt; fileName &lt;&lt; "\"" &lt;&lt; " " &lt;&lt; noOfLines &lt;&lt; " lines removed.\n";
        delete[] array;
    }

    void removeLines()
    {
        getFileName();
        TotalLinesToRemove();

        std::ifstream fileIn;
        std::string line;
        int lineCounter{ 0 };
        int* array = new int[noOfLines];

        fileIn.open(fileName);
        if (!fileIn)
        {
            std::cerr &lt;&lt; fileName &lt;&lt; " can not be opened." &lt;&lt; std::endl;
            exit(1);
        }
        while (fileIn.is_open())
        {
            while (std::getline(fileIn, line))
            {
                lineCounter++;
                buf.push_back(line);
            }
            for (int i{ 0 }; i &lt; noOfLines; ++i)
            {
                array[i] = getRandomNumber(0, lineCounter - 1);
                buf.erase(buf.begin() + (array[i]));
                --lineCounter;
            }
            writeTo("temp.txt", buf);
            fileIn.close();
        }
    }
private:
    std::string getFileName()
    {
        std::cout &lt;&lt; "Enter name of text file: ";
        std::cin &gt;&gt; fileName;

        return fileName;
    }
    int TotalLinesToRemove()
    {
        std::cout &lt;&lt; "Enter the number of lines to remove: ";
        std::cin &gt;&gt; noOfLines;

        return noOfLines;
    }
    int getRandomNumber(int min, int max)
    {
        std::mt19937 seed{ static_cast&lt;std::mt19937::result_type&gt;(std::time(nullptr)) };
        std::uniform_int_distribution&lt;&gt; rand(min, max);
        return rand(seed);
    }
};

int main()
{
    LineEditor file;
    file.removeLines();
    return 0;
}


I am fairly beginner to intermediate at C++ and want to improve.
Use the force library Luke!

Your code does use std::vector, but for reasons I don't understand, it also uses new to allocate a manually managed dynamic array as well. You don't seem to gain anything from this, so I'd advise using std::vector throughout.

I'd also look at the standard library's algorithms. Some of them (e.g., std::remove_copy_if) could be put to excellent use for this task.

Avoid std::endl

std::endl not only writes a new-line to a stream (which is what you usually want) but also flushes the stream (which you usually don't want). If you do that very often, it can slow a program significantly.

In your case, you using it when writing to std::cerr, which automatically flushes in any case, so the flushing done by std::endl is unnecessary in any case.

Random generator seeding

When you're using &lt;random&gt; (a good idea) the generally accepted way of seeding a generator is by using std::random_device. This is usually a truly random device (e.g., reading from /dev/random) so it normally does a substantially better job of seeding than using the system time.

As @1201ProgramAlarm already pointed out, you normally want to seed a generator exactly once, then just generate numbers from it.

Unfortunately, doing a good job of seeding a random number generator is a somewhat nontrivial task. I posted some code that demonstrates what I believe is one reasonable possibility in a previous answer. The short summary is that regardless of whether you use time(nullptr) or std::random_device, if you only use a 32-bit seed, you're limiting yourself to only \$2^{32}-1\$ possible sequences that you can generate, which is a huge limitation compared to the full capabilities of MT19377.

Single Responsibility Principle

It seems to me that your LineEditor does more than I'd like a single class to do. For most practical purpose, the entire program is embodied in that one class. That doesn't strike me as entirely ideal. I'd rather see individual pieces responsible for the individual parts of doing the job.

This also fits better with having clearly defined layers of abstraction. For example, you might have one layer that deals with the file name, and a separate one that deals with the actual content of the file.

Convenient Interface

For most uses, I'd rather run the program something like removeLines foo.txt 10, rather than doing removeLines, then having to separately walk through a questionairre (so to speak) to tell it the name of the file and number of lines to remove. Of course, depending on the situation, that might be better as something like removeLines -f foo.txt -n 10 (especially if this were part of a larger program that might have more/other command line arguments as well.

If you are going to ask questions interactively, I'd ask for the filename first, then read in the file, then ask for the number of lines to remove. This will allow you to check how many lines you've actually read, which (in turn) you can use to advise the user as to the maximum number of lines they can have removed (e.g., "Please enter the number of lines to remove (1-723): ").

Constructor Definition

You've currently defined your constructor with no member initializer list and nothing in the body. It's not clear what you're hoping to accomplish by defining it explicitly rather than just accepting what the compiler would define if you didn't define one at all. Until/unless your ctor will actually accomplish something, it's probably better not to define it explicitly.

Possible Code

#include &lt;iostream&gt;
#include &lt;vector&gt;
#include &lt;string&gt;
#include &lt;set&gt;
#include &lt;fstream&gt;
#include &lt;iterator&gt;
#include "rand.h"

struct InputSpec {
    std::string filename;
    std::size_t count;

    InputSpec(int argc, char **argv) { 
        if (argc != 3)
            throw std::runtime_error("Usage: removeLines &lt;filename&gt; &lt;count&gt;");

        filename = argv[1];
        count = std::atoi(argv[2]);

        if (count == 0)
            throw std::runtime_error("numbers of lines to remove can't equal 0");
    }
};

int main(int argc, char **argv) { 
    InputSpec input{argc, argv};

    std::ifstream infile(input.filename);
    std::vector&lt;std::string&gt; lines;  
    std::string line;
    while (std::getline(infile, line))
        lines.push_back(line);

    int maxLines = std::min(input.count, lines.size());

    generator g(maxLines);
    std::set&lt;int&gt; removals;

    while (removals.size() &lt; maxLines)
        removals.insert(g());

    int current_line = 0;
    std::remove_copy_if(lines.begin(), lines.end(), 
        std::ostream_iterator&lt;std::string&gt;(std::cout, "\n"),
        [&amp;](std::string const &amp;) { return removals.find(++current_line) != removals.end(); });
}


Note that for the moment, this presumes that the number of lines to be removed from the file is small compared to the number of lines in the file. If there's a significant chance that won't be true, you may want to see the selection algorithm in yet another old answer.