24

I have a big object:

class BigObject{
    public int Id {get;set;}
    public string FieldA {get;set;}
    // ...
    public string FieldZ {get;set;}
}

and a specialized, DTO-like object:

class SmallObject{
    public int Id {get;set;}
    public EnumType Type {get;set;}
    public string FieldC {get;set;}
    public string FieldN {get;set;}
}

I personally find a concept of explicitly casting BigObject into SmallObject - knowing that it is a one-way, data-losing operation - very intuitive and readable:

var small = (SmallObject) bigOne;
passSmallObjectToSomeone(small);

It is implemented using explicit operator:

public static explicit operator SmallObject(BigObject big){
    return new SmallObject{
        Id = big.Id,
        FieldC = big.FieldC,
        FieldN = big.FieldN,
        EnumType = MyEnum.BigObjectSpecific
    };
}

Now, I could create a SmallObjectFactory class with FromBigObject(BigObject big) method, that would do the same thing, add it to dependency injection and call it when needed... but to me it seems even more overcomplicated and unnecessary.

PS I'm not sure if this is relevant, but there will be OtherBigObject that will also be able to be converted into SmallObject, setting different EnumType.

  • 4
    Why not a constructor? – edc65 May 5 '15 at 16:15
  • 2
    Or a static factory method? – Brian Gordon May 6 '15 at 1:17
  • Why would you need a factory class, or dependency injection? You've made a false dichotomy there. – user253751 May 6 '15 at 8:55
  • 1
    @immibis - because I somehow did not think about what @Telastyn proposed: .ToSmallObject() method (or GetSmallObject()). A momentary lapse of reason - I knew something is wrong with my thinking, so I've asked you guys :) – Gerino May 6 '15 at 9:04
  • 3
    This sounds like a perfect use case for an ISmallObject interface which is only implemented by BigObject as a means to provide access to a limited set of its extensive data/behavior. Especially when combined with @Telastyn's idea of a ToSmallObject method. – Marjan Venema May 9 '15 at 19:15
0

None of the other answers have it right in my humble opinion. In this stackoverflow question the highest-voted answer argues that mapping code should be kept out of the domain. To answer your question, no - your usage of the cast operator is not great. I would advise to make a mapping service which sits between your DTO and you domain object, or you could use automapper for that.

  • This is a terrific idea. I already have Automapper in place, so it will be a breeze. Only issue I have with it: shouldn't there be some trace that BigObject and SmallObject are somehow related? – Gerino May 6 '15 at 12:17
  • 1
    No I don't see any advantage by coupling BigObject and SmallObject further together other than the mapping service. – Esben Skov Pedersen May 6 '15 at 12:24
  • 7
    Really? An automapper is your solution for design problems? – Telastyn May 6 '15 at 13:20
  • 1
    The BigObject is mappable to SmallObject, they are not really related to each other in classical OOP sense, and code reflects this (both objects exist in domain, mapping ability is set in mapping config along with many others). It does remove dubious code (my unfortunate operator override), it leaves models clean (no methods in them), so yeah, it seems to be a solution. – Gerino May 6 '15 at 15:29
  • 2
    @EsbenSkovPedersen This solution is like using a bulldozer to dig a hole to install your mailbox. Luckily the OP wanted to dig up the yard anyway, so a bulldozer works in this case. However, I wouldn't recommend this solution in general. – Neil May 7 '15 at 7:35
81

It is... Not great. I've worked with code that did this clever trick and it led to confusion. After all, you would expect to be able to just assign the BigObject into a SmallObject variable if the objects are related enough to cast them. It doesn't work though - you get compiler errors if you try since as far as the type system is concerned, they're unrelated. It is also mildly distasteful for the casting operator to make new objects.

I would recommend a .ToSmallObject() method instead. It is clearer about what is actually going on and about as verbose.

  • 18
    Doh... ToSmallObject() seems like the most obvious choice. Sometimes most obvious is the most elusive ;) – Gerino May 5 '15 at 12:16
  • 6
    mildly distasteful is an understatement. It's unfortunate that the language allows this sort of thing to look like a typecast. Nobody would guess it was an actual object transformation unless they wrote it themselves. In a one-person team, fine. If you collaborate with anyone, in the best case it's a waste of time because you have to stop and figure out if it's really a cast, or if it's one of those crazy transforms. – Kent A. May 5 '15 at 13:38
  • 3
    @Telastyn Agreed that it's not the most egregious code smell. But the hidden creation of a new object from on operation that most programmers understand to be an instruction to the compiler to treat that same object as a different type, is unkind to anyone who has to work with your code after you. :) – Kent A. May 5 '15 at 14:45
  • 4
    +1 for .ToSmallObject(). Hardly ever should you override operators. – ytoledano May 5 '15 at 14:55
  • 6
    @dorus - at least in .NET, Get implies returning an existing thing. Unless you've overridden the operations on the small object, two Get calls would return unequal objects, causing confusion/bugs/wtfs. – Telastyn May 5 '15 at 16:25
11

