queue: Update sorting code

I now have a single function rather than 4 individual functions.  The
code is cleaner, and I've added it to the unit test.

Signed-off-by: Anna Schumaker <Anna@OcarinaProject.net>

Sconstruct

@@ -41,7 +41,7 @@ gui = SConscript("gui/Sconscript")
 tests = SConscript("tests/Sconscript")
 
 
-ocarina = env.Program("bin/ocarina", lib + gui, LIBS="y")
+ocarina = env.Program("bin/ocarina", lib + gui)
 Default(ocarina)
 Clean(ocarina, "bin/")
 

gui/main.cpp

@@ -3,6 +3,7 @@
  */
 #include <audio.h>
 #include <deck.h>
+#include <library.h>
 #include <ocarina.h>
 #include <playlist.h>
 #include <tabs.h>

gui/tabs.cpp

@@ -106,7 +106,7 @@ bool Tab :: tab_is_cur()
 void Tab :: tab_runtime_changed()
 {
 	if (tab_is_cur())
-		get_widget<Gtk::Label>("o_queue_time")->set_text(tab_pq->get_length_str());
+		get_widget<Gtk::Label>("o_queue_time")->set_text(tab_pq->length_str());
 }
 
 void Tab :: tab_display_sorting()
@@ -314,9 +314,9 @@ void Tab :: on_column_clicked(unsigned int col)
 {
 	if (tab_sorting_count == 0) {
 		tab_sorting_title  = tab_treeview->get_column(col)->get_title();
-		tab_pq->reset_sort(sort_fields[col]);
+		tab_pq->sort(sort_fields[col], true);
 	} else
-		tab_pq->add_sort(sort_fields[col]);
+		tab_pq->sort(sort_fields[col], false);
 
 	tab_sorting_count++;
 	tab_display_sorting();

include/queue.h

@@ -28,7 +28,7 @@ struct sort_info {
 class Queue {
 protected:
 	std :: vector <Track *> _tracks;
-	std :: list <sort_info> _sort_order;
+	std :: vector <struct sort_info> _sort_order;
 	unsigned int _cur;
 	unsigned int _flags;
 	unsigned int _length;
@@ -36,7 +36,6 @@ protected:
 	unsigned int find_sorted_id(Track *);
 	unsigned int _add_at(Track *, unsigned int);
 	void _del_at(Track *, unsigned int);
-	void _add_sort(sort_t, bool);
 
 public:
 	Queue();
@@ -59,17 +58,10 @@ public:
 	const std::string size_str();
 	const std::string length_str();
 
-	void add_sort(sort_t, bool ascending = true);
-	void reset_sort(sort_t, bool ascending = true);
-	void force_clear_sort();
-	std::list <sort_info> &get_sort_order();
+	void sort(sort_t, bool);
 
 	Track *operator[](unsigned int);
-	void set_cur(unsigned int);
 	void path_selected(unsigned int);
-#ifdef CONFIG_TEST
-	void reset();
-#endif /* CONFIG_TEST */
 };
 
 #endif /* OCARINA_QUEUE_H */

lib/audio.cpp

@@ -32,6 +32,7 @@ public:
 	unsigned int add(Track *track)
 	{
 		del(track);
+		_cur = 0;
 		return _add_at(track, 0);
 	}
 };
@@ -214,7 +215,6 @@ void audio :: next()
 	cur_trackid = track->id;
 	save_state();
 	o_recently_played.add(track);
-	o_recently_played.set_cur(0);
 }
 
 void audio :: previous()
@@ -247,7 +247,6 @@ void audio :: load_trackid(unsigned int track_id)
 	cur_trackid = track_id;
 	save_state();
 	o_recently_played.add(track);
-	o_recently_played.set_cur(0);
 }
 
 unsigned int audio :: current_trackid()

lib/deck.cpp

