9

Despite many years in IT, I still struggle with OO design. One particular problem I seem to keep ending up with is large classes, often containing many hundreds of lines of code.

The OO world talks a lot about SRP, and I could argue that these large classes I end up with are each handling a single responsibility. The complex nature of the app (a scientific data processing system) means that one of these responsibilities requires an awful lot of code! E.g. a class might have a single public method Calculate(), but there could be 15-20 or more private methods to support this operation.

Where possible I will extract methods into separate classes for re-usability, but often there is little or no scope for this. Part of me says I should split these large classes up purely to improve readibility (e.g. grouping similar methods into their own classes), but the times I've done this feels like a wasted exercise - all I've really done is spread the methods around, created more class dependencies, and made things a little more difficult for other devs to find stuff.

But then I read articles where developers talk about how all their classes are small enough to fit on the screen without scrolling, and I worry that I'm doing something wrong! In a previous job, I worked on a system where it had been OO'd to the extreme, and it was common to see classes containing just one method, often with no more than one or two lines of code. Personally I found this difficult to find my way around the code-base and to debug, and for this reason (and sheer number of classes) I could argue that it becomes more difficult to maintain - not the aim of good OO design. I guess it's down to personal taste.

So, is it acceptable to have large classes, or can you suggest other ways that I could deal with them?

Andrew Stephens
  • 1,105
  • 11
  • 19
  • 4
    This is clearly not a duplicate question IMHO. Writing readable and maintainable code isn't just an OO concern... – Robbie Dee Jun 20 '13 at 12:24
  • 1
    Cohesion is something you want to look at for 'large' classes. It looks at 'relatedness' of the elements by usage. Midway through [CouplingAndCohesion](http://c2.com/cgi/wiki?CouplingAndCohesion) there is a good summary of the qualities that encompass cohesion and coupling that you might find useful in exploring the topic. – JustinC Jun 20 '13 at 19:54

3 Answers3

11

have a single public method Calculate(), but there could be 15-20 or more private methods to support this operation.

That does not sound very "large" to me - as long as your 15-20 methods have a reasonable size. At least, not in general. However, to find out if your classes are reasonably-scoped, you will have to show them to another experienced developer and discuss it with him.

But then I read articles where developers talk about how all their classes are small enough to fit on the screen without scrolling, and I worry that I'm doing something wrong!

Don't worry: as long as your methods are small enough to fit on the screen. Okay, some people, including myself, think that's still too long. If you apply SRP and SLA ("single level of abstraction" principle, see, for example, here) to methods, they will typically become shorter than 20 lines.

The more important thing you should ask yourself is: do you suffer from maintainability problems? Do you often need days to find a specific bug because you have trouble isolating the problem due to the complexity of your code? If the answer is "yes", then your classes are probably too large. If the answer is "no", then don't worry about the size of your classes just because you "read some articles of some OO experts". Find our what works for your case.

Greg Kramida
  • 282
  • 2
  • 12
Doc Brown
  • 199,015
  • 33
  • 367
  • 565
2

Some classes are inevitably larger than others so this wouldn't immediately set any alarm bells ringing. However, you may wish look at your class design again if any of the following are true:

  • You have a class that forms the whole (or large part of the) program
  • There are large chunks of code within the class relating to a specific concern
  • Similar code is present in other classes
  • The same parameters with basic data types are being passed around from method to method e.g.

DoSomethingToThis(string name, int rating, datetime when, bool flag) {...}

DoSomethingToThat(string name, int rating, datetime when, bool flag) {...}

DoSomethingToTother(string name, int rating, datetime when, bool flag) {...}

Depending on your implementation language there may also be other ways of breaking up the code e.g. code regions and namespaces.

Robbie Dee
  • 9,717
  • 2
  • 23
  • 53
  • That's useful thanks. I use regions, and had considered partial classes, but again this is akin to breaking up the class, potentially making things harder to find. – Andrew Stephens Jun 20 '13 at 10:52
  • I must confess I've only ever found partial classes useful for forms where I sometimes use them to split the events from the rest of the code. Otherwise I've found them to be fairly pointless from a coding point of view. – Robbie Dee Jun 20 '13 at 13:07
2

Do you write tests to maintain your code? If not, then you'll find your code will probably become much more atomic. However, I'd really focus on what 'Doc Brown' says regarding maintainibility. That's the key thing.

One large class with lots of small methods is probably easier to work with then lots of small classes each with one method. I've recently done a re-write at work taking one large God class of almost 9,000 lines of code and splitting it down. There are still classes that have almost a thousand lines of code in them. It sounds pretty dire when written down like that but at work we're finding it much easier to find and understand things. There are more classes now and more folders but it's tidier.

There is a gap between theory and practice. What looks and sounds right in theory, and usually demonstrated with a simple example, doesn't work when applied to a large real application.

Daniel Hollinrake
  • 740
  • 1
  • 5
  • 13
  • 1
    Thanks for the response. I do write unit tests (although haven't yet progressed to TDD), and have on occasion broken up large classes for the sole purpose of making it easier to test the private methods (which are now public in the new classes). I suspect this probably isn't the "right" approach to take with unit testing though. – Andrew Stephens Jun 20 '13 at 10:56
  • It's hard making the leap to TDD but with regards to getting into writing unit tests I'll make a huge assumption that you're doing what a lot of people do. Also if you are working with existing code the approach you are taking seems the most pragmatic. – Daniel Hollinrake Jun 20 '13 at 11:09