Given this dummy code, which has an if statement with a verbose code (imagine this is a really verbose if statement), I decide to create a util function to make my code cleaner. When I choose to create a util function, should it be implemented as an interface or can I implement the concrete function instead? I may use this same function again in other class.
Dummy code:
export class UserRepository {
constructor (private readonly database: DatabaseProtocol) {}
async findUserByEmail (data: Record<string, any>): Promise<IUser | null> {
const userCollection = this.database.collection('users')
if (Object.keys(data).length === 0) return null
const user = await userCollection.findOne({ email: data.email })
return user
}
}
Concrete implementation:
export class UserRepository {
constructor (private readonly database: DatabaseProtocol) {}
async findUserByEmail (data: Record<string, any>): Promise<IUser | null> {
const userCollection = this.database.collection('users')
if (isEmpty(data)) return null
const user = await userCollection.findOne({ email: data.email })
return user
}
}
Interface implementation:
export class UserRepository {
constructor (
private readonly database: DatabaseProtocol,
private readonly isEmpty: IsEmptyProtocol
) {}
async findUserByEmail (data: Record<string, any>): Promise<IUser | null> {
const userCollection = this.database.collection('users')
if (this.isEmpty(data)) return null
const user = await userCollection.findOne({ email: data.email })
return user
}
}
Private method:
export class UserRepository {
constructor (
private readonly database: DatabaseProtocol
) {}
async findUserByEmail (data: Record<string, any>): Promise<IUser | null> {
const userCollection = this.database.collection('users')
if (this.isEmpty(data)) return null
const user = await userCollection.findOne({ email: data.email })
return user
}
isEmpty (data: Record<string, any>): boolean {
return Object.keys(data).length === 0
}
}
Which option above would be considered the best practice?
There's not really any alternatives implementation you'd want to inject for such helper functions, therefore an interface would be overkill and combersome to use.
If you look at java, they usually have such utility functions in plural classes like Arrays where we can find utilities such as Arrays#asList. JavaScript does the same with for instance Object.entries.
With modules you could just have an objects.ts module and export an isEmpty function to reuse.
One thing to be careful about though is whether "empty" could have different meanings. For instance, perhaps "empty" in another context could mean there could be keys, but they don't count if they reference null or undefined. Therefore, it's important to document the behavior properly.
Furthermore, the empty check is really domain-specific. It seems to represent in your case whether or not there's an email filter present. Therefore, rather than calling isEmpty directly you'd most likely have a private hasEmailCriteria function or something.
However, in the end I also think the signature of findUserByEmail is pretty bad. Why not just accept an email such as findUserByEmail(email: string) instead? Or even better, perhaps findUserByEmail(address: EmailAddress).
Here are my 2 cents:
IsEmpty functionality is needed only for UserRepository class then make it a private method of that class.IsEmpty functionality is needed by some (by not all) classed in your program, then go with the Interface implementation: solution you proposed in your question.IsEmpty functionality is used in all your classes (or in the vast majority), then make it a global class with static methods. I know, I know, Uncle Bob does not allow global state (and rightfully so), but this is an exception. This case is a cross-cutting concern (please see my answer here from more details).