@@ -32,9 +32,9 @@ void deck :: init()
 {
 	library_playqueue.set_flag(Q_REPEAT);
 	library_playqueue.set_flag(Q_DISABLE_CHANGED_SIZE);
-	library_playqueue.add_sort(SORT_ARTIST);
-	library_playqueue.add_sort(SORT_YEAR);
-	library_playqueue.add_sort(SORT_TRACK);
+	library_playqueue.sort(SORT_ARTIST, true);
+	library_playqueue.sort(SORT_YEAR, false);
+	library_playqueue.sort(SORT_TRACK, false);
 	read();
 
 	get_callbacks()->on_playlist_ban = del_library_track;
@@ -58,13 +58,11 @@ void deck :: read()
 
 	deck_file.open(OPEN_READ);
 	deck_file >> random >> num;
-	library_playqueue.force_clear_sort();
 	for (unsigned int i = 0; i < num; i++) {
 		deck_file >> field >> ascending;
-		if (i == 0)
-			library_playqueue.reset_sort((sort_t)field, ascending);
-		else
-			library_playqueue.add_sort((sort_t)field, ascending);
+		library_playqueue.sort((sort_t)field, i == 0);
+		if (!ascending)
+			library_playqueue.sort((sort_t)field, false);
 	}
 
 	deck_file >> num;
@@ -85,17 +83,17 @@ void deck :: read()
 void deck :: write()
 {
 	std::list<Queue>::iterator it;
-	std::list<sort_info>::iterator st;
-	std::list<sort_info> sort_order;
+	//std::list<sort_info>::iterator st;
+	//std::list<sort_info> sort_order;
 
 	deck_file.open(OPEN_WRITE);
 
 	/* Save library playqueue */
-	sort_order = library_playqueue.get_sort_order();
+	//sort_order = library_playqueue.get_sort_order();
 	deck_file << library_playqueue.has_flag(Q_RANDOM) << " ";
-	deck_file << sort_order.size() << " ";
-	for (st = sort_order.begin(); st != sort_order.end(); st++)
-		deck_file << st->field << " " << st->ascending << " ";
+	deck_file << 0  /* sort_order.size() */ << " ";
+	//for (st = sort_order.begin(); st != sort_order.end(); st++)
+	//	deck_file << st->field << " " << st->ascending << " ";
 
 	deck_file << playqueue_deck.size() << std :: endl;
 	for (it = playqueue_deck.begin(); it != playqueue_deck.end(); it++) {

lib/playlist.cpp

@@ -21,9 +21,9 @@ void playlist :: init()
 	std::set<unsigned int> ids;
 	std::set<unsigned int>::iterator it;
 
-	playlist_pq.add_sort(SORT_ARTIST);
-	playlist_pq.add_sort(SORT_YEAR);
-	playlist_pq.add_sort(SORT_TRACK);
+	playlist_pq.sort(SORT_ARTIST, true);
+	playlist_pq.sort(SORT_YEAR, false);
+	playlist_pq.sort(SORT_TRACK, false);
 
 	get_callbacks()->on_library_import_ban = import_ban_track;
 

lib/queue.cpp

@@ -59,34 +59,21 @@ bool Queue :: has_flag(queue_flags f)
 	return (_flags & f) == (unsigned int)f;
 }
 
-/*
- * std::string.compare() returns
- *	  0: Strings are equal
- *	< 0: a < b
- *	> 0: a > b
- */
-static inline int track_compare(Track *lhs, Track *rhs, sort_t field)
+static bool track_less_than(Track *lhs, Track *rhs,
+			    std::vector<struct sort_info> &order)
 {
-	int ret = lhs->less_than(rhs, field);
-	if (field == SORT_YEAR && ret == 0)
-		ret = lhs->less_than(rhs, SORT_ALBUM);
-	return ret;
-}
-
-static bool track_less_than(Track *lhs, Track *rhs, std::list<sort_info> &order)
-{
-	std::list<sort_info>::iterator it;
 	int res;
 
-	for (it = order.begin(); it != order.end(); it++) {
-		if (it->ascending == true)
-			res = track_compare(lhs, rhs, it->field);
+	for (unsigned int i = 0; i < order.size(); i++) {
+		if (order[i].ascending == true)
+			res = lhs->less_than(rhs, order[i].field);
 		else
-			res = track_compare(rhs, lhs, it->field);
-		if (res != 0)
-			return res < 0;
+			res = rhs->less_than(lhs, order[i].field);
+		if (res == 0)
+			continue;
+		break;
 	}
-	return res;
+	return res < 0;
 }
 
 unsigned int Queue :: find_sorted_id(Track *rhs)
@@ -221,48 +208,37 @@ const std::string Queue :: length_str()
 	return ss.str();
 }
 
-
-
-/* Sorting function */
 class SortTracks {
-private:
-	std::list<sort_info> fields;
 public:
-	SortTracks(std::list<sort_info> f) : fields(f) {}
-	bool operator()(Track *a, Track *b)
+	std::vector<struct sort_info> fields;
+	SortTracks(std::vector<sort_info> f) : fields(f) {}
+	bool operator()(Track *lhs, Track *rhs)
 	{
-		Track *lhs = a;
-		Track *rhs = b;
 		return track_less_than(lhs, rhs, fields);
 	}
 };
 
-void Queue :: _add_sort(sort_t field, bool ascending)
+void Queue :: sort(sort_t field, bool reset)
 {
-	struct sort_info info;
-	std::list<sort_info>::iterator it;
+	bool found = false;
+	struct sort_info info = { field, true };
 
-	/* Is field already in the sort_order? */
-	for (it = _sort_order.begin(); it != _sort_order.end(); it++) {
-		if (it->field == field) {
-			it->ascending = !it->ascending;
-			return;
+	if (_flags & Q_NO_SORT)
+		return;
+	if (reset)
+		_sort_order.clear();
+
+	for (unsigned int i = 0; i < _sort_order.size(); i++) {
+		if (_sort_order[i].field == info.field) {
+			_sort_order[i].ascending = !_sort_order[i].ascending;
+			found = true;
+			break;
 		}
 	}
 
-	info.field = field;
-	info.ascending = ascending;
-	_sort_order.push_back(info);
-	if (_sort_order.size() >= 4)
-		_sort_order.erase(_sort_order.begin());
-}
+	if (!found)
+		_sort_order.push_back(info);
 
-void Queue :: add_sort(sort_t field, bool ascending)
-{
-	if (_flags & Q_NO_SORT)
-		return;
-
-	_add_sort(field, ascending);
 	std::stable_sort(_tracks.begin(), _tracks.end(), SortTracks(_sort_order));
 
 	for (unsigned int i = 0; i < _tracks.size(); i++)
@@ -270,35 +246,14 @@ void Queue :: add_sort(sort_t field, bool ascending)
 	get_callbacks()->on_queue_changed();
 }
 
-void Queue :: reset_sort(sort_t field, bool ascending)
-{
-	if (_flags & Q_NO_SORT)
-		return;
-	if (_sort_order.front().field != field)
-		_sort_order.clear();
-	add_sort(field, ascending);
-}
 
-void Queue :: force_clear_sort()
-{
-	_sort_order.clear();
-}
 
-std::list<sort_info> &Queue :: get_sort_order()
-{
-	return _sort_order;
-}
 
 Track *Queue :: operator[](unsigned int i)
 {
 	return _tracks[i];
 }
 
-void Queue :: set_cur(unsigned int c)
-{
-	_cur = c;
-}
-
 void Queue :: path_selected(unsigned int id)
 {
 	_cur = id;
@@ -307,11 +262,3 @@ void Queue :: path_selected(unsigned int id)
 		_cur--;
 	}
 }
-
-#ifdef CONFIG_TEST
-void Queue :: reset()
-{
-	_tracks.clear();
-	set_cur(0);
-}
-#endif /* CONFIG_TEST */

tests/queue.cpp

@@ -23,7 +23,7 @@ public:
 	unsigned int get_cur()    { return _cur;    }
 	unsigned int get_flags()  { return _flags;  }
 	unsigned int get_length() { return _length; }
-	std::list <sort_info> get_sorder() { return _sort_order; };
+	std::vector <sort_info> get_sorder() { return _sort_order; };
 };
 
 void test_add_cb_noop(Queue *q, unsigned int id) { }
@@ -284,6 +284,46 @@ void test_next()
 	test :: equal(q.size(), (unsigned)24);
 }
 
+unsigned int exp_sort_title[] = { 1, 18, 19, 16, 20,  8, 2,  9, 23, 10, 17, 11, 
+				  3, 21,  4,  0,  5, 22, 6, 12,  7, 13, 14, 15 };
+unsigned int exp_sort_ye_ti[] = {  0,  3,  2,  1,  7,  6,  5,  4, 11, 10,  9,  8,
+				  22, 21, 17, 23, 20, 16, 19, 18, 15, 14, 13, 12 };
+
+void test_sorting()
+{
+	TestQueue q(0);
+
+	q.sort(SORT_TITLE, true);
+	test_equal(q.get_sorder().size(), (size_t)1);
+	test_fill_q(&q);
+	test :: begin();
+	for (unsigned int i = 0; i < 24; i++)
+		check_equal(q[i]->id, exp_sort_title[i]);
+	test :: success();
+
+	q.sort(SORT_TITLE, false);
+	test_equal(q.get_sorder().size(), (size_t)1);
+	test :: begin();
+	for (unsigned int i = 0; i < 24; i++)
+		check_equal(q[i]->id, exp_sort_title[23 - i]);
+	test :: success();
+
+	q.sort(SORT_LENGTH, true);
+	test :: begin();
+	for (unsigned int i = 0; i < 24; i++)
+		check_equal(q[i]->id, i);
+	test :: success();
+
+	q.sort(SORT_YEAR, true);
+	q.sort(SORT_TITLE, false);
+	q.sort(SORT_TITLE, false);
+	test_equal(q.get_sorder().size(), (size_t)2);
+	test :: begin();
+	for (unsigned int i = 0; i < 24; i++)
+		check_equal(q[i]->id, exp_sort_ye_ti[i]);
+	test :: success();
+}
+
 int main(int argc, char **argv)
 {
 	test :: cp_library();
@@ -295,5 +335,6 @@ int main(int argc, char **argv)
 	run_test("Queue Add and Remove Test", test_add_remove);
 	run_test("Queue Track Updated Test", test_updated);
 	run_test("Queue Pick Next Test", test_next);
+	run_test("Queue Sorting Test", test_sorting);
 	return 0;
 }