"Ticker" for game timing I designed a "Ticker" based on how I imagine Minecraft's tick system works. My ticker calls a method tickRate times a second and provides a Delta Time value (deltaTime) to the method being called.

The issue I have with my code is that I find the update method (which handles timing) to be ugly but I'm not sure what I can do.

My ticker class is designed to be instantiated and used whenever, it does not have to be used right away (which is why I don't set any time related information in the constructor). This is why there is a check for nextTick and lastTick being 0 inside update, as to ensure that the delta time does not go crazy on the first tick. The issue with this is that it looks a bit ugly, however I'm not sure what I could do. I could move the reset code into the reset method, however that would now mean that the reset method is time dependant, and must be only be called when you intend to use the ticker.

Ticker.java

import java.util.HashSet;
import java.util.Set;

public class Ticker {
    private long lastTick;
    private long nextTick;
    private int tickRate;

    private Set<TickListener> tickListeners = new HashSet<>();

    public Ticker(int tickRate) {
        this.tickRate = tickRate;
    }

    public void addTickListener(TickListener listener) {
        tickListeners.add(listener);
    }

    public void removeTickListener(TickListener listener) {
        tickListeners.remove(listener);
    }

    public void setTickRate(int tickRate) {
        this.tickRate = tickRate;
    }

    public int getTickRate() {
        return tickRate;
    }

    public void reset() {
        lastTick = 0;
        nextTick = 0;
    }

    public boolean update() {
        long currentTime = System.currentTimeMillis();

        if (currentTime >= nextTick) {
            long targetTimeDelta = 1000L / tickRate;

            if (lastTick == 0 || nextTick == 0) {
                lastTick = currentTime - targetTimeDelta;
                nextTick = currentTime;
            }

            float deltaTime = (float) (currentTime - lastTick) / targetTimeDelta;

            for (TickListener listener : tickListeners) {
                listener.onTick(deltaTime);
            }

            lastTick = currentTime;
            nextTick = currentTime + targetTimeDelta;

            return true;
        }

        return false;
    }
}

TickListener.java

public interface TickListener {
    void onTick(float deltaTime);
}

Example usage

Ticker ticker = new Ticker(20); // 20 ticks per second

ticker.addTickListener(new TickListener() {

    @Override
    public void onTick(float deltaTime) {
        System.out.println(String.format("Ticked with deltaTime %f", deltaTime));
    }
});

while (true) {
    ticker.update();
}

This is one of the cleanest code I have come across in a while. Congrats to your lovely, well-written code!

Just some points:

I don't like seeing this:

public class Ticker {
    private long lastTick;
    // ...
    }
}

It is more readable (IMO) when you put spaces after the beginning and before the end:

public class Ticker {

    private long lastTick;
    // ...
    }

}

I don't see a reason why not to do so, as the rest of your code is very well spaced.
This method:

public boolean update() {
    long currentTime = System.currentTimeMillis();

    if (currentTime >= nextTick) {
        long targetTimeDelta = 1000L / tickRate;

        if (lastTick == 0 || nextTick == 0) {
            lastTick = currentTime - targetTimeDelta;
            nextTick = currentTime;
        }

        float deltaTime = (float) (currentTime - lastTick) / targetTimeDelta;

        for (TickListener listener : tickListeners) {
            listener.onTick(deltaTime);
        }

        lastTick = currentTime;
        nextTick = currentTime + targetTimeDelta;

        return true;
    }

    return false;
}

Just my opinion, but I feel uncomfortable with that many spaces in code. The only place I would use spaces would be:

Between Node declarations in JavaFX, or JWhatever declarations in Swing.
Where there is an obvious change in what the code will do.

So it would be more like:

public boolean update() {
    long currentTime = System.currentTimeMillis();
    if (currentTime >= nextTick) {
        long targetTimeDelta = 1000L / tickRate;
        if (lastTick == 0 || nextTick == 0) {
            lastTick = currentTime - targetTimeDelta;
            nextTick = currentTime;
        }

        float deltaTime = (float) (currentTime - lastTick) / targetTimeDelta;
        for (TickListener listener : tickListeners) {
            listener.onTick(deltaTime);
        }

        lastTick = currentTime;
        nextTick = currentTime + targetTimeDelta;
        return true;
    }
    return false;
}


Shopify monitor that utilizes redis for caching each product This is just a simple monitor for shopify products for an array of sites. It involves parsing the /products.json of the site, checking if each product ID is already stored as a key in redis and if it isn't it needs to notify a group of discord members (haven't implemented yet). Really appreciate any feedback given!

const axios = require('axios'),
      fs = require('fs'),
      redis = require('redis');

const client = redis.createClient();

let trackingF = fs.readFileSync("../tracking.json");
let proxiesF = fs.readFileSync("../proxies.json");

let keywords = JSON.parse(trackingF).keywords.map(x => {
     return x.toLowerCase();
});

let sites = JSON.parse(trackingF).sites.map(x => {
    return x.toLowerCase();
});

let proxies = JSON.parse(proxiesF).proxies;

const getProducts = (url) => {
    return new Promise((resolve, reject) => {
        axios.get(`${url}/products.json`, rProxy()).then(response => {
           resolve(response.data.products);
        }).catch(err => {reject(err)});
    });
 }

const cache = (product) => {
    client.get(product.id, (err, reply) => {
       if (err) console.log(err);
       if (!reply) {
           client.set(product.id, JSON.stringify(product));
           console.log("NEW PRODUCT: " + product.id);
           return;
       }
    });
}

const rProxy = () => {
    return proxies[Math.floor(Math.random() * Math.floor(proxies.length))];
}

const checkStock = () => {
    console.log("[MONITOR] CHECKING");
    for (let i = 0; i < sites.length; i++) {
        getProducts(sites[i]).then(products => {
           products.forEach(product => {
               cache(product)
           });
        }).catch(err => console.log(err));
    }
}

setInterval(() => checkStock(), 5000);

const axios = require('axios'),
      fs = require('fs'),
      redis = require('redis');

Personally, I discourage the use of a single var/let/const for declaring a variable. Firstly, they're not portable. Let's say you want to move fs somewhere else. Instead of copy-pasting the entire line, you end up copying that line and tacking on var/let/const anyways.

Next, say you already had redis. if you wanted to add axios, the Git changelog will look like:

- const redis = require('redis');
+ const axios = require('axios'),
+       redis = require('redis');

or this:

- const redis = require('redis');
+ const redis = require('redis'),
+       axios = require('axios');

Instead of just this:

+ const axios = require('axios');
  const redis = require('redis');

or this:

  const redis = require('redis');
+ const axios = require('axios');

You never touched redis, yet it's highlighted in the commit. This is because you had to add the ,. This ends up being unnecessary noise in the commit, and annoying in code reviews and pull requests.

Next up, I recommend a const-first policy. The priority should be const first, then let if the variable has to mutate (e.g. loop counters). Then var if you absolutely have to have a function-scoped variable defined in some nested block for whatever reason.

This block-scoped variables reduce cognitive overhead, avoiding having to remember that what variables exist in the upper scopes, and to avoid accidentally clobbering a variable elsewhere. Small change in process, but has big benefits down the line.

Node.js has native support for async/await. So you no longer have to deal with the clunky syntax of promises.

Additionally, if you have functions that use the Node-style callback API (an async function that takes a callback whose first argument is an error and the second being the result), Node has utils.promisify() which returns a promise-based version of it.

for (let i = 0; i < sites.length; i++) {
    getProducts(sites[i]).then(products => {
       products.forEach(product => {
           cache(product)
       });
    }).catch(err => console.log(err));
}

You can replace this with an array.map() and Promise.all(). Put the promises returned by getProducts() in an array, pass that array to Promise.all() which returns a promise, await that promise to listen for the completion.

setInterval(() => checkStock(), 5000);

I'd wait for checkStock() to complete first before launching the next run. You wouldn't want to hammer the site every 5 seconds. They might not respond in time, and your requests may accumulate. You might be rate-limited or worse, blocked, before you know it.

Here's how I'd write it:

const axios = require('axios')
const fs = require('fs')
const redis = require('redis')

const trackingF = fs.readFileSync("../tracking.json")
const proxiesF = fs.readFileSync("../proxies.json")

const client = redis.createClient()
const clientGetAsync = utils.promisify(client.get)

const toLower = x => x.toLowerCase()

const keywords = JSON.parse(trackingF).keywords.map(toLower)
const sites = JSON.parse(trackingF).sites.map(toLower)
const proxies = JSON.parse(proxiesF).proxies;

const rProxy = () => proxies[Math.floor(Math.random() * Math.floor(proxies.length))]

const getProducts = async url => {
  const response = await axios.get(`${url}/products.json`, rProxy())
  return response.data.products
}

const cache = async product => {
  try {
    const reply = await clientGetAsync(product.id)
    if (reply) return
    client.set(product.id, JSON.stringify(product))
    console.log("NEW PRODUCT: " + product.id)
  } catch (e) {
    console.error(e)
  }
}

const checkStock = async delay => {
  console.log("[MONITOR] CHECKING")

  const promises = sites.map(async site => {
    try {
      const products = await getProducts(site)
      await Promise.all(products.map(cache))
    } catch (e) {
      console.error(e)
    }
  })

  await Promise.all(promises)

  setTimeout(() => checkStock(delay), delay)
}

checkStock(5000).catch(e => console.error(e))


Implementation of a Custom Database Class Following Database Class is what I'm using as a wrapper to connect with MySqli in a application, and below the class is how i'm implementing it currently I'm not sure if this is the right way to handle it or how I can further improve.

class Database {
/**
 * @var $conn: type Mysqli
 **/

private $conn = null;
private $stmt = null;

/**
 * @__construct: establishes a new Connection to db
 **/

function __construct() {
    $this->conn = new mysqli(
        DatabaseHelper::DB_HOST,
        DatabaseHelper::DB_USERNAME,
        DatabaseHelper::DB_PASSWORD,
        DatabaseHelper::DB_NAME
    );
}

/**
 * @ret: $conn
 **/
public function connect() {
    return $this->conn ? true : false;
}

public function prepare($query) {
    $this->stmt = $this->conn->prepare($query);
    if ($this->stmt) {
        return true;
    } else {
        return false;
    }
}

public function set_params($sql_str, ...$params) {
    // bind the sql string
    $bind_args = array();
    $bind_args[] = $sql_str;

    foreach ($params as $param_key => $param_val) {
        $bind_args[] = &$params[$param_key]; // pass the reference to bind_Args
    }

    if (call_user_func_array(array($this->stmt, 'bind_param'), $bind_args)) {
        return true;
    } else {
        return false;
    }
}

public function exec_query() {
    $ret = $this->stmt->execute();
    $this->stmt->close();
    return $ret ? $ret : false;
}

public function exec_query_array() {

    $ret = $this->stmt->execute();
    if ($ret) {
        $this->stmt->store_result();
        $response = array();
        while($r = $this->getResultByArray($this->stmt)) {
            $response[] = $r;
        }
        $this->stmt->close();

        return $response;
    } else {
        return false;
    }
}

private function getResultByArray($stmt) {
    if($stmt->num_rows>0) {
        $result = array();
        $md = $stmt->result_metadata();
        $params = array();
        while($field = $md->fetch_field()) {
            $params[] = &$result[$field->name];
        }
        call_user_func_array(array($stmt, 'bind_result'), $params);
        if($stmt->fetch())
            return $result;
    }
    return null;
}}

