New Design Guideline: Avoid Protected Static

re: New Design Guideline: Avoid Protected Static

Jeroen Frijters — Thu, 09 Sep 2004 14:39:00 GMT

This is one of those "I got carried away" nonsense guidelines. There is nothing wrong with protected statics, you just have to know what you're doing. The example is obviously stupid, but that doesn't mean you should never use protected statics.

re: New Design Guideline: Avoid Protected Static

Brad Abrams [MSFT] — Thu, 09 Sep 2004 14:59:00 GMT

OK -- I am game... what do you think is a good use of protected statics?

re: New Design Guideline: Avoid Protected Static

Nathan Neitzke — Thu, 09 Sep 2004 16:04:00 GMT

I have a comment about your example. It is with the annotation that instanced data could not be accessed.

What if Malicious inherited from Child? Would it then succeed even in the CLR?

Thanks.

re: New Design Guideline: Avoid Protected Static

Joe — Thu, 09 Sep 2004 17:05:00 GMT

I agree with Jeroen.

"protected static" means you explicitly want all subclasses to be able to access the value. Of course a subclass should and would not store its own sensitive data in a protected static property.

As for a use of "protected static", the most obvious thing would be a readonly value supplied by a base class and used by all subclasses, such as configuration data.

re: New Design Guideline: Avoid Protected Static

Nicholas Allen — Thu, 09 Sep 2004 20:46:00 GMT

I've never actually seen this problem Brad describes in real world code. What I have seen several times though is a field that is protected, static, but not read only. The field is usually then written to by several subclasses with the obvious, sometimes fatal, result.

I use protected statics quite a bit, primarily to keep the namespace clean. I don't care if someone gets access; I just want to improve the API usability by not publishing to the documentation, code editors, or a reader some elements that they don't need to think about when using the class. This gives a clear separation about what's important to external users as opposed to extenders.

re: New Design Guideline: Avoid Protected Static

Steven Padfield — Fri, 10 Sep 2004 18:46:00 GMT

> OK -- I am game... what do you think is a good use of protected statics?
> 9/9/2004 7:59 AM | Brad Abrams [MSFT]

Personally, I agree with Brad. I've never used protected statics and I probably never will. And if I ever see them in someone's code, I'll immediately begin thinking about how to refactor them away. If Microsoft chooses to remove them from the CLS, I probably won't miss them.

If you have to provide subclass access to a private static, provide a protected instance accessor.

re: New Design Guideline: Avoid Protected Static

Jeroen Frijters — Fri, 10 Sep 2004 19:07:00 GMT

My response was a little knee-jerk, because design guidelines are no substitute for actually knowing what you're doing. I feel that anybody that doesn't understand that the code in the example is a bad idea really has no business writing code, but I'm a bit of a snob that way ;-)

As to practical use for protected static, I was thinking of readonly fields (constants that aren't primitive, or maybe you don't want the versioning semantics of constants) that are only useful for subclasses.

As to examples, just search the BCL:
mscorlib:
System.Threading.WaitHandle::InvalidHandle
System.Security.Util.SiteString::m_separators
System.Security.Util.StringExpressionSet::m_emptyList
System.Security.Util.StringExpressionSet::m_separators
System.Security.Util.StringExpressionSet::m_trimChars
System.Security.Util.StringExpressionSet::m_directorySeparator
System.Security.Util.StringExpressionSet::m_alternateDirectorySeparator
System.Security.Util.DirectoryString::m_illegalDirectoryCharacters
System.Runtime.Remoting.Identity::IDFLG_DISCONNECTED_FULL
System.Runtime.Remoting.Identity::IDFLG_DISCONNECTED_REM
System.Runtime.Remoting.Identity::IDFLG_IN_IDTABLE
System.Runtime.Remoting.Identity::IDFLG_CONTEXT_BOUND
System.Runtime.Remoting.Identity::IDFLG_WELLKNOWN
System.Runtime.Remoting.Identity::IDFLG_SERVER_SINGLECALL
System.Runtime.Remoting.Identity::IDFLG_SERVER_SINGLETON

