# Simple survey application

I've completed the application and it works against all tests I've thrown at it. Now, I'd like to see if there's a way I can optimise the application and lower the code footprint.

The application is intended to be used for collecting input and collating this into an email, to be used later.

All variables are pulled from a Properties file, just in-case you're wondering.

UserInput

    public class UserInput
{
public static InputStream file = null;
public static String name;
public static String fName;
public static String sName;
public static String ans1;
public static String ans2;
public static String ans3;
public static String ans4;
public static String ans5;

@SuppressWarnings({ "resource", "unused" })
public static void main(String[] args) throws NullPointerException
{
try
{
file = new FileInputStream("C:/Exercise 7 - Emails/configuration.properties");

Scanner nameVar1 = new Scanner(System.in).useDelimiter("\\n");
Scanner nameVar2 = new Scanner(System.in).useDelimiter("\\n");

Properties props = new Properties();

String recipient1 = props.getProperty("email1");
String recipient2 = props.getProperty("email2");
String recipient3 = props.getProperty("email3");

String host = props.getProperty("host");
String port = props.getProperty("port");
String from = props.getProperty("from");

String iterate= props.getProperty("noOfEmails");
int noOfEmails = Integer.parseInt(iterate);
String number = props.getProperty("noOfQuest");
int noOfQuest = Integer.parseInt(number);

String[] questionArr = new String[5];
questionArr[0] = props.getProperty("Q1");
questionArr[1] = props.getProperty("Q2");
questionArr[2] = props.getProperty("Q3");
questionArr[3] = props.getProperty("Q4");
questionArr[4] = props.getProperty("Q5");

//System.out.println("Recipient1: "+recipient1+" Recipient2: "+recipient2+" Recipient3: "+recipient3);

if(questionArr[0].isEmpty()||questionArr[1].isEmpty()||questionArr[2].isEmpty()||questionArr[3].isEmpty()||questionArr[4].isEmpty())
{
System.out.println("FILE_ERROR: Please ensure all question fields contain questions. These cannot be blank." + "\n" +
"File Location: C:\\Exercise 7 - Emails\\");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}

if(noOfQuest==0 || noOfQuest>5)
{
System.out.println("FILE_ERROR: Please ensure line \"noOfQuest\" in file is greater than 0, and 5 or less." + "\n" +
"File located: C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}

if(recipient1.isEmpty())
{
System.out.println("FILE_ERROR: Please populate field \"email1\" in configuration.properties");
try
{
}
catch(InterruptedException e)
{
}
System.exit(0);
}
if(recipient2.isEmpty())
{
System.out.println("FILE_ERROR: Please populate field \"email2\" in configuration.properties");
try
{
}
catch(InterruptedException e)
{
}
System.exit(0);
}
if(recipient3.isEmpty())
{
System.out.println("FILE_ERROR: Please populate field \"email3\" in configuration.properties");
try
{
}
catch(InterruptedException e)
{
}
System.exit(0);
}

if(noOfEmails==0)
{
System.out.println("FILE_ERROR: Please ensure line \"noOfEmails\" is between 1 & 3" +
" File Located: C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}
if(noOfEmails>3)
{
System.out.println("FILE_ERROR: Please ensure line \"noOfEmails\" is between 1 & 3" +
" File Located: C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}
if(host.isEmpty())
{
System.out.println("FILE_ERROR: Please ensure line \"host\" is a valid hostname." +
" File Located: C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}

if(port.isEmpty())
{
System.out.println("FILE_ERROR: Please ensure line \"port\" is a valid port." +
" File Located: C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}
if(from.isEmpty())
{
" File Located: C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch(InterruptedException t)
{
}
System.exit(0);
}

String prefinalMess = "Thank you for your time so far, this is the final question";
String finalMess = "Thank you for completing this Survey - your responses will be processed";

do
{
System.out.println("You will now be prompted to answer a number of questions." + "\n");
fName = nameVar1.next();
{
System.out.println("INPUT ERROR: Please do not begin your first name with a blank space");
continue;
}
else
{
{
System.out.println("INPUT ERROR: Please do not begin your first name with a number");
continue;
}
}
{
System.out.println("INPUT ERROR: Please ensure you do not leave first name blank");
continue;
}
break;
}
while(true);
do
{
sName = nameVar2.next();
{
System.out.println("INPUT ERROR: Please do not begin your surname with a blank space");
continue;
}
else
{
{
continue;
}
}
{
System.out.println("INPUT ERROR: Please ensure you do not leave surname name blank");
continue;
}
break;
}
while(true);

String userName = fName.trim() + " " + sName.trim();

Scanner ans = new Scanner(System.in).useDelimiter("\\n");

int iLoop = 0;

int iCount=0;

String[] ansArray = new String[5];

for(iLoop=0; iLoop<noOfQuest; iLoop++)
{
do
{
if(noOfQuest==1)
{
if(iCount==0)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(prefinalMess);
System.out.println(questionArr[0]);
ansArray[0]=ans.next();
{
System.out.println("Please ensure you enter a response");
continue;
}

}
}
if(noOfQuest==2)
{
if(iCount==0)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(questionArr[0]);
ansArray[0]=ans.next();
{
System.out.println("Please ensure that you enter a response for question 1");
continue;
}
iCount++;

}
if(iCount==1)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(prefinalMess);
System.out.println(questionArr[1]);
ansArray[1]=ans.next();
{
System.out.println("Please ensure that you enter a response for question 2");
continue;
}
iCount++;

}
}

if(noOfQuest==3)
{
if(iCount==0)
{
System.out.println("Count is on [" +iCount+"]");
System.out.println(questionArr[0]);
ansArray[0]=ans.next();
{
System.out.println("Please ensure that you enter a response for question 1");
continue;
}
iCount++;

}
if(iCount==1)
{
System.out.println(questionArr[1]);
ansArray[1]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 2");
continue;
}
iCount++;

}
if(iCount==2)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(prefinalMess);
System.out.println(questionArr[2]);
ansArray[2]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 3");
continue;
}
iCount++;

}
}
if(noOfQuest==4)
{
if(iCount==0)
{
System.out.println("Count is on [" +iCount+"]");
System.out.println(questionArr[0]);
ansArray[0]=ans.next();
{
System.out.println("Please ensure that you enter a response for question 1");
continue;
}
iCount++;

}
if(iCount==1)
{
System.out.println("Count is on [" +iCount+"]");
System.out.println(questionArr[1]);
ansArray[1]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 2");
continue;
}
iCount++;

}
if(iCount==2)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(questionArr[2]);
ansArray[2]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 3");
continue;
}
iCount++;

}
if(iCount==3)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(prefinalMess);
System.out.println(questionArr[3]);
ansArray[3]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 4");
continue;
}
iCount++;

}
}
if(noOfQuest==5)
{
if(iCount==0)
{
System.out.println("Count is on [" +iCount+"]");
System.out.println(questionArr[0]);
ansArray[0]=ans.next();
{
System.out.println("Please ensure that you enter a response for question 1");
continue;
}
iCount++;

}
if(iCount==1)
{
System.out.println("Count is on [" +iCount+"]");
System.out.println(questionArr[1]);
ansArray[1]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 2");
continue;
}
iCount++;

}
if(iCount==2)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(questionArr[2]);
ansArray[2]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 3");
continue;
}
iCount++;

}
if(iCount==3)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(questionArr[3]);
ansArray[3]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 4");
continue;
}
iCount++;

}
if(iCount==4)
{
System.out.println("Count is on ["+iCount+"]");
System.out.println(prefinalMess);
System.out.println(questionArr[4]);
ansArray[4]=ans.next();
{
System.out.println("Please ensure that you have entered a response for question 5");
continue;
}
}
iCount++;

}

