1

Purpose

How do I through any method, for the purposes of a unit test, make sure that a derived class does not refer to any properties in the base class? I understand that Reflection won't cut it, here. Could I create a mock of the base class somehow and observe if a property is called at the wrong time? Or any other way?

Background

I have a series of classes that are participating in serialization. There is a natural hierarchy of parts and pieces, so that for example, a Chunk1 knows how to serialize itself (starting, ending, delimiters) but will delegate the serialization of its inner parts to a Blob that itself serializes several lines.

Here is the interface that all the parts implement:

public interface ICoolSerializable {
   void Serialize(Writer w);
}

And given this desired serialization result:

Chunk1:/Line1
/Line2

There is a Chunk1 class that is responsible for "Chunk1:" and inherits from the Blob class, which in turn is responsible for "/Line1", the newline, and "/Line2". (Both implement ISerializable.)

Note: please assume for the sake of the question that I truly do want an is-a relationship, and it is correct for the Chunk1 to inherit from the Blob (the Blob can be used in many different chunks, and the Chunk1 just determines how the Blob is interpreted, but not how it is serialized beyond the initial label).

The Problem

I see a potential gotcha for me or another developer in the future writing more classes like this and attempting to copy the pattern. Since the constructor of Chunk1 accepts an IEnumerable of Line items to pass to its base Blob, the developer will have in mind how the base is constructed, and might easily make this mistake in the Chunk1 serialize method:

public override void Serialize(Writer w) {
   w.Write("Chunk1:");
   w.WriteEnumerable(Lines); // wrong, this is a forbidden base.Lines!
}

This would yield the wrong serialization result (missing the slashes):

Chunk1:Line1
Line2

Full disclosure: I did make this mistake, and then initially "fixed" it by writing "/" before each Line from the derived class. Of course, the moment another class inherited from the base, it also was missing the slashes—I'd fixed it the wrong way.

The Question

So how can I inspect the Serialize method or take any other measure to ensure that base.Lines is never accessed from within it? Instead of the wrong way above, it needs to work like this:

public override void Serialize(Writer w) {
   w.Write("Chunk1:");
   base.Serialize(w); // Remember to let the superclass decide how to serialize itself
}

This pattern is not global throughout. Not all classes implementing my ICoolSerializable interface have sub-parts, nor do all of them inherit from anything else. In some cases, it may make sense to wrap another class instead of subclass from it.

Some Thoughts

For those interested, since strings are implicitly convertible to ICoolSerializable, I wish I could do this:

public override void Serialize(Writer w) {
   w.WriteCoolSerializables,
      "Chunk1:",
      base
   }
}

However, base here cannot refer to the base instance, and if I cast the current class as its parent, it still wouldn't work because the derived Serialize method (it's override!) would be called and thus cause a loop, eventually resulting in a stack overflow.

Update

I suspect that the right answer will be refactoring, but I'm not sure how that refactoring will work right now. I suspect that I may lean more heavily on Reflection, and on the serialization process working through properties or a returned series of property-or-value-accessing objects, rather than on a procedural statement. This would enable the property-accessing-objects to be inspected to see what they're referring to. This could also enable the parent class to indicate (through attributes or an attribute-like method that returns information) how it relates to any child class, a sort of template that says "the child class may only hook onto my serialization components at the head", which can then be enforced.

Community
  • 1
  • 1
ErikE
  • 48,881
  • 23
  • 151
  • 196
  • Side note: it is often considered code smell to mix multiple concerns - like "being piece of data" and "serialize in one/many formats"... Consider if existing serialization formats/frameworks work four your objects (JSON/XML are common .Net choices that are pretty flexible in configuring what should be serialized). – Alexei Levenkov Aug 17 '15 at 22:39
  • Thank you @Alexei, however the serialization format is required by the business. It is a highly arcane and nonsensical format, but I have no choice. The serialization here doesn't exist to port *this* object across the wire and back to the same form, but to cross a boundary to another system, so it's functioning like a service layer--it just so happens that one side wants custom archaic serialization instead of a modern format. – ErikE Aug 17 '15 at 22:45
  • 1
    Makes sense. You still may want to consider if externalizing serialization (i.e. with Visitor pattern if hierarchy is small enough) would make code safer/let you localize code that caused you ask the question into one place. – Alexei Levenkov Aug 17 '15 at 23:24