While I can see why you would need to have a SmallObject, I would approach the problem differently. My approach to this type of issue is to use a Facade. Its sole purpose is to encapsulate BigObject and only make available specific members. In this way, it is a new interface on the same instance, and not a copy. Of course you may also want to perform a copy, but I would recommend that you do so through a method created for that purpose in combination with the Facade (for instance return new SmallObject(instance.Clone())).

Facade has a number of other advantages, namely ensuring that certain sections of your program can only make use of the members made available through your facade, effectively guaranteeing that it cannot make use of what it shouldn't know about. In addition to this, it also has the enormous advantage that you have more flexibility in changing BigObject in future maintenance without having to worry too much about how it is used throughout your program. So long as you can emulate the old behavior in some form or another, you can make SmallObject work the same as it did before without having to change your program everywhere BigObject would have been used.

Note, this means BigObject does not depend on SmallObject but rather the other way around (as it should be in my humble opinion).

  • The only advantage you mentioned that a facade has over copying fields to a new class is avoiding the copying (which is probably not a problem unless the objects have an absurd amount of fields). On the other hand it has the disadvantage that you must modify the original class every time you need to convert to a new class, unlike a static conversion method. – Doval May 5 '15 at 13:18
  • @Doval I suppose that's the point. You wouldn't convert it to a new class. You'd create another facade if that's what you require. Changes made to BigObject need only be applied to the Facade class and not everywhere where it is used. – Neil May 5 '15 at 13:30
  • One interesting distinction between this approach and Telastyn's answer is whether responsibility for generating SmallObject lies with SmallObject or BigObject. By default, this approach forces SmallObject to avoid dependencies on private/protected members of BigObject. We can go a step further and avoid dependencies on private/protected members of SmallObject by using a ToSmallObject extension method. – Brian May 5 '15 at 14:54
  • @Brian You risk cluttering BigObject that way. If you wanted to do something similar, you'd vouche for creating a ToAnotherObject extension method within BigObject? These should not be the concerns of BigObject since, presumably, it is already large enough as it is. It also allows you to separate BigObject from the creation of its dependencies, meaning you could use factories and the like. The other approach strongly couples BigObject and SmallObject. That may be fine in this particular case, but it isn't best practice in my humble opinion. – Neil May 5 '15 at 15:00
  • 1
    @Neil Actually, Brian explained it wrong, but he is right - extension methods do get rid of the coupling. It's no longer BigObject being coupled to SmallObject, it's just a static method somewhere that takes an argument of BigObject and returns SmallObject. Extension methods really are just syntactic sugar to call static methods in a nicer way. The extension method is not part of BigObject, it's a completely separate static method. It's actually a pretty good use of extension methods, and very handy for DTO conversions in particular. – Luaan May 6 '15 at 8:57
6

There is a very strong convention that casts on mutable reference types are identity-preserving. Because the system generally does not allow user-defined casting operators in situations where an object of the source type could be assigned to a reference of the destination type, there are only a few cases where user-defined casting operations would be reasonable for mutable reference types.

I would suggest as a requirement that, given x=(SomeType)foo; followed sometime later by y=(SomeType)foo;, with both casts being applied to the same object, x.Equals(y) should always and forevermore be true, even if the object in question was modified between the two casts. Such a situation could apply if e.g. one had a pair of objects of different types, each of which held an immutable reference to the other, and casting either object to the other type would return its paired instance. It could also apply with types that serve as wrappers to mutable objects, provided that the identities of the objects being wrapped were immutable, and two wrappers of the same type would report themselves as equal if they wrapped the same collection.

Your particular example uses mutable classes, but does not preserve any form of identity; as such, I would suggest that it is not an appropriate usage of a casting operator.

1

It might be okay.

A problem with your example is that you use such example-ish names. Consider:

SomeMethod(long longNum)
{
  int num = (int)longNum;
  /* ... */

Now, when you've a good idea what a long and int means, then both the implicit cast of int to long and the explicit cast from long to int are quite understandable. It's also understandable how 3 becomes 3 and is just another way to work with 3. It's understandable how this will fail with int.MaxValue + 1 in a checked context. Even how it will work with int.MaxValue + 1 in an unchecked context to result in int.MinValue isn't the hardest thing to grok.

Likewise, when you cast implicitly to a base type or explicitly to a derived type its understandable to anyone who knows how inheritance works what is happening, and what the result will be (or how it might fail).

Now, with BigObject and SmallObject I don't have a sense of how this relationship works. If your real types where such that the casting relationship where obvious, then casting might indeed be a good idea, though a lot of the time, perhaps the vast majority, if this is the case then it should be reflected in the class hierarchy and the normal inheritance-based casting will suffice.

  • Actually they're not much more than what's provided in question - but for example BigObject might describe an Employee {Name, Vacation Days, Bank details, Access to different building floors etc.}, and SmallObject might be a MoneyTransferRecepient {Name, Bank details}. There is a straightforward translation from Employee to MoneyTransferRecepient, and there is no reason to send to banking application any more data than needed. – Gerino May 6 '15 at 12:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.