break;
}
while(true);
}

System.out.println(finalMess);
EmailSend sendEmail = new EmailSend();
}
catch (IOException e)
{
+ "C:/Exercise 7 - Emails/configuration.properties");
try
{
}
catch (InterruptedException r)
{
}
System.exit(0);
}
catch(NullPointerException |NumberFormatException npe)
{
System.out.println("Please be advised: Properties file must contain the following fields:" + "\n" +
"Q1=, Q2=, Q=3, Q=4, Q=5, host=, port=, email1=, email2=, email3=, from=, noOfEmails=, noOfQuest." + "\n"
+ "Please ensure these fields are present, each on their own line with input immediately after \"=\".");
try
{
}
catch (InterruptedException e)
{
}
System.exit(0);
}
}

/*****************************************************\
|Variable: String fName                               |
|Function: Checks if the String is a space character  |
\*****************************************************/
{
if(fName.substring(0,1).equals(" "))
{
return true;
}
else
{
return false;
}
}
/*******************************************\
|Variable: String fName                     |
|Function: Checks if the String is numeric  |
\*******************************************/
{
if(Character.isDigit(fName.charAt(0)))
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(fName.trim().isEmpty())
{
return true;
}
return false;
}
/*******************************************************\
|Variable: String sName                               |
|Function: Checks if the String is a space character  |
\*****************************************************/
{
if(sName.substring(0,1).equals(" "))
{
return true;
}
else
{
return false;
}
}
/*******************************************\
|Variable: String sName                     |
|Function: Checks if the String is numeric  |
\*******************************************/
{
if(Character.isDigit(sName.charAt(0)))
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(sName.trim().isEmpty())
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(ansArray[0].trim().isEmpty())
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(ansArray[1].trim().isEmpty())
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(ansArray[2].trim().isEmpty())
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(ansArray[3].trim().isEmpty())
{
return true;
}
return false;
}
/*****************************************\
|Variable: String sName                   |
|Function: Checks if the String is blank  |
\*****************************************/
{
if(ansArray[4].trim().isEmpty())
{
return true;
}
return false;
}
}


