Problem
Its a conference service code from controller to DAO for conference application.
it contains controller, conference service, dao and entity class, I am seeking for any improvement or code review comments for this.
/**
* This class is responsible for representing as a conference features, this will be the entry point for the conference management system
*
*/
public class ConferenceFacadeController {
private ConferenceService conferenceService;
private VotingService votingService;
private DashBoardService dashBoardService;
private UserService userService;
public ConferenceFacadeController(ConferenceService conferenceService, VotingService votingService,
DashBoardService dashBoardService, UserService userService) {
super();
this.conferenceService = conferenceService;
this.votingService = votingService;
this.dashBoardService = dashBoardService;
this.userService = userService;
}
/**
* This method is responsible for calling conference creation.
* @param meetingName
* @param meetingDescription
* @param organizedBy
* @param conferenceSlides
* @param scheduledTime
* @param attendees
*/
public void createConference(String meetingName, String meetingDescription, String organizedBy,
List<ConferenceSlide> conferenceSlides, LocalDateTime scheduledTime, List<Attendee> attendees) {
ConferenceDetail conferenceDetail = new ConferenceDetail(UUID.randomUUID().timestamp(), meetingName,
meetingDescription, organizedBy, attendees, conferenceSlides, "New", scheduledTime);
conferenceService.createConference(conferenceDetail);
}
/**
* This method is responsible for calling update conference.
* @param updatConferenceDetail
*/
public void updateConferenceDetails(ConferenceDetail updatConferenceDetail) {
conferenceService.updateConferenceDetails(updatConferenceDetail);
}
/**
* This method is responsible for calling cancel conference
* @param cancelledConferenceDetail
*/
public void cancelConference(ConferenceDetail cancelledConferenceDetail) {
conferenceService.cancleConference(cancelledConferenceDetail);
}
/**
* This method is responsible for calling voting service.
* @param voteUpOrDown
*/
public void voteForConference(VoteUpOrDown voteUpOrDown) {
votingService.addVoting(voteUpOrDown);
}
/**
* This method is responsible for providing dash board object.
* @param user
* @return
* @throws UserNotFoundException
*/
public DashBoard getDashBoardData(User user) throws UserNotFoundException{
if(! userService.isUserExists(user)) {
throw new UserNotFoundException();
}
return dashBoardService.getDashBoardData(user);
}
}
////////////////////////////////
/**
* This is a service class which is responsible for performing all the operation for conference.
*
*
*/
public class ConferenceService {
private ConferenceDao conferenceDao;
private ConferenceSlideService conferenceSlideService;
private AttendeeService attendeeService;
private NotificationService notificationService;
public ConferenceService(ConferenceDao conferenceDao, ConferenceSlideService conferenceSlideService,
AttendeeService attendeeService,NotificationService notificationService) {
this.conferenceDao = conferenceDao;
this.conferenceSlideService = conferenceSlideService;
this.attendeeService = attendeeService;
this.notificationService=notificationService;
}
/**
* This method is to create conference.
* @param conferenceDetail
*/
public void createConference(ConferenceDetail conferenceDetail) {
attendeeService.addAttendee(conferenceDetail.getAttendeeList());
conferenceSlideService.addConferenceSlide(conferenceDetail.getConferenceSlide());
conferenceDao.createConference(conferenceDetail);
Notification notification=new Notification(conferenceDetail.getStatus(),conferenceDetail,NotifyStatus.NEW_MEETING);
notificationService.notifyAttendees(notification);
}
/**
* This method is to cancel conference.
* @param cancelledConferenceDetail
*/
public void cancleConference(ConferenceDetail cancelledConferenceDetail) {
cancelledConferenceDetail.setStatus("Cancelled");
conferenceDao.cancelConference(cancelledConferenceDetail);
Notification notification=new Notification(cancelledConferenceDetail.getStatus(),cancelledConferenceDetail,NotifyStatus.CANCEL_MEETING);
notificationService.notifyAttendees(notification);
}
/**
* This method is to update conference
* @param updatConferenceDetail
*/
public void updateConferenceDetails(ConferenceDetail updatConferenceDetail) {
conferenceDao.updateConference(updatConferenceDetail);
Notification notification=new Notification(updatConferenceDetail.getStatus(),updatConferenceDetail,NotifyStatus.UPDATED_MEETING);
notificationService.notifyAttendees(notification);
}
/**
* Get all the organized conference of a user.
* @param user
* @return
*/
public List<ConferenceDetail> getAllConferenceOrganizedBy(User user) {
return conferenceDao.getAllConferenceOrganizedBy(user);
}
/**
* This method is responsible for getting conference by date
* @param date
* @return
*/
public List<ConferenceDetail> getConferencesByDate(LocalDateTime date) {
return conferenceDao.getConferenceByDate(date);
}
}
/**
* This is a DAO class for ConferenceDao.
*
*
*/
public class ConferenceDao {
/**
* This method is responsible for creation of conference.
* @param conferenceDetail
* @return
*/
public ConferenceDetail createConference(ConferenceDetail conferenceDetail) {
throw new UnsupportedOperationException();
}
/**
* This method is responsible for updation of conference.
* @param updateConferenceDetail
* @return
*/
public ConferenceDetail updateConference(ConferenceDetail updateConferenceDetail) {
throw new UnsupportedOperationException();
}
/**
* This method is responsible for cancelation of conference.
* @param cancelledConferenceDetail
* @return
*/
public boolean cancelConference(ConferenceDetail cancelledConferenceDetail) {
throw new UnsupportedOperationException();
}
/**
* This method is responsible for getting all the organized conference of a user
* @param user
* @return
*/
public List<ConferenceDetail> getAllConferenceOrganizedBy(User user) {
throw new UnsupportedOperationException();
}
/**
* This method is responsible for getting conference by date.
* @param date
* @return
*/
public List<ConferenceDetail> getConferenceByDate(LocalDateTime date) {
throw new UnsupportedOperationException();
}
}
/**
* This class is representing DTO object for conference.
*
*
*/
public class ConferenceDetail implements Serializable {
/**
*
*/
private static final long serialVersionUID = 1L;
private long id;
private long meetingId;
private String meetingName;
private String meetingDescription;
private String organizedBy;
private List<Attendee> attendeeList;
private List<ConferenceSlide> conferenceSlide;
private LocalDateTime scheduledTime;
private String status;
public ConferenceDetail(long meetingId, String meetingName, String meetingDescription, String organizedBy,
List<Attendee> attendeeList, List<ConferenceSlide> conferenceSlide, String status,LocalDateTime scheduledTime) {
super();
this.meetingId = meetingId;
this.meetingName = meetingName;
this.meetingDescription = meetingDescription;
this.organizedBy = organizedBy;
this.attendeeList = attendeeList;
this.conferenceSlide = conferenceSlide;
this.status = status;
this.scheduledTime=scheduledTime;
}
public LocalDateTime getScheduledTime() {
return scheduledTime;
}
public String getStatus() {
return status;
}
public void setStatus(String status) {
this.status = status;
}
public long getId() {
return id;
}
public long getMeetingId() {
return meetingId;
}
public String getMeetingName() {
return meetingName;
}
public String getMeetingDescription() {
return meetingDescription;
}
public String getOrganizedBy() {
return organizedBy;
}
public List<Attendee> getAttendeeList() {
return attendeeList;
}
public List<ConferenceSlide> getConferenceSlide() {
return conferenceSlide;
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((attendeeList == null) ? 0 : attendeeList.hashCode());
result = prime * result + ((conferenceSlide == null) ? 0 : conferenceSlide.hashCode());
result = prime * result + (int) (id ^ (id >>> 32));
result = prime * result + ((meetingDescription == null) ? 0 : meetingDescription.hashCode());
result = prime * result + (int) (meetingId ^ (meetingId >>> 32));
result = prime * result + ((meetingName == null) ? 0 : meetingName.hashCode());
result = prime * result + ((organizedBy == null) ? 0 : organizedBy.hashCode());
result = prime * result + ((scheduledTime == null) ? 0 : scheduledTime.hashCode());
result = prime * result + ((status == null) ? 0 : status.hashCode());
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
ConferenceDetail other = (ConferenceDetail) obj;
if (attendeeList == null) {
if (other.attendeeList != null)
return false;
} else if (!attendeeList.equals(other.attendeeList))
return false;
if (conferenceSlide == null) {
if (other.conferenceSlide != null)
return false;
} else if (!conferenceSlide.equals(other.conferenceSlide))
return false;
if (id != other.id)
return false;
if (meetingDescription == null) {
if (other.meetingDescription != null)
return false;
} else if (!meetingDescription.equals(other.meetingDescription))
return false;
if (meetingId != other.meetingId)
return false;
if (meetingName == null) {
if (other.meetingName != null)
return false;
} else if (!meetingName.equals(other.meetingName))
return false;
if (organizedBy == null) {
if (other.organizedBy != null)
return false;
} else if (!organizedBy.equals(other.organizedBy))
return false;
if (scheduledTime == null) {
if (other.scheduledTime != null)
return false;
} else if (!scheduledTime.equals(other.scheduledTime))
return false;
if (status == null) {
if (other.status != null)
return false;
} else if (!status.equals(other.status))
return false;
return true;
}
}
Solution
I would suggest folowing:
1) Controller’s methods are responsible not only for service call but for request parameter validations. Im getDashBoardData you make validation but what about the other methods.
2) Calling service from service (sample: attendeeService.addAttendee(...) in ConferenceService) is dangerous approach because you can end up in cyclic dependencies. In other words conferenceService depends on attendeeService and attendeeService depends on servicex and servicex depends on conferenceService. Usually it can be solved by extraction common code common logic into dedicated class. So by my ippinion we should not call service from service.
3) Use lombock library if you can (@Getter, @Setter, @EqualsAndHashCode, @RequiredArgsConstructor). It will make your code clean and highly readable.
4) When you create conference it is good to return at least ID (or created object).
5) Consider to use DTO with mappers (for example mapstract ). It allows you to separate and control external API and internal implementation.