views:

121

answers:

5

I'm looking through some existing code in a project I'm working on, and I found a class that is implemented as:

public class ThingOne
{
    private int A;

    private int B;

    [NonSerialized]
    private System.Timers.Timer timer1;
}

Shouldn't it look more like this?

[Serializable]
public class ThingOne
{
    private int A;

    private int B;

    [NonSerialized]
    private System.Timers.Timer timer1;
}

Or is there some additional benefit to adding [NonSerialized] even when the class itself is not Serializable?

+4  A: 

NonSerialized will have no effect when Serializable is not used. By default, classes and their members are non-serializable.

The only advantage of declaring something NonSerialized when the class isn't serialized is under the circumstances that the class is inherited by a Serialized object, and then the inherited member will be non-serializable.

From MSDN:

'NonSerialized' attribute will not affect this member because its containing class is not exposed as 'Serializable'.

By default, classes and their members are non-serializable. The NonSerializedAttribute attribute is only needed if a member of a serializable class should not be serialized.

Greg
+7  A: 

Or is there some additional benefit to adding [NonSerialized] even when the class itself is not Serializable?

The class isn't sealed, so another class could inherit from that object. That class could be marked as Serializable, and then the NotSerializable attribute would come into play. (although as pointed out not for private members).

Remember you can check attributes by reflection too. It may not be used by the runtime to check what should and should not be serialized, it could be used as a marker for something else in the program dealing with some sort of custom serialization (I'm not saying this is a good idea in the least).

Kevin
Exactly what i was going to write just better put :-) !!
InSane
`timer1` is `private`, so would this only apply if it was `protected` instead?
FrustratedWithFormsDesigner
yes, see my update about reflection.
Kevin
Serialising just by adding [Serialized] requires that the base be serialisable. Serialising with the use of ISerlizable, will make the nature of the base class irrelevant, unless it's used in the implementation, with the same caveats. Either way, either the serialisation will fail, or the field's attribute won't matter.
Jon Hanna
+3  A: 

I agree with Greg, MSDN states it in a similar manner, citing references is a good idea..

"By default, classes and their members are non-serializable. The NonSerializedAttribute attribute is only needed if a member of a serializable class should not be serialized."

http://msdn.microsoft.com/en-us/library/dwys85sk(VS.80).aspx

Michael Eakins
I cited that exact reference in my answer.
Greg
After the fact, nice reference, [edit].
Michael Eakins
+4  A: 

MSDN SerializeAttribute states that "Apply the SerializableAttribute attribute to a type to indicate that instances of this type can be serialized. " This implies that without it, the class cannot be serialized. I believe I have attempted this and the serialize will throw an exception if it is attempted on a NonSerializable Type.

Jimmie Clark
+5  A: 

I can think of two reasons:

  1. It could be vital that the field isn't serialised. Hence if in the future the class is made serialisable, this won't introduce a bug, inefficiency or security issue, because without it marking the class serialisable will also do so for the field.

  2. They could be doing some sort of custom use of the attribute

In case 2 it'll be clear from elsewhere in the code that this is what's happening. Number 1 is good practice though.

Case 1 is good practice, it can be worth balancing YAGNI ("You Aren't Gonna Need It" - not doing work "in case it's needed later") with considering "okay, but if I do need it later, it'll be a disaster if someone misses that this field is an exception.

So, while it has no effect here, it is definitely a good practice for scenarios where it begins to have an effect.

Edit: Another possibility is that it is cruft from a previous version where it was indeed serialisable or the author was in two minds at the time and it was never entirely "finished" (is working code ever entirely finished?). Just because something is in code, doesn't mean it was meant to be that way. Still, if it's really important that something not be serialised, I still say it's good practice to mark this for the reason given above.

Jon Hanna
+1 for the comment about this being good practice
RevolXadda