and accessing it in this way

 public function findUserByMobile($mobile) {
    $db = new Database();
    $query = "SELECT * FROM users WHERE mobile = ?";
    $sql_str = "s";
    if ($db->connect()) {
        if ($db->prepare($query)) {
            if ($db->set_params($sql_str, $mobile)){
                $ret = $db->exec_query_array();
                if ($ret) {
                    return $ret;
                } else {
                    return false;
                }
            } else {
                return false;
            }
        } else {
            return false;
        }
    } else {
        return false;
    }
}

This code is also available here.

Returning Early / Guard Clauses

Returning early is very easy to do and can severely improve your code.

Looking at findUserByMobile, it is nearly impossible to say which else closes which if on first glance. In this case it doesn't matter that much as all else do the same thing, but it is still not nice to read.

If you just invert the if and return, the code is a lot nicer:

 public function findUserByMobile($mobile) {
    $db = new Database();
    if (!$db->connect()) {
        return false;
    }

    $query = "SELECT * FROM users WHERE mobile = ?";
    if (!$db->prepare($query)) {
        return false;
    }

    $sql_str = "s";
    if (!$db->set_params($sql_str, $mobile) ){
        return false;
    }

    $ret = $db->exec_query_array();
    if (!$ret) {
        return false;
    }
    return $ret;
}

If you don't care what went wrong, you could also combine some of the ifs. In practice, you probably do care though.

Error Handling

Just returning false on error can make debugging quite difficult. Did the query fail because you couldn't connect to the database? Because the query is wrong? Because the parameter is wrong? Because no result could be found? We just don't know. Think about throwing exceptions instead.

Database connections

You really do not want to create a new database connection for each query. Instead, pass the database object to the method and reuse it.

Misc

connect sounds like it would connect something. Instead, it returns something. It is customary to name the method isConnected.
don't shorten variable names. ret is more difficult to read than return or result, metadata is better than md, etc.
always use curly brackets, even for one-line statements.


Simple singleton database connection pool I'm studying design patterns, and to demonstrate a singleton, I've implemented a primitive database connection pool.

ConnectionPool.java

package com.levent.connpool;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;

public class ConnectionPool {

    private final static int MAX_CONNECTIONS = 8;
    private static ConnectionPool instance = null;  // lazy loading
    private static Connection[] connections = new Connection[MAX_CONNECTIONS];
    private static String dbUrl = "jdbc:derby://localhost:1527/memory:levoDB/singletonDemo;create=true";
    private static int counter;

    private ConnectionPool() { }

    public static ConnectionPool getInstance() {
        if(instance == null) {
            synchronized(ConnectionPool.class) {
                if(instance == null) {
                    instance = new ConnectionPool();
                    initializeConnections();
                    counter = 0;
                }
            }
        }

        return instance;
    }

    private static void initializeConnections() {
        for(int i = 0; i < MAX_CONNECTIONS; i++) {
            try {
                connections[i] = DriverManager.getConnection(dbUrl);
            } catch (SQLException e) {
                e.printStackTrace();
            }
        }
    }

    public static Connection getConnection() {
        counter++;
        if(counter == Integer.MAX_VALUE)
            counter = 0;

        return connections[counter%MAX_CONNECTIONS];
    }

}

ConnectionPoolDemo.java

package com.levent.connpool;

import java.sql.Connection;
import java.sql.SQLException;

public class ConnectionPoolDemo {

    public static void main(String[] args) {
        ConnectionPool pool = ConnectionPool.getInstance();

        Connection[] connections = new Connection[17];

        for(int i = 0; i < connections.length; i++)
            connections[i] = pool.getConnection();

        int preIndex = connections[0].getClass().getCanonicalName().length();
        for(int i = 0; i < connections.length; i++)
            System.out.printf("Connection %2d : %s\n", i+1, connections[i].toString().substring(preIndex));

        for(int i = 0; i < connections.length; i++)
            try {
                connections[i].close();
            } catch (SQLException e) {
                e.printStackTrace();
            }
    }

}

The output is as follows:

Connection  1 : @27f8302d
Connection  2 : @6438a396
Connection  3 : @e2144e4
Connection  4 : @6477463f
Connection  5 : @3d71d552
Connection  6 : @1cf4f579
Connection  7 : @18769467
Connection  8 : @46ee7fe8
Connection  9 : @27f8302d
Connection 10 : @6438a396
Connection 11 : @e2144e4
Connection 12 : @6477463f
Connection 13 : @3d71d552
Connection 14 : @1cf4f579
Connection 15 : @18769467
Connection 16 : @46ee7fe8
Connection 17 : @27f8302d

Explanation

The aims is to have distributed connections that's why on each getConnection() method call, on of the each connections are returned with the use of mod. The maximum connection size is determined by the MAX_CONNECTIONS which is set to 8 in this example and all 17 connections retrieved from the singleton pool class are recurring instances of the 8 connections stored in the ConnectionPool class.

I know that this code is not a big shot, but my aim was to demonstrate a simple singleton in a real world example. I'm waiting for your ideas and your criticism.

Your connection pool is not threadsafe / concurrently usable, if you are sure you do not need it to be ignore my points 1. to 3.
Your pool does not offer connection locking (mutual exclusive usage of one Connection object) which is usually implemented by making the connection inaccessible after getConnection() and making it eligible for takeout after calling its close() method. You close the raw connections manually, leaving the pool in a broken state.
This is problematic: https://stackoverflow.com/questions/9428573/
Your counter int might return the same int for two concurrent calls at the same time. Use AtomicInteger / AtomicLong instead.
Your pool does not implement connection testing: Your connections could expire at any time. You need to replace broken Connections with new Connections or your program might receive dead Connection objects!
You might want to implement Closeable to close all connections at once when your application shuts down and clear the instance.
You might want to throw if initializeConnections fails, because what are you supposed to do with a broken object?
Hardcoding URLs that might change is not considered good practice


Get two array from array of objects I have an array of objects:

const dataset = [
  {name: 'Marie', year: 12, country: 'Canada'},
  {name: 'Jeff', year: 18, country: 'France'},
  {name: 'Joss', year: 10, country: 'Spain'},
  {name: 'Carrie', year: 45, country: 'Belgium'},
  {name: 'Linda', year: 32, country: 'France'},
]

I want two array: one with names and one with countries:

const names = ['Marie', 'Jeff', 'Joss', 'Carrie', 'Linda']
const countries = ['Canada', 'France', 'Spain', 'Belgium', 'France']

So, I usually do:

const names = dataset.map(e => e.name)
const countries = dataset.map(e => e.country)

Is there a simplest way possibly using a single loop?

I'm not sure this is any simpler but it does use only a single loop:

 const dataset={ ... }
 const names=[], countries=[]
 for (var d of dataset) {
     names.push(d.name);
     countries.push(d.country);
 }

Surprisingly, according to jsben.ch, this approach is about 6 times faster than running map twice.


PHP - Deliberate loose == check and assigning to if statement if ($v->getMedical()) {
    $visitorHasMedical = $v->getMedical()->getMedCon() == true; // $visitorHasMedical will be false if "falsey" i.e. empty string or NULL
} else {
    $visitorHasMedical = false;
}

Where getMedical() is a OneToOne Doctrine relationship that might not exist.

Does this snippet read OK? Firstly, I'm deliberately doing a loose == check as this could be NULL or empty string.

Secondly, is assigning $visitorHasMedical to what is essential an if statement OK?

Quite so. Except for the trailing comment getting beyond edge. But it can be written in much simpler way anyway.

$medical = $v->getMedical();
$visitorHasMedical = ($medical && $medical->getMedCon());


Convert rows with multiple values to row with single value I have a dataframe with measurement data of different runs at same conditions. Each row contains both the constant conditions the experiment was conducted and all the results from the different runs.

Since I am not able to provide a real dataset, the code snippet provided below will generate some dummy data.

I was able to achieve the desired output, but my function transform_columns() seems to be unecessary complicated:

import pandas as pd
import numpy as np

np.random.seed(seed=1234)

df = pd.DataFrame(np.random.randint(0, 100, size=(100, 6)), columns=['constant 1', 'constant 2', 1, 2, 3, 4])

def transform_columns(data):
    factor_columns = []
    response_columns = []
    for col in data:
        if isinstance(col, int):
            response_columns.append(col)
        else:
            factor_columns.append(col)
    collected = []
    for index, row in data.iterrows():
        conditions = row.loc[factor_columns]
        data_values = row.loc[response_columns].dropna()
        for val in data_values:
            out = conditions.copy()
            out['value'] = val
            collected.append(out)
    df = pd.DataFrame(collected).reset_index(drop=True)
    return df

print(transform_columns(df))

Is there any Pythonic or Pandas way to do this nicely?

It is probably easier to work with the underlying Numpy array directly
than through Pandas. Ensure that all factor columns comes before all
data columns, then this code will work:

import pandas as pd
import numpy as np

np.random.seed(seed=1234)

n_rows = 100
n_cols = 6
n_factor_cols = 2
n_data_cols = n_cols - n_factor_cols
arr = np.random.randint(0, 100, size=(n_rows, n_cols))
factor_cols = arr[:,:n_factor_cols]
data_cols = [arr[:,i][:,np.newaxis] for i in range(n_factor_cols, n_cols)]
stacks = [np.hstack((factor_cols, data_col)) for data_col in data_cols]
output = np.concatenate(stacks)

The above code assumes that order is not important. If it is, then use
the following instead of np.concatenate:

output = np.empty((n_rows * n_data_cols, n_factor_cols + 1),
                  dtype = arr.dtype)
for i, stack in enumerate(stacks):
    output[i::n_data_cols] = stack

This is the best I can do, but I wouldn't be surprised if someone
comes along and rewrites it as a Numpy one-liner. :)


Showing a JDialog whenever there is an exception I have made a method which I use to show the exception error through a JDialog window whenever there is one in my program.