System.Windows.Forms:
System.Windows.Forms.ScrollableControl::ScrollStateAutoScrolling
System.Windows.Forms.ScrollableControl::ScrollStateHScrollVisible
System.Windows.Forms.ScrollableControl::ScrollStateVScrollVisible
System.Windows.Forms.ScrollableControl::ScrollStateUserHasScrolled
System.Windows.Forms.ScrollableControl::ScrollStateFullDrag
System.Windows.Forms.DataGridRow::xOffset
System.Windows.Forms.DataGridRow::yOffset
System.Windows.Forms.DateTimePicker::DefaultTitleBackColor
System.Windows.Forms.DateTimePicker::DefaultTitleForeColor
System.Windows.Forms.DateTimePicker::DefaultMonthBackColor
System.Windows.Forms.DateTimePicker::DefaultTrailingForeColor
System.Windows.Forms.FileDialog::EventFileOk
System.Windows.Forms.FontDialog::EventApply
System.Windows.Forms.PropertyGridInternal.GridEntry::InvalidPoint
System.Windows.Forms.PropertyGridInternal.GridEntry::DisplayNameComparer
System.Windows.Forms.PropertyGridInternal.GridEntry::NOTIFY_RESET
System.Windows.Forms.PropertyGridInternal.GridEntry::NOTIFY_CAN_RESET
System.Windows.Forms.PropertyGridInternal.GridEntry::NOTIFY_DBL_CLICK
System.Windows.Forms.PropertyGridInternal.GridEntry::NOTIFY_SHOULD_PERSIST
System.Windows.Forms.PropertyGridInternal.GridEntry::NOTIFY_RETURN
System.Windows.Forms.PropertyGridInternal.GridEntry::OUTLINE_ICON_PADDING
System.Windows.Forms.PropertyGridInternal.DocComment::CBORDER
System.Windows.Forms.PropertyGridInternal.DocComment::CXDEF
System.Windows.Forms.PropertyGridInternal.DocComment::CYDEF
System.Windows.Forms.PropertyGridInternal.DocComment::MIN_LINES
System.Windows.Forms.PropertyGridInternal.PropertyGridCommands::wfcMenuGroup
System.Windows.Forms.PropertyGridInternal.PropertyGridCommands::wfcMenuCommand
System.Windows.Forms.PropertyGridInternal.PropertyGridView::InvalidPoint
System.Windows.Forms.PropertyGridInternal.PropertyGridView::InvalidPosition
System.Windows.Forms.PropertyGridInternal.PropertyGridView::EDIT_INDENT
System.Windows.Forms.PropertyGridInternal.PropertyGridView::OUTLINE_INDENT
System.Windows.Forms.PropertyGridInternal.PropertyGridView::OUTLINE_SIZE
System.Windows.Forms.PropertyGridInternal.PropertyGridView::PAINT_WIDTH
System.Windows.Forms.PropertyGridInternal.PropertyGridView::PAINT_INDENT
System.Windows.Forms.PropertyGridInternal.PropertyGridView::ROWLABEL
System.Windows.Forms.PropertyGridInternal.PropertyGridView::ROWVALUE
System.Windows.Forms.PropertyGridInternal.PropertyGridView::MAX_LISTBOX_HEIGHT
System.Windows.Forms.PropertyGridInternal.PropertyGridView::ERROR_NONE
System.Windows.Forms.PropertyGridInternal.PropertyGridView::ERROR_THROWN
System.Windows.Forms.PropertyGridInternal.PropertyGridView::ERROR_MSGBOX_UP
System.Windows.Forms.ComponentModel.Com2Interop.Com2IDispatchConverter::none

re: New Design Guideline: Avoid Protected Static

Jeroen Frijters — Fri, 10 Sep 2004 19:10:00 GMT

Oops. Some of those are actually constants.

Here is the correct list:
mscorlib:
System.Threading.WaitHandle::InvalidHandle
System.Security.Util.SiteString::m_separators
System.Security.Util.StringExpressionSet::m_emptyList
System.Security.Util.StringExpressionSet::m_separators
System.Security.Util.StringExpressionSet::m_trimChars
System.Security.Util.StringExpressionSet::m_directorySeparator
System.Security.Util.StringExpressionSet::m_alternateDirectorySeparator
System.Security.Util.DirectoryString::m_illegalDirectoryCharacters

System.Windows.Forms:
System.Windows.Forms.DateTimePicker::DefaultTitleBackColor
System.Windows.Forms.DateTimePicker::DefaultTitleForeColor
System.Windows.Forms.DateTimePicker::DefaultMonthBackColor
System.Windows.Forms.DateTimePicker::DefaultTrailingForeColor
System.Windows.Forms.FileDialog::EventFileOk
System.Windows.Forms.FontDialog::EventApply
System.Windows.Forms.PropertyGridInternal.GridEntry::InvalidPoint
System.Windows.Forms.PropertyGridInternal.GridEntry::DisplayNameComparer
System.Windows.Forms.PropertyGridInternal.PropertyGridCommands::wfcMenuGroup
System.Windows.Forms.PropertyGridInternal.PropertyGridCommands::wfcMenuCommand
System.Windows.Forms.PropertyGridInternal.PropertyGridView::InvalidPoint
System.Windows.Forms.PropertyGridInternal.PropertyGridView::InvalidPosition
System.Windows.Forms.ComponentModel.Com2Interop.Com2IDispatchConverter::none

