-1
\$\begingroup\$

I've got some code which converts from type X to Z on a web service. The X is from our database and we convert it to Z using the below code to send to the customer's required format . I've simplified it to keep it really basic.

So, let me give you my unit test:

OrderReleased orderReleased = new OrderReleased();
orderReleased.MessageId = 1234;
orderReleased.OrderId = "123";

// PS: Convert data from our type to customer's data-type
var orderReleased = Converter.Transform(orderReleased);

Assert.AreEqual(orderReleased.MessageId, orderReleasedReflex.message.msgId);
Assert.AreEqual(orderReleased.OrderId, orderReleasedReflex.message.orderId);

Though, there are many fields in these classes and many classes to test. Is there more of an elegent way of performing these test to ensure the data is been copied accross? And ensuring if any new fields are added that the test fails? Thank you.

\$\endgroup\$

closed as off-topic by Sᴀᴍ Onᴇᴌᴀ, Stephen Rauch, Imus, Toby Speight, Dannnno Apr 5 '18 at 14:33

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Sᴀᴍ Onᴇᴌᴀ, Stephen Rauch, Imus, Toby Speight, Dannnno
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Unless you include the implementation of OrderReleased, this question is not on-topic. Please edit your post to include that code and read more about why we need to see the implementation \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Apr 5 '18 at 13:15
  • \$\begingroup\$ Thanks Sam, I was just trying to keep the question very simple and not include all fields but I'll throw a simplified class on the question. \$\endgroup\$ – Paul Apr 5 '18 at 13:26
  • 2
    \$\begingroup\$ Please do not make any changes based on the suggestions offered in the answers (as this invalidates the answer). \$\endgroup\$ – Donald.McLean Apr 5 '18 at 14:12
  • 2
    \$\begingroup\$ Welcome to Code Review! I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. The example code that you have posted is not reviewable in this form because it leaves us guessing at your intentions. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR? \$\endgroup\$ – Dannnno Apr 5 '18 at 14:33
  • \$\begingroup\$ This question cannot be fixed anymore. It's a deadlock. Please ask a new one with the complete code. Do not modify this one because it invalidates the answer. \$\endgroup\$ – t3chb0t Apr 5 '18 at 15:10
3
\$\begingroup\$

Consider Reflection

With Reflection you can iterate over public properties and fields and verify that they're all set as expected, this means your tests will test all new fields and properties as they are added instead of relying on a developer to remember to update not just the converter, but also the converter tests.

Don't re-invent the wheel

The problem you have has been well-solved by Automapper, I recommend using their solution as it has already been thoroughly tested and is likely to be more flexible.

Use var

Using var for variable declarations where the assignment makes the type obvious saves you time in the future when you want to change the type. You already use var in the Act part of your testing pattern, may as well use it in the Arrange section, too.

Comments

The comment:

// PS: Convert data from our type to customer's data-type

Is not particularly helpful. This information can be gleaned from the code, and if the code changes odds are the comment will not be updated and end up out-of-date, actively harming understandability.

I recommend aiming for your comments to explain why you're doing something (when it is not apparent), not how you're doing it, unless your solution is so complex as to not be trivially understood by a maintainer (and in those cases simplify the code where possible instead of just adding comments).

Use an object initialiser

Object initialisers simplify code, so instead of:

var orderReleased = new OrderReleased();
orderReleased.MessageId = 1234;
orderReleased.OrderId = "123";

Use:

var orderReleased = new OrderReleased()
{
    MessageId = 1234,
    OrderId = "123"
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for awesome reply Nick! The reason I don't use object initialisation is it goes 4 levels deep in classes can get really messy and the debugger doesn't help when you forget to initialise an object. I'm going to check out the AutoMapper and see how it goes. Thank you. \$\endgroup\$ – Paul Apr 5 '18 at 13:23

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