OK, so there are now more things here than are suitable for a comment.
tl;dr
Your intuition about what you ought to do (refactoring as you go) is correct.
Your difficulty implementing this - given that you have to work around a poor code review system - comes down to your difficulty manipulating your source code and VCS. Multiple people have said that you can and should split your changes (yes, even within files) into multiple commits, but you seem to have difficulty believing this.
You really can do this. That really is what we're suggesting. You really should learn to get the most out of your editing, source manipulation and version control tools. If you invest the time in learning how to use them well, it makes life much easier.
Workflow/office politics issues
I'd make essentially the same recommendation as GlenH7 that you create two commits - one with only refactorings and (demonstrably or obviously) no functional changes, and one with the functional changes in question.
It may be useful though, if you're finding lots of errors, to pick a single category of error to fix within a single CR. Then you have one commit with a comment like "dedupe code", "fix type-X errors", or whatever. Because this makes a single type of change, presumably in multiple places, it should be trivial to review. It does mean you can't fix every error you find, but may make it less painful to smuggle through.
VCS issues
Splitting the changes made to your working source into multiple commits shouldn't be a challenge.
You haven't said what you're using, but possible workflows are:
if you're using git, you have excellent tools for this
- you can use
git add -i
for interactive staging from the command line
- you can use
git gui
and select individual hunks and lines to stage (this is one of the few VCS related GUI tools I actually prefer to the command line, the other being a good 3-way merge editor)
- you can make lots of tiny commits (individual changes, or fixing the same class of bug in multiple places) and then reorder them, merge them or split them with
rebase -i
if you're not using git, your VCS may still have tooling for this sort of thing, but I can't advise without knowing what system you use
You mentioned you're using TFS - which I believe is compatible with git since TFS2013. It may be worth experimenting with using a local git clone of the repo to work in. If this is disabled or doesn't work for you, you can still import the source into a local git repo, work in there, and use it to export your final commits.
you can do it manually in any VCS if you have access to basic tools like diff
and patch
. It's more painful, but certainly possible. The basic workflow would be:
- make all your changes, test, etc.
- use
diff
to make a (context or unified) patch file with all changes since the last commit
- partition that into two files: you'll end up with one patch file containing refactorings, and one with functional changes
- this isn't entirely trivial - tools such as emacs diff mode may help
- back everything up
- revert to the last commit, use
patch
to replay one of the patch files, commit those changes
- NB. if the patch didn't apply cleanly, you may need to fix up the failed hunks manually
- repeat 5 for the second patch file
Now you have two commits, with your changes partitioned appropriately.
Note there are probably tools to make this patch management easier - I just don't use them since git does it for me.
This is the kind of question that you can ask a dozen different people and get a dozen different answers. I say do the simplest thing that solves your problem: if all you need to do is override GetContracts
, don't even bother creating an abstract class, just derive from your existing class and override it. If you find yourself doing this a whole bunch and things start getting messy, then you can look into other strategies such as interface segregation and composition.
Best Answer
Primary reason to use a class is that there is a concept in domain you are trying to express in your code and you want to codify this concept in some way. Usually, classes are created when you, as a developer, see, through your years of experience, that there is a concept in what you are trying to achieve and you want to explicitly convert this concept into code.
But also, many of the concept show up after you have written the code, quite often as various code smells. There are two examples :
Multiple values being passed around together.
Be it Point or Customer, you can sometimes see that there are multiple (3+) variable all being passes around together. Code usually needs all, or most of those variables and even if it doesn't directly need them, it's dependencies might do. This is great opportunity to introduce a class to encapsulate all those variables. After that, you realize that many of the functions that work on those fields could actually be part of the class itself. And then, you might realize this is a concept within your domain, that you missed when doing analysis or design.
Big functions
It happened multiple times to me, that I found a function that was just too long. But when I tried to divide it into smaller functions, it resulted in all of the function's state being passed around in parameters. This is great opportunity to introduce a class to represent the whole function itself. The function's parameters might become a classe's constructor. The function's variables become fields. And then, you can easily separate the big function into smaller ones, because they all use shared state instead of relying on parameter passing. And then, you realize some of the
if
s andswitch
es might be represented as subclasses instead of code flow constructs, creating rich, maintainable and expressive representation instead of one huge hard-to follow and maintain function. This should make you think : if this piece of code was so complex, shouldn't it be some kind of concept in domain I'm working in?Classes as "structures with functions"
You are saying you have experience with procedural programming. Then you must know about structures and their use cases. Structures and classes overlap in many of those use cases. Some people even say that classes are just structures with functions bolted onto them and that they are no different than procedural programming. While I disagree, it might be good way for you to start thinking about classes. So whenever you would create a structure, you instead create a non-static class. The rest then follows.
Transparent dependencies and control flow
Another problem with static classes is that of dependency. If method of a class is static, then it becomes hard to tell what code actually uses this method. It becomes even worse problem if static state (in form of static fields) is involved. It becomes extremely hard to follow the flow of a code when you don't know what other methods might be involved. If you use non-static classes, it becomes much cleaner, because you know what piece of memory method can modify, instead of working with global state. Actually, global state is the keyword here. It is generally known problems with global state range from pain in the ass to total nightmare. You should be striving to minimize amount of global state in your programs. Usage of non-static classes should be prioritized over static classes whenever possible.