EmailSend

public class EmailSend
{
static InputStream file = null;

{
try
{
Properties props = new Properties();
file = new FileInputStream("C:/Exercise 7 - Emails/configuration.properties");

String host = props.getProperty("host");
String port = props.getProperty("port");

String iterate= props.getProperty("noOfEmails");
int noOfEmails = Integer.parseInt(iterate);
String questions = props.getProperty("noOfQuest");
int noOfQuest = Integer.parseInt(questions);

String recipient1 = props.getProperty("email1");
String recipient2 = props.getProperty("email2");
String recipient3 = props.getProperty("email3");

String from = props.getProperty("from");
Calendar current = Calendar.getInstance();
int hour = current.get(Calendar.HOUR_OF_DAY);
int minute = current.get(Calendar.MINUTE);
int second = current.get(Calendar.SECOND);
int day = current.get(Calendar.DAY_OF_MONTH);
int month = current.get(Calendar.MONTH)+1;
int year = current.get(Calendar.YEAR);
String timeStamp = year+"/"+month+"/"+day+" "+hour+":"+ minute+":"+second;

Properties properties = System.getProperties();
Session session = Session.getDefaultInstance(properties);

MimeMessage message = new MimeMessage(session);
properties.setProperty("mail.smtp.host", host);
properties.setProperty("mail.smtp.port", port);
message.setSubject("Survey Results "+ "- "+timeStamp);

System.out.println("Port is currently [" + port + "]");

for(int i=0; i<noOfEmails; i++)
{
//System.out.println("In Main Loop [" + i + "]");
if(noOfEmails==1)
{
for(int j=0; j<noOfQuest; j++)
{
//System.out.println("In Loop[" + j + "] : recipient1: "+ recipient1);

if(i==0)
{
}
if(noOfQuest==1)
{
}
if(noOfQuest==2)
{
"Response 1: " + answers[0] + "\n" +
}
if(noOfQuest==3)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
}
if(noOfQuest==4)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
"Response 3: " + answers[2] + "\n" +
}
if(noOfQuest==5)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
"Response 3: " + answers[2] + "\n" +
"Response 4: " + answers[3] + "\n" +
}
}
}

if(noOfEmails==2)
{
for(int k=0; k<noOfQuest; k++)
{
//System.out.println("In Loop[" + k + "] : recipient2: "+ recipient2);

if(i==0)
{
}
if(i==1)
{
}
if(noOfQuest==1)
{
}
if(noOfQuest==2)
{
"Response 1: " + answers[0] + "\n" +
}
if(noOfQuest==3)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
}
if(noOfQuest==4)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
"Response 3: " + answers[2] + "\n" +
}
if(noOfQuest==5)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
"Response 3: " + answers[2] + "\n" +
"Response 4: " + answers[3] + "\n" +
}
}
}