For now I can pass it either a String or a subclass of Exception. At first I had two different methods for each one but now I combined them into one.

The showError method:

public static void showError(Object error) {
        String mainMessage = null;
        String title = null;
        if (error instanceof String) {
            mainMessage = (String) error;
            title = "Error!";
        } else if (error instanceof Exception) {
            Exception exceptionError = (Exception) error;
            mainMessage = "Message: " + exceptionError.getMessage()
                    + "\nStackTrace: " + Arrays.toString(exceptionError.getStackTrace());
            title = exceptionError.getClass().getName();
        }

        JOptionPane.showMessageDialog(null, mainMessage, title, JOptionPane.ERROR_MESSAGE);
    }

At first I had two different methods for each one but now I combined
  them into one.

I would have made 3 methods instead of one.

public static void showError(String errorMessage) {
    showError(errorMessage, "Error!");
}

public static void showError(Exception exceptionError) {
    String errorMessage = "Message: " + exceptionError.getMessage()
                + "\nStackTrace: " + Arrays.toString(exceptionError.getStackTrace());
    String title = exceptionError.getClass().getName();
    showError(errorMessage, title);
}

public static void showError(String errorMessage, String title) {
    JOptionPane.showMessageDialog(null, errorMessage, title, JOptionPane.ERROR_MESSAGE);
}

In this way the readability is improved, unnecessary checks are removed as well as casts.
And it is easier to maintain.


Random generator extension for VSCode Background

A VSCode extension that uses Chance.JS to generate random data types.

Concerns

My main concern is that there are 4 levels of function references that feels like a hack. From registerCommand → insertFunc* → randomFunc* → processSelection.
If the code is sensible TypeScript/JavaScript

The Code

export function activate(context: vscode.ExtensionContext) {
    context.subscriptions.push(vscode.commands.registerCommand('randomeverything.int', insertRandomInt));
    context.subscriptions.push(vscode.commands.registerCommand('randomeverything.letters', insertRandomLetters));
    context.subscriptions.push(vscode.commands.registerCommand('randomeverything.lettersAndNumbers', insertRandomLettersAndNumbers));
    context.subscriptions.push(vscode.commands.registerCommand('randomeverything.iPv4Address', insertRandomIPv4Address));
    context.subscriptions.push(vscode.commands.registerCommand('randomeverything.iPV6Address', insertRandomIPV6Address));
    /* More commands... */
}

function insertRandomInt(): void {

    var max:number;
    var min:number;

    Window.showInputBox({prompt: "Please enter [MIN-MAX]", value:"1-100"}).then(
        function(txt){
            if(txt){
                var args = txt.split("-");

                min = Number.parseInt(args[0]);
                max = Number.parseInt(args[1]);

                if(args.length != 2 || isNaN(min) || isNaN(max))
                {
                    Window.showErrorMessage("Invalid format.");
                    return;
                }
                processSelection(randomIntString, [min, max]);
            }
    });

}

function insertRandomLetters(): void {
    processSelection(randomLetters, []);
}

function insertRandomLettersAndNumbers(): void {
    processSelection(randomLettersAndNumbers, []);
}

function insertRandomIPv4Address(): void {
    processSelection(randomIP, ['ipv4']);
}

function insertRandomIPV6Address(): void {
    processSelection(randomIP, ['ipv6']);
}
/* More inserters... */

/**
 * Chance.js Wrappers
 */
function randomIntString(min, max): string {
    var chance = require('chance').Chance();
    var randomVar: Number = chance.integer({
      min: min,
      max: max
    });

    return randomVar.toString();
}

function randomIP(option?:string): string{
    var chance = require('chance').Chance();
    var randomVar:string;

    switch (option) {
        default:
        case 'ipv4':
            randomVar = chance.ip();
            break;
        case 'ipv6':
            randomVar = chance.ipv6();
            break;
    }

    return randomVar;
}

/* More wrappers... */

function processSelection(formatCB, argsCB) {

    let e = Window.activeTextEditor;
    let d = e.document;
    let sel = e.selections;

    e.edit(function (edit) {
        // iterate through the selections
        for (var x = 0; x < sel.length; x++) {
            let txt: string = d.getText(new Range(sel[x].start, sel[x].end));

            if (argsCB.length > 0) {
                txt = formatCB.apply(this, argsCB);
            } else {
                txt = formatCB();
            }

            //insert the txt in the start of the current selection
            edit.insert(sel[x].start, txt);
        }
    });
}

As for the architecture it seems good to me.

As for the TypeScript I have a few minor suggestions:

Avoid using var, use const or let instead.

function randomIntString(min: number, max: number): string {
    const chance = require('chance').Chance();
    const randomVar: number = chance.integer({
      min: min,
      max: max
    });

    return randomVar.toString();
}

It is also preferable to use let for for-cycle control variables.

for (let x = 0; x < sel.length; x++) {
   // body
}

It would help readability and maintainability if you specified the types of all the arguments of all your functions.

function randomIntString(min: number, max: number): string {
    // body
}

Also prefer using lower case type names for built-in primitive types (e. g. number over Number) when specifying the type of something.

You could specify the IP option type more precisely (asuming you use TypeScript >= 2.0). This will help you prevent typos and also improves readability (the code is more self-documenting):

type IpAddressType = 'ipv4' | 'ipv6';
function randomIP(option?: IpAddressType): string{
    // body stays the same
}

When the property of the object being initialized has the same name as the variable assigned to it you can shorten it like so (see the chance.integer call):

var randomVar: number = chance.integer({
      min,
      max
    });


Optimizing mass inserts into SQLite I am trying to optimize inserts from a returned JSON which contains data from multiple tables on the remote server. My code is working, but I just want to figure out a way for it to work better. I need the structure to be very dynamic (i.e. fetching the table names from JSON then looping through to complete the inserts into SQLite). This is so if a table is added in the future, I will not have to change any code.

private boolean prepReturnedData(String newString) throws JSONException {
     long errCheck = 0; // tracks valid db inserts.

     JSONArray columns = new JSONArray();

     JSONObject jsonObj = new JSONObject(newString);
     ArrayList<String> tables = new ArrayList<String>();

     Iterator<?> tableKeys = jsonObj.keys();

     while (tableKeys.hasNext()) {
         tables.add(tableKeys.next().toString());
     }
     for (String table : tables) { // Loop through table names.
         columns = jsonObj.getJSONArray(table);
         for (int index = 0; index < columns.length(); index++) { // Loop through Array of Entries

             ContentValues cv = new ContentValues(); // Figure out where to put this.

             Iterator<?> iter = columns.getJSONObject(index).keys(); // Creates an Iterator containing Column names.
             JSONObject newObj = columns.getJSONObject(index); // Creates a new JSONObject for retrieving column values.

             while (iter.hasNext()) { // Iterates to get Column names.
                 String tColumn = iter.next().toString(); // Stores column name, for retrieval of column value.

                 cv.put(tColumn, newObj.get(tColumn).toString());

             } // end column iterator loop.

             String status = null;
             if (table == "Seizures") {
                 status = "timestamp";
             } else {
                 status = "name";
             }
             try {
                 SQLiteDatabase db = this.getWritableDatabase();
                 errCheck = db.replace(table, status, cv);

                 if (db.isOpen()); {db.close(); };
             } catch (Exception e) {
                 // TODO Auto-generated catch block
                 e.printStackTrace();
             }
         } // end entry loop
     } // end table loop
     if (errCheck >= 0) {
         return true;
     } else {
         return false;
     }
}

This function is within a DatabaseHelper class.

I suggest some optimizations:

Do not initialize columns with empty JSONArray - it will always be overwritten.
Do not iterate twice and do not create overhead ArrayList object you can iterate json object once.
Due to Android specific garbage collection it's better to reuse object than create a new one in each iteration (see where is declared ContentValues cv = new ContentValues(); and cleared then later).
Open database once not in loop -> be sure to close it (finally block).
Your error reporting is leaky - the next db.replace would clear off the former errCheck value - I added some simple handling but I did not know the requirements in that area.

private boolean prepReturnedData(String newString) throws JSONException {
    JSONObject jsonObj = new JSONObject(newString);

    if(jsonObj.length() > 0) {
        Iterator<?> tableKeys = jsonObj.keys();
        SQLiteDatabase db = null;

        try {
            db = getWritableDatabase();
            db.beginTransaction();

            ContentValues cv = new ContentValues();

            String table;
            JSONArray columns;

            String status;
            int columnsLength;
            JSONObject columnJson;
            Iterator<?> columnKeys;

            String tColumn;

            while (tableKeys.hasNext()) {
                table = tableKeys.next().toString();
                columns = jsonObj.getJSONArray(table);
                status = table == "Seizures" ? "timestamp" : "name";
                columnsLength = columns.length();

                for (int i = 0; i < columnsLength; i++) { // Loop through Array of Entries
                    cv.clear();

                    columnJson = columns.getJSONObject(i); // Creates a new JSONObject for retrieving column values.
                    columnKeys = columnJson.keys(); // Creates an Iterator containing Column names.

                    while (columnKeys.hasNext()) {
                        tColumn = columnKeys.next().toString();
                        cv.put(tColumn, columnJson.optString(tColumn, ""));
                    }

                    if(db.replace(table, status, cv) == -1L) {
                        throw new IllegalStateException(
                                "Insert failed for table: " + table +
                                ", contentValues: " + cv.toString());
                    }
                }
            }

            db.setTransactionSuccessful();
        }
        catch(Throwable e) {

            return false;
        }
        finally {
            if (db != null && db.isOpen()) {
                db.endTransaction();
                db.close();
            };
        }
    }

    return true;
}


Calculate AB significance with Control and Treatment Objects I am working on an AB tool in JavaScript using the following:

Calculate the Conversion rate based on the conversions divided by the hits.
Calculate the Z Score by passing in the control and treatment objects.
Determine the confidence of the Z Score with Cumulative Normal Distribution.

How accurate is this?

/**
  Example input:

  var a = {
    'hits' : 15,
    'conversions' : 2
    };

  var b = {
     'hits' : 15,
     'conversions' : 6
  };
 */

/**
 * Calculation of conversion rate
 * @param t
 * @returns {number}
 */
function cr(t) {
    return t.conversions / t.hits;
}

/**
 * Calculation of score
 * @param c
 * @param t
 */
function calcZScore(c, t) {
    var z = cr(t) - cr(c);
    var s = (cr(t) * (1 - cr(t))) / t.hits + (cr(c) * (1 - cr(c))) / c.hits;
    return Math.sqrt(s);
}

/**
 * Calculate the Cumulative Normal Distribtion
 *
 * @param x
 * @returns {number}
 */
