wouterfennis / coderefactorfw Goto Github PK

View Code? Open in Web Editor NEW

0.0 0.0 1.0 4.02 MB

For ASP Assignment

Java 57.65% HTML 21.79% JavaScript 10.77% CSS 9.79%

coderefactorfw's People

Contributors

Watchers

Forkers

michielio

coderefactorfw's Issues

HTML string verkleinen

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

Twee if statements kan naar een enkele

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;
        }

Een INT hernoemen

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

constante maken regel 208

ClientServlet.java

message = "Onverwachte fout opgetreden, client niet gevonden.";

Overal constante maken waar dat mogelijk is

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);

Onnodige string cast regel 31

LoginSupportServlet.java
Onnodige string cast regel 31

option = (req.getParameter("option") != null) ? (String) req.getParameter("option") : "";

Integer parse en onnodige comment

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();

constante maken

ClientServlet.java

regel 131 en 195
message += " Overzicht door een onbekende fout niet herlanden, overzicht is niet up-to-date met de database.";

Aparte methode loginsupportservlet

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")) {

Hernoemen van te korte variabelen

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

Het vullen van JSON met User data niet steeds opnieuw programmeren

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.

Methode naam veranderen en nieuwe methodes maken

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.

Opdelen van één methode naar meerdere methodes

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

Username meteen trimmen

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;
                    }
                }

Refresh familymember overview table by updating JSON, FamilyServlet.

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");
        }

DoPost mist override notatie volgens Sonarlint

LoginSupperServlet.java

DoPost()
Add the "@Override" annotation above this method signature

Constante maken en onnodige comment

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();

Meteen trimmen email

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;
                    }
                }

Insert Familymember in db. FamilyServlet.

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));

Verhelpen van "Magic numbers" in de code

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);

Verwijder onnodige commentaar regels

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

Eventueel in methode

ClientServlet.java

regel 291

                for (Client c : currentUser.getDbController().getAllClientsOfUser(u)) {
                    if (client.getClient_id() == c.getClient_id()) {
                        socialworkerClient = u;
                        break;
                    }
                }

Onnodige comments verwijderen overal

ClientServlet.java
//e.printStackTrace();

Integer cast aanpassen

In de ClientServlet.java

regel 76, gebruik Integer.ParseInt ipv valueOf
int clientID = Integer.valueOf((String) req.getParameter("currentID"));

FamilyServlet Overzicht Creeeren

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

Create Or Change Family Member Object

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 :

Een voor CreateFamilyMemberObject, geeft een FamilyMember object terug.

Try en catch bij foward

ClientServlet.java

Try en catch aanbevolen door Sonarlint regel 88

reqDisp.forward(req, resp);

Return waarden juist benoemen

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

FamilyServlet Check if familymemberId is in the current session

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

Update FamilyMember FamilyServlet

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);

Update networks in the session, FamilyMemberServlet.

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) { }

Extracten naar nieuwe methode

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.

Onnodige comment verwijderen

ClientServlet.java

regel 113
message += "Zorgprofessional niet gevonden."; //e.printStackTrace();

Constante variabelen maken

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

Cast naar String onnodig

In de ClientServlet.java

Deze String cast is niet nodig regel 45.
option = (req.getParameter("option") != null) ? (String) req.getParameter("option") : "";

For loop met te kleine variabelen

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.

Eventueel in aparte methode

In ClientServlet.java

vanaf regel 65, controle welke optie zou in een aparte methode kunnen eventueel
// Check wich option is choosen

Check Username and Password Null

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");
}

Refactor het SWITCH statement

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;
    }

Onduidelijke variable namen herschrijven

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

AdminFilter Overzicht Creeeren

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.

missende Override notatie

ClientServlet.java

de doPost methode

Add the "@Override" annotation above this method signature

Aanmaken van constanten

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!

FamilyServlet convert String date to date object

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);

wouterfennis / coderefactorfw Goto Github PK

coderefactorfw's People

Contributors

Watchers

Forkers

coderefactorfw's Issues

Recommend Projects

Recommend Topics

Recommend Org