re: New Design Guideline: Avoid Protected Static

Keith Patrick — Sat, 11 Sep 2004 01:58:00 GMT

Does this apply exclusively to protected static properties/fields, or does it go for methods as well? If I have a method that doesn't use instance data, even protected, I'll often just tack a static to it, but should I not be doing this for some reason?

re: New Design Guideline: Avoid Protected Static

Steven Padfield — Tue, 14 Sep 2004 00:02:00 GMT

Actually, I just found myself using a protected static method today.

Originally, I had a class with a private static method. Then I had to refactor this into two separate subclasses with a common base abstract class. Yet both subclasses still needed access to that private static method. Here were my choices:

1) Make the method public static. This violates encapsulation.

2) Copy the method to each of the subclasses. This results in duplicated code.

3) Make the method a protected instance method. Since the method uses no instance data, does not employ polymorphism, and conceptually has no relationship to specific instances of the class, this is a bad idea and could lead to confusion later.

4) Make the method protected static. This is the decision I made, because it avoids the previous three pitfalls.

Maybe the guideline would be better restated as "Avoid Protected Static FIELDS".

Just my $0.02.

re: New Design Guideline: Avoid Protected Static

Eric Newton — Tue, 14 Sep 2004 01:27:00 GMT

Another irritation:

internal protected modifiers dont do what you'd expect... the internal keyword takes precedence over the protected modifier, even though you might need for a subclass to be able to call, but you'd like to give the assembly a way to call into it as well...

for example:
<code>
public class Order
{
private int _orderId; //orderId should never be settable from outside the assembly
//but what if somebody subclasses the Order? It would be nice to let the subclass
//be able to set it... AND let the data classes be able to set it when
// Loading an order or Creating a new order [linking the instance to a record in the db

public int OrderId
{ get { return this._orderId; } }
internal protected void Set_OrderId(int value) { this._orderId=value; }

}
public class MyOrders //class handles database interaction
{
public MyOrder LoadMyOrder(int orderId)
{
MyOrder o = new MyOrder();
// build a dbcommand to read a record into a datareader
o.Set_OrderId( (int) dr["OrderId"] ); //impossible to do this without making OrderId public somehow
}
///<summary>Inserts the order into the db</summary>
///<returns>the id of the new order. (OrderId property is also set.)</returns>
public int Insert(MyOrder order)
{
//build a dbcommand to insert the order into the database...
//set the OrderId to the new record?
order.Set_OrderId( (int) dr["OrderId"] ); //another impossible call
return order.OrderId;
}
}
</code>
---------------------------
make a new project, referencing the above assembly

<code>
public class MyOrder : Order
{
public DateTime MyOrderSpecificProperty { get; set; }

}
public class MyOrders //class handles database interaction
{
public MyOrder LoadMyOrder(int orderId)
{
MyOrder o = new MyOrder();
// build a dbcommand to read a record into a datareader
o.Set_OrderId( (int) dr["OrderId"] ); //impossible to do this without making OrderId public somehow
}
///<summary>Inserts the order into the db</summary>
///<returns>the id of the new order. (OrderId property is also set.)</returns>
public int Insert(MyOrder order)
{
//build a dbcommand to insert the order into the database...
//set the OrderId to the new record?
order.Set_OrderId( (int) dr["OrderId"] ); //another impossible call
return order.OrderId;
}
}
</code>

re: New Design Guideline: Avoid Protected Static

Eric Newton — Tue, 14 Sep 2004 01:28:00 GMT

ah crap, i screwed up the first set of MyOrders should read Orders... sorry guys! grrr

re: New Design Guideline: Avoid Protected Static

Eric Newton — Tue, 14 Sep 2004 01:31:00 GMT

Another irritation:

internal protected modifiers dont do what you'd expect... the internal keyword takes precedence over the protected modifier, even though you might need for a subclass to be able to call, but you'd like to give the assembly a way to call into it as well...