function cumNorDist(x) {
    var b1 = 0.319381530;
    var b2 = -0.356563782;
    var b3 = 1.781477937;
    var b4 = -1.821255978;
    var b5 = 1.330274429;
    var p = 0.2316419;
    var c = 0.39894228;

    if (x >= 0.0) {
        t = 1.0 / ( 1.0 + p * x );
        return (1.0 - c * Math.exp(-x * x / 2.0) * t *
        ( t * ( t * ( t * ( t * b5 + b4 ) + b3 ) + b2 ) + b1 ));
    } else {
        t = 1.0 / ( 1.0 - p * x );
        return ( c * Math.exp(-x * x / 2.0) * t *
        ( t * ( t * ( t * ( t * b5 + b4 ) + b3 ) + b2 ) + b1 ));
    }
}

/**
 * Given a conversion rate, calculate a recommended sample
 * size
 * E.g: 0.25 worst, 0.15, 0.05 best at a 95% confidence
 * @param conv
 * @returns {Array}
 */
function sampleSize(conv) {
    var a = 3.84145882689;
    var res = [];
    var bs = [0.0625, 0.0225, 0.0025];
    var len = bs.length;
    for (var i = 0; i < len; i++) {
        res.push(((1 - conv) * a / (bs[i] * conv)));
    }
    return res;
}

/**
 * Calculate the significance between Control and Treatment [A/B/C]
 *
 * @param controlObj
 * @param treatmentObj
 */
function calculateSig(controlObj, treatmentObj) {

    if (typeof controlObj !== 'object') {
        console.log(outputErrorFor('Control Object'));
    }

    if (typeof controlObj !== 'object') {
        console.log(outputErrorFor('Control Object'));
    }

    var zScore;
    var confidence;
    var confidencePercentage;
    var cRatio;

    var cNumHits = controlObj.hits;
    var cNumConver = controlObj.conversions;
    var tNumHits = treatmentObj.hits;
    var tNumConver = treatmentObj.conversions;

    var cConversionRate = (cNumConver / cNumHits) * 100;
    var tConversionRate = (tNumConver / tNumHits) * 100;

    cConversionRate += '%';
    tConversionRate += '%';

    zScore = calcZScore(controlObj, treatmentObj);
    confidence = cumNorDist(zScore);
    confidencePercentage = confidence * 100;

    console.log("Control Hits is:", cNumHits);
    console.log("Control Conversions is:", cNumConver);
    console.log("Treatment Hits is:", tNumHits);
    console.log("Treatment Conversions is:", tNumConver);

    console.log("Results are in:");

    console.log("Control Conversion Rate is:", cConversionRate);
    console.log("Treatment Conversion Rate is:", tConversionRate);

    console.log("Z Score is:", zScore);
    console.log("Confidence is:", confidence);
    console.log("Confident percentage is:", confidencePercentage);
}

/**
 *
 * @param name
 * @returns {string}
 */
var outputErrorFor = function (name) {
    return 'The passed in parameter must be an object and contain two properties: "hits" and "conversions" for: ' + name;
};

This is a ported version of this repo. (credit to the author)

Side note: With the Z Score, how can I find the P Value?

One problem here is in calcZScore, where it should be:

return z/Math.sqrt(s);

Otherwise, it looks like you successfully ported the code. More generally, there are some caveats associated with using the normal assumption for A/B tests. You want to make sure that you have a sufficient number of data points for the normality assumption to hold (often expressed as np > 5 and n(1-p)>5), which the sample size estimator doesn't always seem to satisfy.

You also should also be aware of other incorrect methods of A/B testing, such as repeated significance errors as expressed here: http://www.evanmiller.org/how-not-to-run-an-ab-test.html. In fact, I'd say getting working confidence calculation code is just scratching the surface of proper A/B testing.

Also, assuming you are doing a two-sided hypothesis test, the P-value is simply 1-confidence.

console.log("P-Value is:", 1-confidence);

I compared the results this code produces to the following online A/B calculator and it seemed to match up (although I haven't validated the sample size recommendation, so someone else should probably take a look at that).
https://vwo.com/ab-split-test-significance-calculator/


C++ wrappers for execv(3) Calling execv(3) (and similar functions) in C++ can be
problematic. execv expects an array of const pointers to
non-const char, and string literals are inherently const, so you have
to jump through hoops to convert your literals to non-const char arrays.
And of course in C++ you normally want to use vectors and strings
instead of arrays.

Not finding any standard functions to handle this, I wrote two wrapper
functions to make things easy. They seem to work correctly, in the ways
I thought to test, but I’m fairly new to C++; more experienced
developers may see any number of pitfalls I’ve missed.

My first version just used argv[0] as the path. Then I checked and
found good reasons for path != argv[0] in some cases, so I
overloaded it; one version is more general and one simplifies the call.

#include <cerrno>
#include <cstdio>
#include <cstring>
#include <string>
#include <vector>

#include <unistd.h>

int execv_cpp(const std::string &path,
              const std::vector<std::string> &argv);
int execv_cpp(const std::vector<std::string> &argv);

int main()
{
    // Test some failure modes.

    execv_cpp({});
    perror("execv() 1 failed");

    execv_cpp("/bin/ls", {});
    perror("execv() 2 failed");

    execv_cpp({"/foo/bar/baz"});
    perror("execv() 3 failed");

    execv_cpp({"/etc/passwd"});
    perror("execv() 4 failed");

    // Test valid calls.

    if (fork() == 0) {
        execv_cpp({"/bin/ls", "-lart"});
        perror("forked execv() unexpectedly failed");
        return 0;
    }

    usleep(50000);
    execv_cpp("/bin/ls", {"ls", "-lh"});
    perror("final execv() unexpectedly failed");

    return 0;
}

int execv_cpp(const std::string &path,
              const std::vector<std::string> &argv)
{
    /* Convert arguments to C-style and call execv. If it returns
     * (fails), clean up and pass return value to caller. */

    if (argv.size() == 0) {
        errno = EINVAL;
        return -1;
    }

    std::vector<char *> vec_cp;
    vec_cp.reserve(argv.size() + 1);
    for (auto s : argv)
        vec_cp.push_back(strdup(s.c_str()));
    vec_cp.push_back(NULL);

    int retval = execv(path.c_str(), vec_cp.data());

    int save_errno = errno;
    for (auto p : vec_cp)
        free(p);
    errno = save_errno;
    return retval;
}

int execv_cpp(const std::vector<std::string> &argv)
{
    /* Overloaded. Use first element as path for simpler call. */

    if (argv.size() == 0) {
        errno = EINVAL;
        return -1;
    }

    return execv_cpp(argv[0], argv);
}

Comments are not by any order of significance.

Which operating system standard?

A platform supporting C++ does not necessarily have an execve() call; that depends on <unistd.h>. Are you sure you're not making assumptions about what it contains?

Perhaps you aren't, but you need to double-check and document this.

Wrapping a single operating system call is questionable

If you've written this for your own use, since you execve() all the time like there's no tomorrow :-) ... then ok. Otherwise, developers would probably be hesitant to incorporate a idiosyncratic piece of code which handles exactly one out of dozens of system calls.

Also, even focusing on exec'ing another program, we have:

int execl(const char *path, const char *arg, ...);
int execlp(const char *file, const char *arg, ...);
int execle(const char *path, const char *arg, ...);
int execv(const char *path, char *const argv[]);
int execvp(const char *file, char *const argv[]);
int execvpe(const char *file, char *const argv[], char *const envp[]);

I'm not saying you need to implement a "Cpp-ified" version of all of these - on the contrary, it would probably be a bad idea - but you chose the less-general version.

Don't add utility/library functions to the global namespace

Application code uses (or rather, may use) the global namespace; library code uses a dedicated namespace. What you've written is supposed to be used repeatedly, not just once - so it's essentially library code, right? Put it in some namespace (e.g. util or unix etc.) A benefit of that is that you don't have to append the artificial name extension, i.e. unix::exec sounds nicer than execve_cpp.

Confusing parameters in your second variant

If I look at your second variant:

int execv_cpp(const std::vector<std::string> &argv);

It would not be clear to me whether the element of that vector should be the name of the binary I wish to execute. You may think you're making things more convenient for the client code, but people would rather type a few more characters than have to contend with semantic ambiguity.

If you're Cpp'ifying - do so for errors as well

In C++, passing invalid arguments typically causes an exception to be thrown; and you don't need to return anything, since you never return (as opposed to throwing an exception). Thus, for example:

void execv_cpp(const std::vector<std::string> &argv)
{
    if (argv.size() == 0) {
        throw std::invalid_argument(
            "At least one argument must be provided, being the path of "
            "the binary to execute");
    execv_cpp(argv[0], argv);
}

but of course, you're making it easy for the calling code to get it wrong by being willing to take an empty argv in the first place.

Don't force people to use std::vector's

A vector is just one specific container. There's no reason to assume the calling code is using vectors; and you're not even actually using the vector - you're converting it into a C-style array.

  in C++ you normally want to use vectors... instead of arrays.

Not true. You sometimes want to use them.

Don't force people to use std::strings

  of course in C++ you normally want to use... strings...

Sometimes, sometimes not. An std::string is just one way - albeit the default way - to represent strings. Remember, it's in the library - it's not int he langue. And std::string involves memory allocation policy (stack & heap etc.) which might not be appropriate to everyone. You should be more flexible, e.g. by templating on the string type:

template<typename S>
void execv_cpp(const std::vector<S> &argv);

or by using string views (C++17, experimental in C++14).

Consider a C++'ish interface for a bounded number of arguments

If you know the number of arguments at compile time, you can just have the following:

template <typename Args...>
void execv_cpp(std::string_view binary, Args&&... arguments);

and use std::to_string(), or operator<< into a stringstream, on each of the arguments in a loop. That gives maximum flexibility to the calling code, and makes exec() ing a binary basically the same as calling an arbitrary function:

execv_cpp(my_binary, 1, "two", std::optional<int>(3), 4.0);

Now, true, in this case you would be using up memory for string conversion results, but in the fixed-number-of-arguments case it's acceptable, I would say.

No need to duplicate strings

In the answer you linked to, it specifically says it's safe to const_cast the char pointers. So no need for the strdup().


Haskell permutations matching criteria How would one do the following in Haskell:

  Return all permutations of a list where one element comes before the
  another element (cannot assume that elements of the list can be
  ordered)?

My solution was to do:

sLeftOf l r lss =
    [ ls
    | ls <- lss
    , DL.findIndex (l==) ls <= DL.findIndex (r==) ls
    ]

for somewhere to the left of and

sDirectLeftOf l r lss =
  [ls
  | ls <- lss
  , DL.findIndex (l==) ls  == fmap (\x-> x - 1) (DL.findIndex (r==) ls)
  ]

for directly to the left of which works,

*Main Lib> sLeftOf 2 3 (permutations [1..3])
[[1,2,3],[2,1,3],[2,3,1]]

*Main Lib> sDirectLeftOf 2 3 (permutations [1..4])
[[1,2,3,4],[2,3,1,4],[4,2,3,1],[2,3,4,1],[4,1,2,3],[1,4,2,3]]

But I don't like these. The findIndex seems un-Haskelly and the fmap on the result of findIndex feels just wrong. Anyone has better ways to do this? For two lists there is a nice method using guard/zip and elem.

mylist = do
  x <- permutations ["a","b","c"]
  y <- permutations ["1","2","3"]

  leftOf "b" x "3" y
  return $ zip x y
  where
    leftOf x xs y ys = guard $ (x,y) `elem` zip xs (tail ys)
    leftOf' x xs y ys = guard $ (x,y) `elem` (aux xs (tail ys))
    aux a b@(_:ys) = (zip a b) ++ aux a ys
    aux _ []       = []

reqA = (map (map fst )) mylist
reqB = (map (map snd )) mylist
required = zip reqA reqB

The first leftOf is immediately to left of and the second is somewhere to the left of; but this won't work for a single list.

The findIndex seems un-Haskelly and the fmap on the result of findIndex feels just wrong.

There's nothing wrong with fmap.

One thing I would advise is replacing (\x -> x - 1) with (minus 1). Also, [ls | ls <- lss, p ls] is probably best rewritten to use filter.

If you format your code more nicely, I think you'd find it reasonably appealing:

g = findIndex . (==)

p l r ls = g l ls <= g r ls

sLeftOf l r lss = filter (p l r) lss

sDirectLeftOf l r lss = filter (p l r) $ fmap (minus 1) (g ls)

  The first leftOf is immediately to left of and the second is somewhere to the left of; but this won't work for a single list.

Seems like a fairly contrived solution. elem and zip are still \$O(n)\$ so I don't think this accomplishes what you intended. If you're concerned about performance, I'd suggest using a keyed map so that lookups become \$O(\log n)\$ - searching an array will be \$O(n)\$ just like searching a list.

Also, you define

leftOf' x xs y ys = guard $ (x,y) `elem` (aux xs (tail ys))
aux a b@(_:ys) = (zip a b) ++ aux a ys
aux _ []       = []

which is not used anywhere. Passing the -Wall option to GHC will warn you any time you do something like this.


Perl module to deal with numbered nested objects I wrote a small perl module that dealing with PHP-ish array structures, and I'm going to release it to CPAN.

I want it to be reviewed before the release about the module naming and the code itself.

Could you please give me improvements?  Any suggestions are appreciated.

https://github.com/ernix/p5-Object-Squash

use strict;
use warnings;
package Object::Squash;
# ABSTRACT: Remove numbered keys from a nested object

use parent 'Exporter';
use List::Util qw/max/;

use version; our $VERSION = version->declare("v0.0.1");

our @EXPORT_OK = qw(squash);

sub squash {
    my $obj = shift;
    return $obj unless ref $obj;

    $obj = _squash_hash($obj);
    $obj = _squash_array($obj);

    return $obj;
}

sub _squash_hash {
    my $obj = shift;
    return $obj unless ref $obj eq 'HASH';

    my @keys = keys %{$obj};

    if (grep {/\D/} @keys) {
        return +{
            map { $_ => squash($obj->{$_}) } @keys,
        };
    }

    my $max = max(@keys) || 0;

    my @ar;
    for my $i (0 .. $max) {
        push @ar, sub {
            return (undef) unless exists $obj->{$i};
            return squash($obj->{$i});
        }->();
    }

    return \@ar;
}

sub _squash_array {
    my $obj = shift;
    return $obj unless ref $obj eq 'ARRAY';

    return (undef) if @{$obj} == 0;
    $obj = squash($obj->[0]) if @{$obj} == 1;

    return $obj;
}

1;
__END__

=head1 NAME

Object::Squash - Remove numbered keys from a nested object

=head1 DESCRIPTION

This package provides B<squash> subroutine to simplify hash/array structures.

I sometimes want to walk through a data structure that consists only of a bunch
of nested hashes, even if some of them should be treated as arrays or single
values.  This module removes numbered keys from a hash.

=head1 SYNOPSIS

=head2 C<squash>

    use Object::Squash qw(squash);
    my $hash = squash(+{
        foo => +{
            '0' => 'nested',
            '1' => 'numbered',
            '2' => 'hash',
            '3' => 'structures',
        },
        bar => +{
            '0' => 'obviously a single value',
        },
    });

$hash now turns to:

    +{
        foo => [
            'nested',
            'numbered',
            'hash',
            'structures',
        ],
        bar => 'obviously a single value',
    };

=head1 AUTHOR

Shin Kojima <shin@kojima.org>

=head1 LICENSE

This program is free software; you can redistribute it and/or
modify it under the same terms as Perl itself.

Just a few remarks about parts which poke the eye,

    push @ar, sub {
        return (undef) unless exists $obj->{$i};
        return squash($obj->{$i});
    }->();

introduces unnecessary subroutine call/overhead which can be replaced with do {} block, or even simpler,

    push @ar, exists $obj->{$i} ? squash($obj->{$i}) : undef;

This one is only matter of taste, so

return $obj unless ref $obj eq 'HASH';

could also be written as

return $obj if ref $obj ne 'HASH';

Usually you don't have to explicitly return undef, as in list context you may end up with list one element long, and that might not be what is intended. Instead just return; or return (); will produce empty list, or undef in scalar context. So

return (undef) if @{$obj} == 0;

could be better as

return () if @{$obj} == 0;

or perhaps,

@$obj or return;


Matrix operations using classes So I recently had a practical exam in which I was told to execute a function of a menu oriented matrix operation program using classes. The code I made was supposedly wrong as I hadn't used classes the way they have to be used. Can anybody help me rectify the error here. I'm a noob (less than 2 months into C++) so please pardon any nooby mistakes.

 #include <iostream>
    using namespace std;
    class matrix3
    {
        int a[10][10],b[10][10],c[10][10],d[10][10],e[10][10],f[10][10],x,y,i,j;
        public :
            void values();
            void transpose();
            void sum();
            void diff();
    };
    void matrix3::values()
    {
        cout << "Enter the rows   "; cin >> x;
        cout << "Enter the columns   "; cin >> y;
        cout << "Enter elements of first matrix\n\n";
        for (i=1; i<=x; i++)
        {
            for ( j=1; j<=y; j++)
            {
                cin >> a[i][j];
            }
        }
        cout << "Enter elements of second matrix\n\n";
        for (i=1; i<=x; i++)
        {
            for (j=1; j<=y; j++)
            {
                cin >> c[i][j];
            }
        }
    }
    void matrix3::sum()
    {
        cout << "Sum of Matrices 1 and 2 is\n";
        for (i=1; i<=x; i++)
        {
            for ( j=1; j<=y; j++)
            {
                e[i][j]=a[i][j]+c[i][j];
                cout << e[i][j] << "";
            }
            cout << endl;
        }

    }
    void matrix3::diff()

    {
    cout << "Difference of Matrices 1 and 2 (1-2) is\n";
        for (i=1; i<=x; i++)
        {
            for ( j=1; j<=y; j++)
            {
                f[i][j] = a[i][j]-c[i][j];
                cout << f[i][j] << "";
            }
            cout << endl;
        }
    }
    void matrix3::transpose()
    {
        cout << "transpose of the matrix is\n";
        for ( i=1; i<=x; i++)
        {
            for ( j=1; j<=y; j++)
            {
                b[i][j] = a[j][i];
                cout << b[i][j] << "";
            }
            cout << endl;
        }
        cout << "Transpose of the second matrix is\n";
        for ( i=1; i<=x; i++)
        {
            for ( j=1; j<=y; j++)
            {
                d[i][j] = c[j][i];
                cout << b[i][j] << "";
            }
            cout << endl;
        }
    }
    int main()
    {
        int input;
        char ch;
        matrix3 m;
        m.values();
        do
         {
        cout << "Enter your choice\n";
        cout << " 1. Sum of 1 and 2\n" << " 2. Difference of 1 and 2\n" << " 3. Transpose of both 1 amd 2\n";
        cin >> input;
        switch (input)
        {
            case 1:
                m.sum();
                break;

            case 2:
                m.diff();
                break;

            case 3:
                m.transpose();
                break;
        }
        cout << "\nDo another y/n?";
        cin >> ch;
        }
        while (ch!= 'n');
        cout << "\n";
        system ("pause");
    return 0;
    }

Partial Answer This does not cover all aspects of your code, so feel free to wait for other responses.

In OOP, it is important to identify the scope of an object. In your case, you have two input matrices and possibly a result matrix for the operation that the user selects. The key point is, that each matrix should be a distinguished instance of your matrix class. What you have is something like a matrix manager, that contains multiple matrices in a single instance.

When you re-design your matrix class to contain a single matrix, you should possibly externalize your user input logic. Also, the sum, diff and transpose functions could be external functions that take two matrices as parameters and return a resulting matrix. However, I guess it would also be ok to have them as member function that only takes the second matrix as parameter.

Consider the following class definition:

#include <vector>

class matrix {
private:
    // matrix with m columns and n rows
    int _m, _n;
    // values, organized as an outer vector of rows
    // and an inner vector of columns
    std::vector<std::vector<int>> _values;

public:
    // initialize matrix by its size
    matrix(int m, int n)
        : _m(m), _n(n)
    {
        // initialize the matrix size
        _values.resize(n);
        for (int i = 0; i < n; ++i)
        {
            _values[i].resize(m);
        }
    }

    // TODO:
    // - other constructors
    // - copy operator
    // - other desired functions
    // - assessors to the matrix elements
};

Maybe you can start from there.


Vigenere cipher in Haskell I implemented the Vigenere cipher in haskell as a part of exercises for  Haskell programming from first principles.

Though the code isn't all that long, I feel it could use some refactoring to simplify things (I feel the code I wrote is ugly). One caveat: string inputs should all be in uppercase.

The vigenere function should be applied like so:

vignere "ALLY" "MEET AT DAWN"

which will create the output:

"MPPRAEOYWY"

As it stands, here's the code I implemented:

import Data.Char

-- exercise chapter 11
encode :: Char -> Int
encode x = ord x - ord 'A'

decode :: Int -> Char
decode x = chr (x + ord 'A')

shift :: (Int -> Int -> Int) -> Int -> Char -> Char
shift f x ch =  decode $ f (encode ch) x `mod` 26

rightShift :: Int -> Char -> Char
rightShift = shift (+)

leftShift :: Int -> Char -> Char
leftShift = shift (-)

encodeString :: String -> [Int]
encodeString str = map encode str

type Secret = String
type PlainText = String
type CipherText = String

vignereString :: Secret -> PlainText -> String
vignereString secret plain = take len $ cycle secret
                          where len = length $ concat $ words plain

vignereCode :: Secret -> PlainText -> [Int]
vignereCode secret plain = encodeString $ vignereString secret plain

vignere :: Secret -> PlainText -> CipherText
vignere secret plain =
  zipWith rightShift code plainNoSpace
  where code = vignereCode secret plain
        plainNoSpace = concat $ words plain

unvignere :: Secret -> CipherText -> PlainText
unvignere secret cipher =
  zipWith leftShift code cipherNoSpace
  where code = vignereCode secret cipher
        cipherNoSpace = concat $ words cipher

It's a nice solution, but very long for a relatively simple task.

First, note that the cipher is called "Vigenère", so vignere would be considered a misspelling.

The type definitions…

type Secret = String
type PlainText = String
type CipherText = String

… provide an illusion of type safety, where there actually isn't any.  They are all just Strings.

All of the helper functions (encode, decode, shift, rightShift, leftShift, and encodeString) could just be reduced to a single function:

import Data.Char (chr, ord)

shift :: (Int -> Int -> Int) -> Char -> Char -> Char
shift op offset ch = numToChar $ (charToNum ch) `op` (charToNum offset)
  where
    charToNum ch = ord ch - ord 'A'
    numToChar n = chr $ (n `mod` 26) + ord 'A'

Hiding the charToNum and numToChar helpers makes it easier to see what calls what.

With the generalized shift function defined, the enciphering and deciphering routines could each be one-liners:

vigenere :: String -> String -> String
vigenere secret = zipWith (shift (+)) (cycle secret) . concat . words

unvigenere :: String -> String -> String
unvigenere secret = zipWith (shift (-)) (cycle secret) . concat . words

Here, I've written them using point-free style to emphasize the fact that vigenere secret acts as an enciphering filter on the text.


Kendo grid optimization I have two kendoGrid(s) named:

var sourcegrid = $('#usersGrid').data("kendoGrid");        //SOURCE GRID
var destinationgrid = $('#teamGrid').data('kendoGrid');    //DESTINATION GRID

In sourcegrid, I have 1000 records and in destinationgrid has no records.

I have a button that transfers data from sourcegrid to destinationgrid.

 sourcegrid.select().each(function () {
        var dataItem = sourcegrid.dataItem($(this));
        destinationgrid.dataSource.add(dataItem);
        sourcegrid.removeRow($(this));
    });

When I'm transferring 500 records to destinationgrid, the process of transferring is so slow. How can this be optimized?

You can use this jsfiddle.net link here.

Capitalisation:
If you're gonna use variables like this: kendoGrid, dataItem, but then have variables like: destinationgrid and sourcegrid, then stick with one (preferably the first, because it's easier to read)
Optimizing:
It seemed for me, that removing this line increased the speed:

