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.

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

replaced CanWrite with GetSetMethod != null #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DaniD3v
Copy link

@DaniD3v DaniD3v commented Oct 6, 2024

CanWrite returns true if the property has a set accessor,
even if the accessor is private, internal (or Friend in Visual Basic), or protected.

Source

This is a problem if someone were to write their Property like this:

public decimal Limit {get; private set;}

canWrite will return true here even tho you can't actually write to it.

This PR only fixes a single occasion of this in 05_Vererbung but a single rg CanWrite shows that this anti-pattern is used across the whole teaching material

PLF/Types - Plf3bhif20191113/Program.cs:            score += AssertTrue(() => info.CanWrite == false);
PLF/Types - Plf3bhif20191113/Program.cs:            score += AssertTrue(() => info2.CanWrite == false);
01 Types/03_Properties.md:            if (typeof(Teacher).GetProperty(nameof(Teacher.Firstname))?.CanWrite == false
01 Types/03_Properties.md:                && typeof(Teacher).GetProperty(nameof(Teacher.Lastname))?.CanWrite == false)
01 Types/03_Properties.md:            if (typeof(Teacher).GetProperty(nameof(Teacher.Longname))?.CanWrite == false
01 Types/03_Properties.md:            if (typeof(Teacher).GetProperty(nameof(Teacher.Shortname))?.CanWrite == false
01 Types/03_Properties.md:            if (typeof(Teacher).GetProperty(nameof(Teacher.IsSchoolEmail))?.CanWrite == false
01 Types/Uebungen/Blog/BlogManager.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);
01 Types/Uebungen/LibraryManager/LibraryManager.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);
01 Types/Uebungen/Anmeldesystem/RegistrationSystem.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);
01 Types/Uebungen/Anmeldesystem/RegistrationSystem.Application/Program.cs:                () => typeof(RegistrationService).GetProperties().Any() && typeof(RegistrationService).GetProperties().All(p => p.CanWrite == false),
01 Types/Uebungen/Anmeldesystem/RegistrationSystem.Application/Program.cs:                () => typeof(Applicant).GetProperties().Any() && typeof(Applicant).GetProperties().All(p => p.CanWrite == false),
01 Types/Uebungen/Anmeldesystem/RegistrationSystem.Application/Program.cs:                () => typeof(AcceptedApplicant).GetProperties().Any() && typeof(AcceptedApplicant).GetProperties().All(p => p.CanWrite == false),
01 Types/Uebungen/Kalender/TeamCalendar.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);
01 Types/Uebungen/HealthChecker/HealthChecker.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);
01 Types/Uebungen/HealthChecker/HealthChecker.Application/Program.cs:                CheckAndWrite(() => typeof(Vaccination).GetProperty(nameof(Vaccination.FirstVaccination))?.CanWrite == false, "FirstVaccination is read-only.");
01 Types/Uebungen/CarRentalService/CarRentalService.Application/TestHelpers.cs:            type.GetProperty(propertyName)?.CanWrite == false;
01 Types/Uebungen/CarRentalService/CarRentalService.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);
02 Linq/02_Filterung/LinqUebung1/LinqUebung1.Application/TestHelpers.cs:            return type.GetProperties().All(p => p.CanWrite == false);

In my commit I also added a method to make the code more DRY.
I don't really get why this isn't done in all tests because those should follow good practices in case students were to learn from them.

@schletz schletz force-pushed the master branch 29 times, most recently from 53f1b5d to 1d4c426 Compare November 29, 2024 20:59
@schletz schletz force-pushed the master branch 9 times, most recently from 0995d2a to 8033e38 Compare December 2, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant