Sudoku verifier program I implemented a Sudoku validation program.
Can this be optimized even further in terms of performance?

public class ValidSudoku {
    public boolean verifySudoku(int arr[][], int num, int i, int j, int row, int col) {
        return (verifyRow(arr, num, i, j, col)
               && verifyCol(arr, num, i, j, row)
               && verifyGrid(arr, num, i, j));

    }

    public boolean verifyRow(int arr[][], int num, int i, int j, int col) {
        int k;
        for (k = 0; k < col; k++) {
           if ((arr[i][k] == num) && (j != k)) {
                return false;
            }
        }
        return true;
    }

    public boolean verifyCol(int arr[][], int num, int i, int j, int row){
        int k;
        for (k = 0; k < row; k++) {
            if ((arr[k][j] == num) && (i != k)) {
                return false;
            }
        }
        return true;
    }

    public boolean verifyGrid(int arr[][], int num, int i, int j) {
        int k, l;
        int start_row = i - (i % 3);
        int start_col = j - (j % 3);
        for (k = 0; k < 3; k++) {
            for (l = 0; l < 3; l++) {
                if ((arr[k + start_row][l + start_col] == num) && (i != k + start_row) && (j != l + start_col)) {
                    return false;
                }
            }
        }
        return true;
    }

    public static void main(String args[]) {
        ValidSudoku v = new ValidSudoku();
        int i, j;
        int arr[][] = {{3, 1, 6, 5, 7, 8, 4, 9, 2},
                       {5, 2, 9, 1, 3, 4, 7, 6, 8},
                       {4, 8, 7, 6, 2, 9, 5, 3, 1},
                       {2, 6, 3, 4, 1, 5, 9, 8, 7},
                       {9, 7, 4, 8, 6, 3, 1, 2, 5},
                       {8, 5, 1, 7, 9, 2, 6, 4, 3},
                       {1, 3, 8, 9, 4, 7, 2, 5, 6},
                       {6, 9, 2, 3, 5, 1, 8, 7, 4},
                       {7, 4, 5, 2, 8, 6, 3, 1, 9}};
        for (i = 0; i < 9; i++) {
            for (j = 0; j < 9; j++) {
                if (!v.verifySudoku(arr, arr[i][j], i, j, 9, 9)) {
                    System.out.print(arr[i][j] + "nay");
                }
            }
        }
    }
}

The methods verifyRow, verifyCol, verifyGrid, and verifySudoku are all only accessed inside this class. Therefore, there is no point in making them public right?

If there do happen to be other classes, just they aren't shown here, you could probably leave verifySudoku as public because it makes sense for another class to possible access those.

I think Sudoku should be it's own, immutable, separate class. It should probably hold the 2D int array that is the sudoku board.

Now thinking about, the should be the class with the method verifySudoku (I'd call it isValid) because the method is to validate a sudoku board and the Sudoku class is holding a board to being with.

Here is what I came up with for the class:

public class Sudoku {
    private final int[][] board;
    private final int rows;
    private final int columns;

    public Sudoku(int[][] board, int rows, int columns) {
        this.board = board;
        this.rows = rows;
        this.columns = columns;
    }

    public boolean isValid(...) {
        [code]
    }

    private boolean verifyGrid(...) {
        [code]
    }

    private boolean verifyRow(...) {
        [code]
    }

    private boolean verifyCol(...) {
        [code]
    }
}

public boolean verifyRow(int arr[][], int num, int i, int j, int col) {

The argument names i and j are not descriptive whatsoever.

int k;
for (k = 0; k < col; k++) {

You do not need to declare the iterator variable outside of the loop. This can and should be turned in to:

for (int k = 0; k < col; k++) {

I think you have a very smart algorithm for verifying if the board.

Here is the way I thought of:

Create an java.util.ArrayList of the numbers 1-9.
In each verify method, copy this ArrayList.
Iterate through the row/column/grid's squares
If the number is in the array list, go to 6
If not, return false
Take the number out of the array list
If the array list is empty, return true
Go to 3

(I believe) This works because, by removing the numbers from the array list, you are making sure that that number is not repeated again in that row, which would make a row invalid if it contained 2+ of the same number.

Here is my implementation in Java:

private boolean verifyRow(int row) {
    List<Integer> numbers = new ArrayList<Integer>(this.allNumbers);

    for(int i = 0; i < this.rows; i++) {
        if(numbers.contains(i)) {
            numbers.remove(new Integer(i)); // using java.util.ArrayList.remove(Object o) so I turned int i to an Integer
            if(numbers.isEmpty()) {
                return true;
            }
        } else {
            return false;
        }
    }
}

This used my Sudoku class idea.
allNumbers is the list of numbers 1-9 to copy.

I am not sure which version is faster.


Inline assembly correctness (especially __volatile__) Here's my inline-assembly functions, which is used my x86 32bit kernel.

static inline uint8_t ckPortInByte(uint16_t port)
{
    uint8_t ret;
    __asm__ __volatile__ ( "in %1, %0" : "=a"(ret) : "d"(port) );
    return ret;
}

static inline void ckPortOutByte(uint16_t port, uint8_t data)
{
    __asm__ __volatile__ ( "out %0, %1" : : "a"(data), "d"(port) );
}

static inline uint32_t ckAsmGetEFlag()
{
    uint32_t ret;
    __asm__ __volatile__ ( "pushfl \n pop %0" : "=g"(ret) );
    return ret;
}

static inline void ckAsmSetEFlag(uint32_t eflag)
{
    __asm__ __volatile__ ( "push %0 \n popfl" : : "g"(eflag) );
}

static inline uint32_t ckGetCr0()
{
    uint32_t ret;
    __asm__ ( "mov %%cr0, %0" : "=r"(ret) );
    return ret;
}

static inline void ckSetCr0(uint32_t value)
{
    __asm__ ( "mov %0, %%cr0" : : "r"(value) );
}

static inline uint32_t ckGetCr3()
{
    uint32_t ret;
    __asm__ ( "mov %%cr3, %0 " : "=r"(ret) );
    return ret;
}

static inline void ckSetCr3(uint32_t value)
{
    __asm__ ( "mov %0, %%cr3" : : "r"(value) );
}

static inline void ckAsmCpuid(uint32_t code, CpuidStruct *cs)
{
    __asm__
    (
        "cpuid"
        : "=a"(cs->eax), "=b"(cs->ebx), "=c"(cs->ecx), "=d"(cs->edx)
        : "a"(code)
    );
}

static inline void ckAsmCli()
{
    __asm__ __volatile__ ( "cli" );
}

static inline void ckAsmSti()
{
    __asm__ __volatile__ ( "sti" );
}

static inline void ckAsmStiHlt()
{
    __asm__ __volatile__ ( "sti \n hlt" );
}

static inline void ckAsmHlt()
{
    __asm__ __volatile__ ( "hlt" );
}

It seems that everything is ok.

However, recently I had a bug, which is related to these functions, especially __volatile__ correctness. (Can you see __volatile__ in ckPortInByte and ckPortOutByte? They didn't exist before I fixed it...)

So, I'm really afraid now - Is there anything wrong? (i.e. unnecessary or missing __volatile__)

Your use of volatile appears to be correct, but I think you should do a couple of things for safety:

Add a clobber constraint for the flags (AKA "condition codes") to ckAsmSetEFlag. I believe this is "cc" for x86 machines.
Add clobber constraints for CR0 and CR3 to ckSetCr0 and ckSetCr3, respectively. [edit] This appears to be impossible with GCC. Not sure if this is a problem or not.[/edit]

While GCC's inline assembly syntax is very powerful, it can be horribly unintuitive. Since you're writing a kernel and messing with control registers, you really should be checking the assembly listings for every module you compile, to verify that the compiler is honouring your intentions.


Is there a better way to thread this class function? I have a class bar that keeps track of N instances of class foo in a std::map (so N = map.size()).
When I call bar::func I want to have N threads that call foo::foo_func.

foo::foo_func requires multiple arguments though, namely the instance of bar that it's related to.

I was thinking of doing something like:

void * _threaded_foo_func(void *);

struct box {
  bar * the_bar;
  foo * the_foo;
};

class bar {
  class sub_bar {
    // stuff
  };

  map<foo*, sub_bar*> foo_mappings;

  void func() {
    pthread_t threads[ foo_mappings.size() ];
    map<foo*, sub_bar*>::iterator it = foo_mappings.begin();
    int i=0;
    for(it; it != foo_mappings.end() && i < foo_mappings.size(); ++it, ++i) {
      box * args = new args();
      args->the_bar = this;
      args->the_foo = it->first;
      pthread_create( &threads[i], NULL, _threaded_foo_func, (void*) args);
    }

    for(int i=0; i<bar_mappings.size(); ++i) {
      pthread_join(threads[i], NULL);
    }

  }

};

void * _threaded_foo_func(void * args) {
  box * b = (box*) args;
  bar * the_bar = b->the_bar;
  foo * the_foo = b->the_foo;
  the_foo->foo_func(the_bar);
  return NULL;
}

My questions are:

Is there a better way to do this? Cleaner? Thoughts on using
fork()?
Does this look like a poor design of the relationship
between foo & bar?

To make this fun, you're only allowed to use pthread.h, no C++11 stuff :)

How many instances of foo are you going to have?  Spawning a large number of threads or forking a large number of processes isn't going to scale well in terms of both CPU and memory resources.

Another approach that generally has better scalability is a thread pool.  The basic idea is that you have a fixed number of threads that pull and execute tasks from a queue, resulting in better resource utilization.


JSON variable renamer I made this program to rename variables in json files, part of the functions rely on recursivity which i'm not confortable with so I would like some feedback on readability and performance.

import configparser

def seek_in_array(json_array, conversions):
    """seeks all data in a json array"""
    converted = []
    for it_list in range(len(json_array)):
        if isinstance(json_array[it_list], dict):
            converted.append(seek_and_convert(json_array[it_list], conversions))
        elif isinstance(json_array[it_list], list):
            converted.append(seek_in_array(json_array[it_list], conversions))
        else:
            converted.append(json_array[it_list])
    return(converted)

def seek_and_convert(json_file, conversions):
    """seeks all data in a json file an converts it"""
    converted = {}
    for key in json_file.keys():
        if conversions.has_option('CONVERSIONS', key):
            option = conversions.get('CONVERSIONS', key)
        else:
            option = key
        if isinstance(json_file[key], dict):
            converted[option] = seek_and_convert(json_file[key], conversions)
        elif isinstance(json_file[key], list):
            converted[option] = seek_in_array(json_file[key], conversions)
        else:
            converted[option] = json_file[key]
    return(converted)

def load_conversion_table():
    """loads the configfile containing the conversions to be made"""
    conversions = configparser.ConfigParser()
    conversions.read('conversion_table.ini')
    return conversions

def main():
    conversions = load_conversion_table()
    file = {
              '_id': '006480206',

              'change1': 'HEY'
              'A_THING': 'HEY'
              'an_array': [
                {
                  'A_THING_in_the_array': '00648020600017',
                  'oh_no_,_change_that':'O',
                  'plz_dont_change_me': '12',
                  'listenning_to_disco_music': True,
                  'le_list': {
                    'le_variable': 'baguette !',
                    'le_change': 42,
                  },
                  "that's a trap": {},
                  "change_it_anyway": {},
                  'another_array': [
                    {
                      'change1': 'HANS !',
                      'change2': 'JA !?'
                    },
                    {
                      'nei': 'yas'
                      'change1': 'the very test',
                    },
                    {
                      'change1': 'ran out of idea',
                      'yas_yas': 'nomnomnom'
                    },
                    {
                      'OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOH': 'such long name'
                    }
                  ]
                }
              ]
            }
    print(seek_and_convert(file, conversions))

if __name__ == "__main__":
    main()

Here is the config file you'll need to run the program, put both file in the same folder.
conversion_table.ini:

[CONVERSIONS]
change1 = success1
oh_no_,_change_that = oh_no_,_success_that
plz_dont_change_me = plz_dont_success_me
le_change = le_success
change_it_anyway = success_it_anyway
change2 = success2

looping

Don't loop over indices. the iterattion used by Python is of such simplicity and elegance, and there are some really useful builtin helper functions, that looping over the index (or keys for a dict) is hardly ever necessary

I suggest you check out the excellent 'Looping like a Pro' talk by David Baumgold

def seek_and_convert(json_array, conversions):
    """seeks all data in a json array"""
    converted = []
    for item in json_array:
        if isinstance(item, dict):
            converted.append(seek_and_convert(item, conversions))
        elif isinstance(json_array[it_list], list):
            converted.append(seek_in_array(item, conversions))
        else:
            converted.append(item)
    return(converted)

the same for the dict

def seek_in_array(json_file, conversions):
    """seeks all data in a json file an converts it"""
    converted = {}
    for key, item in json_file.items():
        if conversions.has_option('CONVERSIONS', key):
            option = conversions.get('CONVERSIONS', key)
        else:
            option = key
        if isinstance(item, dict):
            converted[option] = seek_and_convert(item, conversions)
        elif isinstance(item, list):
            converted[option] = seek_in_array(item, conversions)
        else:
            converted[option] = item
    return(converted)

conversions.get

ConfigParser.get has a fallback argument, so instead of

if conversions.has_option('CONVERSIONS', key):
    option = conversions.get('CONVERSIONS', key)
else:
    option = key

This, together with the use of keyword arguments, can be converted to:

option = conversions.get(section='CONVERSIONS', option=key, fallback=key)

For testing and future expansio, it would also make more sense to pass the conversions on as a dict, instead as a ConfigParser object. You use it as a dict anyway.
Together with Hoist the IO, you get something like:

with StringIO(config_str) as config_file:
    config_parser = ConfigParser()
    config_parser.read_file(config_file)
    conversions = dict(config_parser.items(section="CONVERSIONS"))

In your real app, you can use with open(file, "r") as config_file instead of the StringIO

Then further on, all of this:

if conversions.has_option('CONVERSIONS', key):
    option = conversions.get('CONVERSIONS', key)
else:
    option = key

becomes:

option = conversions.get(key, key)

In contrast to your current load_conversion_table method, where you've hardcoded the filename, return a ConfigParser, and need to specify the section each time.

generators

Instead of keeping an intermediary dict or list, you can also yield the items:

def convert_dict(json_file, conversions):
    """seeks all data in a json file an converts it"""
    for key, item in json_file.items():
        option = conversions.get(key, key)
        if isinstance(item, dict):
            yield option, dict(seek_and_convert(item, conversions))
        elif isinstance(item, list):
            yield option, list(seek_in_array(item, conversions))
        else:
            yield option, item

dispatching

Instead of a chain of isinstance conditions, where depending on the type, a certain method is called with the same arguments, you can use the fact that functions are first-class citizens in Python and build a dict of parsers:

from collections import defaultdict

def fallback_parser():
    def parser(item, conversion):
        """parser that just returns the item, without conversion"""
        return item
    return parser

PARSERS = defaultdict(fallback_parser)
PARSERS[dict] = lambda item, conversion: dict(convert_dict(item, conversion))
PARSERS[list] = lambda item, conversion: list(convert_array(item, conversion))
PARSERS

and the parser functions become as simple as:

def convert_dict(json_dict, conversions):
    for key, item in json_dict.items():
        option = conversions.get(key, key)
        yield option, PARSERS[type(item)](item, conversions)

def convert_array(json_array, conversions):
    for item in json_array:
        yield PARSERS[type(item)](item, conversions)

If, later, you want support for datetime or other types, etc, this can be added by simply writing the parser and extending the PARSERS dict.

functools.singledispatch

An alternative is functools.singledispatch

from functools import singledispatch
@singledispatch
def parser(item, conversions):
    """parser that just returns the item, without conversion"""
    return item

@parser.register
def _(json_dict: dict, conversions):
    result = {}
    for key, item in json_dict.items():
        option = conversions.get(key, key)
        result[option] = parser(item, conversions)
    return result

@parser.register
def _(json_array: list, conversions):
    result = []
    for item in json_array:
        result.append(parser(item, conversions))
    return result

If you prefer the generator approach, you can do something like:

@singledispatch
def parser(item, conversions=None):
    """parser that just returns the item, without conversion"""
    return item

def convert_dict(json_dict: dict, conversions: dict = None):
    conversions = {} if conversions is None else conversions
    for key, item in json_dict.items():
        option = conversions.get(key, key)
        yield option, parser(item, conversions)

def convert_array(json_array: list, conversions: dict = None):
    for item in json_array:
        yield parser(item, conversions)

parser.register(
    dict, lambda item, conversions=None: dict(convert_dict(item, conversions))
)
parser.register(
    list, lambda item, conversions=None: list(convert_array(item, conversions))
)


Speeding up spectrum analysis I'm trying to speed up this code that loops over an entire spectrum range. It's using a Raspberry Pi, and a rtl-sdr dongle that covers up to around 1.7 GHz. My goal is to be sweep over that spectrum and gather data from it in a timely manner.

#!bin/usr/python

#gathers from the rtl library needed for this program
from rtlsdr import RtlSdr
import math
from pylab import *
import time
from scipy.fftpack import fft, ifft
import pyqtgraph as pg
from pyqtgraph.Qt import QtGui, QtCore
import numpy as np
import os
import sys

os.system("sudo rmmod dvb_usb_rtl28xxu rtl2832")

#RtlSdr set up
##sdr = RtlSdr()
##
##
##num_samples = 4096
##
##freq_range_lo = 50e6     #experimental center freq for now
##freq_range_hi = 600e6  #1750e6
##freq_step = 1e6

##sample_rate = 2e6
##
###Set initial parameters for RTL-SDR Dongle
##sdr.sample_rate = sample_rate
##freq = 512e6
##sdr.center_freq = freq
##
##
##sdr.freq_correction = 102
##sdr.gain = 0

class Spec_Scan(object):

    samples = np.array([])
    samples_temp = np.array([])
    fft_data = np.array([])
    fft_data_temp = np.array([])

    app = QtGui.QApplication([])
    win = pg.GraphicsWindow(title="Basic plotting examples")
    win.resize(1000,600)
    win.setWindowTitle('pyqtgraph example: Plotting')

    def __init__(self, lo, hi, freq, step, samps):
        self.freq_range_lo = lo
        self.freq_range_hi = hi
        self.freq_step = step
        self.freq = freq
        self.num_samples = samps
        self.sample_rate = 2e6

        self.sdr = RtlSdr()
        self.sdr_default()

    def sdr_default(self):
        self.sdr.center_freq = self.freq
        self.sdr.freq_correction = 101
        self.sdr.gain = 0
        self.sdr.sample_rate = self.sample_rate

    def pyqt(self):
        global curve, p6
        # Enable antialiasing for prettier plots
        pg.setConfigOptions(antialias=True)

        self.win.nextRow()
        p6 = self.win.addPlot(title="Freq plot")

        self.curve = p6.plot(pen='y', clear=True)
        #p6.setRange(xRange = [0, (self.num_samples/2)*(1700e6/self.freq_step)], yRange = [-100,100])
        p6.setRange(xRange = [0, (self.num_samples/2)-48], yRange = [-100,100])

        print 'here'
        timer = pg.QtCore.QTimer()
        print'should begin update'
        timer.timeout.connect(self.update)
        timer.timeout.emit()
        print 'update should have finished'
        timer.start(2)

        if __name__ == '__main__':

            if (sys.flags.interactive != 1) or not hasattr(QtCore, 'PYQT_VERSION'):
                QtGui.QApplication.instance().exec_()

    def update(self):
        global curve, p6

        for self.freq in range(int(self.freq_range_lo), int(self.freq_range_hi), int(self.freq_step)):

            self.sdr.center_freq = self.freq
            self.samples = self.sdr.read_samples(self.num_samples)
            self.fft_data_temp = 20*np.log10(np.abs(np.fft.rfft(self.samples)))
            #self.fft_data_temp = self.fft_data_temp[0:2049]
##            self.fft_data_temp = self.fft_data_temp[1:(self.num_samples/2)]
##            self.fft_data_temp = 2*self.fft_data_temp[2::]
            self.fft_data = np.append(self.fft_data, self.fft_data_temp)
            self.fft_data_temp = 0

        #self.curve.setData(self.fft_data[1::], clear=True)
        self.fft_data = 0

        print 'refresh'

def main():
    num_samples = 4096

    lo = 50e6     #experimental center freq for now
    hi = 1750e6
    step = 1e6
    freq = 425e6

    test = Spec_Scan(lo, hi, freq, step, num_samples)
##    test.pyqt()
    start = time.time()
    test.update()
    end = time.time()
    print str(end-start)

## Start Qt event loop unless running in interactive mode or using pyside.

main()

When the scan itself was timed, the result yielded around 197 seconds which is entirely too long. Is there a way I can speed up the spectrum scan?

I'll add my small contribution even though I'm not really a Python developer (hopefully some day). I've taken your update method and tested it on my computer (desktop, i3-4360@ 3.70GHz × 4 ), without the SDR bit.  And my intuition was right!

Like in most languages allowing you to do it, don't ever do that:

self.fft_data = np.append(self.fft_data, self.fft_data_temp)

The reason is pretty simple: your array does not have a predefined size, so for each call to append, it has to re-allocate the memory to fit the new size. And that's terribly slow.

Let's talk numbers. Without this line of code, doing the FFT and converting the data to dB takes 300ms on my computer. Adding that line, it almost reaches 9s.

So now that we have found this, what to do about it for your application ? It seems that you would like to display the spectrum from 50MHz to 1750MHz, by decomposing it into signals with a 1MHz bandwidth. And for each slice, you generate 4096 samples. I don't know what kind of display you have, but displaying 4096 * 1700 FFT bins is gonna be difficult (especially in a 1000*600 window). You should restrict the size of the spectrum you want to display at once. And speaking just about bands, displaying the end of the HF band, the VHF band and part of the UHF band is not really gonna be interesting. Mostly because you're gonna have trouble finding an antenna that can cover all these bands simultaneously.


Damerau Levenshtein Implementation in Kotlin This is an implementation of the Damerau-Levenshtein distance in Kotlin which I created as an exercise, but might be also useful, if it proves to be correct.

The implementation is based on this Wikipedia article, even though it does not 100% follow the suggested pseudo-code.

fun damerauLevenshteinCount(a: CharSequence, b: CharSequence): Int {
    return damerauLevenshtein(a, b).first
}

/**
 * According to Wikipedia:
 * <cite>Damerau–Levenshtein distance between two words is the minimum number of operations (consisting of insertions,
 * deletions or substitutions of a single character, or transposition of two adjacent characters) required to change one word into the other</cite>
 */
fun damerauLevenshtein(a: CharSequence, b: CharSequence): Pair<Int, Array<IntArray>> {
    requireNotNull(a) { "First string should not be null. Make sure you specify the first string." }
    requireNotNull(b) { "Second string should not be null. Make sure you specify the second string." }
    val aLength = a.length
    val bLength = b.length
    val d = Array(aLength + 1, { IntArray(bLength + 1) })
    val daMap = hashMapOf<Char, Int>()
    val maxdist = a.length + b.length
    for (i in 0..aLength) {
        d[i][0] = i
    }
    for (j in 0..bLength) {
        d[0][j] = j
    }
    for (i in 1..aLength) {
        var db = 0
        for (j in 1..bLength) {
            val k = daMap.getOrDefault(b[j - 1], 0)
            val l = db
            var cost = 0
            if (a[i - 1] == b[j - 1]) {
                db = j
            } else {
                cost = 1
            }
            val substitution = d[i - 1][j - 1] + cost
            val insertion = d[i][j - 1] + 1
            val deletion = d[i - 1][j] + 1
            val transposition = if (k == 0 || l == 0) maxdist else d[k - 1][l - 1] + (i - k - 1) + 1 + (j - l - 1)
            d[i][j] = intArrayOf(substitution, insertion, deletion, transposition).min() as Int
        }
        daMap[a[i - 1]] = i
    }
    return Pair(d[aLength][bLength], d)
}

Here is a unit test which shows the behaviour of this implementation:

@Test
fun damerauLevenshteinDistanceCount() {
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("h1", "h1")).isEqualTo(0)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("gil", "gil")).isEqualTo(0)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("gil", "gill")).isEqualTo(1)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("ca", "abc")).isEqualTo(2) // Differs from optimal string alignment distance distance
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("waht", "what")).isEqualTo(1)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("thaw", "what")).isEqualTo(2)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("waht", "wait")).isEqualTo(1)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("Damerau", "uameraD")).isEqualTo(2)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("Damerau", "Daremau")).isEqualTo(2)
    Assertions.assertThat(wagnerDamerau.damerauLevenshteinCount("Damerau", "Damreau")).isEqualTo(1)
    Assertions.assertThat(wagnerDamerau.osaDistanceCount("waht", "whit")).isEqualTo(2)
    Assertions.assertThat(wagnerDamerau.osaDistanceCount("what", "wtah")).isEqualTo(2)
}

I want to check for its correctness as well if I am using Kotlin in the best possible idiomatic manner.

For determining correctness of your implementation I recommend referring to an existing implementation such as Apache commons-lang or the newer commons-text:

commons-lang:

StringUtils.java:7997-8254
StringUtilsTest.java:2378-2479

commons-text

LevenshteinDistance.java
LevenshteinDistanceTest.java
ParameterizedLevenshteinDistanceTest.java
LevenshteinDetailedDistance.java
LevenshteinDetailedDistanceTest.java
LevenshteinResults.java

As far as your Kotlin code goes, here are my comments:

Kotlin already guarantees that a and b are not null as you've used CharSequence for their type instead of CharSequence?. As such, the requireNotnull statements are unnecessary.
I do not believe you gain much from aLength and bLength. a.length and b.length read clearly and are read in constant time so the additional variables seem unnecessary to me.
You can place the last lambda argument in a function call into parentheses  as you have done but I personally find moving it out of the parentheses easier to read and more idiomatic:

val d = Array(a.length + 1, { IntArray(b.length + 1) })

becomes

val d = Array(a.length + 1) { IntArray(b.length + 1) }

In some languages variable names like maxdist are common and expected but in Kotlin (and Java) I find it more common and expected to have variables names avoid abbreviations, use camel casing, etc. I recommend using maxDistance instead of maxdist.
You are returning a Pair<Int, Array<IntArray>>. This is fine but data classes are so much better and Kotlin makes it so easy to define and use them. I recommend creating your own type to return. e.g.:

data class DamerauLevenshteinResult(val count: Int, val d: Array<IntArray>)


SafeBox StateMachine implemented with Enums does someone has any opinions about this SafeBox StateMachine?

import java.util.Scanner;

public class SafeBoxStateMachine {

    public enum SafeBox {

        ACCEPT{
            private Scanner scan;
            private boolean askPassword(SafeBoxStateMachine safebox){

                System.out.println("Please type a password:");
                scan = new Scanner(System.in);
                String buffer = scan.nextLine();

                if (buffer.equals(safebox.password) == false){
                    ++safebox.counter;
                    return false;
                }
                else {
                    return true;
                }
            }
            public void process(SafeBoxStateMachine safebox){

                boolean res = askPassword(safebox);

                if (res == false)
                    System.out.println("try number:" + safebox.counter);

                if (safebox.counter == 3){

                    safebox.state = SafeBox.PENELTY;
                }

                if (res == true)
                {
                    safebox.state = SafeBox.SUCCESS;
                }
            }
        },
        PENELTY{
            public void process(SafeBoxStateMachine safebox){
                System.out.println("7 seconds penalty!");
                try{
                    Thread.sleep(7000);
                }
                catch(InterruptedException ex) {
                    Thread.currentThread().interrupt();
                }
                safebox.counter = 0;
                safebox.state = SafeBox.ACCEPT;
            }
        },
        SUCCESS{
            public void process(SafeBoxStateMachine safebox){

                System.out.println("Success!");
                safebox.stop = true;
            }

        };

        abstract public void process(SafeBoxStateMachine safebox);
    }

    int counter = 0;
    private SafeBox state = SafeBox.ACCEPT;
    String password;
    boolean stop = false;

    SafeBoxStateMachine (String password){
        this.password = password;
        counter = 0;
    }

    public void run(){
        while (stop == false)
            this.state.process(this);
    }

    public void setPass(String pass){
        this.password = pass;
    }

    public static void main(String[] args) {

        SafeBoxStateMachine pass = new SafeBoxStateMachine("hello");

        pass.run();

    }

}

Make fields immutable when possible

The setPass method is never used in the posted code.
As such, you can make the password field immutable:

private final String password;

Reuse Scanner

ACCEPT.askPassword will recreate the Scanner scan field from System.in every time it's called. This is unnecessary, it would be better to create the scanner only once.
That also means that you can make scan a final field.

Use boolean expressions directly

Instead of code like this:

if (res == false)
// ...

if (res == true)
// ...

Use boolean expressions directly like this:

if (!res)
// ...

if (res)
// ...

Do this everywhere where you used something == true or false.

Combine related if statements

In this code there are two conditions on the value of res:

if (res == false)
    System.out.println("try number:" + safebox.counter);

if (safebox.counter == 3){

    safebox.state = SafeBox.PENELTY;
}

if (res == true)
{
    safebox.state = SafeBox.SUCCESS;
}

The code could be reorganized to make it clearer and to avoid one unnecessary condition:

if (safebox.counter == 3) {
    safebox.state = SafeBox.PENELTY;
}

if (res) {
    safebox.state = SafeBox.SUCCESS;
} else {
    System.out.println("try number:" + safebox.counter);
}

And since safebox.counter is only incremented in case of failure,
the penalty condition can be moved inside the else branch of the second if statement:

if (askPassword(safebox)) {
    safebox.state = SafeBox.SUCCESS;
} else {
    System.out.println("try number:" + safebox.counter);
    if (safebox.counter == 3) {
        safebox.state = SafeBox.PENELTY;
    }
}


Python preprocesser that allows "until" and "unless" statements I've implemented the beginnings of a Python preprocesser I plan to implement sometime in the future, and the below code is my simple prototype. At the moment, it only converts until and unless statements.

py_preproc.py

"""
A simple Python utility that converts
non-valid Python code, like `until`,
and `unless` blocks to valid Python
code.
"""
from sys import argv

try:
    file_path = argv[1]
    execute_results = argv[2]
except IndexError:
    raise Exception("Two argv arguments are required, file_path, and execute_results.")

def open_code_file():
    """
    Opens the code file to do the conversion
    on, and returns the read version as a
    string.
    """
    with open(file_path, "r+") as code_file:
        return code_file.read()

def replace_items(file_string):
    """
    Replace specific pieces of the code_file
    with valid Python code. Currently only
    `until`, and `unless` blocks are replaced.
    """
    return file_string.replace(
        "until", "while not"
    ).replace(
        "unless", "if not"
    )

def evaluate_result(result_string):
    """
    Evaluates the converted result, after
    the code has been converted to valid
    Python code.
    """
    py_string_compiled = compile(result_string, "fakemodule", "exec")
    exec(py_string_compiled)

def main():
    file_string = open_code_file()
    new_file_string = replace_items(file_string)

    if execute_results == "-true":
        evaluate_result(new_file_string)

    elif execute_results == "-false":
        with open(file_path) as file_to_write:
            file_to_write.truncate()
            file_to_write.write(new_file_string)

    else:
        raise Exception("Invalid argument \"{argument}\".".format(argument=execute_results))

if __name__ == '__main__':
    main()

For reference, here's an example of a code file using until and unless, before it's preprocessed.

iterator = 10

until iterator == 0:
    print(iterator)
    iterator -= 1

unless iterator != 0:
    print("iterator == 0")

The above example is then converted to this:

iterator = 10

while not iterator == 0:
    print(iterator)
    iterator -= 1

if not iterator != 0:
    print("iterator == 0")

Finally, here's the command line syntax for running the command, where file_to_execute_or_convert.py is the Python file that you want to run or convert, and execute_or_write is -true if you want to just execute it, or -false if you want to convert it.

python py_preproc.py [file_to_execute_or_convert.py] [execute_or_write]

Remco Gerlich is quite right to point out that this preprocessor does not work on general Python code, because it uses string replacement. Thus unless will be changed to if not in strings (such as regular expressions, templates, and docstrings), potentially breaking the code. The preprocessor itself is just one example of the kind of code which will be broken: it is not good enough to respond that this concern is "silly".
Additionally, Bakuriu is quite right to point out that the transformation of unless to if not is inadequate: it only works on very simple examples. But consider:

a unless b or c else d

which would be changed to:

a if not b or c else d

where the condition is wrong, because not b or c parses as (not b) or c. The correct transformation is:

a if not (b or c) else d

This problem makes it hopeless to attempt this preprocessing at the level of text, or even of tokens: you need a parser to find the end of the condition. Consider an expression like:

a unless b or (c unless d and e else f) else g

which needs to be transformed into:

a if not (b or (c if not (d and e) else f)) else g


Backup document root and MySQL in Linux I wrote this program to backup everything in document root and the MySQL database in Linux environments. It is aimed for Debian/Ubuntu environments to say the least.

zip -r /var/www/html/html-$(date +\%F-\%T).zip /var/www/html -x "*/cache/*"

mysqldump -u root -p --all-databases > /var/www/html/db-$(date +\%F-\%T).sql
zip /var/www/html/db-$(date +\%F-\%T).zip /var/www/html/db-*.sql
rm /var/www/html/db-*.sql

zip -r /var/www/html/html-$(date +\%F-\%T).zip /db-$(date +\%F-\%T).zip /var/www/html/all_zipped.zip
rm -rf db-$(date +\%F-\%T).zip html-$(date +\%F-\%T).zip

I make the assumption that the document root is under /var/www/html/. In the past it was /var/www/ but I don't think the change is going to be reverted or changed in the next 10 years...

Since you are repeatedly using $(date +\%F-\%T), you can store that as a variable (unless you want to also know how long it took the script to jump between statements). Use that variable instead.

Similarly, set a basepath, which in your case, is /var/www/html. In case it changes in the future, it'll be just enough to change it in one place.

At the end, you have

rm -rf db-$(date +\%F-\%T).zip html-$(date +\%F-\%T).zip

which will try to delete those files from CWD, which might not be the same as /var/www/html.

mysqldump has options such as --compact and --compress. Making use of them will result in smaller sql files, resulting in even smaller zips; saving you some disk space.


Is there better alternative to making an optional set of elements from iterable in java? I'm feeling uneasy with this service method that I wrote:

    @Override
    public Optional<Set<TenantDTO>> getAllTenantsOfUsers(Set<Long> usersIds) {

        return Optional.ofNullable(usersIds)
                .map((ids) -> tenantRepository.findAll(QTenant.tenant.users.any().id.in(usersIds)))
                .map((it) -> StreamSupport.stream(it.spliterator(), false))
                .map((strm) -> strm.map((tenant) -> tenantMapper.toDto(tenant)).collect(Collectors.toSet()));

    }

My tenantRepository.findAll returns an iterable of Tenant entity

Is this the better approach to get all performance, with good code?

Both optional collection and a collection of optionals are anti-patterns. There is no scenario where returning Optional<Set<X>> or Set<Optional<X>> is preferable to returning a Set<X> that can be empty.

As @AJNeufeld mentions, a guard condition is a good way to start. In your case. If incoming usersIds are null then return an empty set.

public Set<TenantDTO> getAllTenantsOfUsers(Set<Long> usersIds) {
    if (usersIds == null) {
        return Collections.emptySet();
    }
    // ...
}

Its really confusing when you mix Optional.map and Stream.map in a single call chain. But since the usersIds == null case is handled, you don't need Optional.ofNullable anymore. You can simplify this to just operations over your ids and Tenants.

Something like this:

public Set<TenantDTO> getAllTenantsOfUsers(Set<Long> usersIds) {
    if (usersIds == null) {
        return Collections.emptySet();
    }
    return tenantRepository
            .findAll(QTenant.tenant.users.any().id.in(usersIds))
            .stream()
            .map((tenant) -> tenantMapper.toDto(tenant))
            .collect(Collectors.toSet());
}

Now that getAllTenantsOfUsers returns Set(possibly empty Set), all clients of this code can use it directly instead of caring about handling and unwrapping optional.


Check which numbers satisfy the condition [A*B*C = A! + B! + C!] #include <iostream>
#include <cmath>
using namespace std;
int s = 1; //Serial No.

Should I use recursion for the factorials function?

int Factorial(int n)
{
    int k=1;
    for(int i=1;i<=n;++i)
    {
        k=k*i;
    }
    return k;
}

How I can go about doing this in a single for loop instead of the 3 while loops?

int main()
{
    int a = 1;
    int b = 1;
    int c = 1;
    int Fact1;
    int Fact2;
    int Fact3;
    while (a < 11)
    {
        Fact1 = Factorial(a);
        while (b < 11)
        {
            Fact2 = Factorial(b);
            while (c < 11)
            {
                Fact3 = Factorial(c);
                cout << s << " : ";
                int LHS = Fact1 + Fact2 + Fact3;
                if (LHS == a * b * c)
                {
                    cout << "Pass:" <<"    "<< a << " & " << b << " & " << c << endl;
                }
                else
                {
                    cout << "Fail" /*<<"   "<< Fact1 <<"   "<< Fact2 <<"   "<<Fact3*/<<endl;
                }
                    c++;
                    s++;
            }
            c = 1;
            b++;
        }
        b = 1;
        a++;
    }
    return 0;
}

Also I would love some variable naming tips.

Hello and welcome to Code Review.

Surprisingly, a, b, and c are probably the best variable names you can come up with. They are conventionally given as canonical examples of bad names, but when the task is to implement a mathematical function using the same variable names as in the spec is more important.

As to the single loop question, the short answer is "not easily". You are trying to check all combinations of three independently varying things. That means you naturally want three loops, and anything else would lose out on readability.

Aside from your questions, I have a few observations about this code that might be worth fixing.

When you have a loop that is counting up to something, it is convention to prefer a for loop to a while loop.
Factorial is what is known as a "pure function" which means that its output only depends on its input. Such functions within loops are often good candidates for caching so that you don't have to waste time calculating the same output a hundred and eleven times for each input number.
Both + and * are commutative, which means that if a, b, and c pass then b, a, and c also pass. You should check whether you need to display all reorderings of the same sets of numbers.
It is unclear where 11 has come from. It seems like a magic number, which would benefit from being moved to a named constant and having a comment explaining why that value is used.


Most efficient way to find an entry in a C++ vector I'm trying to construct an output table containing 80 rows of table status, that could be EMPTY or USED as below

+-------+-------+
| TABLE | STATE |
+-------+-------+
|  00   | USED  |
|  01   | EMPTY |
|  02   | EMPTY |
..
..
|  79   | EMPTY |
+-------+-------+

I have a vector m_availTableNums which contains a list of available table numbers. In the below example, I'm putting 20 random table numbers into the above vector such that all the remaining 60 would be empty. My logic below works fine.

Is there a scope for improvement here on the find logic?

#include <iostream>
#include <iomanip>
#include <cmath>
#include <string.h>
#include <cstdlib>
#include <vector>
#include <algorithm>
#include <ctime>

using namespace std;

int main(int argc, const char * argv[]) {
    std::vector<uint8_t> m_availTableNums;
    char tmpStr[50];

    uint8_t tableNum;
    uint8_t randomCount;
    srand(time(NULL));
    for( tableNum=0; tableNum < 20; tableNum++ )
    {
        randomCount = ( rand() % 80 );
        m_availTableNums.push_back( randomCount );
    }

    sprintf(tmpStr, "+-------+-------+");
    printf("%s\n", tmpStr);
    tmpStr[0]='\0';

    sprintf(tmpStr, "| TABLE | STATE |");
    printf("%s\n", tmpStr);
    tmpStr[0]='\0';

    sprintf(tmpStr, "+-------+-------+");
    printf("%s\n", tmpStr);

    tableNum = 0;
    for( tableNum=0; tableNum < 80 ; tableNum++ )
    {
        tmpStr[0]='\0';
        if ( std::find(m_availTableNums.begin(), m_availTableNums.end(), tableNum) != m_availTableNums.end() )
        {
            sprintf(tmpStr, "|  %02d   | EMPTY |", tableNum );
        } else {
            sprintf(tmpStr, "|  %02d   | USED  |", tableNum );
        }

        printf("%s\n", tmpStr);
    }

    tmpStr[0]='\0';
    sprintf(tmpStr, "+-------+-------+");
    printf("%s\n", tmpStr);

    return 0;
}

Header files

It's strange that this code uses the C header <string.h> but the C++ versions of <cmath>, <ctime> and <cstdlib>.  I recommend sticking to the C++ headers except on the rare occasions that you need to compile the same code with a C compiler.  In this case, I don't see anything using <cstring>, so we can probably just drop that, along with <iomanip>, <iostream> and <cmath>.  And we need to add some missing includes: <cstdint> and <cstdio>.

Avoid using namespace std

The standard namespace is not one that's designed for wholesale import like this.  Unexpected name collisions when you add another header or move to a newer C++ could even cause changes to the meaning of your program.

Use the appropriate signature for main()

Since we're ignoring the command-line arguments, we can use a main() that takes no parameters:

int main()

Remove pointless temporary string

Instead of formatting into tmpStr and immediately printing its contents to standard output, we can eliminate that variable by formatting directly to standard output (using the same format string).  For example, instead of:

std::sprintf(tmpStr, "+-------+-------+");
std::printf("%s\n", tmpStr);
tmpStr[0]='\0';

std::sprintf(tmpStr, "| TABLE | STATE |");
std::printf("%s\n", tmpStr);
tmpStr[0]='\0';

std::sprintf(tmpStr, "+-------+-------+");
std::printf("%s\n", tmpStr);

we could simply write:

std::puts("+-------+-------+\n"
          "| TABLE | STATE |\n"
          "+-------+-------+");

And instead of

    tmpStr[0]='\0';
    if ( std::find(m_availTableNums.begin(), m_availTableNums.end(), tableNum) != m_availTableNums.end() )
    {
        std::sprintf(tmpStr, "|  %02d   | EMPTY |", tableNum );
    } else {
        std::sprintf(tmpStr, "|  %02d   | USED  |", tableNum );
    }

    printf("%s\n", tmpStr);

we would have:

    if (std::find(m_availTableNums.begin(), m_availTableNums.end(), tableNum) != m_availTableNums.end()) {
        std::printf("|  %02d   | EMPTY |\n", tableNum);
    } else {
        std::printf("|  %02d   | USED  |\n", tableNum);
    }

Reduce duplication

Most of these statements are common:

        std::printf("|  %02d   | EMPTY |\n", tableNum);
    } else {
        std::printf("|  %02d   | USED  |\n", tableNum);
    }

The only bit that's different is the EMPTY or USED string.  So let's decide that first:

    const char *status =
        std::find(m_availTableNums.begin(), m_availTableNums.end(), tableNum) != m_availTableNums.end()
        ? "EMPTY" : "USED";
    std::printf("|  %02d   | %-5s |\n", tableNum, status);

Prefer nullptr value to NULL macro

The C++ null pointer has strong type, whereas NULL or 0 can be interpreted as integer.

Reduce scope of variables

randomCount doesn't need to be valid outside the first for loop, and we don't need to use the same tableNum for both loops.  Also, we could follow convention and use a short name for a short-lived loop index; i is the usual choice:

for (std::uint8_t i = 0;  i < 20;  ++i) {
    std::uint8_t randomCount = rand() % 80;
    m_availTableNums.push_back(randomCount);
}

for (std::uint8_t i = 0;  i < 80;  ++i) {

Avoid magic numbers

What's special about 80?  Could we need a different range?  Let's give the constant a name, and then we can be sure that the loop matches this range:

constexpr std::uint8_t LIMIT = 80;
...
    std::uint8_t randomCount = rand() % LIMIT;
...
for (std::uint8_t i = 0;  i < LIMIT;  ++i) {

A departure from specification

The description says

  I'm putting 20 random table numbers into the above vector such that, all the remaining 60 would be empty.

That's not exactly what's happening, as we're sampling with replacement from the values 0..79.  There's nothing to prevent duplicates being added (it's actually quite unlikely that there will be exactly 60 empty values).

Reduce the algorithmic complexity

Each time through the loop, we use std::find() to see whether we have any matching elements.  This is a linear search, so it examines elements in turn until it finds a match.  Since it only finds a match one-quarter of the time, the other three-quarters will examine every element in the list, and the time it takes will be proportional to the list length - we say it scales as O(n), where n is the size of the vector.  The complete loop therefore scales as O(mn), where m is the value of LIMIT.

We can reduce the complexity to O(m + n) if we use some extra storage to store the values in a way that makes them easy to test.  For example, we could populate a vector that's indexed by the values from m_availTableNums:

auto by_val = std::vector<bool>(LIMIT, false);
for (auto value: m_availTableNums)
    by_val[value] = true;

for (std::uint8_t i = 0;  i < LIMIT;  ++i) {
    const char *status = by_val[i] ? "EMPTY" : "USED";
    std::printf("|  %02d   | %-5s |\n", i, status);
}

If the range were much larger, we might use an (unordered) set instead of vector<bool>.  We might also choose vector<char> instead of vector<bool> for better speed at a cost of more space.

Simplified code

Here's my version, keeping to the spirit of the original (creating a list of indices, rather than changing to storing in the form we want to use them):

#include <algorithm>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <ctime>
#include <vector>

int main()
{
    constexpr std::uint8_t LIMIT = 80;
    std::vector<std::uint8_t> m_availTableNums;

    std::srand(std::time(nullptr));

    for (std::uint8_t i = 0;  i < 20;  ++i) {
        std::uint8_t randomCount = rand() % LIMIT;
        m_availTableNums.push_back(randomCount);
    }

    std::puts("+-------+-------+\n"
              "| TABLE | STATE |\n"
              "+-------+-------+");

    auto by_val = std::vector<bool>(LIMIT, false);
    for (auto value: m_availTableNums)
        by_val[value] = true;

    for (std::uint8_t i = 0;  i < LIMIT;  ++i) {
        const char *status = by_val[i] ? "EMPTY" : "USED";
        std::printf("|  %02d   | %-5s |\n", i, status);
    }

    std::puts("+-------+-------+");
}


Basic memory pool, alignment, thread safety Mainly for practicing purposes, I'm trying to implement a simple, yet efficient memory pool. It's basically a linked list that is able to grow, with fixed sized members. Each node has a flag if it's free or not. I'm not sure if I'm doing it the right way, and especially confused about how to deal with alignment, and thread safety. I know the basic concept, just don't know how to implement in this case. Could you please help me how to improve my code? This is what I have now:

#include <stdbool.h>
#include "mem_pool.h"

typedef struct block Block;

typedef struct block
{
    Block *next;
    void *ptr;
    bool free;
} Block;

struct mem_pool
{
    Block *head;
    void *memory;
    size_t memory_size;
    size_t memb_size;
};

#define get_ptr_and_increase(pool, size) pool->memory; pool->memory += size

#define add_block_size(pool) pool->memory_size += sizeof(Block) + pool->memb_size

static Block *new_block(MemoryPool *pool, bool is_free)
{
    Block *block = get_ptr_and_increase(pool, sizeof(Block));
    block->ptr = get_ptr_and_increase(pool, pool->memb_size);
    block->next = NULL;
    block->free = is_free;

    return block;
}

MemoryPool *pool_init(size_t memb_size)
{
    MemoryPool *pool = malloc(sizeof(MemoryPool));
    pool->memb_size = memb_size;
    pool->memory_size = 0;
    add_block_size(pool);
    pool->memory = malloc(pool->memory_size);
    pool->head = new_block(pool, true);

    return pool;
}

static Block *find_free(MemoryPool *pool)
{
    Block *current = pool->head;

    while (NULL != current && false == current->free) {
        current = current->next;
    }

    return current;
}

static void add_new_head(MemoryPool *pool)
{
    Block *tmp = pool->head;
    pool->head = new_block(pool, false);
    pool->head->next = tmp;
    add_block_size(pool);
    pool->memory = realloc(pool->memory, pool->memory_size);
}

void *pool_alloc(MemoryPool *pool)
{
    Block *free = find_free(pool);

    if (NULL != free) {
        return free->ptr;
    }
    add_new_head(pool);

    return pool->head->ptr;
}

void pool_free(MemoryPool *pool, void *block)
{
    Block *current = pool->head;

    while (current->ptr != block) {
        current = current->next;
    }

    current->free = true;
}

void pool_destroy(MemoryPool *pool)
{
    free(pool->memory - pool->memory_size);
    free(pool);
}

Serious Bug

The moment you do this:

pool->memory = realloc(pool->memory, pool->memory_size);

all the existing head, next, ptr, and memory pointers become invalid, since they were pointing into the memory that was just freed.  So essentially, your memory pool becomes useless after you expand it.

Unsafe macros

This macro:

#define get_ptr_and_increase(pool, size) pool->memory; pool->memory += size

is unsafe for multiple reasons:

Arguments are not used inside parentheses, e.g. (pool)
The two statements could get split unexpectedly, such as in:

if (condition)
    p = get_ptr_and_increase(pool, size);

I suggest creating a static inline function instead:

static inline void *get_ptr_and_increase(MemoryPool *pool, size_t size)
{
    void *ret = pool->memory;
    pool->memory += size;
    ret += size;
}


Creating a time-course dependent, correlation-based directed graph with Networkx I have a correlation matrix containing 4 time points, each with multiple samples. Each sample is identified with a time point with its name. What I am trying to accomplish here is to create a directed graph using Python's (2.7) Networkx with edges connecting nodes from the last time point to the first time point (6h --> 4h --> 2h --> 1h) given that the correlation value is above a certain threshold. On top of this I would only like to draw edges to nodes only if they have an already existing edge (with exception to the last time point). This will narrow the network the further the path gets.

Additionally since there are only 4 time points, I would like to create a sort of x-axis with each time point and with the respective nodes listed vertically above the x-axis tick marks. Ultimately I would like the graph to look something like a horizontal Christmas tree.

The code I have written works but it was written sloppy and I'm trying my best to compact it. I have listed my code below:

import networkx as nx
import matplotlib.pyplot as plt
from pandas import DataFrame

DG=nx.DiGraph()

corr=DataFrame.from_csv('Correlation_LPS-timecourse.txt', header=0, sep='\t')

timepoint1_allCols=corr.filter(regex=r'(?i)_1h_', axis=0)
timepoint2_allCols=corr.filter(regex=r'(?i)_2h_', axis=0)
timepoint3_allCols=corr.filter(regex=r'(?i)_4h_', axis=0)
timepoint4_allCols=corr.filter(regex=r'(?i)_6h_', axis=0)

timepoint_12=timepoint1_allCols.filter(regex=r'(?i)_2h_', axis=1)
timepoint_23=timepoint2_allCols.filter(regex=r'(?i)_4h_', axis=1)
timepoint_34=timepoint3_allCols.filter(regex=r'(?i)_6h_', axis=1)

threshold = 0.98

for idx, row in timepoint_34.iterrows():
    for i,entry in enumerate(row):
        if entry > threshold:
            DG.add_edge(row.index[i], idx, weight=entry)

for idx, row in timepoint_23.iterrows():
    for i,entry in enumerate(row):
            if entry > threshold and DG.degree(row.index[i]):
                DG.add_edge(row.index[i], idx, weight=entry)

for idx, row in timepoint_12.iterrows():
    for i,entry in enumerate(row):
            if entry > threshold and DG.degree(row.index[i]):
                DG.add_edge(row.index[i], idx, weight=entry)

nx.draw(DG, pos = nx.spring_layout(DG), with_labels=True)
plt.show()

If you're interested in playing with the real data that I am using I have uploaded it here.

This is an awkward way of storing data. You have extremely similar variable names with similar methods of getting the data. You should put them in a list instead, and then you can get them with a list comprehension:

timepoint1_allCols=corr.filter(regex=r'(?i)_1h_', axis=0)
timepoint2_allCols=corr.filter(regex=r'(?i)_2h_', axis=0)
timepoint3_allCols=corr.filter(regex=r'(?i)_4h_', axis=0)
timepoint4_allCols=corr.filter(regex=r'(?i)_6h_', axis=0)

could be

timepoint_allCols = [corr.filter(regex=r'(?i)_{}h_'.format(hour), axis=0)
                     for hour in (1, 2, 4, 6)]

The same with your follow up variables. It's easier to understand and will avoid accidental typos leading you astray. Then it also makes looping over the three results easier:

for idx, row in timepoint_34.iterrows():
    for i,entry in enumerate(row):
        if entry > threshold:
            DG.add_edge(row.index[i], idx, weight=entry)

for idx, row in timepoint_23.iterrows():
    for i,entry in enumerate(row):
            if entry > threshold and DG.degree(row.index[i]):
                DG.add_edge(row.index[i], idx, weight=entry)

for idx, row in timepoint_12.iterrows():
    for i,entry in enumerate(row):
            if entry > threshold and DG.degree(row.index[i]):
                DG.add_edge(row.index[i], idx, weight=entry)

You do need to make some variations but I think it's better to not repeat yourself.

for num, timepoint in enumerate(timepoints[::-1]):
    for idx, row in timepoint.iterrows():
        for i, entry in enumerate(row):
            if entry > threshold and (num == 0 or DG.degree(row.index[i])):
                DG.add_edge(row.index[i], idx, weight=entry)

To explain some things, I'm looping backwards by using the slicing operator [::-1]. I'm also using enumerate along with num == 0 to check if it's your first timepoint so that you know whether or not to use the DG.degree(row.index[i]) test. This is a bit of a hacky solution, but as you're more familiar with the data and process, you could come up with a better and clearer test to determine if the test is necessary to run. The point is, that collapsing it into one loop (even if you need to test for one exception condition) is clearer and less code because the loops were so similar.


Increase efficiency of basic string compressor I was wondering what I can do to increase the performance of this program. The algorithm is pretty slow right now but I was wondering what I can do to increase the efficiency of this program.  I used lists instead of strings to avoid immutable string concatenation.

This program should compress strings by the number of characters within the string

Example:

aaabbbcc --> a3b3c2

Code:

def compressBad(string):
    string = list(string)
    mystr = []
    last = string[0]
    count = 1
    for x in xrange(1,len(string)):
        if string[x] == last:
            count += 1
        else:
            mystr.append(last)
            mystr.append(str(count))
            last = string[x]
            count = 1
    # add the last character
    mystr.append(last)
    mystr.append(str(count))
    return ''.join(mystr)

print compressBad("abaabccc")

output:

a1b1a2b1c3

The following code performs twice as fast as your code does on my machine, both for short strings (e.g. your example) and longer strings.

def compressBetter(string):
    last = string[0]
    result = []
    count = 1
    for c in string[1:]:
        if c == last:
            count += 1
        else:
            result.append(last + `count`)
            count = 1
            last = c

    result.append(last + `count`)
    return ''.join(result)

The main two optimisations are

Using backticks instead of str() for int to str conversion (note that backticks are deprecated and have been removed in Python 3). Some discussion of why they are faster is given here.
Iterating over the string directly rather than indexing. I'm guessing this saves function calls, which have quite a lot of overhead in python.

I find that each optimisation accounts for about half of the performance increase. I am using cPython - the results would probably be very different if you used a different interpreter.


Group by and sum query on multiple columns var fpslist = db.FPSinformations.Where(x => x.Godown_Code != null && x.Godown_Code == godownid).ToList();
var data1 = fpslist.GroupBy(x => x.Ration_Card_Type1)
    .Select(x => new
    {
        CardType_Name = x.Key,
        CardType_Count = x.Sum(y => y.Ration_Card_Count1)
    }).ToList();
var data2 = fpslist.GroupBy(x => x.Ration_Card_Type2)
    .Select(x => new
    {
        CardType_Name = x.Key,
        CardType_Count = x.Sum(y => y.Ration_Card_Count2)
    }).ToList();
var data3 = fpslist.GroupBy(x => x.Ration_Card_Type3)
    .Select(x => new
    {
        CardType_Name = x.Key,
        CardType_Count = x.Sum(y => y.Ration_Card_Count3)
    }).ToList();
var data4 = fpslist.GroupBy(x => x.Ration_Card_Type4)
    .Select(x => new
    {
        CardType_Name = x.Key,
        CardType_Count = x.Sum(y => y.Ration_Card_Count4)
    }).ToList();
var data5 = fpslist.GroupBy(x => x.Ration_Card_Type5)
    .Select(x => new
    {
        CardType_Name = x.Key,
        CardType_Count = x.Sum(y => y.Ration_Card_Count5)
    }).ToList();

var GodownRCCount = data1.Where(x => x.CardType_Name != null).ToList();
var GodownRCCounts = GodownRCCount;
GodownRCCount = data2.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);
GodownRCCount = data3.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);
GodownRCCount = data4.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);
GodownRCCount = data5.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);