editable:   "popup",

Because with your code it popups for each data cell being removed, which is fine, if that's what you want
However, it seems with your code that it doesn't support the movement of more than one data cell at a time.
Note:
If you're wanting the user to select more than one nearly every time, then using popups is fine, but make it one popup per action.


Getting multiple table schema and returning them, using one helper method I want to get two distinct sets of schema from a database without having to create multiple Connection/PreparedStatement/ResultSet objects.

Currently, I have a helper method that queries the database, gets each individual schema, and then returns them together in one ArrayList - please refer to the code below.

private ArrayList<ArrayList<?>> getAttributesAndGroups() {
    ArrayList<ArrayList<?>> retList = new ArrayList<ArrayList<?>>();

    Connection conn = null;
    try {
        Class.forName("com.mysql.cj.jdbc.Driver");
        conn = DriverManager.getConnection(jdbcUrl, userId, password);

        ArrayList<Attribute> listAttrs = new ArrayList<Attribute>();
        String query = "select aid, aname from tbl_attributes order by aid";
        PreparedStatement ps = conn.prepareStatement(query);
        ResultSet rs = ps.executeQuery();
        while (rs.next()) {
            int aid = rs.getInt(1);
            String aname = rs.getString(2);
            Attribute attr = new Attribute(aid, aname);
            listAttrs.add(attr);
        }
        retList.add(listAttrs);

        ArrayList<Group> listGroups = new ArrayList<Group>();
        query = "select gid, gname from tbl_groups order by gid";
        ps = conn.prepareStatement(query);
        rs = ps.executeQuery();
        while (rs.next()) {
            int gid = rs.getInt(1);
            String gname = rs.getString(2);
            Group grp = new Group(gid, gname);
            listGroups.add(grp);
        }
        retList.add(listGroups);

        rs.close();
        ps.close();

    } catch (Exception e) {
        log.error("Exception", e);
    } finally {
        if (conn != null) {
            try {
                conn.close();
            } catch (Exception e) {
                //
            }
        }
    }

    return retList;
}

Where I call my helper method, I do the following:

    ArrayList<ArrayList<?>> lists = getAttributesAndGroups();

    ArrayList<Attribute> listAttrs = (ArrayList<Attribute>) lists.get(ATTRIBUTES);
    request.setAttribute("listAttrs", listAttrs);

    ArrayList<Group> listGroups = (ArrayList<Group>) lists.get(GROUPS);
    request.setAttribute("listGroups", listGroups);

    request.getRequestDispatcher("addUser.jsp").forward(request, response);

(ATTRIBUTES and GROUPS are simple integer constants representing 0 and 1 respectively.)

However, my code is giving me warnings of Type safety: Unchecked cast from ArrayList<capture#1-of ?> to ArrayList<Attribute> (ditto for the other line).

Is there a better approach for what I want to do?

Generics

When you say, ArrayList<ArrayList<?>>, you are saying that there is a type but that you don't know what it is.  So you write it as a ?.

But in your code, you are putting ArrayList<Attribute> and ArrayList<Group> into the larger list.  These are not one type.  So it has to find a type that covers both Attribute and Group.  Absent both extending or implementing some common class or interface, that will be Object.  So ? is actually Object in your code.

It is natural to think that ? is a wildcard that can match multiple things simultaneously.  But it is not.  It represents exactly one type at a time.  You just avoid having to say which type.  As such, I don't think that it is what you want to use here.

Cast by element not collection

The general way to fix this warning on something like

    ArrayList<Attribute> listAttrs = (ArrayList<Attribute>) lists.get(ATTRIBUTES);

would be something like

    List<Attribute> listAttrs = new ArrayList<>();
    for (Object attr : lists.get(ATTRIBUTES)) {
        if (attr instanceof Attribute) {
            listAttrs.add((Attribute) attr);
        }
    }

You can often leave off the instanceof check because you know what the result should be.

I changed from the implementation (ArrayList) to the interface (List), as is customary in Java when you aren't using implementation specific methods unavailable to the interface.

But I wouldn't recommend that code for this specific problem.  You don't really need to cast here.  You control the code at both ends and in between.  Just write it without casts.

Custom return type

The most direct way to fix this specific problem is to replace your return value with a custom class.

class AttributesAndGroups {

    private List<Attribute> attributes;

    private List<Group> groups;

    public void setAttributes(List<Attribute> attributes) {
        this.attributes = attributes;
    }

    public List<Attribute> getAttributes() {
        return attributes;
    }

    public void setGroups(List<Group> groups) {
        this.groups = groups;
    }

    public List<Group> getGroups() {
        return groups;
    }

}

You can use this like

        AttributesAndGroups result = new AttributesAndGroups();
        result.setAttributes(listAttrs);
        result.setGroups(listGroups);
        return result;

and in the caller

        AttributesAndGroups lists = getAttributesAndGroups();
        List<Attribute> listAttrs = lists.getAttributes();
        List<Group> listGroups = lists.getGroups();

No casts (unchecked or otherwise).

There's a lot of boilerplate code in the class.  It's possible to avoid that here.

Pass to the method

Another alternative would be to pass the lists into the helper method rather than have the method initialize them.

        List<Attribute> listAttrs = new ArrayList<>();
        List<Group> listGroups = new ArrayList<>();
        fillAttributesAndGroups(listAttrs, listGroups);
        request.setAttribute("listAttrs", listAttrs);
        request.setAttribute("listGroups", listGroups);

I changed the name to be more representative of the revised implementation.

And then the method signature would be

private void fillAttributesAndGroups(List<Attribute> listAttrs, List<Group> listGroups) {

Now just delete

    ArrayList<ArrayList<?>> retList = new ArrayList<ArrayList<?>>();
        ArrayList<Attribute> listAttrs = new ArrayList<Attribute>();
        retList.add(listAttrs);
        ArrayList<Group> listGroups = new ArrayList<Group>();
        retList.add(listGroups);
    return retList;

And the remainder of your code should function normally.

Again, no casts needed.  And this code is much terser, particularly if this is the only time you call it.

Another alternative would be to pass request into the method, but I won't try to implement that here.  That would allow you to make the lists inside the method again.


Regex to clean text in preparation for word count in PHP EDIT: Here's my totally-revised PHP...

$text = preg_replace("~[^ a-z0-9'-]~"," ",strtolower($INPUT));
for($i=1;$i<strlen($text)-1;$i++) {
    if(preg_match("~['-]~",$text[$i]) && ( !preg_match("~[a-z0-9]~",$text[$i-1]) || !preg_match("~[a-z0-9]~",$text[$i+1])) ){
        $text[$i] = " ";
    }
}
while( preg_match ("~  ~",$text) ) $text = preg_replace( "~  ~", " ", $text);
if(preg_match("~[' -]~",$text[0])) $text = substr($text,1,strlen($text)-1);
if(preg_match("~[' -]~",$text[strlen($text)-1])) $text = substr($text,0,strlen($text)-2);

Now, what I'd said still applies..

This regex seems to work for me, but I'm curious if anyone can think of a breaking case, or tell me anything I did wrong.

(If it seems the code does what I say I want it do to, say so, then for fun you can help me become more neurotic about what I actually want it to do, which hinges on the question of defining an English "word".)

Desired final form of $text: replace, with spaces, all characters in $INPUT except for letters, digits, and any hyphens or apostrophes that are directly between two letters/digits (and hence "part of a word"). Then collapse all whitespace into single spaces, and if necessary, drop the leading and closing space.

End result should be a lowercase series of words separated by spaces, of which some words may contain (entirely "inside" the word) one or more non-consecutive apostraphes, and/or one or more non-consecutive hyphens.

Does this do that, with no exceptions?

After this step, the next part will be to split (or explode, whichever is better) the string by spaces, then generate a list of words and frequencies. I'm pretty sure how to do that on my own (especially because my teacher actually told us two algorithms). In fact I'll probably hand the assignment in before choosing an answer to this; I'm mostly asking out of curiosity.

My thinking is...

English "words" tend to be case insensitive for distinction (though there is the proper noun issue, which I'll just ignore).
Some "words" are digits (The word 88), or include digits, like 2Pac or se7en.
An apostrophe can also be a single quote, so it shouldn't be considered part of the word in case that's the intention, even though it sometimes might be, as with What is goin' on? (Fortunately, "proper" contractions don't begin or end with an apostrophe.)
Hyphens can occur at the beginning or end of a word, but that doesn't make for a distinct word (An eater asked if I was a fire-juggler or -eater has two *eater*s, not one with the hyphen and one without).
Two or more consecutive hyphens are meant as a dash of some kind, separating words
Two consecutive apostrophes are likewise assumed to separate words, despite typos like don''t
In general, digits, hyphens, and apostrophes are the only relevant "non-letter letters" for word-frequency counting.
Despite all my overthinking here, I don't actually want to be maximally thorough, just "good enough" (and "good enough" is the question I'm ovethinking!).

