exercism / exercism/csharp

[authentication-system] Missing test + Identity probably should be class

オープン
#1,904 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
C#
スター
427
フォーク
383
平均マージ
2日 2時間
マージ済み PR(30日)
5

説明

I just mentored this exercise and found out that this exercise should probably be improved.

In Task 3 the user is asked to _"Remove the set accessor or make it private"_ and because Identity is a struct and the properties are strings it can no longer be changed (because structs are implicitly copied) but Task 4 now asks the user to prevent tempering with the Admin property (which is not possible because we prevented that in Task 3, because string is immuatble and Identity is a struct) - maybe make Identity a class and link some resources in the instructions about the difference of structs and classes in C#.

In such simple exercises I would even go as far as not to use structs at all(!) unless the exercise is about structs. Don't get me wrong `structs` are important in C# but such a niche that I would consider them an "advanced concept" because there are so many things that might be confusing for example in structs with mutable reference fields, structs that are larger than 16 bytes, etc.

Task 5 asks the user to make a defensive copy of the developer dictionary before returning it but no test tests for that.
I sent the student the following test to make sure they implement a defensive copy:

```csharp
[Fact]
public void DevelopersCantBeTamperedWith()
{
var authenticator = new Authenticator(new Identity { EyeColor = "green", Email = "admin@ex.ism" });

var devs = authenticator.GetDevelopers() as IDictionary;

try
{
devs.Add("Evil Attacker", new Identity { Email = "evil.attacker@ex.ism", EyeColor = "transparent" });
}
catch (NotSupportedException)
{
// Ignore exception from read-only dictionaries.
}

Assert.False(authenticator.GetDevelopers().ContainsKey("Evil Attacker"));
}
```

Something like that should probably be added to the exercise.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。