Is this an acceptable method of temporarily replacing functions with new versions?

1

In the script below I have created a module LoggedCalculations which ‘hides’ the functions in the module Calculations and provides new functions with the same names.

module Logging =
    open System
    let logEntry remark value =
        printfn "%A: %s %A"
            DateTime.Now
            remark
            value
        value

module Calculations =
    let multiplyBy20 x = x * 20
    let squareIt x = x * x

module LoggedCalculations =
    open Logging
    open Calculations
    let multiplyBy20 = logEntry "Multiplying :" >> multiplyBy20 >> logEntry "Multiplied value ="
    let squareIt = logEntry "Squaring :" >> squareIt >> logEntry "Squared value ="

open LoggedCalculations // Can be swapped with Calculations to disable logging.
squareIt 50

The output looks like this:

28/04/2022 12:56:59: Squaring : 50
28/04/2022 12:56:59: Squared value = 2500

Is this ‘module wrapping/hiding’ an acceptable thing to be doing?
Are there any potential pitfalls that I should be aware of (apart from forgetting to swap the opened module back to Calculations when I don’t want logging)?
Is there a better way to do it?

Note: There are probably much better ways of logging stuff but it’s the ‘hiding’ of functions that I’m curious about here.

2

This is a pretty opinionated topic, but I’m happy to share my opinions :smile:.

I’ve made use of this feature before, for example in an extreme case by pretty much globally shadowing the “partial” functions in the container modules (like Seq.head here) to attach a compiler warning to the function (I used this trick in a codebase where we try to stick to just total functions). I haven’t run into any pitfalls beyond what I’m sure you’re already aware of - it can cause surprise if folks are expecting the original version. The scope can be important. I’ll shadow a function as a local binding inside another small function without a second thought. I’ll think twice before I shadow a function in a good size module. And I’ll only globally shadow something after a lot of thought and a really good reason. If scoped to a module of decent size, I would probably want a better reason to shadow something besides just logging (unless I was debugging something, and intended to revert it), preferring to just give it a new name instead. But there are cases where I will think it worthwhile to shadow something inside a module.

Not everyone agrees with my sentiments though. For example, I do a lot of programming in PureScript, which also allows for shadowing names like that, but any shadowing results in a compiler warning by default. So there are certainly folks out there that think that shadowing is a bad practice in general because of the potential confusion.

I particularly like to shadow when some computation invalidates the original value, e.g.,

let fn xs = 
  // shadow `xs` here
  let xs = xs |> Array.filter isValid
  // anywhere below this point, it would be a bug to use the original `xs`
  // because it potentially contains invalid values
  ...
3

Thanks for the advice.

I don’t understand all of it yet but, from what I do understand, it looks like I could be setting myself up for a nasty fall if I continue doing things this way as a matter of course.

Giving functions unique names sounds like the better way until I really need to shadow things under specific conditions.

I had a feeling that I might have been going about this the wrong way as it didn’t feel quite right, so it’s good to get some confirmation that I was right to question myself.

So much to learn, but always interesting to do so.

Cheers.

1 Like