if(noOfEmails==3)
{
for(int l=0; l<noOfQuest; l++)
{
//System.out.println("In Loop[" + l + "] : recipient3: "+ recipient3);

if(i==0)
{
System.out.println(recipient1);
}
if(i==1)
{
System.out.println(recipient2);
}
if(i==2)
{
System.out.println(recipient3);
}

if(noOfQuest==1)
{
}
if(noOfQuest==2)
{
"Response 1: " + answers[0] + "\n" +
}
if(noOfQuest==3)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
}
if(noOfQuest==4)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
"Response 3: " + answers[2] + "\n" +
}
if(noOfQuest==5)
{
"Response 1: " + answers[0] + "\n" +
"Response 2: " + answers[1] + "\n" +
"Response 3: " + answers[2] + "\n" +
"Response 4: " + answers[3] + "\n" +
}
}
}
Transport.send(message);
//System.out.println("Sent: " + message);
}
}
catch(MessagingException f)
{
System.out.println("FILE_ERROR: Please ensure hostname is correct in file.");
try
{
}
catch (InterruptedException e)
{
}
System.exit(0);
}
catch(IOException e)
{
try
{
}
catch (InterruptedException r)
{
}
System.exit(0);
}
}
}


For now, just a couple of points:

## Long class and method

Your main method is ~500 lines long. That is definitely too much.

Extract Duplicate Code

The first thing you should do is extract duplicate code to functions. For example, you have this pattern multiple times:

    if(something)
{
System.out.println("some message");
try
{
}
catch(InterruptedException e)
{
}
System.exit(0);
}


Often, the boolean check is, if a string is empty. So your method could look like this:

    private void checkAndHandleEmtpyString(String string, Long sleep, String message) {
if (string.isEmpty()) {
System.out.println(message);
try {
} catch (InterruptedException e) {
}
System.exit(0);
}
}


More than One -> List

Whenever you have more than one of something, it is generally a good idea to put it in a list, and then handle the general case of more than one, instead of handling cases one, two, ...

So for your case, it might look something like this:

private StringBuilder assembleMessage(String username, String[] answers) {
StringBuilder messageText = new StringBuilder();
messageText.append("\n");
for (int j = 0; j < noOfQuest; j++) {
messageText.append("Response ");
messageText.append(j);
messageText.append(":");
messageText.append("\n");
}
return messageText;
}


And then use it like this:

message.setText(assembleMessage(username, answers));


This alone will probably get rid of about 120 lines of code in your EmailSend class :)

If you then also store your recipients in an array, all your nested for-if-for-ifs (and thus most of the EmailSend class) could probably look like this:

        // load properties

for (int i = 0; i < recipientCount; i++) {
}

Transport.send(message);


Split code up in functions

Splitting your code in different functions doesn't necessarily reduce the amount of code, but it will increase readability. You could have functions such as loadProperties, checkInput, etc.

Main

Your main method should really only do one thing: start the program. Everything else should be happening in properly named methods.

## OOP

