patternjavaMinor
Activity and Relaxation flow
Viewed 0 times
andactivityrelaxationflow
Problem
I am a total newbie when it comes to coding. I have been plugging away at getting my mind around Java for about a week.
Background
I have been sick for a while with symptoms that resemble some form of chronic fatigue, and my doctor handed me a flow chart that tells me how much activity I should do before taking an equal sized break, based on how I feel.
One look at the flow chart and I realized that she could have just as well handed me a flowchart of a program heh. So I decided to make my first little program in Java.
The code its self was done in a couple of hours (I am a bit of a newbie, remember), but for about a week now, I have spent time learning and improving it when I learn something new and relevant to the project.
My goal here:
I want to learn to code and follow conventions correctly. So I dare not write another program without peer review first, in case I'm doing things wrong or inefficient. I would appreciate any and all comments that can help me improve or encourage me.
Disclaimer stuff:
Other than that, I hope you can dissect this thing and offer some good corrections or alternative methods so I can start coding again! :)
```
package activity;
import java.util.Scanner;
// @author Jack
public class Activity {
//x; = time in minutes.
static int x;
//Flag to stop the loop which encloses the following indented.
static boolean timeInMinutesGiven = false;
static boolean validReply = false;
static String timeRealisticIs;
static boolean userAcceptsRejectsInput = false;
static String dayIs;
static boolean goodBa
Background
I have been sick for a while with symptoms that resemble some form of chronic fatigue, and my doctor handed me a flow chart that tells me how much activity I should do before taking an equal sized break, based on how I feel.
One look at the flow chart and I realized that she could have just as well handed me a flowchart of a program heh. So I decided to make my first little program in Java.
The code its self was done in a couple of hours (I am a bit of a newbie, remember), but for about a week now, I have spent time learning and improving it when I learn something new and relevant to the project.
My goal here:
I want to learn to code and follow conventions correctly. So I dare not write another program without peer review first, in case I'm doing things wrong or inefficient. I would appreciate any and all comments that can help me improve or encourage me.
Disclaimer stuff:
- The time in minutes is shown as
xsimply because that's how it was on the flow chart. Though no, "x" isn't super 'readable'. :)
- The last
StringBuilder atthe bottom should probably be turned into aSystem.out.printfso that I can control those decimal points (I just ran the program and input 32 minutes and seen all those digits for the first time when the build was finished.. heh).
Other than that, I hope you can dissect this thing and offer some good corrections or alternative methods so I can start coding again! :)
```
package activity;
import java.util.Scanner;
// @author Jack
public class Activity {
//x; = time in minutes.
static int x;
//Flag to stop the loop which encloses the following indented.
static boolean timeInMinutesGiven = false;
static boolean validReply = false;
static String timeRealisticIs;
static boolean userAcceptsRejectsInput = false;
static String dayIs;
static boolean goodBa
Solution
Documentation
Documentation is overestimated. The only truth lies in the executable parts of your source code. The way code emerges the same way documention has to emerge and that is an experienced problem. Often these elements diverge when development goes on so documentation begins to lie about the things happening in the executable parts so it confuses more than it helps. Documentation often becomes an alibi for bad code.
My suggestion is as soon as you are dealing with interfaces you should learn how to write JavaDoc for central interfaces that define the development responsibility borders or system borders. So source code should consequently be documented at those borders.
Sometimes it is necessary to give an inline hint what is the intention. But most of the time your code should express exactly the requirement so no further documentation is neccessary.
But why I am talking so much? It is because you should not create comments like "This loop prompts user to give a value for 'x', time in minutes" if the following loop prompts user to give a value for 'x', time in minutes.
Naming
We have several mechanisms to make complex things more ascertainable without falling into a delusion of documentation. One very important thing is "Naming".
From some occuring names you are not able to evaluate a meaning. If you translate "x" for the user during output to "time In minutes" why not simply name the variable "timeInMinutes".
Another problem is the abstraction level of names. Often abstraction levels are mixed. That relates to control flow and names as well. Your "validReply" can be evaluate to a lot of meanings as reply is not very concrete. But you are handling a concrete case: a (maybe very optimistic) validation if a "time in minutes" was successfully entered: "hasGotValidTimeInMinutes" for example would be a variable name of the current abstraction level.
Modularization
To make a developers life easier you should not flood your brain with information. Keep your stack "small". Your main-method is NOT small. Asking for "timeInMinutes" is one part. Asking for "Are you sure?" another. Those things can be viewed separately with "timeInMinutes" as the only connection to "Are you sure?"
Modularization can take place variously. In your case I would start to "extract" a method. This is a typical IDE supported operation but you also can do it by hand. You should watch a video on youtube how to do this for the IDE you use. For your "timeInMinutes"-loop where you get the time in minutes you should mark the whole loop and perform an "extract method" refactoring operation either with the help of the IDE or by hand. The name of the new method should be something like "getTimeInMinutes".
You should slice the main-method into several other methods each method doing on well-defined thing. What we are doing here is applying the so called single responsibility principle (SRP) in a very basic way. The main task in SRP is to achieve a 1 to 1 relationship from a code fragment to a responsibility. Things that belong together should be together. Things that do not belong together should be separated.
You should continue to extract methods and separate the responsibilities.
Method local variables
Currently you are using the variable "validReply" only within the new method "getTimeInMinutes". So why removing it from the global scope and declaring it locally in the method.
Why should we do that? Currently you know that the variable "validReply" is only used by this code fragment. But other developers hav to evaluate that Their mind have to process much more information to identify this relationship between the variable and the algorithm. Their mind has to hide other global variables and consider them as not relevant.
In those small scenarios it is less a problem. But in professional software development with thousands of classes maximal neccessary scope of ANYTHING is the Holy Grail. So put your variables in the scope they are needed but keep them as hidden as possible.
The "validReply" variable should be declared within the new extracted method.
Encapsulation
Here we mean that things have an inner state and an outer state and the inner state is totally hidden to us and our efforts to directly modify it.
If we take the new extracted method as an example. It is currently not well encapsulated. The variable "x" or now "timeInMinutes" can be easily modifed by other algorithms which is called "side effect".
We should introduce a variable with the same name (timeInMinutes) in the new method "getTimeInMinutes". Furthermore we will introduce a return value and assign the returned value to the gobal scope variable "timeInMinutes". We now successfully improved the encapsulation of the method. It is not perfect but it will do the job.
Now I want to show the final result of our refactorings:
```
public class Activity {
...
static int timeInMinutes;
...
public static void main(String[] args) {
..
Documentation is overestimated. The only truth lies in the executable parts of your source code. The way code emerges the same way documention has to emerge and that is an experienced problem. Often these elements diverge when development goes on so documentation begins to lie about the things happening in the executable parts so it confuses more than it helps. Documentation often becomes an alibi for bad code.
My suggestion is as soon as you are dealing with interfaces you should learn how to write JavaDoc for central interfaces that define the development responsibility borders or system borders. So source code should consequently be documented at those borders.
Sometimes it is necessary to give an inline hint what is the intention. But most of the time your code should express exactly the requirement so no further documentation is neccessary.
But why I am talking so much? It is because you should not create comments like "This loop prompts user to give a value for 'x', time in minutes" if the following loop prompts user to give a value for 'x', time in minutes.
Naming
We have several mechanisms to make complex things more ascertainable without falling into a delusion of documentation. One very important thing is "Naming".
From some occuring names you are not able to evaluate a meaning. If you translate "x" for the user during output to "time In minutes" why not simply name the variable "timeInMinutes".
Another problem is the abstraction level of names. Often abstraction levels are mixed. That relates to control flow and names as well. Your "validReply" can be evaluate to a lot of meanings as reply is not very concrete. But you are handling a concrete case: a (maybe very optimistic) validation if a "time in minutes" was successfully entered: "hasGotValidTimeInMinutes" for example would be a variable name of the current abstraction level.
Modularization
To make a developers life easier you should not flood your brain with information. Keep your stack "small". Your main-method is NOT small. Asking for "timeInMinutes" is one part. Asking for "Are you sure?" another. Those things can be viewed separately with "timeInMinutes" as the only connection to "Are you sure?"
Modularization can take place variously. In your case I would start to "extract" a method. This is a typical IDE supported operation but you also can do it by hand. You should watch a video on youtube how to do this for the IDE you use. For your "timeInMinutes"-loop where you get the time in minutes you should mark the whole loop and perform an "extract method" refactoring operation either with the help of the IDE or by hand. The name of the new method should be something like "getTimeInMinutes".
You should slice the main-method into several other methods each method doing on well-defined thing. What we are doing here is applying the so called single responsibility principle (SRP) in a very basic way. The main task in SRP is to achieve a 1 to 1 relationship from a code fragment to a responsibility. Things that belong together should be together. Things that do not belong together should be separated.
You should continue to extract methods and separate the responsibilities.
Method local variables
Currently you are using the variable "validReply" only within the new method "getTimeInMinutes". So why removing it from the global scope and declaring it locally in the method.
Why should we do that? Currently you know that the variable "validReply" is only used by this code fragment. But other developers hav to evaluate that Their mind have to process much more information to identify this relationship between the variable and the algorithm. Their mind has to hide other global variables and consider them as not relevant.
In those small scenarios it is less a problem. But in professional software development with thousands of classes maximal neccessary scope of ANYTHING is the Holy Grail. So put your variables in the scope they are needed but keep them as hidden as possible.
The "validReply" variable should be declared within the new extracted method.
Encapsulation
Here we mean that things have an inner state and an outer state and the inner state is totally hidden to us and our efforts to directly modify it.
If we take the new extracted method as an example. It is currently not well encapsulated. The variable "x" or now "timeInMinutes" can be easily modifed by other algorithms which is called "side effect".
We should introduce a variable with the same name (timeInMinutes) in the new method "getTimeInMinutes". Furthermore we will introduce a return value and assign the returned value to the gobal scope variable "timeInMinutes". We now successfully improved the encapsulation of the method. It is not perfect but it will do the job.
Now I want to show the final result of our refactorings:
```
public class Activity {
...
static int timeInMinutes;
...
public static void main(String[] args) {
..
Code Snippets
public class Activity {
...
static int timeInMinutes;
...
public static void main(String[] args) {
...
while (!timeInMinutesGiven) {
...
timeInMinutes = getTimeInMinutes();
...
}
...
}
private static int getTimeInMinutes() {
int timeInMinutes = 0;
boolean hasGotValidTimeInMinutes;
do {
System.out.print("How long (in minutes) can you be active " + "today before symptoms start to appear?: ");
try {
Scanner input = new Scanner(System.in);
timeInMinutes = input.nextInt();
hasGotValidTimeInMinutes = true;
} catch (Exception e) {
System.out.println("ERROR: A whole number was expected.");
hasGotValidTimeInMinutes = false;
}
} while (!hasGotValidTimeInMinutes);
return timeInMinutes;
}
}Context
StackExchange Code Review Q#155542, answer score: 3
Revisions (0)
No revisions yet.