Skip to content

Commit 46cc506

Browse files
JeremyCaneyJeremyCaney
committed
Default to sealed for protected override members
If we override a `virtual` member and provide our own implementation, we should default to `sealing` that member to prevent further derivations from modifying that logic unless we've _explicitly_ reviewed that particular case and determined that it is appropriate for derived classes to override the logic—and, potentially, not call `base`. In most of these cases, we just implemented `protected override` without specific consideration of (further) derived implementations. In many of these cases, such as `SqlTopicRepository`, we don't anticipate derivations. In others, such as `KeyedTopicCollection<T>`, we do, but not of `protected override` members, such as `GetKeyForItem()` or `InsertItem()`.
1 parent fe609ff commit 46cc506

8 files changed

Lines changed: 16 additions & 16 deletions

File tree

‎OnTopic.Data.Sql/SqlTopicRepository.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ bool persistRelationships
525525
| METHOD: MOVE TOPIC
526526
\-------------------------------------------------------------------------------------------------------------------------*/
527527
/// <inheritdoc />
528-
protected override void MoveTopic(Topic topic, Topic target, Topic? sibling) {
528+
protected override sealed void MoveTopic(Topic topic, Topic target, Topic? sibling) {
529529

530530
/*------------------------------------------------------------------------------------------------------------------------
531531
| Validate parameters
@@ -576,7 +576,7 @@ protected override void MoveTopic(Topic topic, Topic target, Topic? sibling) {
576576
| METHOD: DELETE TOPIC
577577
\-------------------------------------------------------------------------------------------------------------------------*/
578578
/// <inheritdoc />
579-
protected override void DeleteTopic(Topic topic) {
579+
protected override sealed void DeleteTopic(Topic topic) {
580580

581581
/*------------------------------------------------------------------------------------------------------------------------
582582
| Validate parameters

‎OnTopic.ViewModels/Collections/TopicViewModelCollection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public TopicViewModelCollection<TItem> GetByContentType(string contentType) {
6868
/// </summary>
6969
/// <param name="item">The <see cref="Topic"/> object from which to extract the key.</param>
7070
/// <returns>The key for the specified collection item.</returns>
71-
protected override string GetKeyForItem(TItem item) {
71+
protected override sealed string GetKeyForItem(TItem item) {
7272
Contract.Requires(item, "The item must be available in order to derive its key.");
7373
return item.Key?? "";
7474
}

‎OnTopic/Attributes/AttributeValueCollection.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ internal void SetValue(
495495
/// An AttributeValue with the Key '{item.Key}' already exists. The Value of the existing item is "{this[item.Key].Value};
496496
/// the new item's Value is '{item.Value}'. These AttributeValues are associated with the Topic '{GetUniqueKey()}'."
497497
/// </exception>
498-
protected override void InsertItem(int index, AttributeValue item) {
498+
protected override sealed void InsertItem(int index, AttributeValue item) {
499499
Contract.Requires(item, nameof(item));
500500
if (_topicPropertyDispatcher.Enforce(item.Key, item)) {
501501
if (!Contains(item.Key)) {
@@ -530,7 +530,7 @@ protected override void InsertItem(int index, AttributeValue item) {
530530
/// </remarks>
531531
/// <param name="index">The location that the <see cref="AttributeValue"/> should be set.</param>
532532
/// <param name="item">The <see cref="AttributeValue"/> object which is being inserted.</param>
533-
protected override void SetItem(int index, AttributeValue item) {
533+
protected override sealed void SetItem(int index, AttributeValue item) {
534534
Contract.Requires(item, nameof(item));
535535
if (_topicPropertyDispatcher.Enforce(item.Key, item)) {
536536
base.SetItem(index, item);
@@ -551,7 +551,7 @@ protected override void SetItem(int index, AttributeValue item) {
551551
/// When an <see cref="AttributeValue"/> is removed, <see cref="IsDirty(Boolean)"/> will return true—even if no remaining
552552
/// <see cref="AttributeValue"/>s are marked as <see cref="AttributeValue.IsDirty"/>.
553553
/// </remarks>
554-
protected override void RemoveItem(int index) {
554+
protected override sealed void RemoveItem(int index) {
555555
var attribute = this[index];
556556
DeletedAttributes.Add(attribute.Key);
557557
base.RemoveItem(index);
@@ -568,7 +568,7 @@ protected override void RemoveItem(int index) {
568568
/// When an <see cref="AttributeValue"/> is removed, <see cref="IsDirty(Boolean)"/> will return true—even if no remaining
569569
/// <see cref="AttributeValue"/>s are marked as <see cref="AttributeValue.IsDirty"/>.
570570
/// </remarks>
571-
protected override void ClearItems() {
571+
protected override sealed void ClearItems() {
572572
DeletedAttributes.AddRange(Items.Select(a => a.Key));
573573
base.ClearItems();
574574
}
@@ -581,7 +581,7 @@ protected override void ClearItems() {
581581
/// </summary>
582582
/// <param name="item">The <see cref="Topic"/> object from which to extract the key.</param>
583583
/// <returns>The key for the specified collection item.</returns>
584-
protected override string GetKeyForItem(AttributeValue item) {
584+
protected override sealed string GetKeyForItem(AttributeValue item) {
585585
Contract.Requires(item, "The item must be available in order to derive its key.");
586586
return item.Key;
587587
}

‎OnTopic/Collections/KeyedTopicCollection{T}.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public KeyedTopicCollection(IEnumerable<T>? topics = null) : base(StringComparer
7070
/// A {typeof(T).Name} with the Key '{item.Key}' already exists. The UniqueKey of the existing {typeof(T).Name} is
7171
/// '{GetUniqueKey()}'; the new item's is '{item.GetUniqueKey()}'.
7272
/// </exception>
73-
protected override void InsertItem(int index, T item) {
73+
protected override sealed void InsertItem(int index, T item) {
7474

7575
/*------------------------------------------------------------------------------------------------------------------------
7676
| Validate parameters
@@ -115,7 +115,7 @@ protected override void InsertItem(int index, T item) {
115115
/// </summary>
116116
/// <param name="item">The <see cref="Topic"/> object from which to extract the key.</param>
117117
/// <returns>The key for the specified collection item.</returns>
118-
protected override string GetKeyForItem(T item) {
118+
protected override sealed string GetKeyForItem(T item) {
119119
Contract.Requires(item, "The item must be available in order to derive its key.");
120120
return item.Key;
121121
}

‎OnTopic/Collections/TopicMultiMap.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public bool Remove(string key, Topic topic) {
147147
/// </summary>
148148
/// <param name="item">The <see cref="KeyValuesPair{TKey, TValue}"/> object from which to extract the key.</param>
149149
/// <returns>The key for the specified collection item.</returns>
150-
protected override string GetKeyForItem(KeyValuesPair<string, TopicCollection> item) {
150+
protected override sealed string GetKeyForItem(KeyValuesPair<string, TopicCollection> item) {
151151
Contract.Requires(item, "The item must be available in order to derive its key.");
152152
return item.Key;
153153
}

‎OnTopic/Internal/Reflection/MemberInfoCollection{T}.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ internal MemberInfoCollection(Type type, IEnumerable<T> members) : base(StringCo
7575
/// <param name="index">The zero-based index at which <paramref name="item" /> should be inserted.</param>
7676
/// <param name="item">The <see cref="MemberInfo" /> instance to insert.</param>
7777
/// <exception cref="ArgumentException">The Type '{Type.Name}' already contains the MemberInfo '{item.Name}'</exception>
78-
protected override void InsertItem(int index, T item) {
78+
protected override sealed void InsertItem(int index, T item) {
7979

8080
/*------------------------------------------------------------------------------------------------------------------------
8181
| Validate parameters
@@ -109,7 +109,7 @@ protected override void InsertItem(int index, T item) {
109109
/// </summary>
110110
/// <param name="item">The <see cref="Topic"/> object from which to extract the key.</param>
111111
/// <returns>The key for the specified collection item.</returns>
112-
protected override string GetKeyForItem(T item) {
112+
protected override sealed string GetKeyForItem(T item) {
113113
Contract.Requires(item, "The item must be available in order to derive its key.");
114114
return item.Name;
115115
}

‎OnTopic/Internal/Reflection/TypeMemberInfoCollection.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ internal MemberInfoCollection<T> GetMembers<T>(Type type) where T : MemberInfo =
124124
/// <exception cref="ArgumentException">
125125
/// The TypeMemberInfoCollection already contains the MemberInfoCollection of the Type '{item.Type}'.
126126
/// </exception>
127-
protected override void InsertItem(int index, MemberInfoCollection item) {
127+
protected override sealed void InsertItem(int index, MemberInfoCollection item) {
128128

129129
/*------------------------------------------------------------------------------------------------------------------------
130130
| Validate parameters
@@ -154,7 +154,7 @@ protected override void InsertItem(int index, MemberInfoCollection item) {
154154
/// </summary>
155155
/// <param name="item">The <see cref="Topic"/> object from which to extract the key.</param>
156156
/// <returns>The key for the specified collection item.</returns>
157-
protected override Type GetKeyForItem(MemberInfoCollection item) {
157+
protected override sealed Type GetKeyForItem(MemberInfoCollection item) {
158158
Contract.Requires(item, "The item must be available in order to derive its key.");
159159
return item.Type;
160160
}

‎OnTopic/Lookup/TypeCollection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ internal TypeCollection(IEnumerable<Type>? types = null) : base(StringComparer.I
5252
/// </summary>
5353
/// <param name="item">The <see cref="Type"/> object from which to extract the key.</param>
5454
/// <returns>The key for the specified collection item.</returns>
55-
protected override string GetKeyForItem(Type item) {
55+
protected override sealed string GetKeyForItem(Type item) {
5656
Contract.Requires(item, "The item must be available in order to derive its key.");
5757
return item.Name;
5858
}

0 commit comments

Comments
 (0)