[C#] constants new exercise #1877
Conversation
Co-authored-by: Erik Schierboom <[email protected]>
Co-authored-by: Erik Schierboom <[email protected]>
Co-authored-by: Erik Schierboom <[email protected]>
There was a problem hiding this comment.
I like the idea of the refactoring exercise. My only concern would be that for a student it would be a relatively boring exercise I feel. I was thinking if perhaps we could introduce another concept in this exercise, possibly the fact that there are read-only collection types?
languages/csharp/exercises/concept/constants/.docs/introduction.md
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,18 @@ | |||
| The `const` modifier can be (and generally should be) applied to any field where its value is known at compile time and will not change during the lifetime of the program. | |||
There was a problem hiding this comment.
I like how briefly you've described it. The const modifier can also be applied to variables, but I assume you'll add that to the after.md file (which makes sense).
|
|
||
| ## 5. Ensure that the developers cannot be tampered with | ||
|
|
||
| At present the dictionary containing the hard coded privileged developer identities is returned by a call to `GetDevelopers()`. This is not ideal as the caller can modify the dictionary. Find a way to prevent the caller modifying the details of admin on the `Authenticator` object. |
There was a problem hiding this comment.
Will students know about ReadOnlyDictionary? If not, this might be something that we could introduce in this exercise too to make the exercise a bit more interesting.
|
|
||
| ## 3. Remove set accessor or make it private for any appropriate field | ||
|
|
||
| Remove the `set` accessor or make it `private` for any appropriate property on the `Authenticator` class or `Identity` struct. |
There was a problem hiding this comment.
In the example, I don't see this being applied to any properties. Am I mis-reading the code?
There was a problem hiding this comment.
You're right. The students would not have responded well to a "trick" question. Changed GetAdmin() to a property and changed task 3 to require immutability. It wouldn't be a bad place to introduce the concept except that we already have 3 concepts.
| @@ -0,0 +1,21 @@ | |||
| The authentication system that you last saw in (TODO cross-ref-tba) is in need of some attention. You have been tasked with cleaning up the code. | |||
|
|
|||
| ## 1. Set appropriate fields and properties to const | |||
There was a problem hiding this comment.
This is a bit of a technical description, but I must admit that I could not come up with a nice name either. Maybe something like "Ensure that constant values cannot be tampered with"
There was a problem hiding this comment.
I think that making variables readonly is consistent with your wording which itself mentions constants. I think the test here is to find all the appropriate candidates. I am taking the view that the syntax is trivial.
| @@ -0,0 +1,21 @@ | |||
| The authentication system that you last saw in (TODO cross-ref-tba) is in need of some attention. You have been tasked with cleaning up the code. | |||
There was a problem hiding this comment.
Maybe you could mention that you'll not only be cleaning up the code, you'll also be hardening it to improve safety or something like that.
| // However, I'm not sure this should be a test. | ||
| // I can't leave the above explanation in as a) it gives too much away, b) the in-browser | ||
| // student won't see it, so the student won't really understand what the problem is in the event | ||
| // of the test failing. |
There was a problem hiding this comment.
Hah yes, this is a tricky one! I think an analyzer could be really useful here, as I also fail to see a way in which you can have this test and not give away the solution.
There was a problem hiding this comment.
Raised issue exercism/csharp-analyzer#56 and removed test.
…n.md Co-authored-by: Erik Schierboom <[email protected]>
|
closed to finalize - will reopen shortly |
|
All files complete and proofed except csharp/config.json which will be updated before merging. |
Please review Constants.cs - starting point, Example.cs - result of refactoring, introduction.md and instructions.md.
All other files except csharp/config and hints.md are complete apart from proofing and show the direction of travel.