wouterfennis / coderefactorfw Goto Github PK
View Code? Open in Web Editor NEWFor ASP Assignment
For ASP Assignment
ClientServlet.java
De message kan een constante krijgen zodat het makkelijk aan te passen is
regel 131 & 132
message += " Overzicht door een onbekende fout niet herlanden, overzicht is niet up-to-date met de database.";
this.setMessage(MESSAGE_ERROR, message); //e.printStackTrace();
in de OverviewController.java wordt bij:
public JSONArray RefreshOverviewClients(User currentUser)
gegevens uit de database gehaald en in JSON gestopt.
Dit gebeurd voor een administrator en een "normale" gebruiker.
Maar bij de administrator gebeurd naast dit
for (Client c : db.getAllClients()) {
niks anders dan bij een normale gebruiker. Dus kan dit worden geextract naar een aparte methode.
ClientServlet.java
de doPost methode
Add the "@Override" annotation above this method signature
Update FamilyMember in db methode aanmaken.
fm.setMember_id(id);
// update familymember in the database
OverviewController.getInstance().getDb().updateFamilymember(fm);
// update member in the session
ArrayList<Familymember> members= new ArrayList<Familymember>();
for(Familymember f : client.getMyFamilymembers()){
if(f.getMember_id() == fm.getMember_id())
members.add(fm);
else
members.add(f);
}
client.setMyFamilymembers(members);
Let op dat deze functie het huidige id in de sessie moet mee krijgen. De code om het id op te halen staat vlak na de if("update") statement :
// if update was the status
else if (option.equals("update")) {
// get id that needs to update
int id = Integer.valueOf(req.getParameter("familymemberID"));
fm.setMember_id(id);
In de OverviewController.java komt een aantal keer het volgende return statement voor
return returns;
Dit is natuurlijk niet duidelijk en moet daarom hernoemt worden naar een duidelijke variabele
Code voor het inserten van een familymemberobject in de database moet apart aangemaakt worden.
// add familymember object into database
OverviewController.getInstance().getDb()
.addFamilymember(fm, client);
OverviewController.getInstance().getDb().getFamilymembersOfClient(client);
// add member to existing client in session
client.setMyFamilymembers(OverviewController.getInstance().getDb().getFamilymembersOfClient(client));
In de FamilyServlet komt twee keer de onderstaande code voor. Het zou handig zijn als deze code in een aparte methode stond :
if (forename == null || surname == null) {
// set error message and send back
req.setAttribute("message", "Voornaam en/of achternaam zijn onjuist.");
req.setAttribute("messageType", "error");
reqDisp = req.getRequestDispatcher("/socialworker/family/add_edit_family_member.jsp");
}
LoginSupportServlet.java
Meteen trimmen regel 88
String email = req.getParameter("email");
if (email != null && !email.trim().equals("")) {
for (User u : db.getAllUsers()) {
if (u.getEmail().equals(email.trim())) {
user = u;
break;
}
}
In de OverviewController.java wordt aan het begin van de "createJSONNetworks" methode een aantal JSON array's opgezet.
public JSONObject[] createJSONNetworks(Client client) throws JSONException {
// intialize arrays and object
JSONArray netwerkNodes = new JSONArray();
JSONArray netwerkLinks = new JSONArray();
JSONArray netwerks1 = new JSONArray();
JSONArray netwerks2 = new JSONArray();
JSONObject netwerkPerson = new JSONObject();
JSONObject netwerkLink = new JSONObject();
JSONObject netwerkN = new JSONObject();
JSONObject netwerkL = new JSONObject();
ArrayList<Network> clientNetworks = db.getNetworks(
client.getClient_id(), 0);
Het is alleen niet duidelijk wat bijvoorbeeld "netwerkN" en "netwerkL" nu betekenen.
Refactor dit naar duidelijke ENGELSE termen dus niet half NL half EN
Bouw een methode voor de volgende code :
// check for each member if it is equal to the id
for (Familymember fm : client.getMyFamilymembers()) {
if (id == fm.getMember_id()) {
familymember = fm;
break;
}
}
Deze code valt onder de Create methode die aangemaakt moet worden
In de OverviewController.java
in de methode
public JSONObject[] createJSONNetworks(Client client) throws JSONException {
wordt een hoop duplicate code gebruikt.
vanaf regel 144 zie je namelijk dezelfde structuur weer terugkomen.
Probeer de code op te delen in meerdere methodes zodat beide stukken in "createJSONNetworks" hier gebruik van kunnen maken
Schrijf een methode die de onderstaande code uitvoerd. Momenteel staat deze code onderaan de DoPost methode.
// update networks in the session
JSONObject[] networks;
try {
networks = OverviewController.getInstance().createJSONNetworks(client);
req.getSession().setAttribute("familyJSON", OverviewController.getInstance().refreshFamilymember(client));
req.getSession().setAttribute("nodesNetwork", networks[0]);
req.getSession().setAttribute("linksNetwork", networks[1]);
} catch (JSONException e) { }
De code in de methode doFilter in AdminFilter.js kan overzichtelijker worden gemaakt door de verschillende functies die worden uitgevoerd in deze functie om te zetten naar losse functies en aan te roepen vanuit doFilter.
LoginSupportServlet.java
Overal worden messages geset. Door hiervan constante te maken is het makkelijker voor de developer om deze aan te passen in 1x.
Werkzaamheden: overal constante maken waar dat mogelijk is.
this.setMessage("error", "Gebruikersnaam niet correct of niet gevonden in ons systeem.");
req.getRequestDispatcher(PAGE_FORGOT_PASSWORD).forward(req, resp);
}
} else {
this.setMessage("error", "Gebruikersnaam niet correct of niet gevonden in ons systeem.");
if (message.equals("")) { this.setMessage("success", "De gebruikersnaam is verzonden naar je mail."); req.getRequestDispatcher(PAGE_LOGIN).forward(req, resp); } else { this.setMessage("error", message); req.getRequestDispatcher(PAGE_FORGOT_USERNAME).forward(req, resp); } } else { this.setMessage("error", "Email niet correct of niet gevonden in ons systeem."); req.getRequestDispatcher(PAGE_FORGOT_USERNAME).forward(req, resp); } } else { this.setMessage("error", "Email niet correct of niet gevonden in ons systeem."); req.getRequestDispatcher(PAGE_FORGOT_USERNAME).forward(req, resp);
In de MailService.java staat de methode:
public boolean sendMail() {
Deze methode doet alleen naast het verzenden van de mail ook de complete opbouw.
Refactor deze methode zodat de naam niet suggereert dat het alleen verzendt.
Maar verdeel ook stukken van deze methode in kleinere methodes zodat de opbouw ergens anders geregeld wordt.
In de ClientServlet.java
Vanaf regel 51 wordt 2 keer gekeken of administrator is deze kan in een enkele if
// If currentUser is administrator refresh autocomplete that is required to choose socialworker
if (currentUser instanceof Administrator) {
req.getSession().setAttribute("users", OverviewController.getInstance().autoComplete(currentUser));
}
// Check if currentuser is administrator or socialworker to load correct page
if (currentUser instanceof Administrator) {
PAGE_CLIENT_OVERVIEW = "/administrator/" + PAGE_CLIENT_OVERVIEW;
PAGE_CLIENT_ADD_EDIT = "/administrator/" + PAGE_CLIENT_ADD_EDIT;
}
In de ClientServlet.java
Deze String cast is niet nodig regel 45.
option = (req.getParameter("option") != null) ? (String) req.getParameter("option") : "";
In OverviewController.java zit een switch statement. Probeer te kijken of je hier nog optimalisatie uit kan halen.
private JSONObject createLink(ArrayList<Result> myResults)
throws JSONException {
JSONObject link = new JSONObject();
for (Result r : myResults) {
switch (r.getMyAnswer().getAnswer_id()) {
case 1:
link.put("type", 1);
break;
case 2:
link.put("type", 2);
break;
case 3:
link.put("type", 3);
break;
case 4:
link.put("type", 4);
break;
case 5:
link.put("type", 5);
break;
case 6:
link.put("type", 6);
break;
case 7:
link.put("strength", 1);
break;
case 8:
link.put("strength", 2);
break;
case 9:
link.put("strength", 3);
break;
case 10:
link.put("strength", 4);
break;
case 11:
link.put("strength", 5);
break;
case 12:
link.put("distance", 5);
break;
case 13:
link.put("distance", 4);
break;
case 14:
link.put("distance", 3);
break;
case 15:
link.put("distance", 2);
break;
case 16:
link.put("distance", 1);
break;
}
}
return link;
}
ClientServlet.java
regel 113
message += "Zorgprofessional niet gevonden."; //e.printStackTrace();
Voordat de familymember in de database wordt opgeslagen moet het object eerst aangemaakt worden. Schrijf een aparte methode voor deze functionaliteiten.
// get all input
String dateOfBirthString = req.getParameter("dateofbirth");
Date dateOfBirth = null;
try {
java.util.Date date = sdf1.parse(dateOfBirthString);
dateOfBirth = new java.sql.Date(date.getTime());
} catch (ParseException e) {
dateOfBirth = null;
}
String postcode = req.getParameter("postcode");
String street = req.getParameter("street");
String houseNumber = req.getParameter("streetnumber");
String city = req.getParameter("city");
String nationality = req.getParameter("nationality");
String telephoneNumber = req.getParameter("phonenumber");
String mobilePhoneNumber = req.getParameter("mobile");
String email = req.getParameter("email");
// create familymember object
Familymember fm = new Familymember(forename, surname,
dateOfBirth, postcode, street, houseNumber, city,
nationality, telephoneNumber, mobilePhoneNumber, email);
De Code voor het updaten van een Familyobject is vrijwel hetzelfde als de Create code het enige verschil is dat de onderstaande code na het aanmaken van het FamilyMember object komt.
fm.setMember_id(id);
Deze code kan ondervangen worden in de methode waar het FamilMember object daadwerkelijk in de database gestopt wordt.
De volgende methode moet dus aangemaakt worden :
Op de MailService.java
Van de volgende strings (en integers) kunnen ook constanten van worden gemaakt
Zo kan een andere developer deze makkelijk onder elkaar aanpassen
prop.put("mail.smtp.from", "[email protected]");
prop.put("mail.smtp.host", "smtp.gmail.com");
prop.put("mail.smtp.port", 465);
prop.put("mail.smtp.auth", true);
prop.put("mail.smtp.ssl.enable", true);
LET OP dit gebeurt niet alleen bij het voorbeeld. Bijna elke string in deze klasse kan een constante worden!
LoginSupportServlet.java
Onnodige string cast regel 31
option = (req.getParameter("option") != null) ? (String) req.getParameter("option") : "";
Geldt zowel voor OverviewController.java als LoginController.java
Hier komt op verschillende plekke te korte variabelen namen voor. Zoals "db" voor "database" en "oc" voor "overviewController"
public OverviewController(DatabaseInterface db) {
this.db = db;
oc = this;
}
Graag dit hernoemen naar eenduidige, duidelijke termen waardoor ook mogelijk het commentaar verminderd kan worden
ClientServlet.java
regel 291
for (Client c : currentUser.getDbController().getAllClientsOfUser(u)) {
if (client.getClient_id() == c.getClient_id()) {
socialworkerClient = u;
break;
}
}
ClientServlet.java
Try en catch aanbevolen door Sonarlint regel 88
reqDisp.forward(req, resp);
In de ClientServlet.java
regel 76, gebruik Integer.ParseInt ipv valueOf
int clientID = Integer.valueOf((String) req.getParameter("currentID"));
ClientServlet.java
message = "Onverwachte fout opgetreden, client niet gevonden.";
ClientServlet.java
Integer.valueOf moet integer.parseint
message eventueel in constant
vanaf regel 154
int clientID = Integer.valueOf(req.getParameter("clientID"));
this.summary(clientID);
} catch (NumberFormatException e) {
message += "Client niet gevonden."; //e.printStackTrace();
LoginSupperServlet.java
Username meteen trimmen, regel 38
String username = req.getParameter("username");
if (username != null && !username.trim().equals("")) {
for (User u : db.getAllUsers()) {
if (u.getUsername().equals(username.trim())) {
user = u;
break;
}
}
LoginSupportServlet.java
vanaf regel 34 t/m regel 84 in aparte methode
if (option.equals("forgotPassword")) {
vanaf regel 84 t/m 120 in aparte methode
} else if (option.equals("forgotUsername")) {
LoginSupperServlet.java
DoPost()
Add the "@Override" annotation above this method signature
Schrijf een methode die de volgende code uitvoerd. Deze code is nu te vinden aan het einde van de FamilyServlet doPost gedaan worden.
try {
// refresh familymembers overview table by updating the JSON
req.getSession().setAttribute(
"familyJSON",
OverviewController.getInstance()
.refreshFamilymember(client));
} catch (JSONException e) {
req.setAttribute(
"message",
"Kan de gegevens van "
+ client.getForename()
+ " "
+ client.getSurname()
+ " niet goed ophalen, log opnieuw in en probeer het opnieuw.");
req.setAttribute("messageType", "error");
reqDisp = req
.getRequestDispatcher("/socialworker/startscreen_socialworker.jsp");
}
ClientServlet.java
regel 131 en 195
message += " Overzicht door een onbekende fout niet herlanden, overzicht is niet up-to-date met de database.";
ClientServlet.java
//e.printStackTrace();
De volgende regel code in de MailService.java bevat een html opmaak
msg.setContent("<html><head><style>body{width: 100%;height: auto;background-color: white;font-family: 'Arial';}.wrapper{width: 90%;min-width: 365px;margin: 10px auto;padding: 10px;background: #49c0f0; /* Old browsers */background: -moz-linear-gradient(top, #49c0f0 0%, #2cafe3 100%); /* FF3.6+ */background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,#49c0f0), color-stop(100%,#2cafe3)); /* Chrome,Safari4+ */background: -webkit-linear-gradient(top, #49c0f0 0%,#2cafe3 100%); /* Chrome10+,Safari5.1+ */background: -o-linear-gradient(top, #49c0f0 0%,#2cafe3 100%); /* Opera 11.10+ */background: -ms-linear-gradient(top, #49c0f0 0%,#2cafe3 100%); /* IE10+ */background: linear-gradient(to bottom, #49c0f0 0%,#2cafe3 100%); /* W3C */filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#49c0f0', endColorstr='#2cafe3',GradientType=0 ); /* IE6-9 */-webkit-border-radius: 5px;-moz-border-radius: 5px;border-radius: 5px;}.content{padding: 5px;background: rgb(249,252,247); /* Old browsers */background: -moz-linear-gradient(top, rgba(249,252,247,1) 0%, rgba(245,249,240,1) 100%); /* FF3.6+ */background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,rgba(249,252,247,1)), color-stop(100%,rgba(245,249,240,1))); /* Chrome,Safari4+ */background: -webkit-linear-gradient(top, rgba(249,252,247,1) 0%,rgba(245,249,240,1) 100%); /* Chrome10+,Safari5.1+ */background: -o-linear-gradient(top, rgba(249,252,247,1) 0%,rgba(245,249,240,1) 100%); /* Opera 11.10+ */background: -ms-linear-gradient(top, rgba(249,252,247,1) 0%,rgba(245,249,240,1) 100%); /* IE10+ */background: linear-gradient(to bottom, rgba(249,252,247,1) 0%,rgba(245,249,240,1) 100%); /* W3C */filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#f9fcf7', endColorstr='#f5f9f0',GradientType=0 ); /* IE6-9 */-webkit-border-radius: 5px;-moz-border-radius: 5px;border-radius: 5px;}.bold_text{font-weight: bold;}.custom_table{margin-left: 20px;padding: 2px;background-color: #079FD9;}.row {padding: 5px;background-color: #71D1F5;}.data {width: 150px;}</style></head><body><div class='wrapper'><div class='header'><h1>FamilyWeb</h1></div><div class='content'> " + message + "</div></body></html>","text/html" );
Dit zorgt nu voor een hele lange horizontale regel.
Misschien uit een file lezen? Anders een constante maar dan blijft hij wel lang
In bijna alle For loops wordt er gebruik gemaakt van éénletterige variable namen.
In de OverviewController.java
for (Contact c : n.getContacts()) {
Los dit op naar duidelijke en eenduidige namen.
Het onderstaande stukje code komt op een aantal plekken voor in de OverviewController.java
userJSON.put("forename", u.getForename());
userJSON.put("surname", u.getSurname());
userJSON.put("username", u.getUsername());
userJSON.put("dateOfBirth", u.getDateOfBirth());
userJSON.put("isActive", u.isActive());
userJSON.put("postcode", u.getPostcode());
userJSON.put("street", u.getStreet());
userJSON.put("houseNumber", u.getHouseNumber());
userJSON.put("city", u.getCity());
userJSON.put("nationality", u.getNationality());
userJSON.put("telephoneNumber", u.getTelephoneNumber());
userJSON.put("mobilePhoneNumber", u.getMobilePhoneNumber());
userJSON.put("email", u.getEmail());
userJSON.put("employeeNumber", u.getEmployeeNumber());
userJSON.put("user_id", u.getUser_id());
Je zou denken dat dit in een aparte methode geplaatst kan worden en op elke plek die methode aanroepen.
In de ClientServlet.java
Op regel 28 & 29 deze zouden constant gemaakt kunnen worden en alvast een waarde kunnen krijgen. Deze worden nu gevuld in de DoPost() regel 41 en 42 en aangepast vanaf regel 58.
PAGE_CLIENT_OVERVIEW = "client_overview.jsp"; PAGE_CLIENT_ADD_EDIT = "add_edit_client.jsp";
Het is misschien handiger om dan 4 constante te maken, 2 voor de administrator en 2 als het geen administrator is.
Zoiets:
AdministratorClientOverviewPage
AdministratorClientAddEdit
UserClientOverviewPage
UserClientAddEdit
In de Family Servlet staan nu nog de if en else statements met daarna code die uitgevoerd moet worden. Wat ik graag zou willen zien is dat de code na de if en else statements in aparte functies wordt gestopt.
Er zijn drie verschillende gorte methoden die uitgevoerd worden door de code, deze moeten dus sowiezo aangemaakt worden.
Overview/Summary :
// if status was overview
if (option.equals("summary")) {
}
Create :
// if create was selected else if (option.equals("create")) {
Update :
// if update was the status else if (option.equals("update")) {
Code is te vinden op :
https://github.com/wouterfennis/CodeRefactorFW/blob/master/FamilyWeb/src/main/java/servlets/FamilyWeb/FamilyMemberServlet.java
In de FamilyServlet code komt de volgende code twee keer voor vat deze samen in een methode die de string omzet naar een date object. Deze code komt ook voor in de createfamilymemberobject methode die aangemaakt moet worden.
String dateOfBirthString = req.getParameter("dateofbirth");
Date dateOfBirth = null;
try {
java.util.Date date = sdf1.parse(dateOfBirthString);
dateOfBirth = new java.sql.Date(date.getTime());
} catch (ParseException e) {
dateOfBirth = null;
}
Het moet dus het volgende worden :
String dateOfBirthString = req.getParameter("dateofbirth");
Date dateOfBirth = ConvertDateStringToDate(dateOfBirthString);
Op een aantal plekken binnen de OverviewController.java komen een aantal "Magic numbers".
Probeer deze in variable te stoppen (zo nodig een constante). Anders met een duidelijke commentaar regel aangeven waarom het getal daar staat.
clientNode.put("group", 0);
Voor zowel "LoginController.java" als de "OverviewController.java"
Door het hele bestand staat bijna bij elke regel commentaar. Soms is dit Javadoc maar je ziet vooral veel inline commentaar.
Probeer onnodig commentaar te verwijderen of te herschrijven
In ClientServlet.java
vanaf regel 65, controle welke optie zou in een aparte methode kunnen eventueel
// Check wich option is choosen
Er is een int in de OverviewController.java die vaak gebruikt wordt
Toch heet hij:
int i = 0;
Op regel 107
Ik denk dat een betere naam ook meer duidelijk brengt
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.