3

This well-known article criticises Java on the basis that it does not allow you to write functions that do not live in a class. This flaw forces you to write classes with names that look suspiciously like verbs, such as GridCellHander or GarbageDisposalDestinationLocator. In other words, it looks like you've built a class (i.e. a noun) to escort these methods (i.e. verbs). I believe that the article is entirely correct and I feel disturbed when I write a class with a name similar to these, particularly in a language that does not have Java's restrictions.

However, it is utterly standard advice that if you have a small amount of methods within a class that are all the only users of some of the class's state, then those methods and their shared state should all be placed in a different class. This maintains cohesion and keeps your classes small.

How can this advice be followed without writing an "escort" class that clearly only exists to hold these methods?

For example, I'm currently dealing with a grid. Some of its cells represent times. These times need to be sent to my server. I originally had two independent methods for handling them, one for deleting from the server and one for upserting. When I realised that they shared state, I put them and their shared state in a TimeCellHandler class. This let me significantly simplify the parameter list of the methods and let me write helper methods that they share. This follows the guidance described in my second paragraph, but violates the first. I can't just call this class TimeCell, because it plainly doesn't represent those cells; It only represents the small subset of their data needed to talk with the server about them.

Note that this question is language agnostic. The article linked gives the impression that functional languages (e.g. Scheme) can always replace TimeCellHandler with something better, but I'm just as happy to see an OOP or non-OOP answer. I don't care if you have to resort to CLOS or macros. I will be happy with any answer that demonstrates how to group two methods that share state in a sensible way that does not involve naming a class (or your language's most similar concept) in such a way that it makes it obvious that the class only exists to hold the methods.

J. Mini
  • 997
  • 8
  • 20
  • Two methods with a shared state without a class around them can be created. by using closures. I dont' how to implement this in Java, but you find an example in C# in this Q&A , see my [former answer](https://softwareengineering.stackexchange.com/a/444245) – Doc Brown Jun 09 '23 at 22:38
  • 2
    @DocBrown Closures are just a poor man's classes. – J. Mini Jun 09 '23 at 22:41
  • 1
    Been a while since I used them, but Java has inner classes, right? So the methods could go into TimeCell.Handler. – user949300 Jun 10 '23 at 06:04
  • 1
    @J.Mini: better call it "a lightweight alternative to classes" - exactly what you asked for, if I got you right. – Doc Brown Jun 10 '23 at 06:21
  • 1
    `How can this advice be followed without writing an "escort" class that clearly only exists to hold these methods?` by not implementing the code in Java? – Laiv Jun 10 '23 at 07:22
  • @Laiv What would non-Java solution do you have in mind? The methods share state and that state has to go somewhere. – J. Mini Jun 10 '23 at 09:19
  • @user949300 I'm not seeing your point. I've pointed out why a `TimeCell` class would be unwise. – J. Mini Jun 10 '23 at 09:20
  • @DocBrown Solving this with a closure would have the same issues as solving it with a class. There is very little difference between the two concepts. – J. Mini Jun 10 '23 at 09:21
  • @J.Mini: which "issues" do you mean? You were complaining about an extra class (extra boilerplate code) and that you have to invent a name for it where you are struggling with this holy war of "verb vs. noun". A solution based on closures requires less boilerpate and can be done without having to invent any new name - using anonymous functions. So what's your point? BTW, it is debatable of not introducing an extra class name makes things more readable / maintainable. – Doc Brown Jun 10 '23 at 10:05
  • @DocBrown Hmm, I must admit that I never thought about the closure's potential to be anonymous. You might be on to something. – J. Mini Jun 10 '23 at 10:10
  • 1
    @J.Mini The post's owner doesn't give an alternative (another OO language that makes it better) so he's complaining by pleasure. If escort classes are code smell and you are against such a thing (which is legit too), the solution is "don't use Java". Or any OO language which functions are not first-level citizens. We can not complain about hammers not being good at screwing. – Laiv Jun 10 '23 at 11:07
  • @Laiv What solution do you have in mind in a language where functions are first class? – J. Mini Jun 10 '23 at 11:11
  • @J.Mini There is presumably a TimeCell class, containing many other methods, getters, yada yada, that interact with is "main" state. A first glance, one could simply add the delete and upsert methods there. However, if that offends your sense of SRP or good design, one could mentally separate them into an inner class, TimeCell.Handler. I'm not sure if this is a good idea, but its something that might work for your domain and design style. – user949300 Jun 10 '23 at 18:25
  • @user949300 My question says that there isn't a `TimeCell` class. The cells are really cells in a particular column of a `DataGridView`. I just happen to know that those cells are always times. – J. Mini Jun 10 '23 at 18:51
  • @J.Mini Your question says "I can't just call _this class_ TimeCell". (emphasis added). Does the fact that the cells are times have any effect on the delete / upsert? – user949300 Jun 10 '23 at 22:36
  • 1
    @user949300 Not the fact in of itself, but it does determine which SQL procedure I'm using and what I pass to it. – J. Mini Jun 10 '23 at 23:41
  • 1
    Name it anything other than `Handler` - If you need a shorter name, try `TimeCellDbSyncer` - https://wiki.c2.com/?DontNameClassesObjectManagerHandlerOrData – rwong Jun 11 '23 at 01:11
  • 1
    @rwong Some excellent advice I read years ago that often works: if you are ever naming something a **FooHandler** or **FooEntity**, consider dropping the suffix completely and just naming it **Foo**. – user949300 Jun 12 '23 at 02:21
  • [And objects are a poor man's closures.](https://wiki.c2.com/?ClosuresAndObjectsAreEquivalent) – Greg Burghardt Jun 12 '23 at 17:56
  • 1
    Consider the inherent contradiction between _"those methods **and their shared state** should all be placed in a different class"_ and _" an "escort" class that clearly **only** exists to **hold these methods**"_. – Flater Jun 13 '23 at 01:09

2 Answers2

7

So you've followed people's advice and split some logic and state into a class, but you cannot give this new class a good name. Now is a great time to examine the design decisions that lead you to this conundrum.

This part of the question immediately caught my attention:

However, it is utterly standard advice that if you have a small amount of methods within a class that are all the only users of some of the class's state, then those methods and their shared state should all be placed in a different class. This maintains cohesion and keeps your classes small.

This advice is often given when dealing with a very large class, and splitting the class up makes the code easier to reason about. You should be able to identify an abstraction that can be pulled out of that large class, which lives just fine on its own.

As with all advice on software design, you must assess whether this is beneficial to your current code. I also don't see how relentlessly moving code into smaller and smaller classes maintains cohesion. The concept of cohesion applies equally to how well classes cooperate with each other as it does to the members within a single class.

Fields do not need to be used in 100% of the methods in a class for that class to be cohesive. These things need to work well together and compliment one another. If they do that, don't separate them. They belong together (hence they are "cohesive").

Each time you reorganize some code, it should be fairly easy to give it a good name. The methods and internal state combined with your knowledge of the business process you are modeling should guide you when naming this new abstraction.

When you can no longer find a good name, stop. Put that code back where it used to be. It was fine there. There is nothing wrong with a subset of methods in a class accessing a subset of fields.

Another part of the question jumped out at me:

I can't just call this [TimeCellHandler] class TimeCell, because it plainly doesn't represent those cells; It only represents the small subset of their data needed to talk with the server about them.

I can see a couple of missing abstractions which might lead you to better names:

  • What is this small subset of data? Is there a business concept associated with it? Consult with your end users, a subject matter expert, or whoever is gathering requirements for the application. They might already have a good name.

  • Talk to the server? Deletes? Upserts? This sounds like data access logic to me.

You might be missing a business class that represents that subset of data. You also might be missing an abstraction that allows you to access and manipulate data in permanent storage, be that a file system, web service, or database.

Names become easier to identify once you refocus on the business process you are modeling. Try to identify other common categories of logic, like data access. This can lead you to better names, and other opportunities to move code into appropriate abstractions that still maintain cohesiveness.

Then you don't need "escort" classes and this question becomes moot.

Oh, and a quick thought experiment. If you cram a bunch of free floating functions into their own file, what would you name the file? And would that name be any better than the class that only serves as a container for those functions? Why wouldn't you name the class the same as that file?

Greg Burghardt
  • 34,276
  • 8
  • 63
  • 114
4

These times need to be sent to my server. I originally had two independent methods for handling them, one for deleting from the server and one for upserting. When I realised that they shared state, I put them and their shared state in a TimeCellHandler class.

This is fine? Now you just need a name that represents what the code is rather than what it does. TimeCellServerConnection? Something similarly nounlike.

And you might point out that simply changing a name has no impact at all on the code's actual design. There's no change in performance or behavior. I'll leave it as an exercise for the reader the consequences of this fact.

Telastyn
  • 108,850
  • 29
  • 239
  • 365
  • +1. And in case there are any doubts, for me "design" is not just about "performance of behaviour", it is clearly also about choosing expressive names. @J.Mini: though I mentioned the technical possibility of using anonymous function in another comment to avoid the necessity to come up with a name, I doubt it would make the code more readable for this specific case. Introducing a class `TimeCellServerConnection` is probably way more expressive. – Doc Brown Jun 11 '23 at 15:39