Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw ArgumentError when trying to change to a negative Weight. #1719

Merged
merged 8 commits into from
Aug 21, 2024
Merged

Throw ArgumentError when trying to change to a negative Weight. #1719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3c9e995
Add and implement: 'Setting a grade weight to a negative number throw…
Jonas-Sander Aug 21, 2024
13a366e
Add and implement: 'Setting a grade type weight for a subject to a ne…
Jonas-Sander Aug 21, 2024
2e472ec
Add and implement: 'Setting a grade type weight for a term to a negat…
Jonas-Sander Aug 21, 2024
0167e86
Add and implement: 'Setting a subject weight for a term to a negative…
Jonas-Sander Aug 21, 2024
072a750
Merge remote-tracking branch 'origin/main' into grades-weight-error-o…
Jonas-Sander Aug 21, 2024
27ef6ca
Create and use `NonNegativeWeight` class.
Jonas-Sander Aug 21, 2024
90b74d4
Remove redundant code in grades_test.dart
Jonas-Sander Aug 21, 2024
1246bbe
Fix `NonNegativeWeight.fromWeight` constructor
Jonas-Sander Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions app/lib/grades/grades_service/grades_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,11 +752,11 @@ class ConnectedCourse extends Equatable {
});
}

class Weight extends Equatable {
class Weight {
final num asFactor;
num get asPercentage => asFactor * 100;
@override
List<Object?> get props => [asFactor];

bool get isNegative => asFactor < 0;

const Weight.percent(num percent) : asFactor = percent / 100;
const Weight.factor(this.asFactor);
Expand All @@ -766,4 +766,11 @@ class Weight extends Equatable {
String toString() {
return 'Weight($asFactor / $asPercentage%)';
}

@override
bool operator ==(Object other) =>
identical(this, other) || other is Weight && other.asFactor == asFactor;

@override
int get hashCode => asFactor.hashCode;
}
23 changes: 16 additions & 7 deletions app/lib/grades/grades_service/src/grades_repository.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ class FirestoreGradesStateRepository extends GradesStateRepository {
?.gradeComposition
.gradeWeights
.map((key, value) =>
MapEntry(key, value.toWeight()))[dto.id] ??
const Weight.factor(1),
MapEntry(key, value.toNonNegativeWeight()))[dto.id] ??
NonNegativeWeight.factor(1),
);
},
).toIList();
Expand Down Expand Up @@ -259,14 +259,16 @@ class FirestoreGradesStateRepository extends GradesStateRepository {
isFinalGradeTypeOverridden:
termSubject.finalGradeType != subTerm.finalGradeTypeId,
gradeTypeWeightings: termSubject.gradeComposition.gradeTypeWeights
.map((key, value) => MapEntry(GradeTypeId(key), value.toWeight()))
.map((key, value) =>
MapEntry(GradeTypeId(key), value.toNonNegativeWeight()))
.toIMap(),
gradeTypeWeightingsFromTerm: subTerm.gradeTypeWeights
.map((key, value) => MapEntry(GradeTypeId(key), value.toWeight()))
.map((key, value) =>
MapEntry(GradeTypeId(key), value.toNonNegativeWeight()))
.toIMap(),
weightingForTermGrade:
subTerm.subjectWeights[subject.id.value]?.toWeight() ??
const Weight.factor(1),
subTerm.subjectWeights[subject.id.value]?.toNonNegativeWeight() ??
NonNegativeWeight.factor(1),
grades: grades
.where((grade) =>
grade.subjectId == subject.id && grade.termId.value == termId)
Expand Down Expand Up @@ -295,7 +297,7 @@ class FirestoreGradesStateRepository extends GradesStateRepository {
// Change both to num
gradeTypeWeightings: dto.gradeTypeWeights
.map((key, value) =>
MapEntry(GradeTypeId(key), value.toWeight()))
MapEntry(GradeTypeId(key), value.toNonNegativeWeight()))
.toIMap(),
),
)
Expand Down Expand Up @@ -385,6 +387,13 @@ class WeightDto {
);
}

NonNegativeWeight toNonNegativeWeight() {
return switch (type) {
_WeightNumberType.factor => NonNegativeWeight.factor(value),
_WeightNumberType.percent => NonNegativeWeight.percent(value),
};
}

