Fix style, minor code fixes, and add review comments.

9 files changed, 103 insertions, 29 deletions

diff --git a/src/drumgizmo.cc b/src/drumgizmo.cc
index 6b01be2..fcc54ad 100644
--- a/src/drumgizmo.cc
+++ b/src/drumgizmo.cc
@@ -357,6 +357,7 @@ void DrumGizmo::getSamples(int ch, int pos, sample_t* s, size_t sz)
 		}
 
 		{
+			// TODO: We should make all audiofiles reference counted and get rid of this lock.
 			std::lock_guard<std::mutex> guard(af.mutex);
 
 			renderSampleEvent(sample_event, pos, s, sz);
diff --git a/src/events.h b/src/events.h
index 1b3c376..e673329 100644
--- a/src/events.h
+++ b/src/events.h
@@ -53,14 +53,15 @@ using EventIDs = std::vector<EventID>;
 class Event
 {
 public:
-	enum class Type {
+	enum class Type
+	{
 		SampleEvent,
 	};
 
 	Event(Type type, channel_t channel, timepos_t offset = 0)
 		: type(type), channel(channel), offset(offset) {}
 
-	virtual ~Event() {}
+	virtual ~Event() = default;
 
 	EventID id;
 	EventGroupID group_id;
diff --git a/src/events_ds.cc b/src/events_ds.cc
index 988cb21..9edbe16 100644
--- a/src/events_ds.cc
+++ b/src/events_ds.cc
@@ -32,7 +32,7 @@
 
 void EventsDS::remove(EventID event_id)
 {
-	auto const& event_info = id_to_info.get(event_id);
+	const auto& event_info = id_to_info.get(event_id);
 
 	if (event_info.type == Event::Type::SampleEvent)
 	{
@@ -64,12 +64,12 @@ std::size_t EventsDS::numberOfEvents(int ch) const
 	return channel_data.sample_events.size();
 }
 
-EventGroupIDs const& EventsDS::getSampleEventGroupIDsOf(InstrumentID instrument_id) const
+const EventGroupIDs& EventsDS::getSampleEventGroupIDsOf(InstrumentID instrument_id) const
 {
 	return instruments_sample_event_group_ids[instrument_id];
 }
 
-EventIDs const& EventsDS::getEventIDsOf(EventGroupID event_group_id) const
+const EventIDs& EventsDS::getEventIDsOf(EventGroupID event_group_id) const
 {
 	return id_to_group_data.get(event_group_id).event_ids;
 }
@@ -90,7 +90,8 @@ void EventsDS::startAddingNewGroup(InstrumentID instrument_id)
 		group_ids.push_back(current_group_id);
 		id_to_group_data.get(current_group_id).instrument_index = group_ids.size() - 1;
 	}
-	else {
+	else
+	{
 		current_groups_instrument_id.invalidate();
 	}
 }
@@ -106,11 +107,11 @@ void EventsDS::removeGroup(EventGroupID group_id, InstrumentID instrument_id)
 
 	if (instrument_id.valid())
 	{
-		auto index = id_to_group_data.get(group_id).instrument_index;
+		auto instrument_index = id_to_group_data.get(group_id).instrument_index;
 		auto& ids = instruments_sample_event_group_ids[instrument_id];
 
-		id_to_group_data.get(ids.back()).instrument_index = index;
-		ids[index] = ids.back();
+		id_to_group_data.get(ids.back()).instrument_index = instrument_index;
+		ids[instrument_index] = ids.back();
 		ids.pop_back();
 	}
 
diff --git a/src/events_ds.h b/src/events_ds.h
index 4a31f47..1b3a4b6 100644
--- a/src/events_ds.h
+++ b/src/events_ds.h
@@ -38,38 +38,47 @@
 #include "range.h"
 #include "instrument.h"
 
+//! TODO: document class as a whole
+//! TODO: What s does DS stand for?
 // TODO: make it possible to choose sizes
-
-struct EventsDS
+class EventsDS
 {
-	//
-	// member functions
-	//
-	EventsDS() = default;
+public:
+	EventsDS() = default; // TODO: Is this needed?
 
+	//! TODO: document all the (public) things!
 	template <typename T, typename... Args>
 	T& emplace(int ch, Args&&... args);
 
+	//! TODO: document all the (public) things!
 	void remove(EventID event_id);
 
+	//! TODO: document all the (public) things!
 	std::size_t numberOfEvents(int ch) const;
 
+	//! TODO: document all the (public) things!
 	template <typename T>
 	ContainerRange<std::vector<T>> iterateOver(int ch);
 
-	EventGroupIDs const& getSampleEventGroupIDsOf(InstrumentID instrument_id) const;
-	EventIDs const& getEventIDsOf(EventGroupID event_group_id) const;
+	//! TODO: document all the (public) things!
+	const EventGroupIDs& getSampleEventGroupIDsOf(InstrumentID instrument_id) const;
+
+	//! TODO: document all the (public) things!
+	const EventIDs& getEventIDsOf(EventGroupID event_group_id) const;
 
+	//! TODO: document all the (public) things!
 	void startAddingNewGroup(InstrumentID instrument_id = InstrumentID());
 
 private:
-	struct ChannelData {
+	struct ChannelData
+	{
 		std::vector<SampleEvent> sample_events;
 	};
 
 	using ChannelEventIndex = std::size_t;
 
-	struct EventInfo {
+	struct EventInfo
+	{
 		Event::Type type;
 		int ch;
 		ChannelEventIndex channel_event_index;
@@ -77,7 +86,9 @@ private:
 		EventInfo(Event::Type type, int ch, ChannelEventIndex channel_event_index)
 			: type(type), ch(ch), channel_event_index(channel_event_index) {}
 	};
-	struct GroupData {
+
+	struct GroupData
+	{
 		EventIDs event_ids;
 		Event::Type type;
 
@@ -121,7 +132,7 @@ T& EventsDS::emplace(int ch, Args&&... args)
 		return sample_event;
 	}
 
-	assert(false);
+	assert(false); // TODO: This should probably be a static_assert instead?
 }
 
 template <typename T>
diff --git a/src/inputprocessor.cc b/src/inputprocessor.cc
index a29ec77..6249c5b 100644
--- a/src/inputprocessor.cc
+++ b/src/inputprocessor.cc
@@ -44,7 +44,6 @@ InputProcessor::InputProcessor(Settings& settings,
                                Random& random)
 	: kit(kit)
 	, events_ds(events_ds)
-	, is_stopping(false)
 	, settings(settings)
 {
 	// Build filter list
@@ -191,7 +190,7 @@ bool InputProcessor::processOnset(event_t& event, std::size_t pos,
 		}
 	}
 
-	// Mute other instruments in the same group
+	// Mute other instruments in the same instrument/choke group
 	applyChokeGroup(settings, kit, *instr, event, events_ds);
 
 	// Apply directed chokes to mute other instruments if needed
diff --git a/src/inputprocessor.h b/src/inputprocessor.h
index 98623d5..546f348 100644
--- a/src/inputprocessor.h
+++ b/src/inputprocessor.h
@@ -58,7 +58,7 @@ public:
 private:
 	DrumKit& kit;
 	EventsDS& events_ds;
-	bool is_stopping; ///< Is set to true when a EventType::Stop event has been seen.
+	bool is_stopping{false}; ///< Is set to true when a EventType::Stop event has been seen.
 
 	bool processOnset(event_t& event, std::size_t pos, double resample_ratio);
 	bool processChoke(event_t& event, std::size_t pos, double resample_ratio);
diff --git a/src/memory_heap.h b/src/memory_heap.h
index e2901b8..d90c50c 100644
--- a/src/memory_heap.h
+++ b/src/memory_heap.h
@@ -51,9 +51,9 @@ public:
 	template <typename... Args>
 	Index emplace(Args&&... args);
 
-	Index add(T const& element);
+	Index add(const T& element);
 	T& get(Index index);
-	T const& get(Index index) const;
+	const T& get(Index index) const;
 	void remove(Index index);
 
 private:
@@ -62,7 +62,7 @@ private:
 };
 
 template <typename T>
-auto MemoryHeap<T>::add(T const& element) -> Index
+auto MemoryHeap<T>::add(const T& element) -> Index
 {
 	if (free_indices.empty())
 	{
@@ -92,6 +92,8 @@ auto MemoryHeap<T>::emplace(Args&&... args) -> Index
 	return free_index;
 }
 
+// Note: MemoryHeap never really deletes anything -- it just overwrites, so
+// old indices will always return a valid item wrt. memory.
 template <typename T>
 T& MemoryHeap<T>::get(Index index)
 {
@@ -100,7 +102,7 @@ T& MemoryHeap<T>::get(Index index)
 }
 
 template <typename T>
-T const& MemoryHeap<T>::get(Index index) const
+const T& MemoryHeap<T>::get(Index index) const
 {
 	assert(index < memory.size());
 	return memory[index];
diff --git a/test/Makefile.am b/test/Makefile.am
index 56d7fa3..d0f718d 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -7,7 +7,8 @@ TESTS = resource enginetest paintertest configfile audiocache \
 	audiocachefile audiocacheidmanager audiocacheeventhandler \
 	randomtest atomictest syncedsettingstest imagecachetest \
 	semaphoretest drumkitcreatortest bytesizeparsertest notifiertest \
-	dgxmlparsertest domloadertest configparsertest midimapparsertest
+	dgxmlparsertest domloadertest configparsertest midimapparsertest \
+	eventsdstest
 
 EXTRA_DIST = \
 	dgunit.h \
@@ -259,4 +260,13 @@ midimapparsertest_SOURCES = \
 	midimapparsertest.cc \
 	dgtest.cc
 
+eventsdstest_CXXFLAGS = -DOUTPUT=\"eventsdstest\" \
+	$(DEBUG_FLAGS) \
+	-I$(top_srcdir)/src
+eventsdstest_LDFLAGS =
+eventsdstest_SOURCES = \
+	$(top_srcdir)/src/events_ds.cc \
+	eventsdstest.cc \
+	dgtest.cc
+
 endif
diff --git a/test/eventsdstest.cc b/test/eventsdstest.cc
new file mode 100644
index 0000000..2616053
--- /dev/null
+++ b/test/eventsdstest.cc
@@ -0,0 +1,49 @@
+/* -*- Mode: c++ -*- */
+/***************************************************************************
+ *            eventsdstest.cc
+ *
+ *  Sun Feb 16 21:38:59 CET 2020
+ *  Copyright 2020 Bent Bisballe Nyeng
+ *  deva@aasimon.org
+ ****************************************************************************/
+
+/*
+ *  This file is part of DrumGizmo.
+ *
+ *  DrumGizmo is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU Lesser General Public License as published by
+ *  the Free Software Foundation; either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  DrumGizmo is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public License
+ *  along with DrumGizmo; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
+ */
+#include "dgunit.h"
+
+#include "../src/events_ds.h"
+
+class EventsDSTest
+	: public DGUnit
+{
+public:
+	EventsDSTest()
+	{
+		DGUNIT_TEST(EventsDSTest::todo);
+	}
+
+public:
+	void todo()
+	{
+		// TODO: Add some tests here
+		DGUNIT_ASSERT(false);
+	}
+};
+
+// Registers the fixture into the 'registry'
+static EventsDSTest test;