4
\$\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://i.sstatic.net/aQSJf.jpg

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());
 }
 }
}
asked Oct 13, 2020 at 19:16
\$\endgroup\$

1 Answer 1

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.

answered Oct 14, 2020 at 0:42
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.