Dev

[mraniki]

Summary by Sourcery

Add new handlers for FXCM, Oanda, and Schwab, each providing methods for financial operations such as retrieving quotes, account balances, positions, and executing orders.

New Features:

• Introduce a new handler for FXCM, providing methods for obtaining quotes, account balances, account positions, trading asset balances, and executing orders.
• Add a new handler for Oanda, offering similar functionalities as the FXCM handler, including methods for quotes, balances, positions, and order execution.
• Implement a new handler for Schwab, with methods to retrieve quotes, account balances, positions, trading asset balances, and execute orders.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request introduces three new handler classes for different centralized exchange (CEX) clients: FXCM, Oanda, and Schwab. Each handler extends the CexClient base class and implements a common set of methods for interacting with the respective exchange.

File-Level Changes

Change	Details	Files
Addition of new handler classes for FXCM, Oanda, and Schwab exchanges	Created FxcmHandler class extending CexClient Created OandaHandler class extending CexClient Created SchwabHandler class extending CexClient Implemented common methods like get_quote, get_account_balance, get_account_position, get_trading_asset_balance, and execute_order Added placeholder implementations for methods (mostly returning 0 or pass)	cefi/handler/fxcm.py cefi/handler/oanda.py cefi/handler/schwab.py

---

Tips

• Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
• Continue your discussion with Sourcery by replying directly to review comments.
• You can change your review settings at any time by accessing your dashboard:
  • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
  • Change the review language;
• You can always contact us if you have any questions or feedback.

[sourcery-ai]

Hey @mraniki - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider removing commented-out code and implementing actual functionality, even if basic. If these are meant as placeholders, consider using abstract base classes or interfaces instead.
• Improve class-level documentation to explain the purpose of each handler and how they differ from each other. Also, consider adding more meaningful docstrings for methods.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 3 issues found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

[sourcery-ai]

issue (complexity): Consider implementing a common base class for handlers to reduce code duplication.

The current implementation introduces unnecessary complexity through code duplication across multiple handler files. To reduce complexity and improve maintainability, consider refactoring these handlers to use a common base class. This approach will reduce duplication while still allowing for service-specific implementations. Here's a suggested structure:

from abc import ABC, abstractmethod

class BaseHandler(ABC):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)

    @abstractmethod
    async def get_quote(self, instrument):
        pass

    @abstractmethod
    async def get_account_balance(self):
        pass

    @abstractmethod
    async def get_account_position(self):
        pass

    @abstractmethod
    async def get_trading_asset_balance(self):
        pass

    @abstractmethod
    async def execute_order(self, order_params):
        pass

class FxcmHandler(BaseHandler):
    async def get_quote(self, instrument):
        # FXCM-specific implementation
        raise NotImplementedError("get_quote not implemented for FXCM")

    # Implement other methods...

This structure ensures that:

1. Common functionality is centralized in the base class.
2. Service-specific implementations are clearly separated.
3. Unimplemented methods raise NotImplementedError, making their status explicit.

Additionally, replace all instances of pass or return 0 in unimplemented methods with raise NotImplementedError("Method not implemented"). This clearly indicates the status of these methods and prevents silent failures or unexpected behavior.

By adopting this approach, you'll significantly reduce code duplication and improve the overall structure and maintainability of your codebase.

[sourcery-ai]

issue (complexity): Consider implementing a base class for CEX clients and refactoring OandaHandler to inherit from it.

To reduce complexity and improve maintainability, consider the following refactoring:

1. Create a base class for CEX clients:

from abc import ABC, abstractmethod

class BaseCexClient(ABC):
    def __init__(self, **kwargs):
        super().__init__()
        # Common initialization code here

    @abstractmethod
    async def get_quote(self, instrument):
        raise NotImplementedError("get_quote must be implemented")

    @abstractmethod
    async def get_account_balance(self):
        raise NotImplementedError("get_account_balance must be implemented")

    @abstractmethod
    async def get_account_position(self):
        raise NotImplementedError("get_account_position must be implemented")

    @abstractmethod
    async def get_trading_asset_balance(self):
        raise NotImplementedError("get_trading_asset_balance must be implemented")

    @abstractmethod
    async def execute_order(self, order_params):
        raise NotImplementedError("execute_order must be implemented")

2. Refactor OandaHandler to inherit from BaseCexClient:

from .base_cex_client import BaseCexClient

class OandaHandler(BaseCexClient):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        # Oanda-specific initialization here

    async def get_quote(self, instrument):
        # Implement Oanda-specific logic here
        pass

    async def get_account_balance(self):
        # Implement Oanda-specific logic here
        pass

    async def get_account_position(self):
        # Implement Oanda-specific logic here
        pass

    async def get_trading_asset_balance(self):
        # Implement Oanda-specific logic here
        pass

    async def execute_order(self, order_params):
        # Implement Oanda-specific logic here
        pass

This refactoring:

1. Reduces code duplication across different CEX clients.
2. Provides a clear contract for what methods each client should implement.
3. Uses NotImplementedError in the base class to catch unimplemented methods early.
4. Allows for easy addition of new CEX clients in the future.

Implement the Oanda-specific logic in each method instead of using pass or returning 0. This will make the code more robust and prevent silent failures.

[sourcery-ai]

issue (complexity): Consider implementing a common base class for all handlers to reduce code duplication and enforce a consistent interface.

While the current implementation isn't overly complex on its own, it contributes to overall system complexity through code repetition across multiple handler classes. To address this, consider the following improvements:

1. Create a common base class for all handlers (e.g., BaseHandler) to reduce code duplication.
2. Use NotImplementedError for unimplemented methods instead of pass or return 0.

Here's an example of how you could start refactoring:

from abc import ABC, abstractmethod

class BaseHandler(ABC):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)

    @abstractmethod
    async def get_quote(self, instrument):
        raise NotImplementedError("get_quote method must be implemented")

    @abstractmethod
    async def get_account_balance(self):
        raise NotImplementedError("get_account_balance method must be implemented")

    # ... other common methods ...

class SchwabHandler(BaseHandler):
    async def get_quote(self, instrument):
        # Implement Schwab-specific logic here
        pass

    async def get_account_balance(self):
        # Implement Schwab-specific logic here
        pass

    # ... implement other methods ...

This approach:

• Ensures all handlers have a consistent interface
• Reduces code duplication
• Clearly indicates which methods need implementation
• Fails fast if an unimplemented method is called

Consider applying this pattern to your other handler classes (e.g., for FXCM and OANDA) to further reduce overall system complexity.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.43%. Comparing base (83db1ca) to head (407bb64).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
+ Coverage   69.40%   70.43%   +1.02%     
==========================================
  Files           8        8              
  Lines         487      487              
==========================================
+ Hits          338      343       +5     
+ Misses        149      144       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.