Should we create classes just to group properties

Question

I am designing a bit of software that will integrate with our enterprise system using web services. We are dealing with work orders that are needed to maintain equipment.

We have a bit of a disagreement in regards to the structure of the contract. There are two methods on my side, I would like a single work order class with all properties related to the work order in it.

example (simplified)

WO - class
  Number - property
  Description - property
  WarrantyStatus - property
  WarrantySettlementDate - property
  LabourSteps<LabourStep>[] - list property
  Children<WO>[] - list property

On their side, they want to create a work order class, but for each logical group of data, they want to create another class to wrap the properties.

example (simplified)

WO - class
  WOHeader - class
    Number - property
    Description - property
  Warranty - class
    WarrantyStatus - property
    WarrantySettlementDate - property
  LabourSteps<LabourStep>[] - list property
  Children<WO>[] - list property

Now they will have approx 8 classes in the full definition that they want to create all with a few properties in each. This seems a bit over the top to me and has a bit of a code smell or anti pattern feel to it.

For my example, I have two classes, WO and LabourStep.

They have 4 classes for the same thing WO,WOHeader,Warranty,LabourStep. There will only ever be one WOHeader per WO and the same for Warranty.

My one seems far simpler to manage changes (add a new property to WO and thats it), but is less human readable. On their side, The structure is a little more easier to read by a human, but it feel like its pushing a logical view onto the contract the doesn't really belong. And to add a new property potentially requires a modification to the WO and the creation of a new class if a new logical group was added. These extra classes are only there to group properties.

What is the right way to go?

======EDIT==========

Frank - I completely agree with your thoughts and it is the way I approach my development. Except in this case, the third party monolithic enterprise system we are integrating with has been written in a very generic way to try and cater for as many businesses as possible. The data is spread across three tables in a one to one mapping with the majority in one table. There is barely any validation. There is no concept of a "Warranty" object/thing in that system. Its just 3 properties against a WO. All the web service will do is act like a data store, with the WO persisted as a whole. e.g. getWO() updateWO() createWO(). There will never be get/updateWarranty() methods. It's all just one blob of data to read/persist.

On our side, we never use the result directly from web services even if they are our own. We convert them into proper models and then use those. To us a data contract is very different to a model. There is no business logic in the data contract, its purely a container to get data from one part of the system to another without closely coupling them. That means we can easily change to persist to a database instead of a web service and all its needs to do is match the interface of the web service. This also allows us to modify our own models which may even include results from multiple web services without having to worry about being restricted to or changing the format of the data contracts.

So when I think in those terms, Having the data contracts split into WOHeader or WOWarranty classes just doesn't make any sense. It will never used in that way, not even by them. Its just there for a nice logical group of properties that mean nothing by themselves. A region block around those properties would have the same net effect. Labour steps and equipment details however, make a lot of sense to be in separate classes as they are separate entities and not just properties of work orders.

Answer 1

To be honest, your approach is more of a code smell to me than theirs. The point of OO is that you can break down your "big" work order thingy into smaller, and better manageable, parts.

I do not say that the way your colleagues break things down is "correct", or the "best" way - as that requires a lot more knowledge about the problem domain. However, I do already agree from the list of attributes you have given that your work order class is too large. It's prime to become a God-class very soon.

Let's consider one example in more detail: Warranty. You may be thinking "warranty only applies to our work orders, so we cannot reuse it anywhere else" and this may be true, or not. What will be true either way is that your WO class will have methods that specifically deal only with the warranty attributes and are independent of a work order in all but their placement inside that class.

With your approach, you will get a class that has loads of methods, which do not have much in common. By simply recognizing warranty as its own thing/class, you end up with a Warranty class that is very focused, extremely easy to understand and maintain and all that at a very small cost - you just need a Warranty attribute in your WO.

A follow-up question that you or your team may ask next is whether you want to refactor out an interface as well. Is Warranty really limited to work orders, or is it occurring somewhere else too? Do you really need to expose the Warranty class's methods to clients of the WO class? etc. etc.

And a final side note: When you start with your idea of the WO class, then moving towards your colleague's version is a simple matter of refactoring (extract class) and if it shows that you went the wrong way with that you can go back again just as well (inline class). Simply keep in mind, that no design decision will ever survive the test of time. It all should remain fluent and provide you with the ability to make such refactorings, so that you can adapt the design when next requirement (change) comes around the corner.