3 Answers3

1

In this case I wouldn't make your Serialize method inheritable.

protected void SerializeCore(Writer w) { }
public void Serialize(Writer w) {
    SerializeCore(w);
    ...
}

This way you control how your base class is serialised. If you want to be stricter you could use reflection with attributes to perform serialisation.

Example base class for the attributes:

public abstract class CustomSerializeAttribute : Attribute
{
    public abstract void SerializeProperty(Writer w, object value);
}
pmccloghrylaing
  • 1,110
  • 8
  • 12
0

Make the properties in the base class private.

Dave C
  • 211
  • 4
  • 6
  • They can't be private, because when a consumer is working with my `Chunk1` object, it needs to be able to read those properties. For example, at deserialization time, the business layer will get a fully constructed object, see the `Chunk1` in the provided list of chunks, and know that it can read (for example) the `IEnumerable Lines` property to get its business data back out. The class is useless without public properties. – ErikE Aug 17 '15 at 22:08
  • In that case, you could use some sort of proxy. If you sub class Chunk1 again in your unit test and override the Lines property getter, you can keep track of how many times the Lines property has been accessed. This does require you to make Lines virtual, and Chunk1 can't be sealed. This approach isn't ideal, but it's often used by ORMs and Mock frameworks. Obviously if you wanted it to apply to all sub classes of Blob then you'll need to subclass with some pretty horrible reflection, or use a library like castle proxy. – Dave C Aug 17 '15 at 22:22
  • Also, have you considered using a custom static analysis rule instead of adding a unit test? – Dave C Aug 17 '15 at 22:26
  • That sounds like a reasonable avenue. I'll have to go research that, unless you can give me a head start. Does it have access to the parse tree to notice things like member access to the parent class? – ErikE Aug 17 '15 at 22:28
  • I've only done it once before, and it wasn't very pretty. This site helped a lot though: http://www.binarycoder.net/fxcop/html/index.html It sort of has access to the parse tree, but not explicitly. You can find all callers of a method, which is probably a good starting point. – Dave C Aug 17 '15 at 22:33
  • Suggestion in the post does not sound very useful (also valid option) - what one going to do with class that has no accessible properties? – Alexei Levenkov Aug 17 '15 at 22:36
  • I was only suggesting hiding the properties that he didn't want accessed from the base class. For a serializer, it seems an unusual design to expose the data after you've serialized it - the serializer is generally used at the edge of your component to communicate with other components. – Dave C Aug 17 '15 at 22:43
  • The serializer is used at the edge to communicate with another system. We chose to locate the information about how to serialize (the process of which is not regular/consistent, not ordered, not delimited properly, and not sane) in the class being serialized because putting it elsewhere just makes it harder to work with. Through attributes we get some big Reflection benefits at deserialization time, when trying to figure out what pieces of the text stream fit into which properties (it is not straightforward). – ErikE Aug 17 '15 at 22:48
0

If you are willing to wrap the functionality provided by your properties with functions, you could check the caller's source file against a blacklist or whitelist of which files can't/can contain code that accesses those properties.

Within the Blob implementation, for each property (wrapper) you want to monitor you can do something along these lines:

public int GetExampleProp([System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "")
{
    CheckCaller(sourceFilePath);
    return ExampleProp;
}
public void SetExampleProp(int value, [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "")
{
    CheckCaller(sourceFilePath);
    ExampleProp = value;
}

and then check if the call is valid in CheckCaller

private void CheckCaller(string path)
{
    if (!_whitelist.Contains(path)) {
        // report error
    }
}
jwde
  • 642
  • 4
  • 13