3
\$\begingroup\$

I'm using Spring boot - 2.3.3.RELEASE and gradle.

Note- I'm using Factory and Strategy pattern. This project is going to be used as a Library for other projects to import and use the methods exposed by Service layer.

Here is the folder structure: https://imgur.com/a/jYr7wyP

I would appreciate if the community can review this code and give suggestions to make it better.

Demo.class

@EnableAutoConfiguration(exclude={DataSourceAutoConfiguration.class})
@SpringBootApplication
public class Demo {
    public static void main(String[] args) {
        SpringApplication.run(Demo.class, args);
    }
}

Init.java

@Component
public class InitClass implements ApplicationRunner {

    @Autowired
    PhoneService phoneService;

    @Override
    public void run(ApplicationArguments args) throws Exception {
        String title="Title";
        String message="message";
        List<String> phoneNumbers = new ArrayList<>();
        phoneNumbers.add("333-222-1111");
        phoneService.sendNotificationByPhoneNumbers(title, message, phoneNumbers);
    }
}

PhoneService.java

@Service
public class PhoneService {

    @Autowired
    PhoneServiceImpl notificationServiceImpl;

    public void sendNotificationByPhoneNumbers(String title, String message, List<String> phoneNumbers) {
        notificationServiceImpl.sendNotificationByPhoneNumbers(title, message, phoneNumbers);
    }
}

PhoneServiceImpl.java

@Slf4j
@Component
public class PhoneServiceImpl {

    @Value("${notificationService.url}")
    String url;

    public void sendNotificationByPhoneNumbers(String title, String message, List<String> phoneNumbers) {
        PhoneContext phoneContext = new PhoneContext(new SendPhoneByPhoneNumbers(url));
        phoneContext.notify(title, message, phoneNumbers);
    }
}

PhoneContext.java

public class PhoneContext {

    private PhoneStrategy phoneStrategy;

    public PhoneContext(PhoneStrategy phoneStrategy){
        this.phoneStrategy = phoneStrategy;
    }

    public void notify(String title, String message, List<String> employees){
        phoneStrategy.sendNotification(title, message, employees);
    }
}

PhoneStrategy.java

public interface PhoneStrategy {

   public void sendNotification(String title, String message, List<String> listOfEmployeeIdGroupNamePhoneNumbers);
}

SendPhoneByPhoneNumbers.java

@Slf4j
public class SendPhoneByPhoneNumbers implements PhoneStrategy {

    RestTemplate restTemplate;
    String notificationServiceURL;
    BuildHttpRequest buildHttpRequest;

    public SendPhoneByPhoneNumbers(String notificationServiceURL) {
        this.notificationServiceURL = notificationServiceURL;
        this.restTemplate = new RestTemplate();
        this.buildHttpRequest = new BuildHttpRequest();
    }

    @Async
    public void sendNotification(String title, String message, List<String> listOfEmployeeIdGroupNamePhoneNumbers) {
        SmsMessage smsMessage= new SmsMessage(title, message, listOfEmployeeIdGroupNamePhoneNumbers, Collections.emptyList(), Collections.emptyList());

        try {
            HttpHeaders headers = new HttpHeaders();
            headers.set("idToken", buildHttpRequest.getNewToken());

            HttpEntity<SmsMessage> newRequest = new HttpEntity<>(smsMessage, headers);
            restTemplate.postForObject(notificationServiceURL + "/sms/custom", newRequest, String.class);
        } catch (Exception e) {
            log.error(e.getMessage());
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

PhoneService class

In my opinion, the PhoneService should be converted to an interface exposing the methods.

public interface PhoneService {
   void sendNotificationByPhoneNumbers(String title, String message, List<String> phoneNumbers);
}

Then, the PhoneServiceImpl class can implement the interface PhoneService.

@Slf4j
@Service
public class PhoneServiceImpl implements PhoneService {

   @Value("${notificationService.url}")
   String url;

   @Override
   public void sendNotificationByPhoneNumbers(String title, String message, List<String> phoneNumbers) {
      PhoneContext phoneContext = new PhoneContext(new SendPhoneByPhoneNumbers(url));
      phoneContext.notify(title, message, phoneNumbers);
   }
}

Those’s changes will allow you to use inheritance and make the code easier to refactor if, you want to add any new phone services (new implementations). The old implementation was, in my opinion, a bit confusing and did not add any advantages that I can see.

Prefer Constructor-based Dependency Injection / Setter-based Dependency Injection over Field Injection

In my opinion, the Constructor Dependency Injection should be preferred over the Field Injection since it makes the testing easier and more natural (you can pass the mocks directly into the constructor) without using hardcore ways to inject the dependencies.

Also, even in Spring documentation, the Field Injection is not even mentioned as a way to inject and lots of people don’t recommend it.

Never expose the variables directly in classes

In my opinion, it's a bad habit to have public variables in classes, since you don’t control your own data. Try to hide them by adding the private / protected keyword before them and use getters / setters to change the state. This will give you more control over the class own data.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.