@Jeff_Mott ; you have brought up a really deep and interesting topic!
Let’s take a deeper look at my innocent getSplitDecimal() function…
I originally wrote this function just as a way to hide some code in my already bursting 2,300 lines-of-code “article.php” script. Nothing more.
But then after some time, I decided this function could be useful in other places, so I started putting some more effort into it.
And now I feel conflicted about how to best write this function. So, stepping back…
In and of itself, getSplitDecimal()'s job should be to take a DECIMAL number and return the Whole portion and the Fractional portion.
If you pass it a String, then I guess it should throw an error. But if you pass it a NULL or Empty String, I’m not so sure. (The Whole and Fractional portions of a NULL or Empty String are also NULLs, so why not just return nothing?)
For the purposes of building my Star Rating, it will be common that a Comment will have no $avgRating, and so the function would have to deal with NULLs.
My question to you is this…
How restrictive and Draconian should you make a Function?
On one hand, it is great if it throws an error every time there is some issue.
Then again, do you want your Function to barf and bring your whole website to a grinding halt over something petty?!
I have some Functions (e.g. validateSurveyResponse) that absolutely should throw up an error to the user if there an issue (e.g. they hacked the Form and submitted a bogus response).
And then there are other Functions (e.g. getAgeRange) which takes a Date-Of-Birth and returns a more modest “20-something” that if it breaks I don’t really care and will just have it display a “–” in that location on the page.
Follow me??
Back to my getSplitDecimal()…
I think in all cases, if you pass a String to that Function it should complain and throw an error. But I am not so sure that it makes sense to program into the Function an Error for a NULL or Empty String.
In my current application, I would rather do this…
if (!empty($ratingAvg)){
// Rating Exists.
// Check Range.
if (($ratingAvg >= 1) && ($ratingAvg <= 5)){
// Valid Rating.
$getSplitDecimal = getSplitDecimal($ratingAvg);
// Start building Stars here...
$stars = ...
}else{
// Invalid Rating.
$stars = '';
}
}else{
// Rating does Not Exist.
$stars = '';
}
As I see it, it is really the $ratingAvg that I should be sanitizing and validating more so than the results from getSplitDecimal().
The $ratingAvg is coming from a query in MySQL, so it is basically impossible that it could be “Hello”. And if $ratingAvg was a NULL (i.e. nobody has rated the Comment), then I think I need to handle that outside of my getSplitDecimal() function, because if $ratingAvg is NULL then I need to set $stars = ‘’; so that no Stars appear, and that really has nothing to do with this function’s purpose.
Follow me?
So… What are your thoughts on all of this??
I can totally see your earlier points, but then again I can also see where if you “go down the rabbit hole” too quickly and make your Functions too restrictive, that it can work against you…
Sincerely,
Debbie