Commit 54d4418f authored by Andreas Cord-Landwehr's avatar Andreas Cord-Landwehr
Browse files

Cleanup remaining raw pointers in ICourse interface

parent 02e6f301
......@@ -79,9 +79,9 @@ void TestCourseResource::loadCourseResource()
QCOMPARE(course.description(), "Ein Kurs in (hoch-)deutscher Aussprache.");
QVERIFY(course.language() != nullptr);
QCOMPARE(course.language()->id(), "de");
QCOMPARE(course.unitList().count(), 1);
QCOMPARE(course.units().count(), 1);
const auto unit = course.unitList().first();
const auto unit = course.units().first();
QVERIFY(unit != nullptr);
QCOMPARE(unit->id(), "1");
QCOMPARE(unit->title(), QStringLiteral("Auf der Straße"));
......@@ -116,14 +116,14 @@ void TestCourseResource::unitAddAndRemoveHandling()
// begin of test
std::unique_ptr<Unit> unit(new Unit);
unit->setId("testunit");
const int initialUnitNumber = course.unitList().count();
const int initialUnitNumber = course.units().count();
QCOMPARE(initialUnitNumber, 1);
QSignalSpy spyAboutToBeAdded(&course, SIGNAL(unitAboutToBeAdded(Unit*, int)));
QSignalSpy spyAdded(&course, SIGNAL(unitAdded()));
QCOMPARE(spyAboutToBeAdded.count(), 0);
QCOMPARE(spyAdded.count(), 0);
course.addUnit(std::move(unit));
QCOMPARE(course.unitList().count(), initialUnitNumber + 1);
QCOMPARE(course.units().count(), initialUnitNumber + 1);
QCOMPARE(spyAboutToBeAdded.count(), 1);
QCOMPARE(spyAdded.count(), 1);
}
......
......@@ -80,9 +80,9 @@ void TestEditableCourseResource::loadCourseResource()
QCOMPARE(course.description(), "Ein Kurs in (hoch-)deutscher Aussprache.");
QVERIFY(course.language() != nullptr);
QCOMPARE(course.language()->id(), "de");
QCOMPARE(course.unitList().count(), 1);
QCOMPARE(course.units().count(), 1);
const auto unit = course.unitList().first();
const auto unit = course.units().first();
QVERIFY(unit != nullptr);
QCOMPARE(unit->id(), "1");
QCOMPARE(unit->title(), QStringLiteral("Auf der Straße"));
......@@ -114,14 +114,14 @@ void TestEditableCourseResource::unitAddAndRemoveHandling()
// begin of test
std::unique_ptr<Unit> unit(new Unit);
unit->setId("testunit");
const int initialUnitNumber = course.unitList().count();
const int initialUnitNumber = course.units().count();
QCOMPARE(initialUnitNumber, 1);
QSignalSpy spyAboutToBeAdded(&course, SIGNAL(unitAboutToBeAdded(Unit*, int)));
QSignalSpy spyAdded(&course, SIGNAL(unitAdded()));
QCOMPARE(spyAboutToBeAdded.count(), 0);
QCOMPARE(spyAdded.count(), 0);
course.addUnit(std::move(unit));
QCOMPARE(course.unitList().count(), initialUnitNumber + 1);
QCOMPARE(course.units().count(), initialUnitNumber + 1);
QCOMPARE(spyAboutToBeAdded.count(), 1);
QCOMPARE(spyAdded.count(), 1);
}
......@@ -221,10 +221,10 @@ void TestEditableCourseResource::fileLoadSaveCompleteness()
QVERIFY(course.title() == loadedCourse.title());
QVERIFY(course.description() == loadedCourse.description());
QVERIFY(course.language()->id() == loadedCourse.language()->id());
QVERIFY(course.unitList().count() == loadedCourse.unitList().count());
QVERIFY(course.units().count() == loadedCourse.units().count());
Unit *testUnit = course.unitList().constFirst();
Unit *compareUnit = loadedCourse.unitList().constFirst();
auto testUnit = course.units().constFirst();
auto compareUnit = loadedCourse.units().constFirst();
QVERIFY(testUnit->id() == compareUnit->id());
QVERIFY(testUnit->foreignId() == compareUnit->foreignId());
QVERIFY(testUnit->title() == compareUnit->title());
......
......@@ -93,13 +93,9 @@ public:
m_language = language;
emit languageChanged();
}
QList<Unit *> unitList() override
QVector<std::shared_ptr<Unit>> units() override
{
QList<Unit *> rawList;
for (auto unit : m_units) {
rawList.append(unit.get());
}
return rawList;
return m_units;
}
std::shared_ptr<Unit> addUnit(std::unique_ptr<Unit> unit) override
{
......
......@@ -72,9 +72,9 @@ void TestSkeletonResource::loadSkeletonResource()
QCOMPARE(course.description(), "Artikulate Test Course Description");
QVERIFY(course.language() == nullptr); // a skeleton must not have a language
QCOMPARE(course.unitList().count(), 2);
QCOMPARE(course.units().count(), 2);
const auto unit = course.unitList().first();
const auto unit = course.units().first();
QVERIFY(unit != nullptr);
QCOMPARE(unit->id(), "{11111111-b885-4833-97ff-27cb1ca2f543}");
QCOMPARE(unit->title(), QStringLiteral("Numbers"));
......@@ -106,14 +106,14 @@ void TestSkeletonResource::unitAddAndRemoveHandling()
// begin of test
std::unique_ptr<Unit> unit(new Unit);
unit->setId("testunit");
const int initialUnitNumber = course.unitList().count();
const int initialUnitNumber = course.units().count();
QCOMPARE(initialUnitNumber, 2);
QSignalSpy spyAboutToBeAdded(&course, SIGNAL(unitAboutToBeAdded(Unit*, int)));
QSignalSpy spyAdded(&course, SIGNAL(unitAdded()));
QCOMPARE(spyAboutToBeAdded.count(), 0);
QCOMPARE(spyAdded.count(), 0);
course.addUnit(std::move(unit));
QCOMPARE(course.unitList().count(), initialUnitNumber + 1);
QCOMPARE(course.units().count(), initialUnitNumber + 1);
QCOMPARE(spyAboutToBeAdded.count(), 1);
QCOMPARE(spyAdded.count(), 1);
}
......@@ -183,10 +183,10 @@ void TestSkeletonResource::fileLoadSaveCompleteness()
QCOMPARE(loadedCourse.title(), course.title());
QCOMPARE(loadedCourse.description(), course.description());
QCOMPARE(loadedCourse.language(), course.language());
QCOMPARE(loadedCourse.unitList().count(), course.unitList().count());
QCOMPARE(loadedCourse.units().count(), course.units().count());
Unit *testUnit = course.unitList().constFirst();
Unit *compareUnit = loadedCourse.unitList().constFirst();
auto testUnit = course.units().constFirst();
auto compareUnit = loadedCourse.units().constFirst();
QCOMPARE(testUnit->id(), compareUnit->id());
QCOMPARE(testUnit->foreignId(), compareUnit->foreignId());
QCOMPARE(testUnit->title(), compareUnit->title());
......
......@@ -34,7 +34,7 @@
class CourseStub : public ICourse
{
public:
CourseStub(std::shared_ptr<Language> language, QVector<Unit *> units)
CourseStub(std::shared_ptr<Language> language, QVector<std::shared_ptr<Unit>> units)
: m_language(language)
, m_units(units)
{
......@@ -65,9 +65,9 @@ public:
{
return m_language;
}
QList<Unit *> unitList() override
QVector<std::shared_ptr<Unit>> units() override
{
return m_units.toList();
return m_units;
}
QUrl file() const override
{
......@@ -76,7 +76,7 @@ public:
private:
std::shared_ptr<Language> m_language;
QVector<Unit *> m_units;
QVector<std::shared_ptr<Unit>> m_units;
};
// define one virtual method out of line to pin CourseStub to this translation unit
......@@ -95,7 +95,7 @@ void TestTrainingSession::cleanup()
void TestTrainingSession::createTrainingSessionWithoutUnits()
{
auto language = std::make_shared<Language>();
CourseStub course(language, QVector<Unit *>());
CourseStub course(language, QVector<std::shared_ptr<Unit>>());
LearnerProfile::ProfileManager manager;
TrainingSession session(&manager);
session.setCourse(&course);
......@@ -105,8 +105,8 @@ void TestTrainingSession::createTrainingSessionWithoutUnits()
void TestTrainingSession::createTrainingSessionWithEmptySounds()
{
auto language = std::make_shared<Language>();
Unit unitA;
Unit unitB;
std::shared_ptr<Unit> unitA(new Unit);
std::shared_ptr<Unit> unitB(new Unit);
Phrase *phraseA1 = new Phrase;
Phrase *phraseA2 = new Phrase;
Phrase *phraseB1 = new Phrase;
......@@ -117,12 +117,12 @@ void TestTrainingSession::createTrainingSessionWithEmptySounds()
phraseB1->setId("B1");
phraseB2->setId("B2");
phraseA1->setSound(QUrl::fromLocalFile("/tmp/a1.ogg"));
unitA.addPhrase(phraseA1);
unitA.addPhrase(phraseA2);
unitB.addPhrase(phraseB1);
unitB.addPhrase(phraseB2);
unitA->addPhrase(phraseA1);
unitA->addPhrase(phraseA2);
unitB->addPhrase(phraseB1);
unitB->addPhrase(phraseB2);
CourseStub course(language, QVector<Unit *>({&unitA, &unitB}));
CourseStub course(language, QVector<std::shared_ptr<Unit>>({unitA, unitB}));
LearnerProfile::ProfileManager manager;
TrainingSession session(&manager);
session.setCourse(&course);
......@@ -136,10 +136,9 @@ void TestTrainingSession::createTrainingSessionWithEmptySounds()
void TestTrainingSession::createTrainingSessionWithEmptyUnits()
{
auto language = std::make_shared<Language>();
Unit firstUnit;
Unit secondUnit;
CourseStub course(language, QVector<Unit *>({&firstUnit, &secondUnit}));
std::shared_ptr<Unit> unitA(new Unit);
std::shared_ptr<Unit> unitB(new Unit);
CourseStub course(language, QVector<std::shared_ptr<Unit>>({unitA, unitB}));
LearnerProfile::ProfileManager manager;
TrainingSession session(&manager);
session.setCourse(&course);
......@@ -149,13 +148,13 @@ void TestTrainingSession::createTrainingSessionWithEmptyUnits()
void TestTrainingSession::createTrainingSessionWithUnitsAndPhrases()
{
auto language = std::make_shared<Language>();
Unit unit;
std::shared_ptr<Unit> unit(new Unit);
Phrase *firstPhrase = new Phrase();
Phrase *secondPhrase = new Phrase();
unit.addPhrase(firstPhrase);
unit.addPhrase(secondPhrase);
unit->addPhrase(firstPhrase);
unit->addPhrase(secondPhrase);
CourseStub course(language, QVector<Unit *>({&unit}));
CourseStub course(language, QVector<std::shared_ptr<Unit>>({unit}));
LearnerProfile::ProfileManager manager;
TrainingSession session(&manager);
session.setCourse(&course);
......@@ -165,8 +164,8 @@ void TestTrainingSession::createTrainingSessionWithUnitsAndPhrases()
void TestTrainingSession::iterateCourse()
{
auto language = std::make_shared<Language>();
Unit unitA;
Unit unitB;
std::shared_ptr<Unit> unitA(new Unit);
std::shared_ptr<Unit> unitB(new Unit);
Phrase *phraseA1 = new Phrase;
Phrase *phraseA2 = new Phrase;
Phrase *phraseB1 = new Phrase;
......@@ -180,34 +179,34 @@ void TestTrainingSession::iterateCourse()
phraseA2->setSound(QUrl::fromLocalFile("/tmp/a1.ogg"));
phraseB1->setSound(QUrl::fromLocalFile("/tmp/b1.ogg"));
phraseB2->setSound(QUrl::fromLocalFile("/tmp/b2.ogg"));
unitA.addPhrase(phraseA1);
unitA.addPhrase(phraseA2);
unitB.addPhrase(phraseB1);
unitB.addPhrase(phraseB2);
unitA->addPhrase(phraseA1);
unitA->addPhrase(phraseA2);
unitB->addPhrase(phraseB1);
unitB->addPhrase(phraseB2);
CourseStub course(language, QVector<Unit *>({&unitA, &unitB}));
CourseStub course(language, QVector<std::shared_ptr<Unit>>({unitA, unitB}));
LearnerProfile::ProfileManager manager;
TrainingSession session(&manager);
session.setCourse(&course);
// session assumed to initialize with first units's first phrase
QCOMPARE(session.activeUnit(), &unitA);
QCOMPARE(session.activeUnit(), unitA.get());
QCOMPARE(session.activePhrase(), phraseA1);
QVERIFY(&course == session.course());
// test direct unit setters
session.setUnit(&unitA);
QCOMPARE(session.activeUnit(), &unitA);
session.setUnit(&unitB);
QCOMPARE(session.activeUnit(), &unitB);
session.setUnit(unitA.get());
QCOMPARE(session.activeUnit(), unitA.get());
session.setUnit(unitB.get());
QCOMPARE(session.activeUnit(), unitB.get());
// test direct phrase setters
session.setPhrase(phraseA1);
QCOMPARE(session.activePhrase(), phraseA1);
QCOMPARE(session.activeUnit(), &unitA);
QCOMPARE(session.activeUnit(), unitA.get());
session.setPhrase(phraseB1);
QCOMPARE(session.activePhrase(), phraseB1);
QCOMPARE(session.activeUnit(), &unitB);
QCOMPARE(session.activeUnit(), unitB.get());
// test number of actions
auto actions = session.trainingActions();
......@@ -217,11 +216,11 @@ void TestTrainingSession::iterateCourse()
// test phrase iterators: accept iterator
session.setPhrase(phraseA1);
QCOMPARE(session.activeUnit(), &unitA);
QCOMPARE(session.activeUnit(), unitA.get());
QCOMPARE(session.activePhrase(), phraseA1);
QVERIFY(session.hasNext());
session.accept();
QCOMPARE(session.activeUnit(), &unitA);
QCOMPARE(session.activeUnit(), unitA.get());
QCOMPARE(session.activePhrase(), phraseA2);
session.accept();
QCOMPARE(session.activePhrase(), phraseB1);
......@@ -231,12 +230,12 @@ void TestTrainingSession::iterateCourse()
// test phrase iterators: skip iterator
session.setPhrase(phraseA1);
QCOMPARE(session.activeUnit(), &unitA);
QCOMPARE(session.activeUnit(), unitA.get());
QCOMPARE(session.activePhrase(), phraseA1);
QVERIFY(!session.hasPrevious());
QVERIFY(session.hasNext());
session.skip();
QCOMPARE(session.activeUnit(), &unitA);
QCOMPARE(session.activeUnit(), unitA.get());
QCOMPARE(session.activePhrase(), phraseA2);
session.skip();
QCOMPARE(session.activePhrase(), phraseB1);
......
......@@ -163,14 +163,13 @@ IEditableCourse * EditorSession::displayedCourse() const
void EditorSession::updateDisplayedUnit()
{
auto course = displayedCourse();
Unit * unit{ nullptr };
if (course != nullptr) {
auto units = course->unitList();
auto units = course->units();
if (!units.isEmpty()) {
unit = units.constFirst();
setUnit(units.constFirst().get());
return;
}
}
setUnit(unit);
}
Unit * EditorSession::unit() const
......@@ -216,10 +215,17 @@ Phrase * EditorSession::previousPhrase() const
if (index > 0) {
return m_phrase->unit()->phraseList().at(index - 1);
} else {
Unit *unit = m_phrase->unit();
int uIndex = unit->course()->unitList().indexOf(unit);
auto unit = m_phrase->unit();
int uIndex{ -1 };
for (int i = 0; i < unit->course()->units().size(); ++i) {
auto testUnit = unit->course()->units().at(i);
if (testUnit->id() == unit->id()) {
uIndex = i;
break;
}
}
if (uIndex > 0) {
return unit->course()->unitList().at(uIndex - 1)->phraseList().last();
return unit->course()->units().at(uIndex - 1)->phraseList().last();
}
}
return nullptr;
......@@ -235,9 +241,16 @@ Phrase * EditorSession::nextPhrase() const
return m_phrase->unit()->phraseList().at(index + 1);
} else {
Unit *unit = m_phrase->unit();
int uIndex = unit->course()->unitList().indexOf(unit);
if (uIndex < unit->course()->unitList().length() - 1) {
Unit *nextUnit = unit->course()->unitList().at(uIndex + 1);
int uIndex{ -1 };
for (int i = 0; i < unit->course()->units().size(); ++i) {
auto testUnit = unit->course()->units().at(i);
if (testUnit->id() == unit->id()) {
uIndex = i;
break;
}
}
if (uIndex < unit->course()->units().length() - 1) {
auto nextUnit = unit->course()->units().at(uIndex + 1);
if (nextUnit->phraseList().isEmpty()) {
return nullptr;
}
......
......@@ -56,7 +56,7 @@ public:
* @brief Lazy loading unit list
* @return list of units in course
*/
virtual QList<Unit *> unitList() = 0;
virtual QVector<std::shared_ptr<Unit>> units() = 0;
virtual QUrl file() const = 0;
Q_SIGNALS:
......@@ -65,7 +65,7 @@ Q_SIGNALS:
void descriptionChanged();
void languageChanged();
void unitAdded();
void unitAboutToBeAdded(Unit*,int);
void unitAboutToBeAdded(std::shared_ptr<Unit>,int);
void unitsRemoved();
void unitsAboutToBeRemoved(int,int);
};
......
......@@ -303,7 +303,7 @@ QDomDocument CourseParser::serializedDocument(ICourse *course, bool trainingExpo
QDomElement unitListElement = document.createElement(QStringLiteral("units"));
// create units
for (Unit *unit : course->unitList()) {
for (auto unit : course->units()) {
QDomElement unitElement = document.createElement(QStringLiteral("unit"));
QDomElement unitIdElement = document.createElement(QStringLiteral("id"));
......@@ -410,7 +410,7 @@ bool CourseParser::exportCourseToGhnsPackage(ICourse *course, const QString &exp
return false;
}
for (auto *unit : course->unitList()) {
for (auto unit : course->units()) {
for (auto *phrase : unit->phraseList()) {
if (QFile::exists(phrase->soundFileUrl())) {
tar.addLocalFile(phrase->soundFileUrl(), phrase->id() + ".ogg");
......
......@@ -246,22 +246,11 @@ void CourseResource::setLanguage(std::shared_ptr<Language> language)
std::shared_ptr<Unit> CourseResource::addUnit(std::unique_ptr<Unit> unit)
{
emit unitAboutToBeAdded(unit.get(), d->m_units.count() - 1);
d->m_units.append(std::move(unit));
std::shared_ptr<Unit> storedUnit(std::move(unit));
emit unitAboutToBeAdded(storedUnit, d->m_units.count() - 1);
d->m_units.append(storedUnit);
emit unitAdded();
return d->m_units.last();
}
QList<Unit *> CourseResource::unitList()
{
if (d->m_courseLoaded == false) {
d->loadCourse(this);
}
QList<Unit *> rawList;
for (auto unit : d->m_units) {
rawList.append(unit.get());
}
return rawList;
return storedUnit;
}
QVector<std::shared_ptr<Unit>> CourseResource::units()
......
......@@ -99,9 +99,7 @@ public:
QUrl file() const override;
QList<Unit *> unitList() override;
QVector<std::shared_ptr<Unit>> units();
QVector<std::shared_ptr<Unit>> units() override;
Q_SIGNALS:
void idChanged();
......
......@@ -109,11 +109,6 @@ void EditableCourseResource::setLanguage(std::shared_ptr<Language> language)
m_course->setLanguage(language);
}
QList<Unit *> EditableCourseResource::unitList()
{
return m_course->unitList();
}
QUrl EditableCourseResource::file() const
{
return m_course->file();
......@@ -161,6 +156,11 @@ std::shared_ptr<Unit> EditableCourseResource::addUnit(std::unique_ptr<Unit> unit
return m_course->addUnit(std::move(unit));
}
QVector<std::shared_ptr<Unit>> EditableCourseResource::units()
{
return m_course->units();
}
bool EditableCourseResource::isModified() const
{
return m_modified;
......
......@@ -108,16 +108,14 @@ public:
bool exportCourse(const QUrl &filePath);
std::shared_ptr<Unit> addUnit(std::unique_ptr<Unit> unit) override;
QVector<std::shared_ptr<Unit>> units() override;
bool isModified() const;
QUrl file() const override;
void setFile(const QUrl &) {}
QList<Unit *> unitList() override;
Q_INVOKABLE Unit * createUnit();
Q_INVOKABLE Phrase * createPhrase(Unit *unit);
......
......@@ -85,7 +85,7 @@ public:
QVector<std::shared_ptr<Unit>> units();
std::shared_ptr<Unit> appendUnit(std::unique_ptr<Unit> unit);
std::shared_ptr<Unit> appendUnit(std::shared_ptr<Unit> unit);
/**
* @return the skeleton resource as serialized byte array
......@@ -115,9 +115,9 @@ QVector<std::shared_ptr<Unit>> SkeletonResourcePrivate::units()
return m_units;
}
std::shared_ptr<Unit> SkeletonResourcePrivate::appendUnit(std::unique_ptr<Unit> unit) {
std::shared_ptr<Unit> SkeletonResourcePrivate::appendUnit(std::shared_ptr<Unit> unit) {
units(); // ensure that units are parsed
m_units.append(std::move(unit));
m_units.append(unit);
return m_units.last();
}
......@@ -284,10 +284,11 @@ bool SkeletonResource::exportCourse(const QUrl &filePath)
std::shared_ptr<Unit> SkeletonResource::addUnit(std::unique_ptr<Unit> unit)
{
emit unitAboutToBeAdded(unit.get(), d->units().count() - 1);
auto sharedUnit = d->appendUnit(std::move(unit));
std::shared_ptr<Unit> storedUnit(std::move(unit));
emit unitAboutToBeAdded(storedUnit, d->units().count() - 1);
d->appendUnit(storedUnit);
emit unitAdded();
return sharedUnit;
return storedUnit;
}
std::shared_ptr<Language> SkeletonResource::language() const
......@@ -302,13 +303,9 @@ void SkeletonResource::setLanguage(std::shared_ptr<Language> language)
Q_UNREACHABLE();
}
QList<Unit *> SkeletonResource::unitList()
QVector<std::shared_ptr<Unit>> SkeletonResource::units()
{
QList<Unit *> rawList;
for (auto unit : d->units()) {
rawList.append(unit.get());
}
return rawList;
return d->units();
}
QUrl SkeletonResource::file() const
......
......@@ -59,7 +59,7 @@ public:
void setDescription(QString description) override;
std::shared_ptr<Language> language() const override;
void setLanguage(std::shared_ptr<Language> language) override;
QList<Unit *> unitList() override;
QVector<std::shared_ptr<Unit>> units() override;
QUrl file() const override;
bool exportCourse(const QUrl &filePath);
......
......@@ -51,8 +51,8 @@ void TrainingSession::setCourse(ICourse *course)
return;
}
m_course = course;
if (m_course && m_course->unitList().count() > 0) {
setUnit(m_course->unitList().constFirst());
if (m_course && m_course->units().count() > 0) {
setUnit(m_course->units().constFirst().get());
}
// lazy loading of training data
......@@ -69,8 +69,8 @@ void TrainingSession::setCourse(ICourse *course)
goal,
m_course->id()
);
const auto unitList = m_course->unitList();
for (Unit *unit : qAsConst(unitList)) {
const auto unitList = m_course->units();
for (auto unit : qAsConst(unitList)) {
const auto phraseList = unit->phraseList();
for (Phrase *phrase : qAsConst(phraseList)) {