variable_get('Email', getAdminMail()),
variable_get('From', t('Comment@Count.Notifier')),
variable_get('Subject', t('Your pending Comment count')),

These variable names are far too general.

They should always start with the module name - comment_count_notify - and then end with something descriptive. I suggest instead of "email" you do "recipient_email" so in total comment_count_notify_recipient_email.

A similar review & change needs to be done on all variables in this module.

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: Better variable names - » Better variable and function names

I noticed this function - get_count - which is a problem as well. It should be named something more descriptive like comment_count_notify_get_comment_count.

getAdminMail also has this problem. And I'm not sure "admin email" is descriptive. It's really just the recipient who may or may not be an admin.

All function names should be reviewed: are they descriptive of what is actually happening? Do the function names start with the module name?

fotuzlab’s picture

fotuzlab commented
Status: Active » Needs review

Greg,
I have changed the variable and function names.
function getAdminMail(now comment_count_notify_get_admin_mail) is descriptive of what it does. It retrieves the site admin's or uid=1's email for the default value for the field named comment_count_notify_recipent_mail(erstwhile email). It has already been commented appropriately.
Kindly review it.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

This looks "fixed" to me - thanks!

fotuzlab’s picture

fotuzlab commented
Version: » 6.x-1.0
Status: Fixed » Closed (fixed)