Weight toWeight() {
return switch (type) {
_WeightNumberType.factor => Weight.factor(value),
Expand Down
13 changes: 7 additions & 6 deletions app/lib/grades/grades_service/src/grades_service_internal.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class _GradesServiceInternal {
newTerm = newTerm.addSubject(subject);
}

newTerm = newTerm.changeWeighting(id, weight);
newTerm = newTerm.changeWeighting(id, weight.toNonNegativeWeightOrThrow());
_updateTerm(newTerm);
}

Expand All @@ -234,8 +234,8 @@ class _GradesServiceInternal {
required GradeTypeId gradeType,
required Weight weight,
}) {
final newTerm = _term(termId)
.changeWeightingOfGradeTypeInSubject(id, gradeType, weight);
final newTerm = _term(termId).changeWeightingOfGradeTypeInSubject(
id, gradeType, weight.toNonNegativeWeightOrThrow());
_updateTerm(newTerm);
}

Expand Down Expand Up @@ -326,7 +326,8 @@ class _GradesServiceInternal {
.subjects
.where((element) => element.grades.any((grade) => grade.id == id))
.first;
final newTerm = _term(termId).changeWeightOfGrade(id, subject.id, weight);
final newTerm = _term(termId).changeWeightOfGrade(
id, subject.id, weight.toNonNegativeWeightOrThrow());

_updateTerm(newTerm);
}
Expand All @@ -341,8 +342,8 @@ class _GradesServiceInternal {
{required TermId termId,
required GradeTypeId gradeType,
required Weight weight}) {
final newTerm =
_term(termId).changeWeightingOfGradeType(gradeType, weight: weight);
final newTerm = _term(termId).changeWeightingOfGradeType(gradeType,
weight: weight.toNonNegativeWeightOrThrow());
_updateTerm(newTerm);
}

