Penalty shootout rules

The team kicking first in penalty shootout enjoys a huge advantage, winning roughly 60% of the time. A recent paper (A comparison of penalty shootout designs in soccer) discusses and compares the three rules devised to overcome the unfairness, which are:

1. Alternating (ABBA) Rule: the order of the first two penalties (AB) is the mirror image of the next two (BA), and this sequence is continued even in the possible sudden death stage of penalty shootouts (the sixth round of penalties is started by team B, the seventh by team A, and so on).
2. Catch-Up Rule: the order of the penalties in a given round (including the sudden death) is the mirror image of the previous round except if the first team failed and the second scored in the previous round when the order of the teams remains unchanged.
3. Adjusted Catch-Up Rule: the first five rounds of penalties, started by team A, are kicked according to the Catch-Up Rule, but team B is the first kicker in the sudden death (sixth round) such that the first mover is alternated in this stage.

Let's skip the mathematical computation and jump directly to the conclusion: the Catch up rule does not outperform the Alternating rule while the Adjusted Catch-up rule is fairer than both the Catch up rule and the Alternating rule.

I wrote a python program (https://github.com/sunmingtao/penalty-shootout) to simulate these rules. The assumption is the first kicker has a success rate of 3/4 while the second kicker has a success rate of 2/3. 

The result of the Alternating rule and Catch up rule is very close to that on the paper. However, my program indicates the gap of the result between the Adjusted Catch-up rule and the other two rules is not as large as the paper claims. In fact, the result seems too close to reveal any statistical significance.



In the Euro 2008 quarter final of Italy against Spain, Buffon won the coin toss and yet chose to kick second. We all know the sad outcome for Italy. I wonder if Buffon was too just proud to go first.

Python Sonarqube with unit test coverage

Prerequisite

Sonarqube is installed (Create a new project named 'shootout')
Sonar scanner is installed
Anaconda is installed

Install nose and coverage

pip install nose
pip install coverage

Python Project structure

main.py
sample/
    hello.py
testcase/
    test.py

Sonar property file

create sonar-project.properties under the root folder

Contents of the file

sonar.projectKey=shootout #corresponds to the project name in Sonarqube
sonar.projectName=Penalty shootout
sonar.projectVersion=1.0
sonar.language=py
sonar.python.xunit.reportPath=nosetests.xml
sonar.python.coverage.reportPaths=coverage.xml
sonar.python.xunit.skipDetails=true
sonar.coverage.exclusions=testcase/**/* 

Activate the Anaconda virtual environment

Note: It appears that nose can only run in the virtual environment, hence the need of the activation.

conda init

Generate nosetests.xml and coverage.xml

coverage erase 

nosetests --with-coverage --with-xunit
coverage xml

Analyze the project

sonar-scanner

View the result

http://localhost:9000

Troubleshooting

https://github.com/sunmingtao/sample-code/issues/11
https://github.com/sunmingtao/sample-code/issues/12
https://github.com/sunmingtao/sample-code/issues/13
https://github.com/sunmingtao/sample-code/issues/14
https://github.com/sunmingtao/sample-code/issues/15
https://github.com/sunmingtao/sample-code/issues/16

JMockit incompatible with JDK 10/11

Running Unit tests under JDK 10/11 produces error below. It was OK under JDK 8.

[ERROR] Errors: 
[ERROR]   BusinessRepositoryTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound Could...
[ERROR]   BusinessRepositoryTest>AbstractRepositoryTest.setUp:23 » ExceptionInInitializer
[ERROR]   BusinessUserRepositoryTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound C...
[ERROR]   BusinessUserRepositoryTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound C...
[ERROR]   TransactionRepositoryTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound Co...
[ERROR]   TransactionRepositoryTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound Co...
[ERROR]   TransactionRepositoryTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound Co...
[ERROR]   BusinessCustomerLinkServiceTest>AbstractRepositoryTest.setUp:23 » NoClassDefFound
[ERROR]   BusinessServiceTest.save:30 » NullPointer
[INFO] 
[ERROR] Tests run: 9, Failures: 0, Errors: 9, Skipped: 0

The contents of AbstractRepositoryTest is nothing but a Mockup of a public static method

@BeforeEach
void setUp() throws Exception {
    new MockUp<CafamilyUtils>() {
        @mockit.Mock
        public String getLoggedInUserID() {
            return "system";
        }
    };
}

The version of jmockit is too old (1.24) in pom.xml. It needs to be upgraded to a much newer version (1.47). Besides JMockit also requires the -javaagent JVM initialization parameter to be used, according to
https://jmockit.github.io/tutorial/Introduction.html#runningTests

<dependencies>
   <dependency>
      <groupId>org.jmockit</groupId>
      <artifactId>jmockit</artifactId>
      <version>${jmockit.version}</version>
      <scope>test</scope>
   </dependency>
</dependencies>

<plugins>
   <plugin>
      <artifactId>maven-surefire-plugin</artifactId>
      <version>2.22.2</version> <!-- or some other version -->
      <configuration>
         <argLine>
            -javaagent:${settings.localRepository}/org/jmockit/jmockit/${jmockit.version}/jmockit-${jmockit.version}.jar
         </argLine>
      </configuration>
   </plugin>
</plugins>

Static method MockUp effect is global

Use mockit.MockUp.MockUp() to mock public static methods. One caveat is that the mocked behaviour applies to other test methods as well. (test2() prints "abc", which is surprising, isn't it?)

@ExtendWith(value = {MockitoExtension.class})
@RunWith(JUnitPlatform.class)
class MyTest {
    
    @Test
    void test1() throws IOException {
        new MockUp<System>() {
            @mockit.Mock
            public String getProperty(String key){
                return "abc";
            }
            
        };
        
        assertThat(System.getProperty("a"), is("abc"));
    }
    
    @Test
    void test2() throws IOException {
        System.out.println(System.getProperty("a")); //Prints abc
    }
    

}

Junit 5 temp directory quick start

The tempDirectory extension makes it quite handy to test file reading/writing functions.

Maven dependency

<dependency>
   <groupId>org.junit-pioneer</groupId>
   <artifactId>junit-pioneer</artifactId>
   <version>0.1.2</version>
   <scope>test</scope>
</dependency>

Java

@Test
@ExtendWith(TempDirectory.class)
void withProperties(@TempDir Path tempDir) throws IOException {
    Path path = tempDir.resolve("env.properties");
    String properties = "a=b";
    Files.write(path, properties.getBytes());
    Map<String, Object> props = EnvPropertiesLoader.loadEnvProperties();
    assertThat(props.get("a"), is("b"));
}

Double-checked locking should not be used

Extracted from sonarqube

Double-checked locking is the practice of checking a lazy-initialized object's state both before and after a synchronized block is entered to determine whether or not to initialize the object.
It does not work reliably in a platform-independent manner without additional synchronization for mutable instances of anything other than float or int. Using double-checked locking for the lazy initialization of any other type of primitive or mutable object risks a second thread using an uninitialized or partially initialized member while the first thread is still creating it, and crashing the program.

There are multiple ways to fix this. The simplest one is to simply not use double checked locking at all, and synchronize the whole method instead. With early versions of the JVM, synchronizing the whole method was generally advised against for performance reasons. But synchronized performance has improved a lot in newer JVMs, so this is now a preferred solution. If you prefer to avoid using synchronized altogether, you can use an inner static class to hold the reference instead. Inner static classes are guaranteed to load lazily.

Noncompliant Code Example

@NotThreadSafe
public class DoubleCheckedLocking {
    private static Resource resource;

    public static Resource getInstance() {
        if (resource == null) {
            synchronized (DoubleCheckedLocking.class) {
                if (resource == null)
                    resource = new Resource();
            }
        }
        return resource;
    }

    static class Resource {

    }
}

Compliant Solution

@ThreadSafe
public class SafeLazyInitialization {
    private static Resource resource;

    public synchronized static Resource getInstance() {
        if (resource == null)
            resource = new Resource();
        return resource;
    }

    static class Resource {
    }
}

With inner static holder:

@ThreadSafe
public class ResourceFactory {
    private static class ResourceHolder {
        public static Resource resource = new Resource(); // This will be lazily initialised
    }

    public static Resource getResource() {
        return ResourceFactory.ResourceHolder.resource;
    }

    static class Resource {
    }
}

Using "volatile":

class ResourceFactory {
  private volatile Resource resource;

  public Resource getResource() {
    Resource localResource = resource;
    if (localResource == null) {
      synchronized (this) {
        localResource = resource;
        if (localResource == null) {
          resource = localResource = new Resource();
        }
      }
    }
    return localResource;
  }

  static class Resource {
  }
}

Swift non-optional, optional (?), and optional(!)

class Country {
    let name: String
    var capitalCity: City!
    init(name: String, capitalName: String) {
        self.name = name
        self.capitalCity = City(name: capitalName, country: self)
    }
    deinit {
        print ("country deinit")
    }
}

class City {
    let name: String
    unowned let country: Country
    init(name: String, country: Country) {
        self.name = name
        self.country = country
    }
    deinit {
        print ("city deinit")
    }
}

var country: Country = Country(name: "Canada", capitalName: "Ottawa")
print ("country capital city is \(country.capitalCity.name)")

Why the exclamation mark is needed after City?

Without making it the variable optional, the code won't compile, because the country instance won't be considered fully initialised and therefore cannot be passed as an argument to initiate the city instance (country: self).

Can exclamation mark be replaced by question mark?

Yes, it can. But to access this property, it needs to be unwrapped. So country.capitalCity.name is to be replaced by country.capitalCity!.name. If we know for sure a property will never be null, it's better to be defined as City! rather than City?.