-
Notifications
You must be signed in to change notification settings - Fork 0
2. Functions
GOOD: - Maximum 100 lines
BETTER:- should fit in one screen
BEST:- not more than 20 lines
GOOD:
if(...) doSomething();
else doSomethingElse();Topdown-Rule: Code should be readable like a story from Top to Bottom.
Stepdown-Rule: Behind every function, there are other functions in the next lower abstraction level.
BAD:
public createCars(){
createOpel();
Audi audi = new Audi();
}GOOD:
public createCars(){
createOpel();
createAudi();
}BAD:
public Money calculatePay(Employee e) throws InvalidEmployeeType {
switch (e.type) {
case COMMISSIONED:
return calculateCommissionedPay(e);
case HOURLY:
return calculateHourlyPay(e);
case SALARIED:
return calculateSalariedPay(e);
default:
throw new InvalidEmployeeType(e.type);
}
}Reasons:
- Big Function. Gets even bigger, if new types are added
- Does more than one thing.
- Violates against SRP. (More than one reason for a change.)
- Violates against Open Closed Princip (OCP). (Must be changed, if new types are added)
- There could be unlimited functions with same structure like:
isPayday(Employee e, Date date)SOLUTION: put switch statements inside Abstract Factory
public abstract class Employee {
public abstract boolean isPayday();
public abstract Money calculatePay();
public abstract void deliverPay(Money pay);
}
-- -- -- -- -- -- -- -- -
public interface EmployeeFactory {
public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType;
}
-- -- -- -- -- -- -- -- -
public class EmployeeFactoryImpl implements EmployeeFactory {
public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType {
switch (r.type) {
case COMMISSIONED:
return new CommissionedEmployee(r);
case HOURLY:
return new HourlyEmployee(r);
case SALARIED:
return new SalariedEmploye(r);
default:
throw new InvalidEmployeeType(r.type);
}
}
}- name should describe the action of the function
- long names are better than long comments or short, but not very meaningful names
- be consistent in your names (use same phrases, nouns, verbs
- the less arguments, the better (easier to understand and test)
- BEST: 0 arguments, MAXIMUM: 3 arguments
- use Input Arguments instead of Output Arguments
case 1 - ask a question about the argument
boolean fileExists("MyFile")case 2 - transform the argument and return it
InputStream fileOpen("MyFile")case 3 - Function call is an event; use the argument to alter the state of the system; no output argument
void passwordAttemptsFailedNtimes(int attempts)if possible, use only those 3 cases, when using Monadic Functions
- hard to understand,
- function does more than one thing (2 cases -> true and false)
- in some cases it's the right solution, like
Point p = new Point(0,0)- but other than that, it should be avoided
- AVOID! But there are some exceptions like:
assertEquals(1.0, amount, .001)If possible, arguments should be wrapped into a class, like:
Circle makeCircle(double x, double y, double radius); //before
Circle makeCircle(Point center, double radius); //after and bettermonad functions:use nice verb/noun pair for function and argument:
writeField(name);here we know, that 'name' is a field, that is being written
other functions: encode the argument names into the function names:
BAD:
assertEquals(expected, actual); GOOD:
assertExpectedEqualsActual(expected, actual); This way we don't have to remember the order.
BAD: "Your function promises to do one thing, but it also does other hidden things" (S.44) Example:
public boolean checkPassword(String userName, String password){
...// logic for checking password
Session.initialize();
}The function name does not imply that it initializes a session!
GOOD:
checkPasswordAndInitializeSession(userName, password);BETTER: Function should only do ONE thing
Generally you should not use Output Arguments! BAD:
// this function appends a footer to the argument (report)
public void appendFooter(StringBuffer report){...}GOOD:
it's the task of this to work as an 'output argument'
report.appendFooter();BAD: This function below sets an attribute with a value (command) and returns true (query), if it was successful and false, if attribute doesn't exist.
public boolean set(String attribute, String value);This will lead to:
if(set("username","unclebob"))...GOOD:
if(attributeExists("username")){ // query
setAttribute("username", "unclebob"); //instruction
}command-functions returning Error Codes are violating againts command-query-separation and will lead to deeply nested if statements
BAD:
if(deletePage(page) == E_OK){
if(registry.deleteReference(page.name) == E_OK){
...Use try/catch to separate error processing code from 'normal processing' code. Also the blocks should be oneliner(function call).
GOOD:
// task of this function is error processing **only**, therefore it should only contain try and catch/finally block
public void delete(Page page){
try {
deletePageAndAllReferences(page);
}
catch (Exception e){
logError(e);
}
}Avoid using Error Codes, because they need an enum or class where they are defined. Problem: other classes are dependent of this enum. if enum is changed -> recompiling, redeployment
public enum Error {
OK,
INVALID,
NO_SUCH,
}Use Exceptions instead, because new Exceptions are subclasses of Exception-> no recompiling, redeployment
-> Open-Closed-Principle(OCP)
This one is obvious. Duplicate code will make your code unnecessarily bigger and the effort is great, when you have to change something. "Duplication may be the root of all evil in software." S.48
For Bigger Functions, go for following rule:
"Dijkstra said that every function, and every block within a function, should have one entry and one exit." S.48
-> only 1 return statement in a function
-> no break or continue statements in a loop
-> no goto
Just start to write your function. It doesn't have to be clean right from the start.
Refactor and make your code clean afterwards!
"Master programmers think of systems as stories to be told rather than programs to be written." (S.50)