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
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.
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.
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.
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.
精彩评论