for example:
<code>
public class Order
{
private int _orderId; //orderId should never be settable from outside the assembly
//but what if somebody subclasses the Order? It would be nice to let the subclass
//be able to set it... AND let the data classes be able to set it when
// Loading an order or Creating a new order [linking the instance to a record in the db

public int OrderId
{ get { return this._orderId; } }
internal protected void Set_OrderId(int value) { this._orderId=value; }

}
public class Orders //class handles database interaction
{
public Order LoadOrder(int orderId)
{
Order o = new Order();
// build a dbcommand to read a record into a datareader
o.Set_OrderId( (int) dr["OrderId"] ); //can call because in same assembly
}
///<summary>Inserts the order into the db</summary>
///<returns>the id of the new order. (OrderId property is also set.)</returns>
public int Insert(MyOrder order)
{
//build a dbcommand to insert the order into the database...
//set the OrderId to the new record?
order.Set_OrderId( (int) dr["OrderId"] ); //another impossible call
return order.OrderId;
}
}
</code>
---------------------------
make a new project, referencing the above assembly

<code>
public class MyOrder : Order
{
public DateTime MyOrderSpecificProperty { get; set; }
internal new void Set_OrderId(int orderId) //try to use the protected aspect of the member
{ base.Set_OrderId(orderId); /* again, impossible */ }

}
public class MyOrders //class handles database interaction
{
public MyOrder LoadMyOrder(int orderId)
{
MyOrder o = new MyOrder();
// build a dbcommand to read a record into a datareader
o.Set_OrderId( (int) dr["OrderId"] ); //impossible to do this without making OrderId public somehow
}
///<summary>Inserts the order into the db</summary>
///<returns>the id of the new order. (OrderId property is also set.)</returns>
public int Insert(MyOrder order)
{
//build a dbcommand to insert the order into the database...
//set the OrderId to the new record?
order.Set_OrderId( (int) dr["OrderId"] ); //another impossible call
return order.OrderId;
}
}
</code>

The idea is to force users of Orders to go through a hoop to get a valid Order that represents an Order record in the db, hence no setting OrderId publically... but you run into an inheritance problem...

re: New Design Guideline: Avoid Protected Static

Steven Padfield — Tue, 14 Sep 2004 19:02:00 GMT

Hey Eric,

I would say you should have a protected property so that it can be accessed and mutated by subclasses, but then also have an alternative mutator method marked as internal for other classes within the assembly to set/get the property's underlying data without needing to actually reference the protected property itself.

public class Order
{
// private field
private int _orderID;

// anyone can read the value
public int OrderID
{
get { return _orderID; }
}

// derived classes from any assembly use this property to set the order id
protected int InternalOrderID
{
get { return _orderID; }
set { _orderID = value; }
}

// non-derived classes within this assembly use this method to set the order id
internal int SetOrderID(int value)
{
_orderID = value;
}
}

re: New Design Guideline: Avoid Protected Static

Matthew W. Jackson — Wed, 15 Sep 2004 06:38:00 GMT

Yeah, I tend to follow Steven's pattern. I tend to avoid "protected internal", since it's really kind of a screwball visibility (there are lots of "exceptions" to the normal rules to accomodate it). I prefer a protected method and an internal method, usually with the same name appended with "Internal".

I could go for a few more visibilities in the language and runtime, though, since I don't always like exposing internals to everybody else in my library. I'd like to add the "protected and internal" visibility to C#, but there's not really a good syntax for it (except maybe an awkward "protected & internal"). I'd also like a visibility for nested classes that was only accessible to an outer class that contains it. But, what do you call such a thing?

I suppose a true "friend" visibility would solve both of these problems.

The Loss of Friends

Steven Padfield — Thu, 16 Sep 2004 00:22:00 GMT

http://67.174.60.122/blog/archive/2004/09/15/155.aspx

re: New Design Guideline: Avoid Protected Static

Steven Padfield — Thu, 16 Sep 2004 18:53:00 GMT

Hey Eric,

I did some experimenting and I found that protected internal does indeed do what you want it to do: it only allows a member to be called from the same assembly or from a derived class. This is consistent with the MSDN documentation.

I tried loading your example into the IDE, and after filling in some of the blanks, it compiles fine. I didn't have to change any of your access modifiers.

-Steven

protected static » Blog Archive » This just in!

protected static » Blog Archive » This just in! — Fri, 26 May 2006 07:18:28 GMT

PingBack from http://www.blog.protectedstatic.com/?p=5

Brad Abrams New Design Guideline Avoid Protected Static | patio umbrella

Brad Abrams New Design Guideline Avoid Protected Static | patio umbrella — Thu, 18 Jun 2009 06:59:38 GMT

PingBack from http://patioumbrellasource.info/story.php?id=312