I think many of the problems in your code - such as the length - stem from the fact that you are not using OOP.

This also means that your code will be very hard to adapt later on when requirements change. For example, what if there can be more than 5 question/answers? Then you have to change your code in very very many places.

Or what if there are more than 3 recipients?

So you might want to add classes such as Email, EmailBody (or EmailMessage), Answer, etc. And also generally but things in lists (see More than One -> List).

## Misc

• All your fields are public, this doesn't really seem necessary.
• use better naming: what does s/f stand for in sName/fName? What does iCount count? What is an ans? Questions like these should not come up with good naming.
• IsLeadingCharNumfName(fName)==true can be written as IsLeadingCharNumfName(fName)
• if(cond) { return true; } else { return false; } can be written as return cond;
• In Java, it is customary to put the opening { on the same line.
• use more spaces (all formatting issues can easily be fixed with any IDE).
• using System.exit is generally not recommended.
• too many continue and break statements can hint to bad code design.
• try-catch blocks should be around as small a scope as possible, that way it's easier to see what might actually fail.
• use else-if instead of multiple ifs for clarity.
• do not hardcode file paths inside methods. At a minimum, I would put them in a final class level field, so they are easier to find.
• Well explained. Only thing I disagree is putting the opening { on the same line. Putting it on a new line makes it much more easier to follow the structure and to find the appropriate } (especially in more complex nested blocks). But I think that's just a matter of taste Feb 11 '15 at 14:28
• Thanks for the suggestion, I've implemented your answer, but as a matter of curiosity, why is the use of System.exit(0) not recommended? for this case when the error is encountered the application should close. Feb 12 '15 at 12:41
• @BenignReaver because then you are stuck with it. maybe in the future, you don't want to exit, but get more user input, or do something completely different. of you handle it with exceptions, this should be easily changed. But if your code just exists in multiple places, it will become a lot harder. It's also hard to follow the controll flow of a program, if it exits in multiple places.
– tim
Feb 12 '15 at 12:56
• @tim Thanks for the clarification. I've been reading up on this method, and for the current state it is acceptable, as the file needs to be updated whenever this method is encountered. In instances where user input needs to be re-obtained, I use boolean Methods within a do-while loop, where the control state is while(true), surrounded by a try-cat/nested/multi-catch Feb 12 '15 at 13:05
1. Style:

Use spacing and indentation properly. You indentation is almost good, except for the first two lines (and those following from that error).

Also, use spaces and new lines to make code more readable. Refer to the Java Google Style-Guide for good practices.

if(questionArr[0].isEmpty()||questionArr[1].isEmpty()||questionArr[2].isEmpty()||questionArr[3].isEmpty()||questionArr[4].isEmpty()


write

if (questionArr[0].isEmpty() || questionArr[1].isEmpty()
|| questionArr[2].isEmpty() || questionArr[3].isEmpty()
|| questionArr[4].isEmpty())


A good IDE such as Eclipse will do this for you automatically.

2. Avoid using suppressWarnings. Refer to Effective Java by Joshua Bloch for more detail here. But in general, if you use suppressWarnings, use it for as little scope as possible. I.e. instead of suppressing warnings for the whole main method use it only for:

@SuppressWarnings("unused");
EmailSend sendEmail = new EmailSend();


Also, add a comment for the reason you are suppressing warnings here and not getting rid of them. Most of the time, for new code you can avoid the warning in the first place. In this case, you can simply get rid of the unused code.

3. Don't declare variables before you really need them. By declaring them earlier, you are unnecessarily increasing scope and increasing the chance of errors.

For example:

int iLoop = 0;

int iCount = 0;

String[] ansArray = new String[5];

for (iLoop = 0; iLoop < noOfQuest; iLoop++) {...}


There is no reason to declare iLoop outside the for loop. Simply do:

for (int iLoop = 0; iLoop < noOfQuest; iLoop++) {...}


You can immediately see how the scope of iLoop has been reduced this way and how readability is improved as well.