Introduce questions to make them realise that what they are doing is wrong. For example, ask these sort of questions:
Why did you decide to make that a global variable?
Why did you give it that name?
That’s interesting. I usually do mine this way because [Insert reason why you are better]
Does that way work? I usually [Insert how you would make them look silly]
I think the ideal way of going about this is subtly asking them why they code a certain way. You may find that they believe that there are benefits to other methods. Unless I knew the reason for their coding style was due to misinformation I would never judge my way as better without good reason. The best way to go about this is to just ask them why they chose that way; be sure to sound interested in their reasoning, because that is what you need to attack, not their ability.
A coding standard will definitely help, but if it were the answer to every software project then we’d all be sipping cocktails on our private islands in paradise. In reality, we’re all prone to problems and software projects still have a low success rate. I think the problem would mostly stem from individual ability rather than a problem with convention, which is why I’d suggest working through the problems as a group when a problem rears its ugly head.
Most importantly, do NOT immediately assume that your way is better. In reality, it probably is, but we’re dealing with another person’s opinion and to them there is only one solution. Never say that your way is the better way of doing it unless you want them to see you as a smug loser.
The goal is not for you to teach your team how to code better. It’s to establish a culture of learning in your team. Where each person looks to the others for help in becoming a better programmer.
Globals: Do you think we’ll ever want to have more than one of these? Do you think we will want to control access to this?
Mutable state: Do you think we’ll want to manipulate this from another thread?
long functions: My brain isn’t big enough to hold all of this at once. How can we make smaller pieces that I can handle?
bad names: I get confused easily enough when reading clear code; when names are misleading, there’s no hope for me.
If the coder feel attacked -> Game Over: You (both) Lose
In conclusion, monolithic functions can have far-reaching consequences and are often a symptom of major design deficiencies. Whenever I encounter code that is an absolute joy to read, it’s elegance is immediately apparent. And guess what: the functions are often very short in length.
The ideal number of arguments for a function is zero (niladic). Next comes one (monadic) followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.
Imagine the difficulty of writing all the test cases to ensure that all various combinations of arguments work properly.
If more, consider creating a factory
public class Customer {
private final String firstName;
private final String surname;
private final String ssn;
Customer(CustomerBuilder builder) {
if (builder.firstName == null) throw new NullPointerException("firstName");
if (builder.surname == null) throw new NullPointerException("surname");
if (builder.ssn == null) throw new NullPointerException("ssn");
this.firstName = builder.firstName;
this.surname = builder.surname;
this.ssn = builder.ssn;
}
public String getFirstName() { return firstName; }
public String getSurname() { return surname; }
public String getSsn() { return ssn; }
}
public class Client {
public void doSomething() {
Customer customer = customer()
.withSurname("Smith")
.withFirstName("Fred")
.withSsn("123XS1")
.build();
}
}Setting unused pointers to NULL is a defensive style, protecting against dangling pointer bugs. If a dangling pointer is accessed after it is freed, you may read or overwrite random memory. If a null pointer is accessed, you get an immediate crash on most systems, telling you right away what the error is.
For local variables, it may be a little bit pointless if it is “obvious” that the pointer isn’t accessed anymore after being freed, so this style is more appropriate for member data and global variables. Even for local variables, it may be a good approach if the function continues after the memory is released.
To complete the style, you should also initialize pointers to NULL before they get assigned a true pointer value.
break is allowed only at top of loop (clause)Probably the most documented case is the guard clause, also known as assert or precondition. The idea is that when you have something to assert in the beginning of a method — do this using a fast return.
public void executeTimer( String timerId ) {
logger.debug( "Executing timer with ID {}", timerId ); TimerEntity timerEntity = timerRepository.find( timerId ); logger.debug( "Found TimerEntity {} for timer ID {}", timerEntity, timerId ); if( timerEntity == null )
return; Timer timer = Timer.fromEntity( timerEntity );
timersInvoker.execute( timer );
}public void executeTimer( String timerId ) {
logger.debug( "Executing timer with ID {}", timerId ); TimerEntity timerEntity = timerRepository.find( timerId ); logger.debug( "Found TimerEntity {} for timer ID {}", timerEntity, timerId ); executeTimer( timerEntity );
}
private void executeTimer( TimerEntity timerEntity ) {
if( timerEntity == null )
return; Timer timer = Timer.fromEntity( timerEntity );
timersInvoker.execute( timer );
}public void example() {
FileUtils.createFile("name.txt", "file contents", false);
FileUtils.createFile("name_temp.txt", "file contents", true);
}
public class FileUtils {
public static void createFile(String name, String contents, boolean temporary) {
if(temporary) {
// save temp file
} else {
// save permanent file
}
}
}public void example() {
FileUtils.createFile("name.txt", "file contents");
FileUtils.createTemporaryFile("name_temp.txt", "file contents");
}
public class FileUtils {
public static void createFile(String name, String contents) {
// save permanent file
}
public static void createTemporaryFile(String name, String contents) {
// save temp file
}
}public class Bird {
private enum Species {
EUROPEAN, AFRICAN, NORWEGIAN_BLUE;
}
private boolean isNailed;
private Species type;
public double getSpeed() {
switch (type) {
case EUROPEAN:
return getBaseSpeed();
case AFRICAN:
return getBaseSpeed() - getLoadFactor();
case NORWEGIAN_BLUE:
return isNailed ? 0 : getBaseSpeed();
default:
return 0;
}
}
private double getLoadFactor() {
return 3;
}
private double getBaseSpeed() {
return 10;
}
}public abstract class Bird {
public abstract double getSpeed();
protected double getLoadFactor() {
return 3;
}
protected double getBaseSpeed() {
return 10;
}
}
public class EuropeanBird extends Bird {
public double getSpeed() {
return getBaseSpeed();
}
}
public class AfricanBird extends Bird {
public double getSpeed() {
return getBaseSpeed() - getLoadFactor();
}
}
public class NorwegianBird extends Bird {
private boolean isNailed;
public double getSpeed() {
return isNailed ? 0 : getBaseSpeed();
}
}You’ll probably notice this further, when each time you invoke the method you need to add this if-statement because the business logic says so
"" or []Tolerance: It’s necessary to be defensive at the outer parts of your codebase, but being defensive inside your codebase probably means the code that you are writing is offensive. Don’t write offensive code.
Solution: Use a NullObject or Optional type instead of ever passing a null. An empty collection is a great alternative.
public boolean horrible(boolean foo, boolean bar, boolean baz) {
if (foo) {
if (bar) {
return true;
}
}
if (baz) {
return true;
} else {
return false;
}
}To
Problem: Functions are always taking the same parameters
Problem: tired of seen deeply nested long statements
class Organization {
constructor(data) {
this._name = data.name;
this._country = data.country;
}
get name() {return this._name;}
set name(arg) {this._name = arg;}
get country() {return this._country;}
set country(arg) {this._country = arg;}
}The caller is happyer
When can you divide on class in two ? Here:
Heritance is made to mutualize varaibles / functions
Problem: Long declaration line
inverse of Remove Middle Man
Inverse of extract
As class
Problem: Caller always sanitize/concat in/out of function call
Opposite of something (remove parameters)
In function call
Works with fields / methods
Also called Clause
Keywords: Streams, Functional progamming
With constants
That is why it is better to pass the full object
Of input
Together, far from call
aliases Consolidate Duplicate Conditional Fragments
Work with the ass little nesting level as possible -> return from loop early
const orderData = orderString.split(/\s+/);
const productPrice = priceList[orderData[0].split("-")[1]];
const orderPrice = parseInt(orderData[1]) * productPrice;
const orderRecord = parseOrder(order);
const orderPrice = price(orderRecord, priceList);
function parseOrder(aString) {
const values = aString.split(/\s+/);
return ({
productID: values[0].split("-")[1],
quantity: parseInt(values[1]),
});
}
function price(order, priceList) {
return order.quantity * priceList[order.productID];
}