maxfridbe
maxfridbe

Reputation: 5970

Bad method names and what it says about code structure

(Apologies in advance if this is a re-post but I didn't find similar posts)

What bad method name patterns have you seen in code and what did it tell you about the code.

For instance, I keep seeing:

public void preform___X___IfNecessary(...);

I believe that this is bad because the operation X has an inversion of conditions. Note that this is a public method because classes methods might legitimately require private helpers like this

Upvotes: 4

Views: 1070

Answers (4)

maxfridbe
maxfridbe

Reputation: 5970

Another one I noticed recently, A bunch of private methods of the form:

private void SOMETHINGBecauseOf__a__(..);
private void SOMETHINGBecauseOf__b__(..);
private void SOMETHINGBecauseOf__c__(..);

I can't think of a good reason to have BecauseOf in a method nor to do sort-of the same SOMETHING. This looks like a good case for a switch/if statement in one method.

Upvotes: 1

tc.
tc.

Reputation: 33602

thing(), doThing(), and reallyDoThing()

I see this when people aren't entirely clear about the things a function is supposed to do. Maybe it checks if any action is needed first, or it updates caches, or it sends change notifications. Who knows?

Sometimes this is due to a reluctance to change method names. I hate this. Functions should do what they sound like they'll do. I change the name if I'm changing functionality significantly anyway, so it forces me to fix up all the callers.

Upvotes: 2

P Daddy
P Daddy

Reputation: 29537

Sometimes, developers just seem to have a problem using concise wording. I had one who named a procedure

InsertImportQueueRecord

I hated that name. I changed it to

ImportItem

The former not only uses tedious wording to express a simple concept, but unnecessarily reveals implementation details. Callers did not need to know that a queue was used, and if they did, I would have named it something like QueueItemImport, or ScheduleImport to point out that the import of the item was being queued or scheduled. Also, the concept of inserting a record is language in the implementation, not of the problem, and should be avoided.

Upvotes: 3

David Neale
David Neale

Reputation: 17058

If a concise method name cannot be worked out then it's a good indication that the method is trying to do too much and refactoring should be considered.

An obvious example would be ValidateFormData_PersistToDB_SendEmail().

Although as I'm a C# developer I wouldn't dare use underscores anyway.

Upvotes: 1

Related Questions