Cleaning Up Generated Code: Refactor or Map?

Question

Context:

I recently had to deal with a class file generated by XSD.exe. It was 3500 lines long with ridiculously-verbose class / variable names (think someRidiculouslyLongPrefixThenMaybeOneThingUniqueAtTheEnd - difficult to compare at a glance with someRidiculouslyLongPrefixThenMaybeOneOtherThingChanged) and annotations all over the place. Bottom line is it took me ages to work out what the heck was going on. I read it and thought I would never put my name next to something so... Un-clean.

Question:

1) Is it bad practice to mess with generated code (i.e. clean it).

2) Would it be better practice to write a mapper to map the generated classes to my own nice, clean classes (which I could then get to work with, quite happily)?

EDIT:

Thanks for all the comments.

If I was actually going to do anything interesting with it (i.e. if there were domain objects which were anything but transport objects) then I think I'd map them to 'cleaner' classes, which I'd have to do anyway to get any kind of functionality out of them. In this case the classes are effectively DTOs so perhaps it makes sense that the naming matches the corresponding elements. As stated, I don't need to touch it - just to call accessors / mutators before passing the data down to another layer for processing.

For now, I think I'll leave them well alone.

Answer 1

The danger with refactoring generated code to clearn and tidy it is that if it is regenerated again by the tool by yourself or another developer then the changes would be lost.

Your team could get yourselves in a position where you would be generating the code in another file and copying it into the cleaned version and refactoring to apply changes which just takes time and resource. (I've been there with the original version of Entity Framework.)

If you cannot live with the names generated, either change the source it generates from or do as you suggest in #2.

Answer 2

As a general rule, never touch generated code, because doing so means promising that you will never generate it again, or you'll have to redo all the changes you've made. If you want the generated code to look nicer, you have to automate both the code generation and the cleanup; for example, if you generate an XML file somewhere, you might want to run it through xmlindent as part of your build process.

For similar reasons, generated code does not belong in source control. You put the data and rules under source control, and let your build script handle the code generation.

Any changes to the generated code have to go through the code generator, that is, if you want the generated code to look different, change the code generator's inputs, the code generator itself, or apply scriptable post-processing. But don't hand-edit.

An exception are 'scaffolding'-style code generators, where you run the generator once to give you a skeleton from which you build further. With those, you'll never run the generator again for the same file, so you just treat the generated file as a regular source file and forget that it comes from a generator.

Answer 3

I totally agree to @NikolaiDante (+1) answer.

In dotnet/c# you have the concept of "partial classes": The sourcecode for the "class to use" consist of 2 seperate files a generated part and of a manually added/edited part. Your api-beautifications go inte the "manual" file that won-t be overwritten by the codegenerator.

In the java/hybris world they use inheritance for the none-generated-code:

You have for example a class Customer with your "api-beautifications" that inherits from "GeneratedCustomer".