Old code:

$text = preg_replace( "~((?<![a-z0-9])[-'](?![a-z0-9]))|([^ a-z0-9'-])~", " ", strtolower($INPUT));
while(preg_match("~  ~",$text)) $text = str_replace("  ", " ", $text);
if($text[0] == " ") $text = substr($text,1,strlen($text)-2);
if($text[strlen($text)-1] == " ") $text = substr($text,0,strlen($text)-2);

Do it in two steps. First replace all the stuff you deem to be "junk" with spaces, then collapse the spaces and hyphens down to a single space. You can express this with a reduce through a series of patterns that collapse to a space.

Some output from a play in Boris.

[7] boris> function clean($input) {
[7]     *>   $patterns = array(
[7]     *>     '/[^\w\r\n -]/',
[7]     *>     '/[\r\n -]{2,}/'
[7]     *>   );
[7]     *>   return array_reduce(
[7]     *>     $patterns,
[7]     *>     function($text, $re) { return preg_replace($re, ' ', $text); },
[7]     *>     $input
[7]     *>   );
[7]     *> }
[8] boris> clean('abc def &% f');
// 'abc def f'
[9] boris> clean('abc-def &% f');
// 'abc-def f'
[10] boris> clean('abc-def-&%-f');
// 'abc-def f


Unit testing a REST API in Go I am learning Go by writing a simple CRUD REST API using gotilla/mux and Gorm.

I would like to get some feedback on the application code and on unit tests in particular. Please suggest me what to improve, what to do better, what not to do and generally tell me what comes to your mind.

func TestGetAllStudents(t *testing.T) {
    app := initApp()
    st := student{ID: "id-1", Age: 20, Name: "John Doe"}
    app.db.Save(st)
    req, _ := http.NewRequest("GET", "/students", nil)
    r := httptest.NewRecorder()
    handler := http.HandlerFunc(app.getAllStudents)

    handler.ServeHTTP(r, req)

    checkStatusCode(r.Code, http.StatusOK, t)
    checkContentType(r, t)
    checkBody(r.Body, st, t)
}

func TestAddStudent(t *testing.T) {
    app := initApp()
    var rqBody = toReader(`{"name":"John Doe", "age":20}`)
    req, _ := http.NewRequest("POST", "/students", rqBody)
    r := httptest.NewRecorder()
    handler := http.HandlerFunc(app.addStudent)

    handler.ServeHTTP(r, req)

    checkStatusCode(r.Code, http.StatusCreated, t)
    checkContentType(r, t)
    checkProperties(firstStudent(app), t)
}

func TestUpdateStudent(t *testing.T) {
    app := initApp()
    app.db.Save(student{ID: "id-1", Age: 25, Name: "Peter Doe"})
    var rqBody = toReader(`{"name":"John Doe", "age":20}`)
    req, _ := http.NewRequest("PUT", "/students/id", rqBody)
    req = mux.SetURLVars(req, map[string]string{"id": "id-1"})
    r := httptest.NewRecorder()
    handler := http.HandlerFunc(app.updateStudent)

    handler.ServeHTTP(r, req)

    checkStatusCode(r.Code, http.StatusOK, t)
    checkContentType(r, t)
    checkProperties(firstStudent(app), t)
}

func TestDeleteStudent(t *testing.T) {
    app := initApp()
    app.db.Save(student{ID: "id-1", Age: 20, Name: "John Doe"})
    req, _ := http.NewRequest("DELETE", "/students/id", nil)
    req = mux.SetURLVars(req, map[string]string{"id": "id-1"})
    r := httptest.NewRecorder()
    handler := http.HandlerFunc(app.deleteStudent)

    handler.ServeHTTP(r, req)

    checkStatusCode(r.Code, http.StatusOK, t)
    checkContentType(r, t)
    checkDbIsEmpty(app.db, t)
}

func initApp() App {
    db, _ := gorm.Open("sqlite3", ":memory:")
    db.AutoMigrate(&student{})
    return App{db: db}
}

func firstStudent(app App) student {
    var all []student
    app.db.Find(&all)
    return all[0]
}

func toReader(content string) io.Reader {
    return bytes.NewBuffer([]byte(content))
}

func checkStatusCode(code int, want int, t *testing.T) {
    if code != want {
        t.Errorf("Wrong status code: got %v want %v", code, want)
    }
}

func checkContentType(r *httptest.ResponseRecorder, t *testing.T) {
    ct := r.Header().Get("Content-Type")
    if ct != "application/json" {
        t.Errorf("Wrong Content Type: got %v want application/json", ct)
    }
}

func checkProperties(st student, t *testing.T) {
    if st.Name != "John Doe" {
        t.Errorf("Name should match: got %v want %v", st.Name, "Peter Doe")
    }
    if st.Age != 20 {
        t.Errorf("Age should match: got %v want %v", st.Age, 20)
    }
}

func checkBody(body *bytes.Buffer, st student, t *testing.T) {
    var students []student
    _ = json.Unmarshal(body.Bytes(), &students)
    if len(students) != 1 {
        t.Errorf("Wrong lenght: got %v want 1", len(students))
    }
    if students[0] != st {
        t.Errorf("Wrong body: got %v want %v", students[0], st)
    }
}

func checkDbIsEmpty(db *gorm.DB, t *testing.T) {
    var students []student
    db.Find(&students)
    if len(students) != 0 {
        t.Errorf("Student has not been deleted")
    }
}

Here are the concerns I already have (not sure, how relevant they are):

I didn't manage to mock DB, that's why I am using an in-memory DB
The verification code was too verbose, I extracted it into separate functions. Is it something you would do?
I didn't manage to test negative scenarios (as a result of the first concern).
I don't like having to call mux.SetURLVars to inject a path variable. How to do this better?
I didn't manage to test the code that initializes Mux routers

Here are some notes I made while reading your code.

For a real-world code review, code should be correct, maintainable, reasonably efficient, and, most importantly, readable.

Writing code is a process of stepwise refinement.

Start by looking at two similar methods.

go-angular/server/app.go:

func (a *App) addStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    err := json.NewDecoder(r.Body).Decode(&s)
    s.ID = uuid.New().String()
    if err != nil {
        sendErr(w, http.StatusBadRequest, err.Error())
    } else {
        err = a.db.Save(&s).Error
        if err != nil {
            sendErr(w, http.StatusInternalServerError, err.Error())
        } else {
            w.WriteHeader(http.StatusCreated)
        }
    }
}

func (a *App) updateStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    err := json.NewDecoder(r.Body).Decode(&s)
    if err != nil {
        sendErr(w, http.StatusBadRequest, err.Error())
    } else {
        s.ID = mux.Vars(r)["id"]
        err = a.db.Save(&s).Error
        if err != nil {
            sendErr(w, http.StatusInternalServerError, err.Error())
        }
    }
}

Write them in more readable form.

func (a *App) addStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    var s student
    err := json.NewDecoder(r.Body).Decode(&s)
    if err != nil {
        sendErr(w, http.StatusBadRequest, err.Error())
        return
    }
    s.ID = uuid.New().String()
    err = a.db.Save(&s).Error
    if err != nil {
        sendErr(w, http.StatusInternalServerError, err.Error())
        return
    }
    w.WriteHeader(http.StatusCreated)
}

func (a *App) updateStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    var s student
    err := json.NewDecoder(r.Body).Decode(&s)
    if err != nil {
        sendErr(w, http.StatusBadRequest, err.Error())
        return
    }
    s.ID = mux.Vars(r)["id"]
    err = a.db.Save(&s).Error
    if err != nil {
        sendErr(w, http.StatusInternalServerError, err.Error())
        return
    }
    w.WriteHeader(http.StatusOK)
}

Write more readable code.

  Go Code Review Comments: Indent Error
  Flow.

  Try to keep the normal code path at a minimal indentation, and indent
  the error handling, dealing with it first. This improves the
  readability of the code by permitting visually scanning the normal
  path quickly.

Use var to declare variables that are decoding targets. They have zero value(s).

var s student

Don't do things before they are necessary.

s.ID = uuid.New().String()

The s.ID value is discarded in addStudent if there is an error in decoding. Wait until it is needed, as you did in updateStudent.

In addStudent you returned a status.

w.WriteHeader(http.StatusCreated)

Return a status in updateStudent.

w.WriteHeader(http.StatusOK)

And so on.

Make sure that your app.go code is good before worrying about testing.


Sliding Window Spliterator in Java I implemented a custom Sliding Window Spliterator in Java.
Do you have any ideas how this could be improved?

Thinking if replacing the buffer with a plain array could have a positive impact on the performance side of things if done correctly.

I'm not going to use that code in any production system, just exercising.

public class WindowSpliterator<T> implements Spliterator<Stream<T>> {

    static <T> Stream<Stream<T>> windowed(Collection<T> stream, int windowSize) {
        return StreamSupport.stream(new WindowSpliterator<>(stream, windowSize), false);
    }

    private final Queue<T> buffer;

    private final Iterator<T> sourceIterator;
    private final int windowSize;

    private WindowSpliterator(Collection<T> collection, int windowSize) {
        this.buffer = new ArrayDeque<>(windowSize);
        this.sourceIterator = Objects.requireNonNull(collection).iterator();
        this.windowSize = windowSize;
    }

    @Override
    public boolean tryAdvance(Consumer<? super Stream<T>> action) {
        if (windowSize < 1) {
            return false;
        }

        while (sourceIterator.hasNext()) {
            buffer.add(sourceIterator.next());

            if (buffer.size() == windowSize) {
                action.accept(Arrays.stream((T[]) buffer.toArray(new Object[0])));
                buffer.poll();
                return sourceIterator.hasNext();
            }
         }

        if (!buffer.isEmpty()) {
            action.accept(buffer.stream());
        }

        return false;
    }

    @Override
    public Spliterator<Stream<T>> trySplit() {
        return null;
    }

    @Override
    public long estimateSize() {
        return Long.MAX_VALUE;
    }

    @Override
    public int characteristics() {
        return ORDERED | NONNULL;
    }
}

I'm curious why you are using:

action.accept(Arrays.stream((T[]) buffer.toArray(new Object[0])));

It seems to me that action.accept(buffer.stream()) would work here, as long as the action.accept() fully processes the produced stream before returning.

Given the stream 1, 2, 3, 4 and the window size 3, the code looks like it will produce  [1,2,3], [2,3,4], and then due to the code outside the while loop, it looks like it wants to produce [3,4] as one last hurrah!

Except, tryAdvance has already returned false from return sourceIterator.hasNext(), so tryAdvance should not be called again.  So the code after the while loop looks effectively unreachable.

But it can be reached ... if the window never fill up completely in the first case, in which case it will produce exactly one stream of less than the full window size.  Is this intended behaviour?  If so, you really need to add comments explaining how that code is reached, and why it is possible not to reach it.

If less than a full window of output is possible, then shouldn't the Spliterator produce [1], [1,2], [1,2,3], [2,3,4], [3,4], [4] ... building up to the full window size and then tapering off once the end of the source stream has been reached?  Wouldn't that be more consistent?

An ArrayDeque will (based on its name) internally use an array, probably with head and tail indices.  I'm not sure how much you can improve it by implementing it with your own "plain array".  Sounds like you'd be reinventing the wheel.  Although using an array of windowSize*2-1 to avoid the wrap-around might give you a slight speedup.

estimateSize() could query the collection for its size, compute the number of windows it will be producing and return an actual value, instead of simply returning MAX_VALUE.


Very basic tuple implementation I've been messing with metaprogramming and variadic templates in C++, and I came up with this very primitive implementation of a tuple:

constexpr bool GreaterThanZero(int N)
{
    return N > 0;
}

template <int, typename...>
struct Helper;

template <int N, typename Head, typename... Tail>
struct Helper<N, Head, Tail...>
{
    typedef typename Helper<N-1, Tail...>::type type;
};

template <typename Head, typename... Tail>
struct Helper<0, Head, Tail...>
{
    typedef Head& type;
};

template <int, typename...>
class TupleImpl;

template <>
class TupleImpl<-1>
{

};

template <typename Head>
class TupleImpl<0, Head>
{
protected:
    Head head;

public:
    template <int Depth>
    Head& get()
    {
        static_assert(Depth == 0, "Requested member deeper than Tuple");
        return head;
    }

    template <int Depth>
    const Head& get() const
    {
        static_assert(Depth == 0, "Requested member deeper than Tuple");
        return head;
    }
};

template <int N, typename Head, typename... Tail>
class TupleImpl<N, Head, Tail...>
{
protected:
    Head head;
    TupleImpl<N-1, Tail...> tail;

public:
    template <int M>
    typename std::enable_if<M == 0, Head&>::type get()
    {
        return head;
    }

    template <int M>
    typename std::enable_if<GreaterThanZero(M), typename Helper<M, Head, Tail...>::type>::type get()
    {
        return tail.get<M-1>();
    }

    template <int M>
    typename std::enable_if<M == 0, const Head&>::type get() const
    {
        return head;
    }

    template <int M>
    typename std::enable_if<GreaterThanZero(M), typename Helper<M, Head, Tail...>::type>::type get() const
    {
        return tail.get<M-1>();
    }
};

template <typename... Elements>
class Tuple : public TupleImpl<sizeof...(Elements)-1, Elements...>
{
public:
    static constexpr std::size_t size()
    {
        return sizeof...(Elements);
    }
};

int main()
{
    using namespace std;

    Tuple<int, int, int, int, int, int, int, int, int, int, int, int, int, int, int, int, int> test;
    Tuple<> test2;

    test.get<0>() = 1;
    test.get<1>() = 2;

    cout << test.size() << endl;
    cout << test.get<0>() << endl;
    cout << test.get<1>() << endl;
}

Being template metaprogramming, of course it looks terrible. Are there any ways I can clean it up? I've been picking up the nitty-gritty details of how templates work by messing around with stuff like this, but I'm sure I'm missing something that would simplify this.

Below is how I would clean this up, or maybe partially re-write [live example]:

// helpers
template <typename T>
struct id { using type = T; };

template <typename T>
using type_of = typename T::type;

template <size_t... N>
struct sizes : id <sizes <N...> > { };

// choose N-th element in list <T...>
template <size_t N, typename... T>
struct Choose;

template <size_t N, typename H, typename... T>
struct Choose <N, H, T...> : Choose <N-1, T...> { };

template <typename H, typename... T>
struct Choose <0, H, T...> : id <H> { };

template <size_t N, typename... T>
using choose = type_of <Choose <N, T...> >;

// given L>=0, generate sequence <0, ..., L-1>
template <size_t L, size_t I = 0, typename S = sizes <> >
struct Range;

template <size_t L, size_t I, size_t... N>
struct Range <L, I, sizes <N...> > : Range <L, I+1, sizes <N..., I> > { };

template <size_t L, size_t... N>
struct Range <L, L, sizes <N...> > : sizes <N...> { };

template <size_t L>
using range = type_of <Range <L> >;

// single tuple element
template <size_t N, typename T>
class TupleElem
{
    T elem;
public:
    T&       get()       { return elem; }
    const T& get() const { return elem; }
};

// tuple implementation
template <typename N, typename... T>
class TupleImpl;

template <size_t... N, typename... T>
class TupleImpl <sizes <N...>, T...> : TupleElem <N, T>...
{
    template <size_t M> using pick = choose <M, T...>;
    template <size_t M> using elem = TupleElem <M, pick <M> >;

public:
    template <size_t M>
    pick <M>& get() { return elem <M>::get(); }

    template <size_t M>
    const pick <M>& get() const { return elem <M>::get(); }
};

template <typename... T>
struct Tuple : TupleImpl <range <sizeof...(T)>, T...>
{
    static constexpr std::size_t size() { return sizeof...(T); }
};

Comments:

With a bit more infrastructure (helper structs that are typically reused here and there) at the beginning, the main tuple implementation becomes just 20 lines.
Instead of a recursive implementation, I have switched to multiple (variadic) inheritance of single tuple elements TupleElem, each with its own id N (so that each element is of unique type) and its own function get(). Hence Tuple's function get() just redirects to the appropriate base class.
Now a specialization of TupleElem for empty types can bring easily the desired empty base optimization without affecting the main tuple implementation.
No specialization is needed for empty tuple. This definition includes empty tuple as a special case.
No static assertions needed, an out-of-range index will give an error anyway (but you can add for better messages of course).

Now, there are so many things missing. I would start from support for rvalue references and, of course, constructors. If you want to get an idea, you can have a look at my own tuple implementation (of which this one here is a miniature) including tuple views, expression templates for lazy evaluation, loops, algorithms, integration with all C++ operators, and much more.

Template metaprogramming does not have to look terrible!


Calculate the centroid of a collection of complex numbers In working on another problem, one component I needed was to calculate the centroid of a collection of complex objects.  The well-known way to calculate this is to simply average the real and imaginary parts.  To accommodate either std::complex<double> or std::complex<float>, I have created the following code as a template.  Everything works as expected.
centroid.cpp
#include <iostream>
#include <complex>
#include <vector>
#include <numeric>

template<class ComplexIterator>
typename ComplexIterator::value_type centroid(ComplexIterator a, ComplexIterator b) {
    return std::accumulate(a, b, typename ComplexIterator::value_type{0})
            / typename ComplexIterator::value_type{b-a};
}

int main() {
    std::vector<std::complex<float>> points{ {4,5}, {30,6}, {20,25} };
    std::cout << centroid(points.begin(), points.end()) << "\n";
}

Sample output:

(18, 12)

My questions

The compiler rightly warns that in the calculation of the denominator in the template, there is a narrowing conversion.  I can't think of a way to elegantly handle that.  Should I just ignore it?
If given an empty set, the code returns a value of (-nan, -nan) which works for me, but for general use, should I throw an exception instead?
Should I use std::enable_if or C++20 requires to constrain the function to require only floating point numbers?
I thought about calling it average because technically, it would happily compute the average of, say, a std::vector<float> but for my purposes, centroid seemed apt.  What do you think of that choice?

Any other ways to improve this would be good as well.

Taking all of the suggestions from the other reviews, I think it's quite improved so I wanted to thank the reviewers and show the final version:
#include <iostream>
#include <complex>
#include <vector>
#include <numeric>
#include <concepts>
#include <ranges>
#include <array>
#include <iterator>

template<typename T>
concept ComplexOrFloat = std::floating_point<T>
    || std::floating_point<typename T::value_type>;

template<typename R>
requires std::ranges::range<R>
    && ComplexOrFloat<std::ranges::range_value_t<R>>
constexpr std::ranges::range_value_t<R> average(R range) {
    if (std::ranges::size(range) == 0)
        throw std::domain_error("Cannot take average of zero-length range");
    using T = std::ranges::range_value_t<R>;
    return std::accumulate(std::ranges::begin(range), std::ranges::end(range), T{0})
        / (T)(std::ranges::size(range));
}

int main() {
    using namespace std::complex_literals;
    constexpr std::array<std::complex<double>,3> points{{ {4,5}, {30,6}, {20,25} }};
    std::cout << average(std::views::all(points)) << "\n";

    constexpr std::array<std::complex<double>,3> points2{
        {  24.0 + 7.0i, 22.0 + 9.0i, 8.0 + 20.0i }
    };
    std::cout << average(std::views::all(points2)) << "\n";

    constexpr double numbers[]{1, -2, 3, 7, -8};
    std::cout << average(std::views::all(numbers)) << "\n";

    const std::vector<float> numbers2{-7, -2, 3, 7, -8};
    std::cout << average(std::views::all(numbers2)) << "\n";

    const std::vector<float> mt{};
    try {
        std::cout << average(std::views::all(mt)) << "\n";
    } catch (std::domain_error& err) {
        std::cerr << "ERROR: " << err.what() << "\n";
    }

#if 0
    int intnumbers[]{1, -2, 3, 7, -8};
    // won't compile because an `int` is not floating point or complex floating point
    std::cout << average(std::views::all(intnumbers)) << "\n";
#endif
}

I'm using C++20 concepts to avoid trying to take an average of a range of ints and returning an int.