The rule S7197 says the following:
This rule reports circular dependencies between source files, including indirect cycles spanning multiple files caused by circular imports.
It triggers with this code sample:
// order.js
import { Customer } from './customer.js';
import { Product } from './product.js';
export class Order {
constructor() {
this.customer = new Customer();
this.products = [];
}
}
// customer.js
import { Order } from './order.js';
export class Customer {
constructor() {
this.orders = [];
}
}
// product.js
import { Order } from './order.js';
export class Product {
constructor() {
this.orders = [];
}
}
And is fixed by this compliant code sample:
// order.js
import { Customer } from './customer.js';
import { Product } from './product.js';
export class Order {
constructor() {
this.customer = new Customer();
this.products = [];
}
}
// customer.js
export class Customer { }
// product.js
export class Product { }
// orderService.js
export function getOrdersByCustomer(customer) {
// Implementation to get orders by customer
}
export function getOrdersByProduct(product) {
// Implementation to get orders by product
}
Why is the Orders By Product relationship extracted to a service, and not the also existing Products by Order one? Even the Customer By Order relationship could be extracted. Why not? Where do you draw the line?
I believe you can’t provide architecture rules without also providing the reason why they exist. People that are comfortable with entity-relationship model are also comfortable with implementing them - they always use the repository pattern anyway and would never trigger the rule to begin with. You rule targets people that are not comfortable with implementing ER models. You can’t just feed them with arbitrary solutions without explaining the reasoning.
What about the Active Record pattern?
The rule will definitely makes codes built around the Active Record pattern explode. I’m not saying this is a bad thing - Active Record should never have existed to begin with - but you can’t ignore the fact that a lot or ORMs are built around entity classes that also act as repositories - and thus provide methods like Product::getOrders.
Typically, TypeORM:
This is their official documentation and it would trigger the rule, and their is no alternative with such a pattern.
Will you officially recommend to not use such a pattern? There are other alternatives - even with TypeORM by using entity schemas instead of decorated classes: will you officially recommend them?
Don’t get me wrong: I would love that. I - and I’m not alone - have been fighting against Active Records and decorated classes usage for years, so having the official support from Sonar would make my life much easier. But, if you do so, you need to be prepared for the shitstorm: the defense force is vindictive.