I have 10 columns in my database like this:

Ration_Card_Type1
Ration_card_count1
Ration_Card_Type2
Ration_card_count2
Ration_Card_Type3
Ration_card_count3
Ration_Card_Type4
Ration_card_count4
Ration_Card_Type5
Ration_card_count5

Now what I want is to get the sum of Ration_Card_Counts and its type from its type.

Expected output:

CardType_Name
CardType_Count

It works fine, but I want to optimize it in max possible way as this will be inside a loop.

First and most obvious issue is usage of ToList(), it's not required and you force creation of multiple (and possibly big) lists moreover you're repeating almost same code again and again. Step by step:

var fpslist = db.FPSinformations.Where(x => x.Godown_Code == godownid);

Unless comparison operator for Godown_Code is broken then you shouldn't need to check for null (in case you may opt for ?. to avoid it). Also you don't need .ToList(), drop it. If you will execute such code inside a loop you may consider to move this outside the loop (to be sure more context is needed).

Now let's make a reusable function (and also avoid to create multiple anonymous types, it's a micro optimization but it also simplifies our code):

static IEnumerable<Tuple<TName, TCount>> CountByType<T, TName, TCount>(
    IEnumerable<T> list,
    Func<T, TName> nameSelector,
    Func<IEnumerable<T>, TCount> aggregator)
{
    return list.Where(x => Object.Equals(nameSelector(x), null) == false)
        .GroupBy(x => nameSelector(x))
        .Select(x => Tuple.Create(x.Key, aggregator(x)));
}

Now your code can be greatly simplified (I assumed you cannot change database columns otherwise everything can be even simpler, see last section of this answer). I also used generics because type of your columns is unknown.

    var data1 = CountByType(fpslist,
        x => x.Ration_Card_Type1,
        x => x.Sum(y => y.Ration_Card_Count1));

    var data2 = CountByType(fpslist,
        x => x.Ration_Card_Type2,
        x => x.Sum(y => y.Ration_Card_Count2));

   // Repeat for each set

   var result = data1
       .Concatenate(data2)
       .Concatenate(data3)
       .Concatenate(data4)
       .Concatenate(data5);

Note that from your code it seems you can have multiple rows with a value for Ration_Card_CountX. If you have just one row then all code will be greatly reduced. Here I used a Tuple<T1, T2> because I do not know exact types of your objects, you'd better use a proper class instead.

Note that you can consume result as-is (an enumeration) or materialize a list (don't forget this if data comes from DB and connection will be disposed, LINQ execution is deferred).

About DB

I do not know if you can change your database schema and I do not know exact layout of your data then this section may not apply in your case.

If your data is in the form:

Ration_Card_Type1   Ration_Card_Count1   Ration_Card_Type2   Ration_Card_Count2
Type1                                5   NULL                                 0
NULL                                 0   Type2                               11
Type1                               12   Type2                                3

Then you have to keep your actual code but I would suggest to do not map 1:1 column names with C# property names (conventions are pretty different) then Ration_Card_Type1 should be RationCardType1 (assuming you can't simply have Type1, Type2 and so on).

If, for example, you have data with this simplified layout:

Ration_Card_Type1   Ration_Card_Count1   Ration_Card_Type2   Ration_Card_Count2
Type1                                5   NULL                                 0
NULL                                 0   Type2                               11
Type1                               12   NULL                                 0

Then you may both optimize and simplify your DB (it will also impact code):

Type   Count
1          5
2         11
1         12

In that case all you need (assuming your C# entities maps 1:1 with DB columns):

var result = db.GroupBy(x => x.Type)
    .Select(x => new { Type = x.Key, Count = x.Sum(y => y.Count) });


Basic word guessing game After doing a simple coding challenge regarding strings, I realized that I have forgotten a few things about them. As such, I decided to do a few basic string manipulation practices for myself.

One of them, which I found in my old Java assignments, was a word guessing game. Very small, nothing special. As such, I decided to re-implement it. The game works as follows: You enter the "secret" word, and then you have 10 tries to guess it.

import java.util.Scanner;

public class guess {

    public static void main(String[] args) {
        int tries = 0;

        boolean iterated = false;
        String temp = "";
        String holder = "";

        Scanner keyboard = new Scanner(System.in);
        System.out.println("Enter your secret word:");
        String word = keyboard.nextLine();

        do {
            System.out.println("Enter your letter guess");
            String guess = keyboard.nextLine();

            for(int i = 0; i < word.length(); i ++) {
                if (guess.equals(Character.toString(word.charAt(i)))) {
                    if(!iterated)
                        temp += Character.toString(word.charAt(i));
                    else {
                        holder = Character.toString(temp.charAt(i)).replace("-", guess);
                        temp = temp.substring(0, i) + holder + temp.substring( i + 1, temp.length());
                    }
                } else {
                    if(!iterated) {
                        temp += "-";
                    }
                }
            }
            tries++;
            iterated = true;
            System.out.println(temp);
            if(temp.equals(word)) {
                System.out.println("You guessed correctly!");
                break;
            }
        }while (tries < 10);

    }
}

Don't do too much in one loop

            for(int i = 0; i < word.length(); i ++) {
                if (guess.equals(Character.toString(word.charAt(i)))) {
                    if(!iterated)
                        temp += Character.toString(word.charAt(i));
                    else {
                        holder = Character.toString(temp.charAt(i)).replace("-", guess);
                        temp = temp.substring(0, i) + holder + temp.substring( i + 1, temp.length());
                    }
                } else {
                    if(!iterated) {
                        temp += "-";
                    }
                }
            }

In this, you handle two entirely different purposes.  You change the existing string to display any occurrence of the character.  And on the first iteration, you fill in the string with dummy characters.  This may be slightly more efficient on that first iteration, but it is less efficient on every other iteration.  Consider how this code looks without the !iterated checks.

            for (int i = 0; i < word.length(); i ++) {
                if (guess.equals(Character.toString(word.charAt(i)))) {
                    holder = Character.toString(temp.charAt(i)).replace("-", guess);
                    temp = temp.substring(0, i) + holder + temp.substring( i + 1, temp.length());
                }
            }

How much harder is it to do the initial fill?  StackOverflow suggests something like

    String temp;
    if (word.length() > 0) {
        char[] array = new char[word.length()];
        Arrays.fill(array, '-');
        temp = new String(array);
    } else {
        temp = "";
    }

Or you could make a method like they do:

    String temp = getStringWithLengthAndFilledWithCharacter(word.length(), '-');

Or any other variants there.

Which code would you rather read?  Those two simple blocks or one large one?

StringBuilder to build a string

You use repeated calls to substring and string concatenation to build your replacement string.  Behind the scenes, Java probably converts this into a StringBuilder.  Why not embrace that and do it yourself?

            if (guess.length() != 1) {
                System.out.println("Guesses must each be a single letter.");
                continue;
            }

            char c = guess.charAt(0);
            for (int i = word.indexOf(c); i >= 0; i = word.indexOf(c, i + 1)) {
                temp.setCharAt(i, c);
            }

This reduces the two line solution to one even simpler line in the loop.

It also eliminates the need to test that the current index is equal to the guessed character.  This reverses the logic, searching for the guess in the string.

I also added an explanatory message for why multicharacter or empty guesses don't work.  The original just silently goes to the next guess in that instance.

Iterate with for loops

Your do/while loop skips the first check at the expense of a more complicated loop.  If you change to a for loop, it's easier to read at the expense of one extra comparison.

Summary

        StringBuilder temp;
        if (word.length() > 0) {
            char[] array = new char[word.length()];
            Arrays.fill(array, '-');
            temp = new StringBuilder(new String(array));
        } else {
            temp = new StringBuilder();
        }

        for (int turn = 0; turn < 10; turn++) {
            System.out.println("Enter your letter guess");
            String guess = keyboard.nextLine();

            if (guess.length() != 1) {
                System.out.println("Guesses must each be a single letter.");
                continue;
            }

            char c = guess.charAt(0);
            for (int i = word.indexOf(c); i >= 0; i = word.indexOf(c, i + 1)) {
                temp.setCharAt(i, c);
            }

            System.out.println(temp.toString());
            if (word.equals(temp.toString())) {
                System.out.println("You guessed correctly!");
                break;
            }
        }

It would be more elegant to break this into more methods, but it will work like this.


Implementing SOLID Principles with C# Asp.net I have started learning about SOLID principles yesterday. I've got a detailed explanation of SOLID principles here. After reading it, and a few other articles, I have tried to apply these principles in one of my projects.
This is a tool for importing data from another system. Requirements are as follows:

Read some settings from SQL Server database.
Call an external system API with read settings.
API returns the result in Json format.
Returned results should be saved in SQL Server Database.

Following code sample is in C# with Asp.Net
I have a form that initiates the import process on a button's click event.
private void btnStart_Click(object sender, EventArgs e)
        {
            IReader _reader = new DBReader();
            Interfaces.IWriter _writer = new Classes.DBWriter();

            Interfaces.IExternalSystemInteraction _externalSystem = new Classes.JSonExternalSystem(_writer);

            Classes.Client _client = new PxImportApplication.Classes.Client(_reader, _externalSystem);
            _client.Invoke();
        }

DBReader class inherits from IReader interface and does the job to read settings from SQL server database. We can create a new class if there is a requirement to read from MySQL database instead of SQLServer.

Similary DBWriter writes into SQL Server database and implements IWriter interface.

JSonExternalSystem class hits the API and reads data into Json format. IWriter interface instance is passed to get the Json data so that it could write that data into the SQL Server DB.

Client class has the following implementation:
class Client
    {
        private IReader _reader;
        private Interfaces.IExternalSystemInteraction _externalSystemInformation;

        public Client(IReader reader, Interfaces.IExternalSystemInteraction externalSystem)
        {
            _reader = reader;
            _externalSystemInformation = externalSystem;

        }

    public void Invoke()
        {
            List<IProperties> lstProperties = _reader.ReadData();
            _externalSystemInformation.HitAPI(lstProperties);
        }
    }

JSONExternalSystem's implementation is as follows:
class JSonExternalSystem : Interfaces.IExternalSystemInteraction
    {

        private Interfaces.IWriter _writer;

        public JSonExternalSystem(Interfaces.IWriter writer)
        {
            _writer = writer;
        }

        #region IExternalSystemInteraction Members

        public void HitAPI(List<IProperties> properties)
        {
           foreach (IProperties property in properties)
            {
                ReadJsonFormat(property.Xproperty);
                _writer.ImportToDataBase();

            }
        }
   }

My question is: By looking at this much code, do you think that I am on the right way to implement SOLID principles? I know that this doesn't show the whole implementation but most of the code for other classes goes more or less like the above given classes. I am sure I must be lacking somewhere as this is just my second day. Kindly correct me.

When you say I have a form that initiates import process on a button's click event, you've coupled your UI with some "business logic". Let's look at that Click handler:

private void btnStart_Click(object sender, EventArgs e)
   {
       IReader _reader = new DBReader();
       Interfaces.IWriter _writer = new Classes.DBWriter();

       Interfaces.IExternalSystemInteraction _externalSystem = new Classes.JSonExternalSystem(_writer);

       Classes.Client _client = new PxImportApplication.Classes.Client(_reader, _externalSystem);
       _client.Invoke();
   }

Naming

This method handles the click of a button, I'd infer a button named btnStart and say that the naming convention you're using for control names is breaking the naming convention for method names - how about naming the button StartButton and handle the Click event in a StartButton_Click, or OnStartButtonClick method? In my opinion the reasons why btnStart is a bad name would be something like:

Hungarian Notation is used, "btn" being a short for Button, which is a type.
Disemvoweling turned "button" into "btn".
camelCase breaks the PascalCase convention for naming methods, because the convention for event handlers is void [objectName]_[eventName](object sender, EventArgs e).

Type qualification

If, at the top of your code file, you added the following:

using PxImportApplication.Classes;
using PxImportApplication.Interfaces;

Your handler code could read like this:

private void btnStart_Click(object sender, EventArgs e)
   {
       var reader = new DBReader();
       var writer = new DBWriter();

       var externalSystem = new JSonExternalSystem(writer);

       var client = new Client(reader, externalSystem);
       client.Invoke();
   }

Fully-qualified types make the code harder to read. I noticed you used _ in front of local variables; I prefer to use the underscore for private fields, if at all.

Dependencies

The problem with this approach, is that by instantiating that form you automatically get a new DBReader and a new DBWriter, with absolutely no way of swapping these two with a mock implementation - actually you don't really have another way of testing this piece of logic, other than F5 and run the thing and click the button and see how it goes.

In fact, the UI is tightly coupled with the DBReader, DBWriter (which should be DbReader and DbWriter), Client and JSonExternalSystem. Whenever you new up something, you've identified a dependency. Most classes of the .net framework are ok to instantiate on the fly like this, other dependencies should be clearly stated, in the form's constructor. Keep the parameterless default constructor so that you won't break the designer, but make another that takes all other dependencies. Too many (4? 5? 6? Your call!) constructor parameters (/dependencies) is a sign that your class is starting to break the Single Responsibility Principle.

I think the form is doing much more than just being a UI here, and the dependencies really are, is dependencies of another type, namely PxImportApplication.Classes.Client.

What the button really needs to do, is instantiate a Client and call the Invoke() method (a confusing name that breaks POLS, Invoke is .net lingo for synchroneously calling a delegate). All this newing up is covering-up the real intent. How about having a click handler doing this:

private void btnStart_Click(object sender, EventArgs e)
{
       var client = _clientFactory.Create();
       client.Invoke(); // todo: rename this method
}

This code means the handler now only depends on a _clientFactory private field:

private readonly IFactory _clientFactory;

And this field can be instantiated from the form's parameterized constructor:

public Form1(IFactory clientFactory) : this()
{
    _clientFactory = clientFactory;
}

The factory class has its own dependencies:

private readonly IReader _reader;
private readonly IExternalSystemInteraction _externalSystem;

public ClientFactory(IReader reader, IExternalSystemInteraction externalSystem)
{
    _reader = reader;
    _externalSystem = externalSystem;
}

public IClient Create()
{
    return new Client(_reader, _externalSystem);
}

The ClientFactory class implements the IFactory interface, which simply exposes a parameterless Create() method - and that's everything the form needs to know.

Your constructor-injected private fields can be made readonly in both Client and JSonExternalSystem classes (should be JsonExternalSystem). You're doing it right in these classes - don't do DI halfway though, do it all the way. Creating instances of types is the role of factory/infrastructure classes and of the application's composition root. If you're using an IoC container you don't have to resolve the dependencies yourself. This means if the IReader that the factory class takes has dependencies of its own, you don't need to bother with them; by letting go of the control you have over the types that a type can instantiate, your code becomes very focused, cohesive. High cohesion, low coupling is what you want to achieve.


Encryption Algorithm in C# I'm coding a "encryption" algorithm for learning and and don't know how good is it or how could I make it better:

private static byte[] encrypt(byte[] data, int session_id, byte[] password)
    {
        int session_key = (session_id & 0xFF);
        if (session_key == 0) session_key = (session_id << 4);
        byte[] output = new byte[data.Length];
        for (int i = 0; i < output.Length; i++)
        {
            int index = getIndex(i, password.Length);
            int num001  = session_key << 2;
            int num002 = (session_id | i) >> (i+1);
            int num003 = function(i, session_key, password[index]);
            byte encrypted = XorByte(data[i], password);
            output[i] = (byte)((encrypted ^ num003) + (num002 % num001) );
        }
        return output;
    }

    private static byte[] decrypt(byte[] data, int session_id, int length, byte[] password)
    {
        int session_key = (session_id & 0xFF);
        if (session_key == 0) session_key = (session_id << 4);
        byte[] output = new byte[data.Length];
        for (int i = 0; i < output.Length; i++)
        {
            int index = getIndex(i, password.Length);
            int num001 = session_key << 2;
            int num002 = (session_id | i) >> (i + 1);
            int num003 = function(i, session_key, password[index]);
            byte decrypted = (byte)(data[i] - (num002 % num001) ^ num003);
            output[i] = XorByte(decrypted, password);
        }
        return output;
    }
    private static int function(int index, int a, int b)
    {
        if (a > b)
            if ((b + index) < a) return a - b - index;
            else return a - b + index;
        else
            if ((a + index) < b) return b - a - index;
        else return b - a + index;
    }

    public static int getIndex(int a, int b) => (a + b) >> 2;

    private static byte XorByte(byte input, byte[] password)
    {
        byte output = input;
        for (int i = 0; i < password.Length; i++) output ^= password[i];
        return output;
    }

Basically, this code takes a byte[] (data) which will be encrypted. The session_id is a randomly-generated number (this is supposed to be used in a client-server app, and every client is assigned an ID), and a password, which is a random generated byte[].

The main functions are encrypt and decrypt, and they call another 3:

function(int index, int a, int b) - this creates a number based on another 3 that is used to XOR each byte the input of the encryption and decryption functions.
getIndex(int a, int b) - this returns the "middle" number (I don't know how to say it in English) of 2 numbers, that are the number of iterations and the password length. This is used to get a byte within the password.
XorByte(byte input, byte[] password) - this applies XOR to a byte recursively for each byte of the password, so the longer the password the harder to break.

Outputs:

Using a 32 byte random password, a random ID between 1000 and 10000, and the input data: "testing string":

UTF-8 bytes:
116  101  115  116  105  110  103   32  115  116  114  105  110  103

encrypted result: ?`R?o1?zqnrjb

UTF-8 bytes:
160   96   82   15  237  111   49  205  122  113  110  114  106   98

Time: 112 ticks, less than 1 ms (encryption and decryption)

With the same inputs done 100.000 times:

Time: 0,08009ms each encryption/decryption, 8009ms overall

Your algorithm effectively combines two simple ciphers: a byte substitution cipher performed by XorByte(), and something that is similar in spirit to xoring with a poor-quality pseudo-random key stream (except for the minor difference due to the use of additive operators instead of xor for the combining step).

Calling XorByte() for each byte is wasteful; you get the same effect by calling XorByte(0, password) once, caching the result and xoring every byte with that. There's a lot more random stumblings in the dark like that. E.g.

int num002 = (session_id | i) >> (i + 1);

There's no need for oring i into the first operand since it will get shifted out completely anyway (unless operator >> implicitly performs a modulo op on its right operand, they way shift instructions do on Intel CPUs).

int index = getIndex(i, password.Length);
...
int num003 = function(i, session_key, password[index]);

This will crash if the input size approaches or exceeds three times the password size, because getIndex() returns half the arithmetic mean.

You tried to make your key stream generator look complex but it is actually very poor. As a result you have one simple cipher (byte substitution, indirectly defined by the password) on top of a complicated but still poor one.

This algorithm will be very safe as long as it doesn't protect anything of interest. As soon as it does, it will get torn to shreds.

Don't forget that your adversaries won't necessarily hobble themselves by using an application-glue language like C#; they could use C++, effectively load all cores of their processors, and process multiple streams on each core via vector instructions (MMX/SSE). Or they could get even more performance by offloading the stuff into their graphics cards (CUDA, OpenCL). IOW, cracking performance could exceed the performance of your own code by five to ten orders of magnitude, especially when combined with a bit of math fu.

Summary: if you randomly throw stuff in a pot then what you get is a right hash.

Post scriptum: Since there weren't any other takers for reviewing this code, a few more considerations and a bit more detail.

It is rarely a good idea to feed passwords and session ids directly into cryptographic computations, because their individual bits are usually not very random (regardless of whether they contain sufficient entropy or not). The standard way of dealing with this is to distill the entropy from password and session id by running them through a hash function (or, more precisely, to apply a password-based key derivation function like PBKDF2). This effectively spreads the entropy over all bits of the resulting hash, which makes it easier to use in further processing. As a first approximation - for testing and experiments - any good hash function will do, even the trusty old MD5 (still my favourite as a general hash function, even though it can't be used for strong crypto anymore).

As was said earlier, one part of the algorithm works pretty much like a stream cipher except for the use of additive operators for the combining step instead of xor. In the following I'll gloss over the resulting differences since they are insignificant, and using xor makes epxlanations easier.

An important consideration for stream ciphers is that key streams must never be reused. If two messages are encrypted with the same key stream then a simple xor of the two cipher texts yields the xor of the two plain texts. I.e.

P  ^ S = C
P' ^ S = C'
C ^ C' = (P ^ S) ^ (P' ^ S) = P ^ P'

A cryptanalyst (attacker) can glean lots of invaluable information from something like that, just like when unhashed passwords are fed directly into crypto algorithms.

Consequently, stream cipher keys must never be reused. That is usually accomplished by adding a salt or nonce when hashing the password to produce the session key; the nonce - which could be a simple counter, or a unique timestamp (i.e. a timestamp plus unique identifying information like machine, process id, thread id and so on), or both. The salt/nonce must be stored with the encryption header, since it is necessary for decryption.

Also, I strongly recommend encapsulating the whole key stream generation in its own class, so that it becomes easier to use and - most importantly - easier to put through rigorous testing independent of other parts. Such a class would yield the key stream byte for byte, and perhaps also in other convenient ways.

Using 'Soopr' as placeholder for the name of the as yet anonymous algorithm, a typical use for en/decipherment might look like this:

var session_key = SooprKDF(password, session_id, nonce);
var key_stream = new SooprKeyStream(session_key);
byte xor_byte = XorByte(0, session_key);

foreach (byte b in input)
   output = b ^ xor_byte ^ key_stream.next_byte();

An important quality indicator for cryptographic algorithms is diffusion. As a simple rule of thumb, flipping a single bit in the key material should affect every output bit with probability 50%. This applies not only to the algorithm as a whole, but most specifically to your key stream generator. Put in a different way: the output of your key stream generator should pass all standard quality tests for pseudo-randomness (most famous: Georgio Marsaglia's Diehard suite). Passing such tests is not sufficient for a good crypto algorithm, but it is necessary.

If you have your key stream generator as a separate class then you easily run some tests to get an impression of its diffusion capability. E.g. create a random session key, generate a number of key stream bytes, flip a bit in the session key, and generate the same number of key stream bytes again. Then you can count the bits that have flipped between the two stream samples.

Let your computer run a whole bunch of such random tests (with varying key stream sample lengths) - perhaps over night - and then look at how often each bit did flip for each key bit, separate per sample length. Every stream bit should have flipped about 50% of the time, for every sample length and every key bit. Tests like this make it easy to find unintended regularities that bleed through, like certain bits that never flip, or not often enough, or too often (which means they are correlated to certain parts of the key - something that shouldn't happen).


Probability of event using Central Limit Theorem + plotting results I've been doing a Udemy course called: "Statistics for Data Science" and I decided to solve one of the homework with Python to kill two birds with one rocket #elon.

The task was:

  The team of traders under your supervision earns profits which can be
  approximated with Laplace distribution. Profits (of any trade) have a
  mean of $95.70 and a std. dev. of $1,247. Your team makes about 100
  trades every week.

  Questions:

  A. What is the probability of my team making a loss in any
  given week?
  B. What is the probability of my team making over $20,000
  in any given week?

As I just started to learn Python I would be happy for some hints and opinions.

# set up
import math
import scipy.stats as st
import matplotlib.pyplot as plt
import numpy as np

# Data
mu = 95.7                           # mean
sigma = 1247                        # standard deviation
n = 100                             # sampling size (trades here)
xcritical1 = 0                      # making a loss
xcritical2 = 20000 / n              # Earning $20k a weak by 100 trades

mu_1 = mu                           # Based on Central Limit Theorem
sigma_1 = sigma / (math.sqrt(n))    # Based on CLT

# Calc
def Z(xcritical, mu, sigma):
    return (xcritical - mu) / sigma     # Standard Score (z-value)

Z1 = Z(xcritical1, mu_1, sigma_1)
Z2 = Z(xcritical2, mu_1, sigma_1)

P1 = st.norm.cdf(Z1)                    # Cumulative Distribution Function for ND
P2 = 1 - st.norm.cdf(Z2)

print('A. Probability of making loss in any given week is', '{0:.4g}'.format(P1*100) + '%')
print('B. Probability of making over $20k in any given week is', '{0:.4g}'.format(P2*100) + '%')

# Plots
def draw_z_score(x, cond, mu, sigma, title):
    y = st.norm.pdf(x, mu, sigma)            # Probability Density function for ND
    z = x[cond]
    plt.plot(x, y)
    plt.fill_between(z, 0, st.norm.pdf(z, mu, sigma))
    plt.title(title)
    plt.text(-300, 0.0020, r'$\mu=' + str(mu_1) + ',\ \sigma=' + str(sigma_1) + '$')
    plt.show()

x = np.arange(-400, 500, 1)      # Fixed interval by experimenting
title1 = 'Probability of making loss: ' + '{0:.4g}'.format(P1*100) + '%'
title2 = 'Probability of earning more than $20k: ' + '{0:.4g}'.format(P2*100) + '%'

draw_z_score(x, x < xcritical1, mu_1, sigma_1, title1)
draw_z_score(x, x > xcritical2, mu_1, sigma_1, title2)

You seem to know about str.format (since you use it), yet you are still doing string addition:

title1 = 'Probability of making loss: ' + '{0:.4g}'.format(P1*100) + '%'
title2 = 'Probability of earning more than $20k: ' + '{0:.4g}'.format(P2*100) + '%'

Just make it a single string:

title1 = 'Probability of making loss: {0:.4g}%'.format(P1*100)
title2 = 'Probability of earning more than $20k: {0:.4g}%'.format(P2*100)

You could also use the % formatting option for floats (described in the fifth table here), which automatically multiplies with 100 and adds the % sign. The number of digits after the decimal point work differently, though, compared to the g format:

"{:.4%}".format(0.5)
# '50.0000%'
"{:.4g}%".format(0.5*100.)
# '50%'
"{:.4g}%".format(0.5123123*100.)
# '51.23%'
"{:.4%}".format(0.5123123)
# '51.2312%'

You should also re-organize your code according to the following scheme:

import something

GLOBAL_CONSTANT = "value"

class Definitions

def functions()

if __name__ == '__main__':
    main()
    # or some small code using the stuff defined above

That last part is there to protect your code from being executed if you want to import some part of this script from another script. At the moment if you want to do from laplace import Z, your whole code would run. With the if __name__ == '__main__' guard, it will only run when you execute this script.

You should add docstrings to your functions, describing what they do, what arguments they take and what they return. Have a look at PEP257 for the guidelines regarding docstrings.

And, in order to answer my own question from the comments:

Yes, it is justified to use the normal distribution here, since the Central Limit Theorem guarantees that the sum of random variables tends towards a normal distribution. The approximation is quite good for \$N = 100\$, as can be seen with the following small script:

import numpy as np
import matplotlib.pyplot as plt

mu = 95.7
sigma = 1247.
n = 100    # how many random variables to sum for each value
N = 1000    # how many values to generate

x_l = np.random.laplace(mu, sigma/np.sqrt(2), (n, N)).sum(axis=0)
x_g = np.random.normal(n*mu, np.sqrt(n)*sigma, N)

plt.hist(x_g, label="gaus")
plt.hist(x_l, label="sum(laplace)", histtype='step')
plt.legend()
plt.show()


Filling two angular $scope variables with data I'm unsure about how to simplify this method properly. The method is basically responsible to allocate data.

The response object is a collection of objects. Each object contains an attribute called order and i have to distinguish between visible and invisible elements. Visible elements are represented by positive integers, invisible elements by a negative integers.

Also it is possible to overwrite the default state if the URL contains an parameter (hidden) with a list of hidden element ids. ?hidden=23,432,3,123

_hasParam() returns boolean

_getParam() returns string (separated by comma [23,432,3,123])

contains() returns boolean

prepare: function (response) {

    var visible = [];
    var invisible = [];

    if (_hasParam('hidden')) {

        var hidden = _getParam('hidden').split(',');

        angular.forEach(response, function (item) {
            if (hidden.contains(item.id)) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        });

    } else {

        angular.forEach(response, function (item) {
            if (item.order === -1) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        });

    }

    $rootScope.visible = visible;
    $rootScope.invisible = invisible;
}

The only thing that stands out a bit is the fact that you do 2 very similar angular.forEach() calls. You could extract these functions out and do only 1 call. I would also make -1 a named constant. I would also declare all variables on top.

What you could consider, but I don't advise it, since it reeks too much of CodeGolf, is to simplify the 2 if statements to 1 single push() call

(hidden.contains(item.id)? invisible : visible).push( item );

This piece of code extracts the filter functions and applies my other suggestions:

prepare: function (response) {

    var visible = [],
        invisible = [],
        HIDE = -1,
        hidden, filter;

    if (_hasParam('hidden')) {
        hidden = _getParam('hidden').split(',');
        /* Hide based on the hidden parameter */
        filter = function (item) {
            if (hidden.contains(item.id)) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        }
    } else {
        filter = function (item) {
        /* Hide based on the order of the item */
            if (item.order === HIDE) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        }
    }

    angular.forEach(response, filter);
    $rootScope.visible = visible;
    $rootScope.invisible = invisible;
}