Untitled

@Service
public class PaymentService {

    // ❌ CODE SMELL: Field Injection
    // WHY: Field injection makes the class harder to test, prevents immutability,
    //      and hides required dependencies.
    // FIX: Use constructor injection instead.
    @Autowired
    private UserRepository userRepository;

    // ❌ CODE SMELL: Another field injection
    // WHY: Same reasons as above. Also makes it unclear which dependencies are mandatory.
    // FIX: Replace with constructor injection.
    @Autowired
    private NotificationService notificationService;

    // ❌ CODE SMELL: Shared mutable static state
    // WHY: Spring services are singletons; static mutable state is unsafe in multithreaded apps.
    //      This cache can cause race conditions and inconsistent results.
    // FIX: Use a proper caching solution (e.g., Spring Cache, Caffeine, Redis).
    private static Map<String, Double> cache = new HashMap<>();


    public double calculateDiscount(String userId, double amount) {

        // ❌ CODE SMELL: Calling .get() on Optional without checking
        // WHY: Throws NoSuchElementException if user not found.
        // FIX: Use orElseThrow(), orElse(), or proper error handling.
        User user = userRepository.findById(userId).get();

        // ❌ CODE SMELL: Naive caching logic
        // WHY: No expiration, no invalidation, not thread-safe.
        // FIX: Use @Cacheable or a real cache provider.
        if (cache.containsKey(userId)) {
            return cache.get(userId);
        }

        double discount = 0;

        // ❌ CODE SMELL: Hardcoded strings for user type
        // WHY: Typos cause bugs; no compile-time safety.
        // FIX: Replace with an enum (e.g., UserType.PREMIUM).
        if (user.getType().equals("PREMIUM")) {
            discount = amount * 0.2;

        // ❌ CODE SMELL: Repeated string comparison logic
        // WHY: Violates Open/Closed principle; adding new types requires modifying this method.
        // FIX: Move discount logic into strategy pattern or user type enum.
        } else if (user.getType().equals("BASIC")) {
            discount = amount * 0.05;
        } else {
            discount = 0;
        }

        cache.put(userId, discount);
        return discount;
    }


    public void processPayment(String userId, double amount) {

        double discount = calculateDiscount(userId, amount);
        double finalAmount = amount - discount;

        // ❌ CODE SMELL: Business logic mixed with side effects
        // WHY: Violates Single Responsibility Principle.
        // FIX: Move notification logic to a domain event or separate orchestrator/service.
        notificationService.sendEmail(
                userId,
                "Payment processed: " + finalAmount
        );

        // ❌ CODE SMELL: Using System.out.println for logging
        // WHY: Not suitable for production; no log levels; no structured logging.
        // FIX: Use a logger (e.g., private static final Logger log = LoggerFactory.getLogger(...)).
        System.out.println("Payment processed for " + userId);
    }
}