This repository has been archived by the owner on Aug 31, 2021. It is now read-only.
[C#] constants new exercise #1877
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| #### const | ||
|
|
||
| The [`const`][constants] 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. | ||
|
|
||
| ```csharp | ||
| private const int num = 1729; | ||
| public const string title = "Grand" + " Master"; | ||
| ``` | ||
|
|
||
| The compiler will guide you as to what expressions can be evaluated at compile-time. Simple arithmetic operations are allowed as is string concatenation. This excludes any evaluation that would require use of the heap or stack so all method calls and references to non-primitive types are not available. | ||
|
|
||
| There is some discussion on the web about the performance advantages of `const` over variables. In the case of a comparison with instance non-const fields there could be a noticeable saving in memory but compared to static variables that is decidedly trivial. Any consideration of CPU performance is likely to be seen by your colleagues as [premature optimization][premature-optimization]. | ||
|
|
||
| A more compelling reason to use `const` is that it enhances a maintainer's ability to reason about the code. Glimpsing that a field is marked as `const` or `readonly` or that a property has no setter allows the maintainer largely to dismiss it from their analysis. It is unlikely to be the seat of bugs. It is unlikely to pose difficulties in a refactoring exercise. This [Stack Overflow comment][so-consts] addresses this. | ||
|
|
||
| The `const` modifier can also be applied to values within methods: | ||
|
|
||
| ```csharp | ||
| public double Area(double r) | ||
| { | ||
| const double π = 3.142; | ||
| return System.Math.Pow((π * r), 2); | ||
| } | ||
| ``` | ||
|
|
||
| Identifying a value with `const` in this way can be useful if it is used multiple times in the method or you want to draw attention to its meaning. There is no performance gain over using literals inline. | ||
|
|
||
| #### readonly | ||
|
|
||
| The [`readonly`][readonly-fields] modifier can be (and generally should be) applied to any field that cannot be made `const` where its value will not change during the lifetime of the program and is either set by an inline initializer or during instantiation (by the constructor or a method called by the constructor). | ||
|
|
||
| ```csharp | ||
| private readonly int num; | ||
| private readonly System.Random rand = new System.Random(); | ||
|
|
||
| public MyClass(int num) | ||
| { | ||
| this.num = num; | ||
| } | ||
| ``` | ||
|
|
||
| Use of the `readonly` modifier is encouraged for the same reasons that apply to `const`. The practice of constraining fields in this way helps maintainers reason about the code. | ||
|
|
||
| Note that adding the `readonly` modifier to a field prevents only the value of the field from being changed. In the case of aggregate types it does not protect the fields or properties of that type. In particular, it does not protect the contents of arrays. | ||
|
|
||
| ```csharp | ||
| private readonly IList list = new List(); | ||
|
|
||
| public void Foo() | ||
| { | ||
| list = new List(); // does not compile | ||
|
|
||
| list.Add("new stuff"); // succeeds at runtime | ||
| } | ||
| ``` | ||
|
|
||
| To ensure that all members of a reference type are protected the fields can be made `readonly` and automatic properties can be defined without a `set` accessor. | ||
|
|
||
| You should examine [read-only collections][readonly-collections] in the Base Class Library. | ||
|
|
||
| For arrays the closest you can get to a read-only version is the [`Array.AsReadOnly<T>()][as-read-only] method. | ||
|
|
||
| #### Defensive Copying | ||
|
|
||
| Reflecting back on the coding exercise, imagine you have a code-base of several hundred thousand lines. You are passed the dictionary of developers into some method you are developing. Perhaps you have been tasked with printing out details of privileged developers. You decide to blank out the eye color in the dictionary to protect the developers' privacy. Unless a [deep copy][so-deep-copy] of the dictionary was made in the `Authenticator.GetDevelopers()` method, or, even better, it was wrapped in a read-only collection then you will have just trashed the authenticator. | ||
|
|
||
| This follows the principle of [defensive copying][defensive-copying]. You can make sure your formal API is not circumvented by avoiding exposure to callers of internal writeable state. | ||
|
|
||
| #### Reference | ||
|
|
||
| - [Readonly fields][readonly-fields]: how to define a readonly field. | ||
| - [Constants][constants]: how to define constants. | ||
|
|
||
| [readonly-fields]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/readonly#readonly-field-example | ||
| [constants]: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/constants | ||
| [so-consts]: https://stackoverflow.com/a/5834473/96167 | ||
| [premature-optimization]: https://wiki.c2.com/?PrematureOptimization | ||
| [so-deep-copy]: https://stackoverflow.com/questions/78536/deep-cloning-objects | ||
| [defensive-copying]: https://www.informit.com/articles/article.aspx?p=31551&seqNum=2 | ||
| [as-read-only]: https://docs.microsoft.com/en-us/dotnet/api/system.array.asreadonly?view=netcore-3.1 | ||
| [readonly-collections]: https://docs.microsoft.com/en-us/dotnet/api/system.collections.objectmodel.readonlycollection-1?view=netcore-3.1 | ||
| [so-deep-copy]: https://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy#:~:text=A%20deep%20copy%20occurs%20when,objects%20to%20which%20it%20refers.&text=Shallow%20copy%20is%20a%20bit,values%20in%20the%20original%20object. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ## General | ||
|
|
||
| - [Readonly fields][readonly-fields]: how to define a readonly field. | ||
| - [Constants][constants]: how to define constants. | ||
|
|
||
| ## 1. Set appropriate fields and properties to const | ||
|
|
||
| - Constants in C# are discussed [here][constants]. | ||
|
|
||
| ## 2. Set appropriate fields to readonly | ||
|
|
||
| - Read-only fields are discussed [here][readonly-fields]. | ||
|
|
||
| ## 3. Remove set accessor or make it private for any appropriate field | ||
|
|
||
| - This [article][properties] discusses C# properties. | ||
|
|
||
| ## 4. Ensure that the admin cannot be tampered with | ||
|
|
||
| - See [this][defensive-copying] discussion on the pattern and purpose of defensive copying. | ||
|
|
||
| ## 5. Ensure that the developers cannot be tampered with | ||
|
|
||
| - Read-only dictionaries are discussed [here][readonly-dictionaries]. | ||
|
|
||
| [readonly-fields]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/readonly#readonly-field-example | ||
| [constants]: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/constants | ||
| [properties]: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties | ||
| [defensive-copying]: https://www.informit.com/articles/article.aspx?p=31551&seqNum=2 | ||
| [readonly-dictionaries]: https://docs.microsoft.com/en-us/dotnet/api/system.collections.objectmodel.readonlydictionary-2?view=netcore-3.1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. Such a cleanup project will not only make life easy for future maintainers but will expose and fix some security vulnerabilities. | ||
|
|
||
| ## 1. Set appropriate fields and properties to const | ||
|
|
||
| This is a refactoring task. Add the `const` modifier to any members of `Authenticator` or `Identity` that you think appropriate. | ||
|
|
||
| ## 2. Set appropriate fields to readonly | ||
|
|
||
| This is a refactoring task. Add the `readonly` modifier to any fields of the `Authenticator` class or the `Identity` struct that you think appropriate. | ||
|
|
||
| ## 3. Ensure that the class cannot be changed once it has been created | ||
|
|
||
| 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 |
||
|
|
||
| ## 4. Ensure that the admin cannot be tampered with | ||
|
|
||
| At present the admin identity field is returned by a call to `Admin`. This is not ideal as the caller can modify the field. Find a way to prevent the caller modifying the details of admin on the `Authenticator` object. | ||
|
|
||
| ## 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 |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| 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 |
||
|
|
||
| ```csharp | ||
| private const int num = 1729; | ||
| public const string title = "Grand" + " Master"; | ||
| ``` | ||
|
|
||
| The `readonly` modifier can be (and generally should be) applied to any field that cannot be made `const` where its value will not change during the lifetime of the program and is either set by an inline initializer or during instantiation (by the constructor or a method called by the constructor). | ||
|
|
||
| ```csharp | ||
| private readonly int num; | ||
| private readonly System.Random rand = new System.Random(); | ||
|
|
||
| public MyClass(int num) | ||
| { | ||
| this.num = num; | ||
| } | ||
| ``` | ||
|
|
||
| #### Read-only collections | ||
|
|
||
| Although the `readonly` modifier prevents the value or reference in a field from being overwritten, the modifier provides no protection for the members of a reference type. | ||
|
|
||
| ```csharp | ||
| readonly List<int> ints = new List<int>(); | ||
|
|
||
| void Foo() | ||
| { | ||
| ints.Add(1); // ok | ||
| ints = new List<int>(); // fails to compile | ||
| } | ||
| ``` | ||
|
|
||
| To ensure that all members of a reference type are protected the fields can be made `readonly` and automatic properties can be defined without a `set` accessor. | ||
|
|
||
| The Base Class Library (BCL) provides some readonly versions of collections where there is a requirement to stop members of a collections being updated. These come in the form of wrappers: | ||
|
|
||
| - `ReadOnlyDictionary<T>` exposes a `Dictionary<T>` as read-only. | ||
| - `ReadOnlyCollection<T>` exposes a `List<T>` as read-only. | ||
|
|
||
| #### Defensive copying | ||
|
|
||
| In security sensitive situations (or even simply on a large code-base where developers have different priorities and agendas) you should avoid allowing a class's public API to be circumvented by accepting and storing a method's mutable parameters or by exposing a mutable member of a class through a return value or as an `out` parameter. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that making variables
readonlyis consistent with your wording which itself mentionsconstants. I think the test here is to find all the appropriate candidates. I am taking the view that the syntax is trivial.