Expand Down
65 changes: 46 additions & 19 deletions app/lib/grades/grades_service/src/term.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class TermModel extends Equatable {
final TermId id;
final DateTime? createdOn;
final IList<SubjectModel> subjects;
final IMap<GradeTypeId, Weight> gradeTypeWeightings;
final IMap<GradeTypeId, NonNegativeWeight> gradeTypeWeightings;
final GradingSystemModel gradingSystem;
final GradeTypeId finalGradeType;
final bool isActiveTerm;
Expand Down Expand Up @@ -96,7 +96,7 @@ class TermModel extends Equatable {
TermModel _copyWith({
TermId? id,
IList<SubjectModel>? subjects,
IMap<GradeTypeId, Weight>? gradeTypeWeightings,
IMap<GradeTypeId, NonNegativeWeight>? gradeTypeWeightings,
WeightDisplayType? weightDisplayType,
GradeTypeId? finalGradeType,
bool? isActiveTerm,
Expand Down Expand Up @@ -151,7 +151,7 @@ class TermModel extends Equatable {
}

TermModel changeWeightingOfGradeType(GradeTypeId type,
{required Weight weight}) {
{required NonNegativeWeight weight}) {
final newWeights = gradeTypeWeightings.add(type, weight);
final newSubjects = subjects.map((s) {
final newSubject = s.copyWith(gradeTypeWeightingsFromTerm: newWeights);
Expand Down Expand Up @@ -236,7 +236,7 @@ class TermModel extends Equatable {
gradingSystem: gradingSystem,
takenIntoAccount: grade.takeIntoAccount,
gradeType: grade.type,
weight: const Weight.factor(1),
weight: NonNegativeWeight.factor(1),
title: grade.title,
details: grade.details,
);
Expand All @@ -246,7 +246,7 @@ class TermModel extends Equatable {
return subjects.any((s) => s.hasGrade(gradeId));
}

TermModel changeWeighting(SubjectId id, Weight newWeight) {
TermModel changeWeighting(SubjectId id, NonNegativeWeight newWeight) {
final subject = subjects.firstWhere((s) => s.id == id);
final newSubject = subject.copyWith(
weightingForTermGrade: newWeight,
Expand All @@ -258,7 +258,7 @@ class TermModel extends Equatable {
}

TermModel changeWeightingOfGradeTypeInSubject(
SubjectId id, GradeTypeId gradeType, Weight weight) {
SubjectId id, GradeTypeId gradeType, NonNegativeWeight weight) {
final subject = subjects.firstWhere((s) => s.id == id);
final newSubject = subject.changeGradeTypeWeight(gradeType, weight: weight);

Expand All @@ -278,7 +278,7 @@ class TermModel extends Equatable {
}

TermModel changeWeightOfGrade(
GradeId id, SubjectId subjectId, Weight weight) {
GradeId id, SubjectId subjectId, NonNegativeWeight weight) {
final subject = subjects.firstWhere((s) => s.id == subjectId);
final newSubject = subject.copyWith(
grades: subject.grades.replaceFirstWhere(
Expand Down Expand Up @@ -342,9 +342,9 @@ class SubjectModel extends Equatable {
final IList<GradeModel> grades;
final GradeTypeId finalGradeType;
final bool isFinalGradeTypeOverridden;
final Weight weightingForTermGrade;
final IMap<GradeTypeId, Weight> gradeTypeWeightings;
final IMap<GradeTypeId, Weight> gradeTypeWeightingsFromTerm;
final NonNegativeWeight weightingForTermGrade;
final IMap<GradeTypeId, NonNegativeWeight> gradeTypeWeightings;
final IMap<GradeTypeId, NonNegativeWeight> gradeTypeWeightingsFromTerm;
final WeightType weightType;
final String abbreviation;
final Design design;
Expand Down Expand Up @@ -385,10 +385,11 @@ class SubjectModel extends Equatable {
this.connectedCourses = const IListConst([]),
this.isFinalGradeTypeOverridden = false,
this.grades = const IListConst([]),
this.weightingForTermGrade = const Weight.factor(1),
NonNegativeWeight? weightingForTermGrade,
this.gradeTypeWeightings = const IMapConst({}),
this.gradeTypeWeightingsFromTerm = const IMapConst({}),
}) {
}) : weightingForTermGrade =
weightingForTermGrade ?? NonNegativeWeight.factor(1) {
gradeVal = _getGradeVal();
}

Expand Down Expand Up @@ -433,7 +434,7 @@ class SubjectModel extends Equatable {
}

SubjectModel changeGradeTypeWeight(GradeTypeId gradeType,
{required Weight weight}) {
{required NonNegativeWeight weight}) {
return copyWith(
gradeTypeWeightings: gradeTypeWeightings.add(gradeType, weight));
}
Expand Down Expand Up @@ -467,10 +468,10 @@ class SubjectModel extends Equatable {
IList<GradeModel>? grades,
GradeTypeId? finalGradeType,
bool? isFinalGradeTypeOverridden,
Weight? weightingForTermGrade,
NonNegativeWeight? weightingForTermGrade,
GradingSystemModel? gradingSystem,
IMap<GradeTypeId, Weight>? gradeTypeWeightings,
IMap<GradeTypeId, Weight>? gradeTypeWeightingsFromTerm,
IMap<GradeTypeId, NonNegativeWeight>? gradeTypeWeightings,
IMap<GradeTypeId, NonNegativeWeight>? gradeTypeWeightingsFromTerm,
WeightType? weightType,
IList<ConnectedCourse>? connectedCourses,
DateTime? createdOn,
Expand Down Expand Up @@ -506,7 +507,7 @@ class GradeModel extends Equatable {
final GradingSystemModel gradingSystem;
final GradeTypeId gradeType;
final bool takenIntoAccount;
final Weight weight;
final NonNegativeWeight weight;
final Date date;
final String title;
final String? details;
Expand Down Expand Up @@ -548,7 +549,7 @@ class GradeModel extends Equatable {
this.createdOn,
}) : assert(originalInput is String || originalInput is num);

GradeModel changeWeight(Weight weight) {
GradeModel changeWeight(NonNegativeWeight weight) {
return copyWith(weight: weight, takenIntoAccount: weight.asFactor > 0);
}

Expand All @@ -561,7 +562,7 @@ class GradeModel extends Equatable {
GradingSystemModel? gradingSystem,
GradeTypeId? gradeType,
bool? takenIntoAccount,
Weight? weight,
NonNegativeWeight? weight,
String? title,
String? details,
DateTime? createdOn,
Expand All @@ -584,3 +585,29 @@ class GradeModel extends Equatable {
);
}
}

/// A [Weight] that is guaranteed to be non-negative.
///
/// Used so that we can enforce with the type system that a weight is non-negative
/// This is better than just checking for non-negativity in the code because it
/// makes it impossible to create a negative weight in the first place.
class NonNegativeWeight extends Weight {
factory NonNegativeWeight.fromWeight(Weight weight) =>
NonNegativeWeight.factor(weight.asFactor);
NonNegativeWeight.factor(num factor) : super.factor(factor) {
if (factor < 0) {
throw ArgumentError('Weight must be non-negative');
}
}
NonNegativeWeight.percent(num percent) : super.percent(percent) {
if (percent < 0) {
throw ArgumentError('Weight must be non-negative');
}
}
}

extension on Weight {
NonNegativeWeight toNonNegativeWeightOrThrow() {
return NonNegativeWeight.fromWeight(this);
}
}
32 changes: 17 additions & 15 deletions app/test/grades/grades_repository_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,14 @@ void main() {
term0110.changeWeightDisplayType(WeightDisplayType.percent);

term0210.changeGradeTypeWeight(
GradeType.vocabularyTest.id, const Weight.factor(1.5));
GradeType.vocabularyTest.id, NonNegativeWeight.factor(1.5));

term0210.subject(const SubjectId('mathe'))
..changeGradeTypeWeight(const GradeTypeId('my-custom-grade-type'),
const Weight.percent(200))
..grade(GradeId('grade-1')).changeWeight(const Weight.factor(0.5))
NonNegativeWeight.percent(200))
..grade(GradeId('grade-1')).changeWeight(NonNegativeWeight.factor(0.5))
..changeWeightType(WeightType.perGrade)
..changeWeightForTermGrade(const Weight.percent(250));
..changeWeightForTermGrade(NonNegativeWeight.percent(250));

final res = repository.data;

Expand Down Expand Up @@ -536,7 +536,7 @@ void main() {
gradingSystem: GradingSystemModel.zeroToFifteenPoints,
gradeType: const GradeTypeId('my-custom-grade-type'),
takenIntoAccount: true,
weight: const Weight.factor(0.5),
weight: NonNegativeWeight.factor(0.5),
date: Date('2024-10-02'),
title: 'hallo',
details: 'hello',
Expand All @@ -555,8 +555,8 @@ void main() {
gradingSystem: GradingSystemModel.zeroToFifteenPoints,
gradeType: const GradeTypeId('vocabulary-test'),
takenIntoAccount: true,
weight: const Weight.factor(1),
// weight: const Weight.factor(0.5),
weight: NonNegativeWeight.factor(1),
// weight: NonNegativeWeight.factor(0.5),
// date: Date('2024-10-02'),
date: Date('2024-10-03'),
title: 'abcdef',
Expand All @@ -565,13 +565,14 @@ void main() {
]),
finalGradeType: const GradeTypeId('school-report-grade'),
isFinalGradeTypeOverridden: false,
weightingForTermGrade: const Weight.factor(2.5),
weightingForTermGrade: NonNegativeWeight.factor(2.5),
gradeTypeWeightings: IMapConst({
const GradeTypeId('my-custom-grade-type'):
const Weight.factor(2.0)
NonNegativeWeight.factor(2.0)
}),
gradeTypeWeightingsFromTerm: IMapConst({
const GradeTypeId('vocabulary-test'): const Weight.factor(1.5)
const GradeTypeId('vocabulary-test'):
NonNegativeWeight.factor(1.5)
}),
weightType: WeightType.perGrade,
abbreviation: 'M',
Expand All @@ -589,8 +590,9 @@ void main() {
)
],
),
gradeTypeWeightings: IMapConst(
{const GradeTypeId('vocabulary-test'): const Weight.factor(1.5)}),
gradeTypeWeightings: IMapConst({
const GradeTypeId('vocabulary-test'): NonNegativeWeight.factor(1.5)
}),
gradingSystem: GradingSystemModel.zeroToFifteenPoints,
finalGradeType: const GradeTypeId('school-report-grade'),
isActiveTerm: true,
Expand Down Expand Up @@ -620,7 +622,7 @@ void main() {
gradingSystem: GradingSystemModel.oneToSixWithPlusAndMinus,
gradeType: const GradeTypeId('my-custom-grade-type'),
takenIntoAccount: false,
weight: const Weight.factor(1),
weight: NonNegativeWeight.factor(1),
date: Date('2024-10-16'),
title: 'hallo',
details: 'ollah',
Expand All @@ -638,7 +640,7 @@ void main() {
gradingSystem: GradingSystemModel.austrianBehaviouralGrades,
gradeType: const GradeTypeId('oral-participation'),
takenIntoAccount: true,
weight: const Weight.factor(1),
weight: NonNegativeWeight.factor(1),
date: Date('2024-10-18'),
title: 'Beep boop',
details: 'robot noises',
Expand All @@ -647,7 +649,7 @@ void main() {
),
finalGradeType: const GradeTypeId('oral-participation'),
isFinalGradeTypeOverridden: true,
weightingForTermGrade: const Weight.factor(1),
weightingForTermGrade: NonNegativeWeight.factor(1),
gradeTypeWeightings: const IMapConst({}),
gradeTypeWeightingsFromTerm: const IMapConst({}),
weightType: WeightType.inheritFromTerm,
Expand Down
Loading
Loading