passing in objects or their members values?

gistfile1.txt

I have to implement a method that does some string formatting based on string property values of two COM objects.
Now there are at least two ways to deal with it...

a) passing in the objects:

public string CreateIdForPartInView(IPartDoc part, IView drawingView) {
	IModelDoc2 model = part as IModelDoc2; 
	string modelPath = model.GetPathName();
	string modelName = Path.GetFileNameWithoutExtension(modelPath);
	string id = string.Format("{0}-1@{1}", modelName, drawingView.Name);
	return id;
}

b) passing in the names only:

public string CreateIdForPartInView(string partName, string drawingViewName) {
	id = string.Format("{0}-1@{1}", partName, drawingViewName);
	return id;
}

When it comes to testing / mocking in version "a" I'm facing the problem that I get null when casting the 
mocked part parameter. Using an explicit cast throws an InvalidCastException also both casts work having 
concrete instances instead of mocks.

Now one could say just pass in IModel2Doc and do the cast outside. 
But that code "outside" may need to be tested also whereas the cast needs to be done there and I have 
other methods where I need both IModelDoc2 and IPartDoc inside.
Should I pass in both then?

When passing in the strings as in version b) I can't ensure getting the correct string values as 
one could pass in arbitrary string values.
Expectations on passed in parameters are less explicit.

But in terms of testing its pretty simple. 

Which version do you prefer? Why?

Definitely NOT b) because it violates "tell don't ask" and exposes too much implementation details to the outside caller.

Testing the a) version with fakes can be achieved using RhinoMocks MultiMocks feature. However I would also suggest not using a) because it feels smelly. What use is the IModelDoc2 cast in your example? As far as I can tell, you don't use the model variable at all. Is that just a side-effect from shortening the example code?

If it's necessary and your code needs it, make it explicit and include it in the parameter list. If it's not necessary at all, don't include IModelDoc2 in the signature.

My quick 2 cents
-Bjoern

The IModelDoc implements the method GetPathName() whereas the interface IPartDoc is specific to parts.
There's a third interface IAssemblyDoc to/from which IModelDoc could also be casted.

The id for an assembly has to be different from the id for a part. That's why I wanted to explicitely pass in a IPartDoc.

Ah ok, then there's an little error in this line

string modelPath = part.GetPathName();

This should read

string modelPath = model.GetPathName();

, right?

I would probably then go for an extension method on IPartDoc ...

Sorry, copy/paste error - fixed it.
Will think about the ext. method idea.
Thanks.

Alex, this is indeed a very interesting scenario. @BjRo I agree with you that b) violates TDA. However, you and me know as well that TDA and SRP have a sort of open hostility. It's - as always - a matter of context when to weigh one over the other.

I think it is crucial to have a viable answer for two important questions:

1. Is the mutability of IPartDoc <-> IModelDoc2 a general (say: "axiomatic") or exceptional (say: "problematic") feature of your domain?
2. what's the level of trust and responsibility of the caller?

I do believe that there's even more to ask, but those two questions are pretty obvious for me in order to have a grounded decision. The first question does in fact adress the importance of the "cast expressiveness" and hence the strength of SRP. If every IPartDoc is by definition an IModelDoc2 (as COM sinks tend to), then the mutation is general. It's then sort of a "public" and "well known" feature. As a consequence, I'd more lean towards TDA and go for option a) - as is.

The "technical issue" of casting would then likely be mitigated with mutimocks or even a handrolled fake. Because the mutability is general, I'd favor the recommendation of @BjRo for an extension method.However, if the mutation has an "exceptional" background, I'd rather go for a static method within the class to perform the cast to get some coherence credits. The TDA preference remains - or more precisely - depends more on the answer to question 2).

If the caller is a trusted caller (in terms of domain knowledge!), probably even with some mediation characteristics, then I'd lean more to version b) - as is. Reasoning: pay into the SRP and KISS piggybank, by explicitly using the bare native types in method signature. Take the cast responsibility outside, preferably onto a third party (-> extension method might be valid). Give the trusted caller the responsibilities.

In contrast, if the caller is untrusted, or if the caller is outside of the bounded context, or even crosses functional boundaries, then i'd lean towards TDA. In theory language: The more caller and callee have a strong DbC relationship, or if there's even an indication towards violation of LoD, then it's plausible for me to opt for option a).

Whatever the context (and the decision) is, it seems to make very good sense for me to declare the cast responsibility (and hence the mutation behavior) explicitly. Your broad options range from converter class over to extension method up to static method.

Just my 2 poor cents.

@ilkerde Exceptionally well put and thoughtful. Couldn't agree more

Ilker, thanks for that profound analysis of my problem. Looking forward to further discussions.