Make getNoteBounds a separate function

include/Note.h

@@ -26,6 +26,7 @@
 #ifndef NOTE_H
 #define NOTE_H
 
+#include <optional>
 #include <QVector>
 
 #include "volume.h"
@@ -244,14 +245,18 @@ private:
 };
 
 
+typedef QVector<Note *> NoteVector;
 
 
-class NoteVector: public QVector<Note*>
+struct NoteBounds
 {
-public:
-	bool getBounds(TimePos& start, TimePos& end, int& lower, int& upper) const;
-	bool transpose(int semitones) const;
+	TimePos start;
+	TimePos end;
+	int lowest;
+	int highest;
 };
 
 
+std::optional<NoteBounds> getNoteBounds(const NoteVector& notes);
+
 #endif

src/core/JournallingObject.cpp

@@ -58,7 +58,6 @@ void JournallingObject::addJournalCheckPoint()
 	if( isJournalling() )
 	{
 		Engine::projectJournal()->addJournalCheckPoint( this );
-		// TODO Engine::getSong()->setModified() ?
 	}
 }
 

src/core/Note.cpp

@@ -236,28 +236,20 @@ bool Note::withinRange(int tickStart, int tickEnd) const
 
 
 
-
-
-
-
-/*! \brief Get the outer bounds of the notes
+/*! \brief Get the start/end/bottom/top positions of notes in a vector
  *
- *  \param start - Will be set to the first time position
- *  \param end - Will be set to the last end position
- *  \param lower - Will be set to the lowest key
- *  \param upper - Will be set to the highest key
- *  \return false if there are no notes
+ *  Returns no value if there are no notes
  */
-bool NoteVector::getBounds(TimePos& start, TimePos& end, int& lower, int& upper) const
+std::optional<NoteBounds> getNoteBounds(const NoteVector& notes)
 {
-	if (empty()) { return false; }
+	if (notes.empty()) { return {}; }
 
-	start = first()->pos();
-	end = start;
-	lower = first()->key();
-	upper = lower;
+	TimePos start = notes.front()->pos();
+	TimePos end = start;
+	int lower = notes.front()->key();
+	int upper = lower;
 
-	for (const Note* note: *this)
+	for (const Note* note: notes)
 	{
 		// TODO should we assume that NoteVector is always sorted correctly,
 		// so first() always has the lowest time position?
@@ -267,26 +259,5 @@ bool NoteVector::getBounds(TimePos& start, TimePos& end, int& lower, int& upper)
 		upper = std::max(upper, note->key());
 	}
 
-	return true;
-}
-
-
-
-
-/*! \brief Transpose all notes in the vector by X semitones
- *
- *  Notes will be hard-clipped to the MIDI note range. To prevent this use getBounds() prior to transposing.
- *
- *  \param semitones Semitones to transpose
- *  \return bool True if notes were transposed
- */
-bool NoteVector::transpose(int semitones) const
-{
-	if (empty() || !semitones) { return false; }
-
-	for (Note* note: *this)
-	{
-		note->setKey(note->key() + semitones);
-	}
-	return true;
+	return NoteBounds{start, end, lower, upper};
 }

src/gui/clips/MidiClipView.cpp

@@ -153,19 +153,17 @@ void MidiClipView::transposeSelection()
 	{
 		if (auto mcv = dynamic_cast<MidiClipView*>(clipview))
 		{
-			TimePos start, end;
-			int low, high;
-			if (mcv->getMidiClip()->notes().getBounds(start, end, low, high))
+			if (auto bounds = getNoteBounds(mcv->getMidiClip()->notes()))
 			{
-				lowest = std::min(low, lowest);
-				highest = std::max(high, highest);
+				lowest = std::min(bounds->lowest, lowest);
+				highest = std::max(bounds->highest, highest);
 			}
 		}
 	}
 
-	int minStep = 0 - lowest;
-	int maxStep = NumKeys - 1 - highest;
-	int semitones = QInputDialog::getInt(this, tr("Transpose"), tr("Semitones to transpose:"), 0, minStep, maxStep, 1);
+	int semitones = QInputDialog::getInt(this, tr("Transpose"), tr("Semitones to transpose by:"),
+		/*start*/ 0, /*min*/ -lowest, /*max*/ (NumKeys - 1 - highest));
+
 	if (semitones == 0) { return; }
 
 	// TODO make this not crash
@@ -174,20 +172,24 @@ void MidiClipView::transposeSelection()
 	QSet<Track*> m_changedTracks;
 	for (ClipView* clipview: selection)
 	{
-		if (auto mcv = dynamic_cast<MidiClipView*>(clipview))
+		auto mcv = dynamic_cast<MidiClipView*>(clipview);
+		if (!mcv) { continue; }
+
+		auto clip = mcv->getMidiClip();
+		if (clip->notes().empty()) { continue; }
+
+		auto track = clipview->getTrackView()->getTrack();
+		if (!m_changedTracks.contains(track))
 		{
-			auto track = clipview->getTrackView()->getTrack();
-			if (!m_changedTracks.contains(track))
-			{
-				track->addJournalCheckPoint();
-				m_changedTracks.insert(track);
-			}
-			auto clip = mcv->getMidiClip();
-			if (clip->notes().transpose(semitones))
-			{
-				emit clip->dataChanged();
-			}
+			track->addJournalCheckPoint();
+			m_changedTracks.insert(track);
 		}
+
+		for (Note* note: clip->notes())
+		{
+			note->setKey(note->key() + semitones);
+		}
+		emit clip->dataChanged();
 	}
 	// At least one clip must have notes to show the transpose dialog, so something *has* changed
 	Engine::getSong()->setModified();
@@ -219,7 +221,7 @@ void MidiClipView::constructContextMenu( QMenu * _cm )
 			tr( "Clear all notes" ), m_clip, SLOT( clear() ) );
 	if (!isBeat)
 	{
-		_cm->addAction(embed::getIconPixmap("scale"), tr("Transpose"), this, SLOT(transposeSelection()));
+		_cm->addAction(embed::getIconPixmap("scale"), tr("Transpose"), this, &MidiClipView::transposeSelection);
 	}
 	_cm->addSeparator();
 

src/tracks/MidiClip.cpp

@@ -347,11 +347,6 @@ void MidiClip::splitNotes(NoteVector notes, TimePos pos)
 
 
 
-
-
-
-
-
 void MidiClip::setType( MidiClipTypes _new_clip_type )
 {
 	if( _new_clip_type == BeatClip ||