Best Practices

// ✅ Good - Each service focused on one domain
@Service
public class SupplierServiceImpl implements SupplierService {
    // Only supplier operations
    public SupplierDTO create(CreateSupplierDTO dto) { }
    public SupplierDTO update(String id, UpdateSupplierDTO dto) { }
    public void delete(String id) { }
}

@Service
public class InventoryItemServiceImpl implements InventoryItemService {
    // Only inventory item operations
    public InventoryItemDTO create(CreateInventoryItemDTO dto) { }
    public InventoryItemDTO updateStock(String id, int newQuantity) { }
}

@Service
public class AnalyticsServiceImpl implements AnalyticsService {
    // Only analytics operations
    public DashboardSummaryDTO getDashboardSummary() { }
    public FinancialSummaryDTO getFinancialSummary() { }
}

// ❌ Bad - Mixed responsibilities
@Service
public class AllInOneService {
    // Supplier operations
    public SupplierDTO createSupplier(CreateSupplierDTO dto) { }
    
    // Inventory operations
    public InventoryItemDTO createItem(CreateInventoryItemDTO dto) { }
    
    // Analytics
    public DashboardSummaryDTO getDashboard() { }
    
    // Too many responsibilities
}

// ✅ Good - Constructor injection
@Service
@RequiredArgsConstructor
public class SupplierServiceImpl implements SupplierService {
    
    private final SupplierRepository repository;
    private final SupplierValidator validator;
    private final SupplierMapper mapper;
    
    // Constructor auto-generated by Lombok
    // Fully testable with mocked dependencies
}

// ❌ Bad - Service locator pattern
@Service
public class SupplierServiceImpl implements SupplierService {
    
    private SupplierRepository repository = 
        ServiceLocator.getService(SupplierRepository.class);
    // Hard to test, hidden dependencies
}

// ❌ Bad - Field injection
@Service
public class SupplierServiceImpl implements SupplierService {
    
    @Autowired
    private SupplierRepository repository;
    // Testability issues with NullPointerExceptions
}

// ❌ Bad - Manual instantiation
@Service
public class SupplierServiceImpl implements SupplierService {
    
    private SupplierRepository repository = new SupplierRepository();
    // Cannot test with mocks, tightly coupled
}

// ✅ Good - Transactional writes
@Service
@RequiredArgsConstructor
public class SupplierServiceImpl implements SupplierService {
    
    private final SupplierRepository repository;
    
    @Transactional
    public SupplierDTO create(CreateSupplierDTO dto) {
        return mapper.toDTO(repository.save(mapper.toEntity(dto)));
    }
    
    @Transactional
    public SupplierDTO update(String id, UpdateSupplierDTO dto) {
        Supplier supplier = repository.findById(id)
            .orElseThrow(() -> new NoSuchElementException("Not found"));
        supplier.setName(dto.getName());
        return mapper.toDTO(supplier);  // Auto-persisted by transaction
    }
    
    @Transactional
    public void delete(String id) {
        repository.deleteById(id);
    }
    
    // Read-only query (optional optimization)
    @Transactional(readOnly = true)
    public SupplierDTO findById(String id) {
        return repository.findById(id)
            .map(mapper::toDTO)
            .orElseThrow(() -> new NoSuchElementException("Not found"));
    }
}

// ❌ Bad - Missing @Transactional
@Service
public class SupplierServiceImpl implements SupplierService {
    
    public SupplierDTO create(CreateSupplierDTO dto) {
        // No transaction - inconsistent state if failure occurs
        return mapper.toDTO(repository.save(mapper.toEntity(dto)));
    }
}

// ✅ Good - Let exceptions propagate
@Service
@RequiredArgsConstructor
public class SupplierServiceImpl implements SupplierService {
    
    @Transactional
    public SupplierDTO create(CreateSupplierDTO dto) {
        // Validation throws exception if fails
        validator.validateUniquenessOnCreate(dto.getName());
        
        // Repository throws exception if fails
        Supplier saved = repository.save(mapper.toEntity(dto));
        
        // Exception propagates to controller → GlobalExceptionHandler
        return mapper.toDTO(saved);
    }
}

// ❌ Bad - Catching and swallowing exceptions
@Service
public class SupplierServiceImpl implements SupplierService {
    
    public SupplierDTO create(CreateSupplierDTO dto) {
        try {
            return mapper.toDTO(repository.save(mapper.toEntity(dto)));
        } catch (Exception e) {
            // Lost error information
            return null;
        }
    }
}

// ❌ Bad - Catching without re-throwing
@Service
public class SupplierServiceImpl implements SupplierService {
    
    public SupplierDTO create(CreateSupplierDTO dto) {
        try {
            validator.validateUniquenessOnCreate(dto.getName());
        } catch (IllegalStateException e) {
            // Silent failure - no error returned to client
            log.error("Validation failed", e);
        }
        
        return mapper.toDTO(repository.save(mapper.toEntity(dto)));
    }
}

// ✅ Good - Validate first
@Service
@RequiredArgsConstructor
@Transactional
public class SupplierServiceImpl implements SupplierService {
    
    public SupplierDTO create(CreateSupplierDTO dto) {
        // 1. Validate input format
        validator.validateRequiredFields(dto);
        
        // 2. Validate business rules (requires DB lookup)
        validator.validateUniquenessOnCreate(dto.getName());
        
        // 3. Only THEN persist
        Supplier supplier = mapper.toEntity(dto);
        return mapper.toDTO(repository.save(supplier));
    }
}

// ❌ Bad - Persistence before validation
@Service
@Transactional
public class SupplierServiceImpl implements SupplierService {
    
    public SupplierDTO create(CreateSupplierDTO dto) {
        // 1. Persist immediately
        Supplier supplier = repository.save(mapper.toEntity(dto));
        
        // 2. Validate too late
        validator.validateUniquenessOnCreate(dto.getName());
        // If validation fails here, data already persisted
        
        return mapper.toDTO(supplier);
    }
}