Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat:Business logic exception categorized as features #266

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

Sathish1891
Copy link
Collaborator

Summary of Changes

** Implemented Structured Exception Handling:**

  • Created a base FeatureException class for managing common feature-related errors.
  • Developed specific exceptions, such as TransfersException, to handle errors related to the transfers feature.

Introduced Enums for Features and Error Descriptors:

  • Added Feature enum to centralize available features (e.g., TRANSFERS, REMOTE_DEPOSITS, PAYMENTS).
  • Added ErrorDescriptor enum to standardize error descriptors (e.g., INSUFFICIENT_FUNDS, ACCOUNT_NOT_FOUND).

Enhanced Clarity and Maintainability:

  • Organized exception handling for better clarity and consistency across the application.
  • Improved type safety by using enums, ensuring only valid feature names and error descriptors are used.

- Documentation:

  • Provided comprehensive JavaDoc comments for classes and methods to facilitate understanding and usage.


import com.mx.path.core.common.request.Feature;

public class FeatureMapper {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a FeatureMapper class. The Feature enum contains these strings already. And it's overriding the toString() method to return the name. Could probably use that instead https://github.com/mxenabled/path-core/blob/master/common/src/main/java/com/mx/path/core/common/request/Feature.java#L113

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature should just be provided as a constructor arg. No need to map anything.

* transfer-related errors.
* </p>
*/
public class TransfersException extends FeatureException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public class TransfersException extends FeatureException {
public class TransferException extends FeatureException {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more consistent to make there singular.

* Returns the name of the feature associated with this exception.
*/
@Override
protected String getFeatureName() {
Copy link
Collaborator

@mattnichols mattnichols Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this. Just convert the enum to a string when placing in header.

Suggested change
protected String getFeatureName() {

*/
public abstract class FeatureException extends PathRequestException {

private final String feature;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private final String feature;
private final Feature feature;

* Initializes headers for the exception.
*/
private void initialize() {
withHeader("MX-Feature", feature);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
withHeader("MX-Feature", feature);
withHeader("MX-Feature", feature.toString());

public abstract class FeatureException extends PathRequestException {

private final String feature;
private final String errorDescriptor;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with this. Just store as an enum and convert to a string when placing in header.

Suggested change
private final String errorDescriptor;
private final ErrorDescriptor errorDescriptor;

*/
private void initialize() {
withHeader("MX-Feature", feature);
withHeader("MX-Feature-Error", errorDescriptor);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
withHeader("MX-Feature-Error", errorDescriptor);
withHeader("MX-Feature-Error", errorDescriptor).toString();

* @param errorDescriptor The error descriptor for this exception.
*/
public TransfersException(String userMessage, ErrorDescriptor errorDescriptor) {
super(userMessage, Feature.TRANSFERS, errorDescriptor.getDescriptor());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
super(userMessage, Feature.TRANSFERS, errorDescriptor.getDescriptor());
super(userMessage, Feature.TRANSFERS, errorDescriptor);

private final String feature;
private final String errorDescriptor;

protected FeatureException(String userMessage, Feature feature, String errorDescriptor) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected FeatureException(String userMessage, Feature feature, String errorDescriptor) {
protected FeatureException(String userMessage, Feature feature, ErrorDescriptor errorDescriptor) {

Copy link
Collaborator

@mattnichols mattnichols left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good start. 👍

@Sathish1891 Sathish1891 merged commit 017620c into master Oct 23, 2024
4 checks passed
@Sathish1891 Sathish1891 deleted the sathish/feature_exception branch October 23, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants