0

I have a vector<string> container but the strings are all numbers

EDIT: I tried this:

so now the logic seems to be borked

cleaned this up but experimenting with various attempts to cast std::string to an int is tough lamba was useless and I have run out of ideas for casting std::string without some bug surfacing

template<typename Iterator>void bubbleSort(Iterator first, Iterator last){
    Iterator i, j;
    for (i = first; i != last; i++)
        for (j = first; j < i; j++)
            if (*i < *j)
                std::iter_swap(i, j); // or std::swap(*i, *j);
}

My code to read the source data is

void loadgames(void) { // read the game app id's
ifstream inFile;
ofstream outFile;
string s;
inFile.open("game-list.txt");
if (inFile.is_open()) {
    while (std::getline(inFile, s)) {
        if(s.length() > 0)
            gamelist.push_back(s); 
    };
    inFile.close();
}
//  bubbleSort(gamelist.begin(),gamelist.end());
outFile.open("game-list.txt");
if (outFile.is_open()) {
    for (i = gamelist.begin(); i != gamelist.end(); i++) {
        outFile << *i << endl;
    }
}
outFile.close();

}

The call to is the problem of sorting my vector

bubbleSort(gamelist.begin(),gamelist.end());
  • *do i need to use a custom function to compare? using std::sort* ? Yes, but it would be a fairly trivial function. – Adrian Mole Feb 08 '22 at 18:09
  • Does this answer your question? [How to sort with a lambda?](https://stackoverflow.com/questions/5122804/how-to-sort-with-a-lambda) – Chris Feb 08 '22 at 18:11
  • 1
    "*but the strings are all numbers*" - then why not use `std::vector` instead, and simply parse the strings with `std::stoi()` or equivalent when you are inserting them into the `vector`? – Remy Lebeau Feb 08 '22 at 18:11
  • the data comes from notepad as text so to sort as numeric makes more sense to me, a lambda is one approach but I am struggling with problems in visual studio 2022 – Hardcore Games Feb 08 '22 at 18:14
  • @HardcoreGames "*the data comes from notepad as text*" - so what? You are responsible for inserting the data into your `vector`, so you can transform it however you want to make things easier on yourself. "*I am struggling with problems*" - such as? That would be useful information that you should add as an [edit] to your question. – Remy Lebeau Feb 08 '22 at 18:17
  • 3
    The nasty part is which goes first: 1 or 10? With a number, the ordering is obvious. As a string you might be surprised. – user4581301 Feb 08 '22 at 18:18
  • How many elements are there in this `vector`? As @RemyLebeau suggested, it may be better to use a `vector` instead, even if only temporarilly ([example](https://godbolt.org/z/3YTjcKW13)) - also, you should compare `std::string`s in your comparator - not `vector`s - read the answer you've gotten carefully. – Ted Lyngmo Feb 08 '22 at 18:59
  • There potentially could be 10,000 elements, the project is 64-bit so I am not worried – Hardcore Games Feb 08 '22 at 20:03
  • 1
    @HardcoreGames Please don't keep changing the question, adding code that you got from the answer. That makes this Q&A really hard to get help from in the future. I suggest you revert your changes where you've copied code from the answer - at least the latest edit. If something is borked, it's not in the code we now see in the question. Note: If the conversion from `string` to `int` fails, `stoi` throws an exception. Have you verified that the data in the `vector` is correct? – Ted Lyngmo Feb 08 '22 at 20:05
  • sorry I am still puzzled so I was posting updates – Hardcore Games Feb 08 '22 at 20:19
  • 1
    @HardcoreGames You can ADD new information to your question, but please don't CHANGE past information, it invalidates prior comments/answers and makes it difficult to follow the conversation. – Remy Lebeau Feb 08 '22 at 20:25
  • I have removed useless code and replaced it with functioning code pieces – Hardcore Games Feb 09 '22 at 01:18
  • @HardcoreGames and in doing so, you have INVALIDATED prior comments/answers AGAIN! We warned you NOT to do that! Please roll back your recent edits, and then APPEND the new details, DO NOT replace the original content! – Remy Lebeau Feb 09 '22 at 02:39

1 Answers1

6

As you already know, you can use std::sort() with a custom comparator. The problem is your compare is setup wrong. Its parameters need to be the container's value_type, not the container type. And you have an erroneous . in it.

Try this instead:

std::vector<std::string> gamelist;
// fill gamelist as needed...

auto compare = [](const std::string &a, const std::string &b){
    return std::stoi(a) < std::stoi(b);
};

std::sort(gamelist.begin(), gamelist.end(), compare);

Online Demo

However, I would suggest using std::vector<int> instead (or appropriate integer type, depending on your string contents), and simply parse the std::strings into integers with std::stoi() (or equivalent) before inserting them into the std::vector, eg:

std::vector<int> gamelist;
// fill gamelist as needed...
gamelist.push_back(std::stoi(someString));
...
std::sort(gamelist.begin(), gamelist.end());

Online Demo

This will consume less memory and sort much faster, and it will reduce the complexity of the sort since you will be incurring the overhead of converting strings to integers only one time up front, not on every iteration of the sort algorithm (potentially parsing the same strings over and over).

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • after sorting I was intending to write the text file back as notepad friendly text – Hardcore Games Feb 08 '22 at 18:17
  • I have already written a lot of code based on the std::string but I simply wanted to sort the list as (int) – Hardcore Games Feb 08 '22 at 18:22
  • Nothing stops you from converting a vector of strings into a vector of ints before sorting, and then converting back afterwards. But whatever. My answer already shows you how to sort a vector of strings – Remy Lebeau Feb 08 '22 at 18:26
  • 2
    @HardcoreGames `std::sort` needs `O(nlogn)` comaparisons and the proposed solution transforms the strings for every single comparison. I would not be surprised if transforming to a `std::vector`, sorting, then transforming it back, is less expensive, because it only needs `O(n)` transformations – 463035818_is_not_an_ai Feb 08 '22 at 18:27
  • sort(gamelist.begin(), gamelist.end()[](const string& a, const string& b) bool{ return (int)a < (int)b;) } does not seem to be working right? – Hardcore Games Feb 08 '22 at 18:29
  • 1
    @HardcoreGames You can't type-cast a `string` to an `int` like that, you must use a conversion function instead, like `std::stoi()`, as shown in my answer. Also, your syntax is wrong, as it is missing a comma after `end()`, a `->` before `bool`, and the `}` is in the wrong place. – Remy Lebeau Feb 08 '22 at 18:30
  • For some reason the code abends when I call sort. Not sure what is up outside of a VS bug or 50. – Hardcore Games Feb 08 '22 at 20:00
  • @HardcoreGames "*the code abends when I call sort*" - meaning what, exactly? – Remy Lebeau Feb 08 '22 at 20:26
  • sort with the basic iterators works but adding the custom auto causes it to fail, converted bubble sort to iterators so now this works like sts::sort – Hardcore Games Feb 08 '22 at 20:41
  • @HardcoreGames "*but adding the custom auto causes it to fail*" - what do you mean by "custom auto"? What kind of failure exactly? BE SPECIFIC! – Remy Lebeau Feb 08 '22 at 20:47
  • there seems to be problems with atoi() – Hardcore Games Feb 08 '22 at 22:28
  • @HardcoreGames `atoi()` works just fine (but you should use `stoi()` instead). So the problem has to be with your strings. But since you haven't provided a [mcve], it is hard to troubleshoot exactly what is wrong. – Remy Lebeau Feb 08 '22 at 22:51
  • I have tried all manor of things to cast the string to an int for comparison in the short, even c_str() caused problems with stoi/atio so I am wondering if I need to use &*i and &*j for my iterators to get the sort values – Hardcore Games Feb 08 '22 at 22:55
  • I abandoned the lambda in favor of the template which I hope to be able to massage into functionality but its brutal to get it working – Hardcore Games Feb 08 '22 at 23:03
  • @HardcoreGames The problem is not with `stoi`/`atoi` itself, or lambda vs template. It is clear that your strings likely *do not* contain what you are expecting, so you need to debug and fix them from the beginning. Again, if you would provide a [mcve] AND the data you are actually working with, someone is more likely to help you with that. – Remy Lebeau Feb 08 '22 at 23:21
  • values are one number per line that comes from a PHP interface to a database, typical values with 40, 350, 1345, 7623 and so on up to maybe 1850020 – Hardcore Games Feb 08 '22 at 23:48
  • @HardcoreGames That doesn't explain what the database looks like, or how the numbers are getting into your strings. What part of [mcve] is unclear to you? – Remy Lebeau Feb 08 '22 at 23:53
  • I appended the read function ^ – Hardcore Games Feb 08 '22 at 23:56
  • @HardcoreGames You didn't show the ACTUAL contents of `game-list.txt`. Also see [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/) So it is likely you are pushing an invalid string into your `gamelist` after the file is done being read. – Remy Lebeau Feb 09 '22 at 00:06
  • in my code i check that the input file is opened fine when the while looks checks for the end of the text so there are two conditions being checked – Hardcore Games Feb 09 '22 at 01:14
  • @HardcoreGames Your `while` loop is simply wrong (did you bother to read [the link](https://stackoverflow.com/questions/5605125/) I provided?) so there may be 1 final call to `getline()` that will return a blank string, which you push unconditionally into your `gamelist`, which can then break your `atoi`/`stoi` handling. The loop MUST be changed to `while (std::getline(inFile, s)) { gamelist.push_back(s); }` to operate *properly* (assuming there are no blank lines in the file) – Remy Lebeau Feb 09 '22 at 01:44
  • I have added another conditional to check for an empty string, the syntax is correct – Hardcore Games Feb 09 '22 at 02:19
  • I use !eof which you may have missed – Hardcore Games Feb 09 '22 at 02:27
  • @HardcoreGames I didn't miss it. USING `while(!eof)` IS WRONG TO USE where there hasn't been a prior I/O read to update the EOF state. You clearly do not understand how `eof()` actually works. It is only valid **after** an I/O read, not **before**, as you were originally treating it. But whatever. I'm getting tired of this conversation, you are just dragging this out longer than it needs to be. I'm done. – Remy Lebeau Feb 09 '22 at 02:42
  • I changed the code as you suggested and it seems to have solved the sort issues – Hardcore Games Feb 09 '22 at 02:54