Multiprocess, multithreaded read write on a single file I've got a job to create a library that would be able to support multithreaded and multi process read and write to a single file. On that single file we would store C# models in an array in JSON format. It would basically work as a mini file system JSON DB.

When the application that uses this lib starts, it would load existing models (if they do exist) in memory, so that we can query the data in memory. Adding, updating and removing of objects would require to flush the data to that single file. I assume that the hardest thing here to achieve would be synchronization between threads and processes. I actually managed to do that with mutex. Currently, I only have two methods, one for writing, and one for reading:

public void Write (string text)
{
    var appGuid = "{FBE403EA-0D59-48D6-941D-E74E0F20B482}";
    var mutexId = $"Global\\{{{appGuid}}}";

    bool createdNew;

    var allowEveryoneRule = new MutexAccessRule(new SecurityIdentifier(WellKnownSidType.WorldSid, null), MutexRights.FullControl, AccessControlType.Allow);
    var securitySettings = new MutexSecurity();
    securitySettings.AddAccessRule(allowEveryoneRule);

    using ( var mutex = new Mutex(false, mutexId, out createdNew, securitySettings) )
    {
        var hasHandle = false;
        try
        {
            hasHandle = mutex.WaitOne(Timeout.Infinite, false);
            using ( var fileStream = new FileStream("test.txt", FileMode.Truncate, FileAccess.Write, FileShare.Read) )
            {
                var buffer = Encoding.UTF8.GetBytes(text);
                fileStream.Write(buffer, 0, buffer.Length);
                fileStream.Close();
            }
        }
        finally
        {
            if ( hasHandle )
                mutex.ReleaseMutex();
        }
    }
}

public string Read ( )
{
    var appGuid = "{FBE403EA-0D59-48D6-941D-E74E0F20B482}";
    var mutexId = $"Global\\{{{appGuid}}}";

    bool createdNew;

    var allowEveryoneRule = new MutexAccessRule(new SecurityIdentifier(WellKnownSidType.WorldSid, null), MutexRights.FullControl, AccessControlType.Allow);
    var securitySettings = new MutexSecurity();
    securitySettings.AddAccessRule(allowEveryoneRule);

    using ( var mutex = new Mutex(false, mutexId, out createdNew, securitySettings) )
    {
        var hasHandle = false;
        try
        {
            hasHandle = mutex.WaitOne(Timeout.Infinite, false);
            using ( var fileStream = new FileStream("test.txt", FileMode.OpenOrCreate, FileAccess.Read, FileShare.Read) )
            {
                var text = "";
                var stringBytes = new byte[fileStream.Length];
                fileStream.Read(stringBytes, 0, (int)fileStream.Length);
                text = Encoding.UTF8.GetString(stringBytes);
                fileStream.Close();
                return text;
            }
        }
        finally
        {
            if ( hasHandle )
                mutex.ReleaseMutex();
        }
    }
}

To test these methods I implemented them in a WPF app, which had two buttons and a textbox. One button would synchronously read a single file in an infinite loop with a wait of 10 ms between cycles and dispatch the text to the textbox. The other button would write to that single file also in an infinite loop with a wait of 10 ms between cycles. In that loop I would download a random text and synchronously write the text to the file.

When there are two instances of that WPF app and they both ran the loops both read and write would work really fast. But after only two more instances read and write would slow down considerably.

Is there anything I can do in these methods to get any boost in performance?

The primary need of this lib is that it always needs to flush data to a file. That's why i'm using Timeout.Infinite when calling WaitOne from a mutex. I can't let it to fail when it tries to access a mutex.

Is there anything i can do in these methods to get any boost in performance?

Yes, You can reimplement them, using the 'memory-mapped files' (MMf) to solve both problems of parallel concurrency and access sharing.

using (MemoryMappedFile file = MemoryMappedFile.CreateFromFile("test.bin"))
using (MemoryMappedViewStream stream = file.CreateViewStream())
{
    Read(stream);
}

There are two ways of accessing data on a memory mapped file – using a MemoryMappedViewStream and an MemoryMappedViewAccessor.
 The former is optimised to provide sequential access (e.g. when reading or writing a block of data) and the later is optimised for random access reading and writing.

Also, for 'production code' consider the following:

Performance can be greatly improved by always writing data sequentially and in large chunks
The chunk size is less important for small amounts of data (less than a kilobyte)
There is a cost to creating and disposing accessors, this cost becomes acceptable for reading files below 32K.
There is benefit of keeping hold of a stream, wich will remain significant.
When reading a large stream the performance is equivalent to raw memory access


Counting consecutive streaks in random numbers I heard that when you toss a die often enough, you could get the value "one" a million times in a row.

I am a beginner in Python 3 and I tried to write a little program that generates 1 or 2 randomly and counts how often the value "one" has occured successively and how big this count could get.

You can also print out the program run or write it to a file to understand the result.

I would like to know if there are possibilities to improve the code quality by using tricks and mechanisms that Python offers to me or if I can reduce something / make something better in general.

from random import randint
import os.path

iterations = int(input("number of iterations: "))
file_with_details = input("create a log file? (y/n): ")
details = input("printing out details about the program run? (y/n): ")
log_file = "is not existing"

# generate folder and log file for program run information
if file_with_details == "y":
    if not os.path.isdir("./logs"):
        os.mkdir("./logs")
    file_number = 1
    file_name = "./logs/log" + str(file_number) + ".txt"
    while os.path.isfile(file_name):
        file_number += 1
        file_name = "./logs/log" + str(file_number) + ".txt"
    log_file = open(file_name, "w")

# start of random experiment
biggest_succession = 0
actual_succession = 0
random_number_before = 0

for i in range(iterations):
    random_number = randint(1, 2)
    if details == "y":
        print("generated number:", random_number)
    if file_with_details == "y":
        log_file.write("generated number:" + str(random_number) + "\n")

    if random_number == 1 and random_number_before == 1:
        actual_succession += 1
        if details == "y":
            print("actual succession:", actual_succession)
        if file_with_details == "y":
            log_file.write("actual_succession:" + str(actual_succession) + "\n")
    else:
        if actual_succession > biggest_succession:
            biggest_succession = actual_succession
            if details == "y":
                print("biggest succession has changed:", biggest_succession)
            if file_with_details == "y":
                log_file.write("biggest succession has changed:" + str(biggest_succession) + "\n")
        actual_succession = 0
    random_number_before = random_number

print("the number one was generated", biggest_succession, "times behind each other")
if file_with_details == "y":
    log_file.write("the number one was generated " + str(biggest_succession + 1) + " times behind each other")
    log_file.close()

If your program is going to be run via cli only, then I'd suggest that you look into argparse library.
Split the code snippet into different functions. Though your current use-case is limited, but suppose you later want to check successive outcomes in a 6 face die? A 10 face die? A 20 face die? I'd suggest asking for number of faces as an argparse param, and default it to 2.
Use the logging library's RotatingFileHandler to control logging to the files.
In the logic, you do not need to store the value of random_number_before. Just checking against a constant value of 1 would suffice in your case:

if random_number == 1:
    actual_succession += 1
else:
    if actual_succession > longest_succession:
        longest_succession = actual_succession
    actual_succession = 0

as the logic is executed for each iteration of the counter, everytime you encounter a number other than 1, your succession counter would be reset to 0.


Email template referencer Which is best practice for creating an enum-like lookup for templates? I enjoy being able to bring up my templates with VS IntelliSense so either works for me.

Option 1 (enum/Dictionary combo):

public enum Template
{
    AccountInformation = 1,
    Registration = 2,
    Signed = 3
}
static readonly IDictionary<Template, String> TemplateData = new Dictionary<Template, String>()
{
    {Template.AccountInformation, "accountinfo.txt"},
    {Template.Registration, "registration.txt"},
    {Template.Signed, "signed.txt"}
};

Option 2 (static class):

public static class Template
{
    public static String AccountInformation = "accountinfo.txt";
    public static String Registration = "registration.txt";
    public static String Signed = "signed.txt";
}

Try this:

public enum Template
{
    [StringValue("accountinfo.txt")]
    AccountInformation = 1,
    [StringValue("registration.txt")]
    Registration = 2,
    [StringValue("signed.txt")]
    Signed = 3
}

For more information: String Enumerations in C#

Otherwise, I prefer your second option.


Split large file into smaller files I recently suggested this method for emulating the Unix utility split in Python.

Is there a more elegant way of doing it?

Assume that the file chunks are too large to be held in memory. Assume that only one line can be held in memory.

import contextlib

def modulo(i,l):
    return i%l

def writeline(fd_out, line):
    fd_out.write('{}\n'.format(line))

file_large = 'large_file.txt'
l = 30*10**6  # lines per split file
with contextlib.ExitStack() as stack:
    fd_in = stack.enter_context(open(file_large))
    for i, line in enumerate(fd_in):
        if not modulo(i,l):
           file_split = '{}.{}'.format(file_large, i//l)
           fd_out = stack.enter_context(open(file_split, 'w'))
        fd_out.write('{}\n'.format(line))

I ran the Unix utility time and the Python module cProfile. Here is what I found (methods not comparable, as I was running other processes, but gives a good indication of slow parts of code):

Ugo's method:

tottime filename:lineno(function)
473.088 {method 'writelines' of '_io._IOBase' objects}

485.36 real       362.04 user        58.91 sys

My code:

tottime function
243.532 modulo
543.031 writeline
419.366 {method 'format' of 'str' objects}
1169.735 {method 'write' of '_io.TextIOWrapper' objects}

3207.60 real      2291.42 user        44.64 sys

The Unix utility split:

1676.82 real       268.92 user      1399.16 sys

Unfortunately, as far as I know, there is no chunks methods in the standard library.
But this makes things rather neat.

from itertools import chain, islice

def chunks(iterable, n):
   "chunks(ABCDE,2) => AB CD E"
   iterable = iter(iterable)
   while True:
       yield chain([next(iterable)], islice(iterable, n-1))

l = ...
file_large = 'large_file.txt'
with open(file_large) as bigfile:
    for i, lines in enumerate(chunks(bigfile, l)):
        file_split = '{}.{}'.format(file_large, i)
        with open(file_split, 'w') as f:
            f.writelines(lines)


How to read a complete Unicode code point as String? I tried this so far:

public String readchar (Reader input) throws java.io.IOException
{
  int i16 = input.read(); // UTF-16 as int
  if (i16 == -1) return null;
  char c16 = (char)i16; // UTF-16
  if (Character.isHighSurrogate(c16)) {
    int low_i16 = input.read(); // low surrogate UTF-16 as int
    if (low_i16 == -1)
      throw new java.io.IOException ("Can not read low surrogate");
    char low_c16 = (char)low_i16;
    int codepoint = Character.toCodePoint(c16, low_c16);
    return new String (Character.toChars(codepoint));
  }
  else
    return Character.toString(c16);
}

But it contains unsafe casts. I have no idea how to avoid them. Is it possible?

The javadoc of the read method states:

  Returns: The character read, as an integer in the range 0 to 65535
  (0x00-0xffff), or -1 if the end of the stream has been reached

The char primative is "a single 16-bit Unicode character. It has a minimum value of '\u0000' (or 0) and a maximum value of '\uffff' (or 65,535 inclusive)."

(emphasis mine)

It would appear that your two C-style casts are safe.


Longest palindrome in a given string using LINQ Problem:I need to write an algorithm that will return length of longest possible palindrome from a given string.

So if the input is aabbbccdfg
Program output should be 7. //-->[cabbbac]

Can someone point it out if there are any problems with below code?

public static void GetLongestPalindromeLength()
    {
        var input = Console.ReadLine().ToCharArray();
        var result = 0;
        var countsByChar = input.GroupBy(c => c);
        var oddCounts = countsByChar.Where(g => g.Count() % 2 != 0).Select(g => g.Count()).ToList();
        var evenCounts = countsByChar.Where(g => g.Count() % 2 == 0).Select(g => g.Count()).ToList();
        if (oddCounts.Any())
        {
            var max = oddCounts.Max();
            result += max;
            oddCounts.RemoveAt(oddCounts.FindIndex(e => e == max));
            result += evenCounts.Sum();
            result += oddCounts.Sum(e => e - 1);
        }
        else if (evenCounts.Any())
        {
           result += evenCounts.Sum();
        }
        Console.WriteLine(result);
        Console.ReadKey();
    }

Well, I think your algorithm is correct, but the code is quite bloat. Your code suggest that the longest palindrome size can be calculated by sum of n / 2 * 2 where n is the count of distinct alphabet that exists more than once in the string, and plus 1 if there are any "oddCounts". So you can reduce the code to

    private static int GetLongestPalindrome(string value) {
        var map = value.GroupBy(c => c);
        int result = map.Sum(r => r.Count() / 2 * 2);
        if (map.Any(r => r.Count() % 2 != 0))
        {
            result++;
        }
        return result;
    }

n / 2 * 2 is the way to calculate for the nearest even number toward zero, and it equals to n - 1 where n is positive odd number.


How to write this method with similar checks and multiple returns more elegantly? I have written the following method which works as expected. But is there a way to write this more elegantly in Java 8? Appreciate any feedback. Thanks.

private String get(Details details, Value value){
    Type type = Type.getEvent(details.getType());

    if(type != null){
        if("SAMPLE".equals(type.typeDesc())){
            if(SPIN.equals(details.getSpin())){ // this if looks very similar to last line..
                return BACKUP + SAMPLE + value.getId();
            }
            return DEFAULT_VAL + SAMPLE + value.getId();
        }
        return value.getRoute();
    }
    return StringUtils.contains(details.getSpin(), SPIN) ? BACKUP : DEFAULT_VAL;
}

I think it's a personal thing, but I prefer == null checks with early exits, because that way you have all exception paths at the beginning and the normal execution path is less indented.
For your string concatenation you could make a ternary if and have one return statement less.

private String get(Details details, Value value){
    Type type = Type.getEvent(details.getType());
    if(type == null) {
      return StringUtils.contains(details.getSpin(), SPIN) ? BACKUP : DEFAULT_VAL;
    }

    if("SAMPLE".equals(type.typeDesc())) {
        String prefix = SPIN.equals(details.getSpin()) ? BACKUP : DEFAULT_VAL;
        return prefix + SAMPLE + value.getId();
    }
    return value.getRoute();
}

Now looking at this, you check the spin for equals and contains. Is this intentional? If not you could further reduce the code by pulling the prefix part up like this:

private String get(Details details, Value value){
    String prefix = SPIN.equals(details.getSpin()) ? BACKUP : DEFAULT_VAL;

    Type type = Type.getEvent(details.getType());
    if(type == null) {
      return prefix;
    }

    if("SAMPLE".equals(type.typeDesc())) {

        return prefix + SAMPLE + value.getId();
    }
    return value.getRoute();
}

In general you should use StringBuilder for string concatenation.


AngularJS Todo List - too much controller logic? I am starting an angular app and I was wondering if I am on the right track with this. I worked on my last angular app when I was just a youngun who didn't understand the important of designing the code properly. I failed so hard at keeping the code clean and maintainable, mostly due to not understanding directives or services.

I am wondering if there is a better structure to my error handling in the below code.  Is that too much logic in the controller functions? All I am doing is allowing the adding of items to a list, but not allowing them to be added or removed if the list length is < 5 or > 1 respectively.  If there are other problems besides the controller logic, please let me know.

var toDo = angular.module('toDo',[]);

toDo.factory('listEdit', function() {
return {
    add: function(list, item) {
        list.push(item);
        return list;
    },
    remove: function(list, index) {
        list.splice(index, 1);
        return list;
    },
    count: function(list) {
        return list.length;
    }
}
});

//Error handling methods

toDo.controller('main', ['$scope', 'listEdit', function($scope, listEdit) {
$scope.list = [];

$scope.addToList = function() {
    list_length = listEdit.count($scope.list);
    if(list_length < 5) {
        $scope.list = listEdit.add($scope.list, $scope.todo);
    } else {
        alert('Too many list items');
    }
}

$scope.remove = function(index) {
    list_length = listEdit.count($scope.list);
    if(list_length > 1) {
    $scope.list = listEdit.remove($scope.list, index);
    } else {
        alert("You can't delete this last item");
    }
}

}]);

HTML

    <input type="text" ng-model="todo">
    <button ng-click="addToList()">Add to List</button>

    {{todo}}

This is a funny question because

One of the most readable Angular submissions on CR
There is probably too much logic in the controller indeed

Some observations:

It seems the controller knows too much, it should not know that 5 is the maximum amount of items, or that 1 is the minimum amount of items to be kept
It seems silly to pass the list to list every single time to listEdit, it is not DRY.
I know alert in this case is used because it is a prototype, but still..

I would counter propose something where listEdit is aware of the list after you make a connection, listEdit also knows about limits and gives feedback ( in the form of a string ) whenever it does something to the list.

The controller this way is very small, and easy to understand:

//Controller methods
toDo.controller('main', ['$scope', 'listEdit', function($scope, listEdit) {

  $scope.list = [];
  listEdit.linkList( $scope.list ); //<- Magic happens here

  $scope.addToList = function() {
    $scope.feedback = listEdit.add($scope.todo);
  };

  $scope.remove = function(index) {
    $scope.feedback =  listEdit.remove(index);
  };

}]);

listEdit is then a little bigger:

toDo.factory('listEdit', function() {
  var _list = [],
      maxLength = 5,
      minLength = 1;

  return {
    add: function(item) {
      if( _list.length >= maxLength )
        return 'Too many list items';
      _list.push(item);
      return item + ' is added';
    },
    remove: function(index) {
      if( _list.length == minLength )
        return 'You can\'t delete this last item';
      var item = _list.splice(index, 1)[0];
      return item + ' has been deleted';
    },
    count: function() {
        return internalList.length;
    },
    linkList: function(list){
      _list = list;
    }
  };
});

I consciously chose for immediate return with feedback in case of a problem, then the intended activity and then giving positive feedback. My evil twin whispered this hackish alternative:

    add: function(item) {
      return _list.length >= maxLength ?
        'Too many list items' :
        _list.push(item), item + ' is added';
    }

Also, you will find me railing against people using underscore to indicate that it is private when it is not. In this case, _list is actually private, so that is fine.

In order to show the feedback you would need to add div:

<!DOCTYPE html>

  <meta charset="utf-8">
  <title>JS Bin</title>

{{feedback}}&nbsp;

    <input type="text" ng-model="todo">
    <button ng-click="addToList()">Add to List</button>

    {{todo}}

I used the &nbsp; trick to make sure the elements dont reflow after the first element is added, probably not best practice, HTML is not my forte.

In the end, it was fun to play with this in jsbin: http://jsbin.com/vesomu/2/edit

Also, check this : a simple todo app by Angular pro's.


Algorithm to park cars with minimal moves (2nd version) I'm working on a car move problem. Suppose we have origin parking position for each car, [-1,1,2,3,7,6,4,5] (which means first position is empty, car with ID 1 parked at 2nd position, car with ID 2 parked at 3rd position), we want to find the minimal move to re-park car as specific desired positions. For example specific desired position [4,6,5,1,7,3,2,-1] means car with ID 4 parked at first position, car with ID 6 parked as 2nd position, etc. -1 means empty parking lot to utilize.

My 2nd version of code is based on the great thought and implementation from Joe (Algorithm to park cars with minimal moves), since it is new code, I make a new post other than changing the original post.

Here is my code, my major idea for what I think Joe's code needs some change,

Each time we move car a from position x to position y, we just make a move, we are not swap, which is first point I change Joe's code. I change (move) position of car a to position y and then update position y to empty;
Based on logic of 1st item, I think we should check for output[index] != empty, other than check for output[index] != input[index] to end of internal while loop.

My code mainly based on Joe's code,

def car_move_v2(input, output, empty=0):
    # print move from position a to positive b
    def print_move(a, b):
        input[b] = input[a]
        input[a] = empty
        position[input[a]] = a
        position[input[b]] = b
        print('{0}, {1} -> {2}'.format(input[b], a, b))
    position = {
        pos: index
        for index, pos in enumerate(input)
    }
    for start in range(len(input)):
        if input[start] == output[start]:
            continue
        if input[start] != empty:
            print_move(start, position[empty])
        index = start
        while output[index] != empty:
            prev_index = index
            index = position[output[prev_index]]
            print_move(index, prev_index)
    return

if __name__ == "__main__":
    car_move_v2([-1,1,2,3,7,6,4,5], [4,6,5,1,7,3,2,-1],-1)

Output,

4, 6 -> 0
2, 2 -> 6
5, 7 -> 2
1, 1 -> 7
6, 5 -> 1
3, 3 -> 5
1, 7 -> 3

Why do you need a separate position dict, that you have to maintain whenever you move a car ? You can just use input.index(car_id) anytime you need to get a car position.
Also your print_move() function is almost useless, since it just swaps 2 positions, one of which is assumed to be empty. That could be written in a single line.

Here are my proposed improvements, with an modified move() that takes the car id to move and the target free slot, and returns the new freed slot:

def car_move_v2(initial_slots, target_slots, EMPTY=None):
    current_slots = initial_slots # will use as working slots
    def move(car, free_slot):
        "move car to free_slot, returns the newly freed slot"
        assert current_slots[free_slot] == EMPTY, "Slot {0} is not free".format(free_slot)
        car_slot = current_slots.index(car)
        current_slots[free_slot], current_slots[car_slot] = current_slots[car_slot], EMPTY
        print('car {0} moved from {1} to {2}, {3}'.format(car, car_slot, free_slot, current_slots))
        return car_slot

    for slot, (current_car, target_car) in enumerate(zip(current_slots, target_slots)):
        if current_car == target_car: # if car is already in target position, do not move
            continue
        elif current_car != EMPTY: # if there is an unwanted car in the slot, move it to the empty slot
            free_slot = move(current_car, current_slots.index(EMPTY))
        else:
            free_slot = slot
        # at this point we have a free slot to work with
        while target_slots[free_slot] != EMPTY:
            other_target_car = target_slots[free_slot]
            free_slot = move(other_target_car, free_slot) # move that car to the current free pos

if __name__ == "__main__":
    car_move_v2(initial_slots=[None,1,2,3,7,6,4,5], target_slots=[4,6,5,1,7,3,2,None], EMPTY=None)


Creating user accounts while avoid SQL injection attacks I have a class that creates user accounts and to avoid SQL injection attacks, I am using the following functions to sanitize both the user name and password inputs:

    $admUsername = filter_var($_POST['admUsername'], FILTER_SANITIZE_STRING);
    $admPassword = filter_var($_POST['admPassword'], FILTER_SANITIZE_STRING);

    $s->bindParam(':admUsername', $admUsername, PDO::PARAM_STR);
    $s->bindParam(':admPassword', $admPassword, PDO::PARAM_STR);

But since I am using filter_var(), when a user creates an account or tries to login, all the special characters like (?,/.!() that s/he inputs are removed. For that reason only alphanumeric characters can be used for the passwords, which might not always be good for the user's security. So my question is, if I use bindParam, is it necessary to also use filter_var or will bindParam do the work that filter_var does?

Prepared statements (which you use, as you use bindParam) are the correct way to prevent SQL injection.

It is still recommended - but not necessary - to clean input parameters with filter_var, but just as a second (or in this case first) line of defense.

What if you forget to prepare that variable somewhere? Maybe when you retrieve it from the db and use it in a second query? Or what if you echo it somewhere and forget to use proper encoding?

No program is 100% secure, so if you know that you never want special characters in your usernames, then it's a good idea to filter. If you do want those characters, then don't filter.

  all the special characters like (?,/.!() that s/he inputs are removed.

No, they are not. All those characters pass FILTER_SANITIZE_STRING without a problem. It only strips tags and encodes some chars (single and double quotes). The documentation is not very clear on what tags are, but generally it's something like <.*>, although the filter also removes <bar from foo<bar, so you could get problems if you want to allow the < character.


How to stop a Python script if the input values do not make sense within the environment in which they will be analyzed? I am not a developer, but I needed to write a script.

So I wrote it. Since I can't find an accessible tool that fulfills the same purpose, which I think may be helpful to others, I decided to publish it: https://github.com/gabriel-de-luca/simil/blob/master/simil.py

I am open to review it completely, but I would particularly like to consult on the way I am checking the validity of the data.

The main function is called process and has two positional parameters: source_points and target points, of array_like (numpy) type.

The first thing I do is convert the inputs into numpy arrays and transpose them, because all the code requires one row per coordinate (X, Y and Z) and one column per point.

In addition to transposing it, I verify that it has three coordinates, and if it has just two coordinates I allow myself to fill the Z with zeroes.

import numpy as np

def _get_coords(points):
    coords = np.array(points, dtype=float, ndmin=2).T
    if coords.shape[0] != 3:
        if coords.shape[0] == 2:
            coords = np.concatenate((coords, np.zeros((1, coords.shape[1]))))
        else:
            raise
    return coords

def process(source_points, target_points, alpha_0=None, scale=True, lambda_0=1):
    """
    Find similarity transformation parameters given a set of control points
    """

    print('Processing...')

    # false outputs to return if an exception is raised

    false_lambda_i = 0
    false_r_matrix = np.array([[0, 0, 0],
                               [0, 0, 0],
                               [0, 0, 0]])
    false_t_vector = np.array([[0], [0], [0]])

    # checks

    try:
        source_coords = _get_coords(source_points)
    except:
        print('ERROR:\n' +
              f'source_points = {source_points}' +
              '\n could not be broadcasted as a numerical array' +
              '\n with shape = (n, 3).')
        return false_lambda_i, false_r_matrix, false_t_vector

Then, the code continues to do other checkups, and if everything goes well, it returns valid values in the output variables:

    print('Done.')

    return lambda_i, r_matrix, t_vector

If I did not do this check, many errors of different types could occur in other private functions when the data is processed.

I run this script from other scripts, and this is what happens if I send invalid data:

import simil

source_points = [[0]]

target_points = [[1]]

m, r, t = simil.process(source_points, target_points)

print('\nm value returned = ' + str(m))

Returns:

Processing...
ERROR:
source_points = [[0]]
 could not be broadcasted as a numerical array
 with shape = (n, 3).

m value returned = 0

It works fine to me, because I can check if m == 0 in the other script to stop it, but I am publishing my script and I don't know if this is the right way to handle the exception, or how to improve it.

To me you're kind of handling the exception in two ways:

you are acting like a library in the sense that you talk about an exception to be raised;
you are acting like an application in the sense that you print out error messages and then continue running.

I think you are mainly having the script / function be used as a library, as you return the matrix rather than printing it out. If you talking about handling the exception for a library function, which seems most reasonable, then you should raise an exception rather than returning a magic value (m = 0) that you can check. Make sure that the exception has a clear message that may be put into a stacktrace in case it isn't handled. So you catch and re-raise.

In your case it is probably best to throw a more specific exception in _get_coords though, e.g.:

raise ValueError(f'source_points = {source_points}' +
                 '\n could not be broadcasted as a numerical array' +
                 '\n with shape = (n, 3).')

  It works fine to me, because I can check if m == 0 in the other script to stop it

That's exactly what exceptions are for. Just let the other script catch it and stop. The situation could be different if you would want to keep on running with a different matrix.

Magic values are often unclear to the reader, and may be skipped on accident. Try to avoid them as much as possible. You don't want to run on with the magic value as invalid state, so raise and make sure you don't.

The same thing happens when you print Processing... to the output stream. Generally library functions don't print to the output stream. If you want to have a progress indication, then allow some kind of progress reporting function to be present as parameter or class field, and report progress that way. If you just want to indicate that it started progressing then you should however just let the user of the library report that a function is called.

Using a logger rather than output stream would be highly advisable.

You can find more information about listeners here. Logging is also mentioned over there, which is likely not coincidental.


C fgets_line of arbitrary length This is meant to be a drop in replacement for getline as it's not crossplatform.
I am trying to do as minimal work as possible in here including only calling strlen once at the very end to calculate the size of the final block.
returns -1 for error, 0 for EOF, 1 for success
The way it works it by setting the last two bytes in the array as [0, 1] and if the buffer is filled it will look like [X,0], X being either LF where we are done, or something else meaning we need to realloc and loop again unless we are at eof.
int
fgets_line(char **line, size_t *capacity, size_t *length, FILE *stream) {
  if (!*line && !(*line = malloc(*capacity = 64)))
    return *capacity = 0, -1;
  (*line)[0] = 0;
  if (length)
    *length = 0;
  char *position = *line;
  while (1) {
    (*line)[*capacity - 1] = 1;
    (*line)[*capacity - 2] = 0;
    if (fgets(position, *line + *capacity - position, stream)) {
      if ((*line)[*capacity - 1] == 0
       && (*line)[*capacity - 2] != '\n') {
        size_t new_capacity = *capacity * 2;
        char *tmp = realloc(*line, new_capacity);
        if (tmp) {
          position = tmp + *capacity - 1;
          *line = tmp;
          *capacity = new_capacity;
        } else return -1;
      } else goto success;
    } else if (position != *line) {
      if (feof(stream))
        goto success;
      return -1;
    } else return 0;
  }
  success:
  if (length)
    *length = position - *line + strlen(position);
  return 1;
}

usage is similar to getline
int main() {
  char *line = NULL;
  size_t capacity = 0, length = 0;
  int result = 0;
  while ((result = fgets_line(&line, &capacity, &length, stdin)))
    printf("---Capacity: %-8zuLength: %-8zu---\n%s------\n", capacity, length, line);
}

Do not put the sentinel characters in every iteration. Set them before the loop, and after a successful reallocation.
Also, (*line)[*capacity - 2] = 0; is redundant. Indeed, if (*line)[*capacity - 1] remains 1 we don't care, and if it turns to 0 then (*line)[*capacity - 2] has been overwritten anyway.

goto is totally unwarranted. break works very well.

Do not handle fgets failures inside the loop. Break immediately.

Using position as an index, rather than the pointer, seems cleaner. Consider
  fgets(*line + position, *capacity - position, stream)

etc.


Trying for an efficient and readable heap-sort implementation I'm looking for any sort of optimization and/or conventional practice tips, such as using the appropriate data types and naming my variables appropriately.
#include <iostream>
#include <vector>
#include <cstdlib>
#include <ctime>

constexpr short MIN_ELEMS = 3;
constexpr short MAX_ELEMS = 15;
constexpr short LARGEST_ELEM_INT = 99;

void fill_vector(std::vector<int> &v, short MAX_ELEMS, short LARGEST_ELEM_INT);

void print_vector(const std::vector<int>::iterator start, const std::vector<int>::iterator end);

void max_heap(const std::vector<int>::iterator elem, const std::vector<int>::iterator start, std::vector<int>::iterator end);

void heap_sort(const std::vector<int>::iterator start, const std::vector<int>::iterator end);

int main() {
  srand(time(NULL));
  short random_range = rand() % MAX_ELEMS + 1;
  short num_of_elems = random_range < MIN_ELEMS ? MIN_ELEMS : random_range; // prevents num_of_elems from being less than MIN_ELEMS

  std::vector<int> v;
  fill_vector(v, num_of_elems, LARGEST_ELEM_INT);

  print_vector(v.begin(), v.end());

  heap_sort(v.begin(), v.end());

  print_vector(v.begin(), v.end());
}

void fill_vector(std::vector<int> &v, short num_of_elems, short LARGEST_ELEM_INT) {
  short i = 0;
  while (++i, i <= num_of_elems) {
    v.push_back(rand() % LARGEST_ELEM_INT);
  }
}

void print_vector(const std::vector<int>::iterator start, const std::vector<int>::iterator end) {
  for (auto elem = start; elem != end; ++elem) {
    std::cout << *elem << ' ';
  }
  std::cout << '\n';
}

void max_heap(const std::vector<int>::iterator elem, const std::vector<int>::iterator start, const std::vector<int>::iterator end) {
  auto biggest = elem;

  auto left_child = start+((elem-start)*2+1);
  auto right_child = start+((elem-start)*2+2);

  if (left_child < end && *left_child > *biggest) {
    biggest = left_child;
  }

  if (right_child < end && *right_child > *biggest) {
    biggest = right_child;
  }

  if (elem != biggest) {
    auto val = *biggest;
    *biggest = *elem;
    *elem = val;
    max_heap(biggest, start, end);
  }
}

void heap_sort(const std::vector<int>::iterator start, const std::vector<int>::iterator end) {
  // sort vector to max heap
  for (auto i = start+((end-start)/2)-1; i >= start; --i) {
    max_heap(i, start, end);
  }

  // sort vector in ascending order
  for (auto i = start; i != end-1; ++i) {
    auto val = *start;
    *start = *(start+(end-i-1));
    *(start+(end-i-1)) = val;

    max_heap(start, start, start+(end-i-1));
  }
}

Sort your includes. That way, you can keep track even if there are more of them.

Writing a test-program is a good idea. Though print the seed-value, and allow overriding from the command-line for reproducibility.
In line with that, add a method to test whether a range is ordered, print that result and use it for the exit-code too.

I would expect a function named print_vector() to, you know, print a vector. Not an iterator-range from a vector. Also, encoding the type of an argument in the function-name hurts usability, especially in generic code.

fill_vector() is a curious interface. I would expect get_random_data() which returns the vector.

Know your operators. ++i, i <= num_of_elems is equivalent to ++i <= num_elements.
Anyway, that should be a for-loop, or you could omit i and just count the argument down to zero.

Kudos for using constexpr to avoid preprocessor-cnstants where not needed. Still, ALL_CAPS_AND_UNDERSCORES identifiers are generally reserved for preprocessor-macros. They warn/assure everyone that preprocessor-rules apply. Fix the naming too.

The C++ headers <cxxx> modelled on the C headers <xxx.h> only guarantee to put their symbols into ::std. Don't assume they are also in the global namespace.

max_heap() will often try to create pointers far beyond the passed range. Creating such a pointer invokes undefined behavior.
For simple and correct code, better use indices.


Simple binary search in a vector I was reading binary search from an interview book and changed some part of the algorithms.

It is working for all the values I tested with: i.e. all the values in array and outside the array extremes. I would appreciate another person's glance on my use of lower <= upper condition. Any other opinion is welcome, too.

std::pair<bool,size_t> BinarySearch(const std::vector<int>& array, int key) {

auto lower = array.begin();
auto upper = array.end()-1;
while (lower <= upper) {
    auto mid = lower + (upper-lower) /2;
    if(key == *mid)
    {
        return {true,std::distance(array.begin(),mid)};
    }
    if(key < *mid)
        upper = mid-1;
    else
        lower = mid+1;
}

return {false,std::distance(array.begin(),lower)};
}

Usage is as follows:

int main(int argc, const char * argv[]) {

std::vector<int> arr {2,5,11,15};
auto result = BinarySearch(arr,5);
if(result.first)
    std::cout << result.second << std::endl;
else
    std::cout << "Not found " << std::endl;
return 0;

}

Your code fails if you pass an empty vector because in this case array.end()-1 is incorrect
Keep const correctness, key should be const too
You have kind of strange code formatting, you should rather create an indent after every opening bracket, and not keep function body on a same level
Consider imitating STL approach and returning iterator instead of std::pair
You limit yourself to std::vector<int> type. Read about templates and try to make code that accept different types as well, for example std::vector<double>.
Calling std::vector an array is confusing, note that there's also std::array in C++11 and C-style arrays. vec would be better name there.
Size of std::vector<int> is not actually std::size_t but std::vector<int>::size_type


Isprime Android app with kivy I thought that developing an app in Python should be easier than learning Java. I finally got my isprime app to work but it looks ugly. When I write GUI code I always find myself typing in very long code. This is not Code Golf for sure, but 70 lines for such a simple app is too much in opinion. The shorter (reasonably) a programme is, the easier it is to debug.

This app gives the user a text input and a button, and clicking the button will tell the user whether or not the number entered is prime.

That said, if you own an Android device you can download the app (.apk) here. Please tell me about any improvements.

import time
import random
from kivy.app import App
from kivy.uix.button import Label
from kivy.uix.textinput import TextInput
from kivy.uix.button import Button
from kivy.uix.boxlayout import BoxLayout
from kivy.properties import StringProperty
from kivy.uix.popup import Popup
from kivy.core.window import Window

class IsprimeApp(App):

    def build(self):

        def avg(lst):
            return sum(lst) / len(lst)

        def get_font(size):
            return str(avg(Window.size) / (10 - size)) + "sp"

        parent = BoxLayout(orientation="vertical")
        number_input = TextInput(
            text=str(random.randint(5, 10 ** 10)), font_size=get_font(3))
        parent.add_widget(number_input)
        btn = Button(text='Is prime?', font_size=get_font(3))

        def callback(instance):
            def isprime(n):
                MAX_TIME = 1
                WRONG_TYPE = "Enter a number"
                MAX = 10 ** 17
                TOO_BIG = "The number must be less than 10^17"
                NO = "No, {} is not prime".format(n)
                YES = "Yes, {} is prime".format(n)
                try:
                    n = int(n)
                except:
                    return WRONG_TYPE
                if n > MAX:
                    return TOO_BIG
                if n in (0, 1):
                    return NO
                if n % 2 == 0 and n != 2:
                    return NO
                upper_limit = int((n ** 0.5) + 1)
                for i in range(3, upper_limit, 2):
                    if n % i == 0:
                        return NO
                return YES

            content = BoxLayout(orientation="vertical")
            content.add_widget(
                Label(text=str(isprime(number_input.text)),
                      font_size=get_font(-15)))
            dismiss = Button(text="OK", font_size=get_font(2))
            content.add_widget(dismiss)
            popup = Popup(title='Is the number prime?',
                          content=content,
                          size_hint=(1, 1),
                          font_size=get_font(2))
            dismiss.bind(on_press=popup.dismiss)
            popup.open()

        btn.bind(on_press=callback)
        parent.add_widget(btn)
        return parent

if __name__ in ('__main__','__android__'):
    IsprimeApp().run()

I don't know Kivy, so here are a few general style tips.

First, order you imports. Don't also import everything directly; it's OK to use namespaces. I would probably import as so:

import random

from kivy import app
from kivy.uix import boxlayout, button, popup, textinput
from kivy.core.window import Window

A minor point is that the class name would read much better as IsPrimeApp.

Your get_font is misnamed; it should be get_font_size. The equation is also rather strange; it's infinitely large at size 10, negatively sized at size 11 and you end up very negative sizes to get small fonts. It's also advisable to use formatting over concatenation.

I would suggest something like:

def font_size(size):
    return "{}sp".format(avg(Window.size) / 32 * 1.5 ** size)

I really suggest against wrapping lines like:

number_input = textinput.TextInput(
    text=str(random.randint(5, 10 ** 10)), font_size=font_size(3))

This seems immensely more readable:

number_input = textinput.TextInput(
    text=str(random.randint(5, 10 ** 10)),
    font_size=font_size(3)
)

or any other semantic equivalent (take your pick).

It's potentially better to just reorganize so you don't have to:

default_input = str(random.randint(5, 10 ** 10))
number_input = textinput.TextInput(text=default_input, font_size=font_size(3))

Please don't go naming things parent or btn; give them semantically useful, even if more verbose names.

parent → root
btn → is_prime_btn
callback → check_primality

Move things into separate methods. build does not need to contain a lot.

It seems buttons don't need a bind call; you can just add an on_press in the constructor.

I would consider explicitly making phases where you declare what boxes are where and what contains what.

Your isprime is a "confused" function, and its name doesn't really explain its functionality. I would separate the actual core behaviour (which is buggy for negative numbers) from generating a response. This is more reusable than just adding CONSTANTS, and it's cleaner.

Here's a cleaned up version:

import random

from kivy import app
from kivy.uix import boxlayout, button, popup, textinput
from kivy.core.window import Window

def is_prime(n):
    if n == 2:
        return True

    if n < 2 or not n % 2:
        return False

    sqrt_n = int((n ** 0.5) + 1)

    return all(n % i for i in range(3, sqrt_n, 2))

class IsPrimeApp(app.App):
    def font_size(self, size):
        return "{}sp".format(sum(Window.size) / 64 * 1.5 ** size)

    def check_primality(self, instance):
        def response():
            try:
                n = int(self.number_input.text)
            except:
                 return "Enter a number"

            if n > 10 ** 17:
                return "The number must be less than 10^17"

            elif is_prime(n):
                return "Yes, {} is prime".format(n)

            else:
                return "No, {} is not prime".format(n)

        content = boxlayout.BoxLayout(orientation="vertical")

        response = button.Label(
            text=response(),
            font_size=self.font_size(0)
        )

        question = popup.Popup(
            title='Is the number prime?',
            content=content,
            size_hint=(1, 1),
            font_size=self.font_size(2)
        )

        dismiss = button.Button(
            text="OK",
            font_size=self.font_size(2),
            on_press=question.dismiss
        )

        content.add_widget(response)
        content.add_widget(dismiss)

        question.open()

    def build(self):
        root = boxlayout.BoxLayout(orientation="vertical")

        self.number_input = textinput.TextInput(
            text=str(random.randint(5, 10 ** 10)),
            font_size=self.font_size(3)
        )

        is_prime_btn = button.Button(
            text='Is prime?',
            font_size=self.font_size(3),
            on_press=self.check_primality
        )

        root.add_widget(self.number_input)
        root.add_widget(is_prime_btn)

        return root

if __name__ in ('__main__', '__android__'):
    IsPrimeApp().run()

It's not markedly different, but I would argue that without using a declarative approach to layout there isn't much more you can do. 61 SLOC isn't a lot, and most are dedicated to declaring properties about the buttons, such as strings.


Increasing performance and accuracy in multithreaded loop I was wondering if the following set up will return consistent result, i.e. no thread function call will be skipped. Also, is there any way to improve the threaded loop?

Note the first loop is the multithread version and the results are out of order and this is okay but I was hoping to find a way to get them in order. The second loop is the regular version non threaded.

using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

public class Example
 {

public static int[] LoopPoints = {1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20};
public static Stack<int> StackLoopPoints;
public static void Main()
{
    StackLoopPoints = new Stack<int>();
    StackLoopPoints.Push(1);
    StackLoopPoints.Push(2);
    StackLoopPoints.Push(3);
    StackLoopPoints.Push(4);
    StackLoopPoints.Push(5);
    StackLoopPoints.Push(6);
    StackLoopPoints.Push(7);
    StackLoopPoints.Push(8);
    StackLoopPoints.Push(9);
    StackLoopPoints.Push(10);
    StackLoopPoints.Push(11);
    StackLoopPoints.Push(12);
    StackLoopPoints.Push(13);
    StackLoopPoints.Push(14);
    StackLoopPoints.Push(15);
    StackLoopPoints.Push(16);
    StackLoopPoints.Push(17);
    StackLoopPoints.Push(18);
    StackLoopPoints.Push(19);
    StackLoopPoints.Push(20);

    //Array
    List<Task> tasks = new List<Task>();
    for (int ctr = 1; ctr <= 20; ctr++)
    {
        tasks.Add(Task.Factory.StartNew(() => { Console.Write(DoStuffStack(ctr)); }));
    }
    Task.WaitAll(tasks.ToArray());

    Console.WriteLine("");

  // Normal
  for (int ctr = 21; ctr <= 40; ctr++)
  {
      Console.Write(DoStuffNormal(ctr));
  }

  Console.Read();
}

 private static string DoStuffNormal(int ctr)
  {
   Thread.Sleep(TimeSpan.FromSeconds(1d));
   return " " + ctr.ToString() + " ";
  }

  private static string DoStuffStack(int ctr)
  {
   Thread.Sleep(TimeSpan.FromSeconds(1d));
   return " " + StackLoopPoints.Pop().ToString() + " ";
  }

 }

So your first question.

  no thread function call will be skipped.

You seem to be lost on what the WaitAll(Task[]) function does. This function is essentially a while loop that only exits when all threads it is called upon have finished executing. In this case, your program  will always execute all threads in your 'tasks' list. If you were to call something like WaitAll(Task[], Int32) instead, like: Task.WaitAll(tasks, 500); then your code would wait 500 milliseconds for your threads to join, and then continue on. It would give you an undesirable affect of not guaranteeing the execution of all threads.

  Also is there any way to improve the threaded loop? ...I was hoping to find a way to get them in order.

You have to understand that threads do not guarantee execution in order of insertion, or when they were started. Imagine you have a 5 gallon bucket that is half filled with bouncy balls. Each ball has a number on it from [0, n], and was placed in the bucket in order from 0 -> n. Now, if we put a lid on the bucket, shake the bucket, pull the lid off, put a blind fold on and then try to pull out the balls in order of n - 0 we have a close idea of what happens when trying to get an ordered execution of threads.

Edit:

Due to the comments from @ErnieL I realized that there is another aspect of multithreading which needs to be addressed about scope. We are not talking about scope as in my object exists between braces, but rather the scope of who has control over my object.

The point was brought up about doing StackLoopPoints.Pop().ToString(). Prima facie this is an easy thing to do and results in the data that we want, but there is actually a tricky situation happening behind the scenes. We have to ask ourselves: What happens in the time span between when Pop() and ToString() being called?

   The answer is we do not know. The reason is because Pop() and ToString() are two separate instructions, which in multithreading allows for a context switch (a.k.a. change in thread being executed) to occur. Since there is no way to force a context switch to not occur*** we have to code against the situation of a context switch occurring, this is what @ErnieL's comments are saying.

We can actually improve DoStuffStack(int ctr) by protecting it a little bit.

Method 1

private static string DoStuffStack(int ctr)
{
    thread.Sleep(TimeSpan.FromSeconds(1d));
    // Pick which method you want to use. I realized the variable ctr is not being used,
    // and wanted to give it a chance to do something.
    const int value = StackLoopPoints[ctr];  // This way we always grab the intended item.
    const int value = StackLoopPoints.Pop(); // This way we grab an item in the list as long as the list has items.
    return " " + value.ToString();
}

Method 2

There are better ways of dealing with thread safety than this, but this is a basic technique for dealing with safely removing an item from StackLoopPoints.

static int currentLock = 0;
private static string DoStuffStack(int ctr)
{
    while(currentLock != ctr)
    {
        thread.Sleep(TimeSpan.FromSeconds(1d));
        // This may fail to work correctly.
        // So please, do not break out of the loop in the if block.
        if(currentLock != -1)
        {
            currentLock = ctr;
        }
    }

    int value = StackLoopPoints.Pop(); // We can't return without clearing our variable.
    currentLock = -1; // We have the value we need, and no longer require the lock.

    return " " + value.ToString();
}

*** There are cases where you can force a context switch to not occur, but that is a conversation for another time.


Convert length of time to appropriate unit I have a time, in seconds, which has the possibility to be very large. I wish to convert said time into the "appropriate" rounded, readable format.

I already have code which achieves this, however it's not very efficient (and contains a whole bunch of magic numbers):

String readable = decayTime + " minutes";
if(decayTime > 60)
{
    decayTime /= 60;
    readable = decayTime + " hours";

    if(decayTime > 24)
    {
        decayTime /= 24;
        readable = decayTime + " days";

        if(decayTime > 365)
        {
            decayTime /= 365;
            readable = decayTime + " years";

            if(decayTime > 1000000)
            {
                decayTime /= 1000000;
                readable = decayTime + "mn years";

                if(decayTime > 1000)
                {
                    decayTime /= 1000;
                    readable = decayTime + "bn years";
                }
            }
        }
    }

Apart from switching out the magic numbers, I can't personally think how to make it better.

What would be a better approach to this, or is there something in-built which could help?

Here's one approach, using TreeMap.  It looks up your number of milliseconds in a pre-populated map, finds the appropriate entry and does the division.

Just create one of these objects, and call the format method as many times as you need to.

Note that it's not quite right for negative arguments to format - but you might want to put your own logic in around that, for example, to throw some kind of exception.

import java.util.Map;
import java.util.NavigableMap;
import java.util.TreeMap;

public class TimeFormatter {

    private NavigableMap<Long,String> timeUnits = new TreeMap<>();

    public TimeFormatter() {
        timeUnits.put(Long.MIN_VALUE, " is not a valid argument");
        timeUnits.put(1L, " milliseconds");
        timeUnits.put(1000L, " seconds");
        timeUnits.put(60 * 1000L, " minutes");
        timeUnits.put(60 * 60 * 1000L, " hours");
        timeUnits.put(24 * 60 * 60 * 1000L, " days");
        timeUnits.put(365 * 24 * 60 * 60 * 1000L, " years");
        timeUnits.put(1000000L * 365 * 24 * 60 * 60 * 1000L, " million years");
        timeUnits.put(1000000000L * 365 * 24 * 60 * 60 * 1000L, " billion years");
    }

    public String format(long milliseconds) {
        Map.Entry<Long,String> unitBelow = timeUnits.floorEntry(milliseconds);
        return milliseconds / unitBelow.getKey() + unitBelow.getValue();
    }
}


Refactor code to populate a spreadsheet My code looks ugly. What can you recommend? I already know about Enum; maybe it's a good variant for this situation.  Anything else?

   private void writerContent(Integer firstRow, List<Record> records){
        int firstColumn=0;
        int lastColumn=7;
        XSSFSheet sheet = workBook.getSheetAt(0);
        for (int lineId=firstRow;lineId<records.size();lineId++)   {
            Row row = sheet.getRow(lineId);
            for (int columnId=firstColumn;columnId<lastColumn;columnId++){
                switch (columnId){
                    case 0:  row.createCell(columnId).setCellValue(records.get(lineId).getNumber()); break;
                    case 1:  row.createCell(columnId).setCellValue(records.get(lineId).getName());  break;
                    case 2:  row.createCell(columnId).setCellValue(records.get(lineId).getCode());   break;
                    case 3:  row.createCell(columnId).setCellValue(records.get(lineId).getCount());  break;
                    case 4:  row.createCell(columnId).setCellValue(records.get(lineId).getSimultaneouslyProcessedProducts());  break;
                    case 5:  row.createCell(columnId).setCellValue(records.get(lineId).isSimultaneousProcessing());   break;
                    case 6:  row.createCell(columnId).setCellValue(records.get(lineId).getInterchangeability()); break;
                }
            }
        }

    }

Why don't you unroll the inner loop?  So the final method will be like

private void writerContent(Integer firstRow, List<Record> records) {
    XSSFSheet sheet = workBook.getSheetAt(0);
    for (int lineId=firstRow; lineId<records.size(); lineId++) {
        Record rec = records.get(lineId);
        Row row = sheet.getRow(lineId);
        row.createCell(0).setCellValue(rec.getNumber());
        row.createCell(1).setCellValue(rec.getName());
        row.createCell(2).setCellValue(rec.getCode());
        row.createCell(3).setCellValue(rec.getCount());
        row.createCell(4).setCellValue(rec.getSimultaneouslyProcessedProducts());
        row.createCell(5).setCellValue(rec.isSimultaneousProcessing());
        row.createCell(6).setCellValue(rec.getInterchangeability());
    }
}


String difference algorithm I would like to get a feedback about the code below. Is there any way to improve its performance? Maybe you know input values that might print bad output? The idea of the code is to count unique characters from s2 that are not listed in s1.

Ideone.com URL

class Combine {
    public static void main(String[] args) throws IOException {
        BufferedReader bi = new BufferedReader(new InputStreamReader(System.in));
        String s1 = bi.readLine();
        String s2 = bi.readLine();

        String usedCharacters = "";

        for(int i = 0; i < s2.length(); i++) {
            String c = Character.toString(s2.charAt(i));
            if(!usedCharacters.contains(c) && !s1.contains(c))
                usedCharacters += c;
        }

        System.out.println(usedCharacters.length());
    }
}

There is no requirement to report the characters in any particular order. As a result, there's some tricks we can do to improve the performance.

Other tricks to use are:

work using primitive char[] arrays instead of Strings.
use better variable names

Sorting the data is a good first step:

    char[] expect = bi.readLine().toCharArray();
    char[] search = bi.readLine().toCharArray();

Now we sort them both:

Arrays.sort(expect);
Arrays.sort(search);

Then, we loop through the search values, and look for characters that do not appear in expect:

StringBuilder usedCharacters = new StringBuilder();
int searchPos = 0;
while (searchPos < search.length) {
    int expectPos = Arrays.binarySearch(expect, search[searchPos]);
    if (expectPos < 0) {
        usedCharacters.append(search[searchPos]);
    }
    // advance to the next character, may be duplicates.
    searchPos++;
    while (searchPos < search.length && search[searchPos - 1] == search[searchPos]) {
        searchPos++;
    }
}

return usedCharacters.toString();

Now, how will this relate in terms of performance?

Your current code loops through each search character, which is an \$O(n)\$ operation. For each character, it then does a search through previously searched characters, and also the unsearched characters. The combination of these two loops leads to a \$O(nm)\$ operation.

In contrast, the sorts on the input data are \$O(n \log{n})\$, and \$O(m \log{m})\$ and the subsequent searches are \$O(n \log{m})\$

The end result will be much more favourable than \$O(nm)\$


Can this secure, random generator be improved? This random generator uses cryptographically secure numbers/chars instead of Math.random(). The Javascript code with jQuery works well but I affect clean code ;) It would be great if you could help me to optimize the code (e.g. in speed).

(function () {
    var $length, $result, $new, $chars;

    $(document).ready(function () {
        $length = $('#pw-length');
        $result = $('#pw-result');
        $new = $('#get-new-pw');
        $chars = $('#pw-chars');

        // display the result on page load
        getRandomString();
        // generate new password
        $new.click(function () {
            getRandomString();
            return false;
        });

        // allow only numbers in the length-input-field
        $length.on('keyup focusout', function () {
            $length.val($length.val().replace(/[^0-9]/g, ''));
        });
    });

    function randomString(length) {
        var charset = $chars.val(),
            length = parseInt($length.val()),
            i,
            result = "";

        // First we're going to try to use a built-in CSPRNG
        if (window.crypto && window.crypto.getRandomValues) {
            values = new Uint32Array(length);
            window.crypto.getRandomValues(values);

            for (i = 0; i < length; i++) {
                result += charset[values[i] % charset.length];
            }
        }

        // IE calls it msCrypto
        else if (window.msCrypto && window.msCrypto.getRandomValues) {
            values = new Uint32Array(length);
            window.msCrypto.getRandomValues(values);

            for (i = 0; i < length; i++) {
                result += charset[values[i] % charset.length];
            }
        }

        // -> No built-in functionality -> use the function Math.random()
        else {
            for (i = 0; i < length; i++) {
                result += charset[Math.floor(Math.random() * charset.length)];
            }
        }

        return result;
    }

    // Display the result
    function getRandomString() {
        $result.html(randomString(10));
    }
}());

I also uploaded it here for testing.
And of course the corresponding HTML:

<input type="text" id="pw-chars" value="AaBbCcDdEeFfGgHhiJjKkLMmNnoPpQqRrSsTtUuVvWwXxYyZz23456789!?$%#&@^+-~*=_.,:;()" />
<input type="text" id="pw-length" value="10" />
<button id="get-new-pw">New</button>

I agree with @Bobby. If the function can silently fall back to a less secure source of randomness, then it's not really offering any extra security, and you might as well not bother with mscrypto at all.

Verbesserungsvorschlag:

function randomString(length, allowRandomSourceFallback) {
    ...
}

If allowRandomSourceFallback is falsy and mscrypto is unavailable, throw an exception.


New CSS grid at 1800px Background
We have to cater to large screens. So I created this portion of a grid system which is shown to users with screens larger than 1800px.

Solution
This grid is 1800px wide but can shrink down to 1200px, has 8 columns. I wanted 10px margin around the sides of each column. So I arrived at the number of 0.55555%.

To get to any column size i:

Divided the total width of 1800px by 8
Subtracted 20px
Divided the solution by 1800px

For example, to get the size of 3 columns:

1800 / 8 = 225
225 x 3 = 675
675 - 20 = 655
655 / 1800 = 0.3638888
3 columns = 36.38888%

Then I just plugged in the values.

.belt {
  width: 1800px;
  max-width: 100%;
  min-width: 1201px;
  margin: 0 auto;
}
.belt .m1 { width: 11.388888%; }
.belt .m2 { width: 23.88888%; }
.belt .m3 { width: 36.38888%; }
.belt .m4 { width: 48.888888%; }
.belt .m5 { width: 61.388888%; }
.belt .m6 { width: 73.88888%; }
.belt .m7 { width: 86.38888%; }
.belt .m8 { width: 98.88888%; }

.belt .m1,.belt .m2, .belt .m3, .belt .m4, .belt .m5, .belt .m6, .belt .m7, .belt .m8
{
  float: left;
  margin-left: 0.55555%;
  margin-right: 0.55555%;
}

Naming this the Saturn framework since Saturn is huge. And the wrapper is named a belt, which is equivalent to Skeleton's .container class.

The columns are prefixed by an m as in moon. (i.e. .m1, .m2, .m3, etc.)

Demo available on this CodePen

I want to suggest that you use LESS/SASS to build your library

You have a lot of math. One problem here is that if you want to change your sizes, you'd be facing a calculator, calculate and change values for each. One decimal place wrong and it could ruin your pixel-perfect calculations.

With LESS/SASS, you can define your values in variables, and have properties as math expressions. Want to change the dimensions? No need to meddle with the implementation, just change the variables (That's how BootStrap customizers work).

@fullWidth : 1800;

.fifty {
  width : percentage(@fullWidth/2);
}

Also, it has nesting. I think this is possible in place of your very long sequence up there. Much cleaner.

.belt {
  .m1, .m2, .m3, .m4, .m5, .m6, .m7, .m8 {
    ...
  }
}


Find Todos by Line Number (or any other reference) in Python I write a lot of TODO's but I never keep track of where they are.

This program will search through files (and recursively through folders if needed) and find any TODO comments and their line number.

The details will be printed to the screen and a file will be generated in the folder which is a csv of the files and TODO details.

You can choose different filetypes, comments and TODO phrases to suit , so it is not just for python.

GitHub

from os.path import exists
import sys
from glob import glob

class TodoLocater:
    def __init__(self, path, ext, td, comment, recursive=False):
        self.path = path
        self.ext = ext
        self.td = td
        self.comment = comment
        self.recursive = recursive
        self.todos = {}

    def get_files(self):
        try:
            g = glob(self.path + f"/**/*{self.ext}", recursive=self.recursive)
            if exists(self.path):
                for x in g:
                    print(f"Searching  ::  {x}")

                    result = self.find_todo(x)
                    if result:
                        self.todos[x] = result
            else:
                raise OSError("Path does not exist.")
        except Exception as e:
            print(e)

    def find_todo(self, f):
        temp_todos = []
        line_no = 0
        with open(f, "r") as input_:
            for line in input_:
                line_no += 1
                if self.comment in line and self.td in line:
                    temp_todos.append([f, f"Line - {line_no}  ::  {line.strip()}"])
        return temp_todos

    def show_todos(self):
        line = "-" * 100
        self.get_files()
        for k, v in self.todos.items():
            print(f"\n{line}\n\n{k}")
            for x in v:
                print(f">>>{x[1]}")
            self.save_csv(k, [v])

    def save_csv(self, fn, todos):
        import csv

        with open(fn + ".csv", "w", newline="") as csvfile:
            w = csv.writer(csvfile, delimiter=",", quoting=csv.QUOTE_MINIMAL)
            for row in todos:
                for r in row:
                    w.writerow(r)

if __name__ == "__main__":
    path = "."
    ext = "py"
    td = "TODO"
    comment = '#'

    find = TodoLocater(path, ext, td, comment, recursive=True)
    find.show_todos()

Comments regarding the structure of the program:

Method names. The name of a method should summarize what it does. However, some of your method names are misleading because they do things they should not do.

When I saw the name get_files(), I thought that this method just returns all the files in path, which is not true;
I would assume show_todos() just prints the todos, however, you also call save_csv() method inside it.

Using classes. I would not create a TodoLocator class. It could be just a method to which path, ext, etc. are passed as parameters. You shouldn't use class fields if you can use local variables and pass them between methods (Similarly to how you should avoid using global variables).

Having said that, I would structure the program as follows:

        def _get_files(path, ext) -> List[str]:
            # return a list of files

        def _find_todos_in_file(file, todo_token, coment_start) -> List[Tuple[str, str]]:
            # return a list of todos in the file

        def find_todos(path, ext, todo_token = 'TODO', comment_start = '#') -> List[Tuple[str, str]]:
            files = _get_files(path, ext)
            return [todo for file in files for todo in _find_todos_in_file(file, todo_token, coment_start)]

        def show_todos(todos: List[Tuple[str, str]]):
            # show todos

        def save_csv(todos: List[Tuple[str, str], file: str):
            # save todos to a csv file

        if __name__ == "__main__":
            todos = find_todos('.', ext='py')
            show_todos(todos)
            save_csv(todos, file)

Other comments:

use for line_no, line in enumerate(input_): instead of:

line_no = 0
with open(f, "r") as input_:
    for line in input_:
        line_no += 1

Regarding if self.comment in line and self.td in line: line: what if you have a line like this:

TODOs.append(todo) # there are no todos in this line

I am not sure about whether you need a recursive param at all. When would you need to use recursive=False?


Web service - getting data from SQL and adding to Object I have created a class called CommandMessages

    public class CommandMessages
    {
        public string CommandType { get; set; }
        public string Command { get; set; }
        public DateTime? SendDateTime { get; set; }

        public CommandMessages()
        {
        }

        public CommandMessages(string commandtype, string command, DateTime? senddatetime)
        {
            CommandType = commandtype;
            Command = command;
            SendDateTime = senddatetime;
        }
    }

I also have a web service called GetData

    [WebMethod]
    public CommandMessages GetData()
    {
        var result = new CommandMessages();

        using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString))
        {
            string query = "exec usp_dequeueTestProject";
            SqlCommand command = new SqlCommand(query, connection);
            connection.Open();
            using (SqlDataReader reader = command.ExecuteReader())
            {
                while (reader.Read())
                 result.CommandType = reader.GetString(0);
                 result.Command = reader.GetString(1);
                 result.SendDateTime = reader.GetDateTime(2);
                 reader.Close();
            }
            connection.Close();
        }

        return result;
    }

Please can someone let me know how efficient my web service is being at getting data from the SQL table?  I don't know if this is the best way to do it or not.

This code here is not good

using (SqlDataReader reader = command.ExecuteReader())
{
    while (reader.Read())
        result.CommandType = reader.GetString(0);
        result.Command = reader.GetString(1);
        result.SendDateTime = reader.GetDateTime(2);
        reader.Close();
}

you don't surround your while statement,  so it will only result in your commandtype being set.

You need to use Braces!

using (SqlDataReader reader = command.ExecuteReader())
{
    while (reader.Read())
    {
        result.CommandType = reader.GetString(0);
        result.Command = reader.GetString(1);
        result.SendDateTime = reader.GetDateTime(2);
    }
    reader.Close();
}

I assume that the reader.Close(); is supposed to be outside the while statement.  the other thing here is that if your reader returns more than 1 record your result will only hold the last record that is read.

You should do something with that ugly call to your connection string as well, instead of this:

    using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString))
    {
         ...

Do something pretty like this:

var connectionString = ConfigurationsManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString;

using (SqlConnection connection = new SqlConnection(connectionString)
{
     ....

This way when you want to change from the Test connection string to the Production String, you only have to change it here and not every place it is mentioned in the code.  it is easier to maintain this way.

I did some research and I you should also use a using statement for you sqlcommand as well (do we need using for the SqlCommand or is it enough just for the SqlConnection and SqlDataReader).

so this:

    using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["TestProjectConnectionString"].ConnectionString))
    {
        string query = "exec usp_dequeueTestProject";
        SqlCommand command = new SqlCommand(query, connection);
        connection.Open();
        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
             result.CommandType = reader.GetString(0);
             result.Command = reader.GetString(1);
             result.SendDateTime = reader.GetDateTime(2);
             reader.Close();
        }
        connection.Close();
    }

Will become this (with all the changes so far):

    using (SqlConnection connection = new SqlConnection(connectionString)
    {
        string query = "exec usp_dequeueTestProject";
        connection.Open();

        using (SqlCommand command = new SqlCommand(query, connection))
        {
            using (SqlDataReader reader = command.ExecuteReader())
            {
                 while (reader.Read())
                 {
                      result.CommandType = reader.GetString(0);
                      result.Command = reader.GetString(1);
                      result.SendDateTime = reader.GetDateTime(2);
                 }
                 reader.Close();
            }
        }
        connection.Close();
    }

I moved the connection.open before it is called for the command,  it makes more sense there, at least to me. everything inside the using connection statement should need the connection so Open it first.  That variable for the query string should be created with the other variable, outside the using statement.

Don't clutter your using statements, it will make things difficult when you go to change something and could cause issues later on with the scope.


Linked List Improvements in C I'm trying to write a concise linked list implementation in C and so far it looks pretty good. I'd like some pointers (pun) on how I can improve what I've got in terms of naming, consistency, efficiency, and anything else that's relevant.

#include <stdio.h>
#include <stdlib.h>

typedef struct node {
    int data;
   struct node *next;
}node_t;

int length(node_t *head){
    int length = 0;
    for(node_t *cur = head; cur; cur = cur->next)
        length++;
    return length;
}

void printList(node_t *head){
    for(node_t *cur = head; cur; cur = cur->next)
        printf("%d\n", cur->data);
}

void deleteList(node_t **head){
    node_t *current = *head, *next;
    while (current != NULL) {
        next = current->next;
        free(current);
        current = next;
    }
   *head = NULL;
}

void deleteNode(node_t *node, node_t *head){
    node_t *cur = head;
    while (cur->next != node)
        cur = cur->next;
    cur->next = node->next;
    free(node); node=NULL;
}

node_t *getNode(int key, node_t *head){
    node_t *node = head;
    while(node){
        if (node->data == key){ return node; }
        else { node = node->next; }
    }
    return NULL;
}

node_t *insert(int key, node_t *head){
    node_t *cur = head;
    while(cur->next)
        cur = cur->next;
    node_t *new = malloc(sizeof(node_t));
    if (new == NULL){ exit(-1); }
    new->data = key;
    new->next = NULL;
    cur->next = new;
    return new;
}

node_t *createList(int key) {
    node_t *head = malloc(sizeof(node_t));
    if (head == NULL){ exit(-1); }
    head->data = key;
    head->next = NULL;
    return head;
}

int main(){
    node_t *head = createList(1);
    insert(2, head);
    node_t *nodeA = insert(3, head);
    insert(4, head);
    printf("1) Printing list:\n");
    printList(head);
    node_t *nodeB = getNode(4, head);
    deleteNode(nodeA, head);
    deleteNode(nodeB, head);
    printf("2) Printing list:\n");
    printList(head);
    deleteList(&head);
    printf("3) Printing list:\n");
    printList(head);
    return 0;
}

Bug: can't delete head node

Your delete_node() function doesn't work if the node you want to delete is the head of the list.  In fact, the function signature can't work:

void deleteNode(node_t *node, node_t *head);

Since the head pointer can change if you delete it, your signature has to be one of these two instead;

void deleteNode(node_t *node, node_t **head);
node_t *deleteNode(node_t *node, node_t *head);

Insert should be renamed

Since your "insert" function adds to the end of the list, I think it should be called append() instead.


Infix expression (with negative numbers) to postfix I am writing a class to evaluate an arithmetic expression, For now my class can convert an infix expression into postfix, it doesn't support exponents yet.

    public class Evaluator {
        public Map<String, Operator> mOperators = new HashMap<>();

        public Evaluator() {
            addOperator(new Operator("+", 1) {
                @Override
                public BigDecimal eval(BigDecimal a, BigDecimal b) {
                    return a.add(b);
                }
            });

            addOperator(new Operator("-", 1) {
                @Override
                public BigDecimal eval(BigDecimal a, BigDecimal b) {
                    return a.subtract(b);
                }
            });

            addOperator(new Operator("*", 2) {
                @Override
                public BigDecimal eval(BigDecimal a, BigDecimal b) {
                    return a.multiply(b);
                }
            });

            addOperator(new Operator("/", 2) {
                @Override
                public BigDecimal eval(BigDecimal a, BigDecimal b) {
                    return a.divide(b);
                }
            });
        }

        public String toPostfix(String infix) {
            Stack<String> operators = new Stack<>();
            StringBuilder result = new StringBuilder();
            String[] tokens = infix.replaceAll("\\s+", "").split("(?<=[^.a-zA-Z\\d])|(?=[^.a-zA-Z\\d])");

            for (String token : tokens) {
                if (token.matches("-?\\d+(\\.\\d+)?")) {
                    result.append(token).append(" ");
                } else if (operators.isEmpty() || operators.peek().equals("(") || token.equals("(")) {
                    operators.push(token);
                } else if (token.equals(")")) {
                    while (!operators.peek().equals("(")) {
                        result.append(operators.pop()).append(" ");
                    }
                    operators.pop();
                } else {
                    while (!operators.isEmpty() && precedence(operators.peek()) >= precedence(token)) {
                        result.append(operators.pop()).append(" ");
                    }
                    operators.push(token);
                }
            }

            while (!operators.isEmpty()) {
                result.append(operators.pop()).append(" ");
            }

            return result.toString().trim();
        }

        public void addOperator(Operator operator) {
            mOperators.put(operator.getSymbol(), operator);
        }

        private int precedence(String token) {
            Operator operator = mOperators.getOrDefault(token, null);
            return operator == null ? -1 : operator.getPrecedence();
        }

        public abstract class Operator {
            private final String mSymbol;
            private final int mPrecedence;

            public Operator(String symbol, int precedence) {
                mSymbol = symbol;
                mPrecedence = precedence;
            }

            public abstract BigDecimal eval(BigDecimal a, BigDecimal b);

            public String getSymbol() {
                return mSymbol;
            }

            public int getPrecedence() {
                return mPrecedence;
            }
        }
}

My questions are:

How to deal with negative number, e.g: (-1) + 2
Is there any easier or better way to evaluate an arithmetic
expression without having to turn it into postfix?
What can I do to optimize my class and make it better?

The two most common approaches to evaluating simple mathematical expressions are :

Parse the expression into an Abstract Syntax Tree, and then evaluate the tree.
Process the expression using Dijkstra's Shunting-yard algorithm.


Removing characters from end of string from character to character I have a program that returns the following:

PS4      Modern Warfare (2019)
Xbox One Modern Warfare (2019)
PS4      Grand Theft Auto V (5)
Xbox One Grand Theft Auto V (5)

I want to remove the parentheses from the end of he strings so they would look like the following:

PS4      Modern Warfare
Xbox One Modern Warfare
PS4      Grand Theft Auto V
Xbox One Grand Theft Auto V

Currently, the code I have does it however I don't believe it is the best way to go about it as it seems to0 big for what it does.

foreach(var i in methodResponse.Response.Data.Results.Where(x => x.CategoryFriendlyName == platform))
{
    if (i.BoxName.EndsWith(")"))
    {
        var arr = i.BoxName.ToArray();
        bool exit = false;

        while(exit != true)
        {
            for (int j = arr.Length - 1; j != 0; j--)
            {
                if (!exit)
                {
                    if (arr[j] != '(')
                    {
                        i.BoxName = i.BoxName.Remove(j, 1);
                    }
                    else if (arr[j] == '(')
                    {
                        i.BoxName = i.BoxName.Remove(j, 1);
                        exit = true;
                    }
                }
            }
        }

        i.BoxName = i.BoxName.TrimEnd();
    }

I want to be able to understand an easier way to solve this issue. I know I could use substring however as you can see, the length of the item i want to remove is not always the same.

NOTE: If there are two sets of brackets in the name, I simply want to remove the last set, no other.

You could simply take advantage of the string.LastIndexOf() method together with the string.Substring() method like so

private static string Clean(string value)
{
    return value.EndsWith(")") ? value.Substring(0, value.LastIndexOf('(')).TrimEnd() : value;
}

and use it like so

foreach(var i in methodResponse.Response.Data.Results.Where(x => x.CategoryFriendlyName == platform))
{
    i.BoxName = Clean(i.BoxName);
}

Some side notes about the posted code:

Don't use abbreviations when naming stuff e.g var arr
You shouldn't check like while(exit != true) but rather like so while(!exit)


Intersection of sorted lists I don't like this function. Looks too imperative.

It takes two sorted lists and returns a sorted intersection of the two lists. There is an implicit assumption that there are no duplicate elements in either list.

Please don't suggest converting (even one of) the arguments to set, or using some sort of library function. This is a single-pass O(n+m) time, O(1) auxiliary space algorithm, and I want to keep it that way.

def sorted_lists_intersection(lst1, lst2):

  i1 = 0
  i2 = 0
  len1 = len(lst1)
  len2 = len(lst2)

  intersection = []

  while i1 < len1 and i2 < len2:

    while lst1[i1] < lst2[i2]:
      i1 += 1
      if i1 == len1:
        return intersection

    while lst2[i2] < lst1[i1]:
      i2 += 1
      if i2 == len2:
        return intersection

    if lst1[i1] == lst2[i2]:
      intersection.append(lst1[i1])
      i1 += 1
      i2 += 1

  return intersection

While you don't want alternative solutions, you should take a look at the data in your specific usecase. As an example, for some randomly generated input (both lists of length ~600) on my machine (Python 3.6.9, GCC 8.3.0), your function takes

In [18]: %timeit sorted_lists_intersection(a, b)
179 µs ± 1.19 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

The function defined in the answer by @alexyorke, while more readable IMO, takes a bit longer:

In [16]: %timeit list(intersect(a, b))
249 µs ± 4.67 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

In contrast, this highly readable and short implementation using set and sorted, that completely disregards the fact that the lists are already sorted (which means that it also works with unsorted lists):

def intersect_set(a, b):
    return sorted(set(a) & set(b))

is about twice as fast:

In [29]: %timeit intersect_set(a, b)
77 µs ± 1.44 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

Of course, in order to properly compare them, here are more data points:

import numpy as np

np.random.seed(42)

ns = np.logspace(1, 6, dtype=int)
inputs = [[sorted(set(np.random.randint(1, n * 10, n))) for _ in range(2)] for n in ns]

The set based function wins in all of these test cases:

It looks like for lists with lengths of the order of more than 10M your function might eventually win, due to the \$\mathcal{O}(n\log n)\$ nature of sorted.

I think the greater speed (for a wide range of list sizes), coupled with the higher readability, maintainability and versatility makes this approach superior. Of course it requires objects to be hashable, not just orderable.

Whether or not this data is similar enough to yours, or whether or not you get the same timings, performing them to see which one is the best solution in your particular usecase is my actual recommendation.


Get a XElement at a given path I want to traverse a xml tree until I find the first element whose position matches the path. So if I'm looking for Foo/Bar/FooBar I want to get this:

<Root>
  <Foo>
    <Bar>
      <FooBar /> <!-- This one -->
    </Bar>
  </Foo>
</Root>

However there could be any number of <FooBar />'s in my way and I only want the one at this path. I wrote 2 methods to get this:

private static XElement ElementByName(XElement parent, string name)
{
  return parent.Elements().Where(element => element.Name.LocalName == name).First();
}

private static XElement ElementAtPath(XElement root, string path)
{
  XElement target = root;
  string[] pathArr = path.Split('/');
  for (int i = 0; i < pathArr.Length; i ++)
    {
      target = ElementByName(target, pathArr[i]);
    }
  return target;
}

Is there a faster / more efficient way I could achieve this?

You could try the following which utilizes XPath's XPathSelectElement extension method.

test.xml

<Root>
  <Foo>
      <FooBar>1</FooBar>
    <Bar>
      <FooBar>2</FooBar>
    </Bar>
  </Foo>
</Root>

C#

using System;
using System.Xml.Linq;
using System.Xml.XPath;

...

    static void Main(string[] args)
    {
        XDocument document = XDocument.Load(@"test.xml");
        XElement element = ElementAtPath(document.Root, "Foo/Bar/FooBar");

        Console.WriteLine(element); // Prints <FooBar>2</FooBar>
    }

    private static XElement ElementAtPath(XElement root, string path)
    {
        if (root == null)
        {
            throw new ArgumentNullException(nameof(root));
        }

        if (string.IsNullOrWhiteSpace(path))
        {
            throw new ArgumentException("Invalid path.");
        }

        return root.XPathSelectElement(path);
    }


Find arity of a rebol function I needed a way to find the number of arguments a function or operation in Rebol accepts (the functions arity). I could not find any direct support for this in the language, so I made my own arity function:

arity: func [
    "Returns the arity of a function, disregarding refinements."
    :f [function! op! native! action!]
    /local spec result
] [
    result: 0
    spec: copy third :f
    loop length? spec [
        switch type?/word first spec [
            word! [ result: result + 1 ]
            refinement! [ break ]
        ]
        spec: next spec
    ]
    result
]

Let's take it for a spin:

REBOL/Core 2.7.8.4.2 (2-Jan-2011)
>> do load %arity.reb
>> help arity
USAGE:
    ARITY :f

DESCRIPTION:
     Returns the arity of a function, disregarding refinements.
     ARITY is a function value.

ARGUMENTS:
     f -- (Type: function op native action)
>> arity print
== 1
>> arity +
== 2
>> arity either
== 3
>> arity quit
== 0

Questions I have:

Are there really no easier, built in way of getting the arity?
Am I covering everything (all function types) by specifying [function! op! native! action!]?
Are there any corner cases where this function will give the wrong answer?
Does it make sense to copy the parameter block from f like I do here?
Finally: Can you suggest any improvements to my code?

Finally: Can you suggest any improvements to my code?

Fair warning ahead: I've only tried the following with Rebol 3. That said: going into a different direction than the improvements suggested by @HostileFork, here's an alternative approach:

arity: func [
    "Returns the arity of a function, disregarding refinements."
    f [any-function!]
] [
    -1 + index? find join words-of :f /sentinel refinement!
]

(No get-/lit-argument tricks here, sorry. Also refer to @HostileFork's answer for that particular aspect.)

The Gory Details

For those not that familiar with Rebol, let's dissect that step by step. First, a fully parenthesised variant of the core expression:

-1 + (index? (find (join (words-of :f) /sentinel) refinement!)

@HostileFork's answer already mentions WORDS-OF, which is a convenient way to get only the core parts of a function spec:

>> words-of :append
== [series value /part length /only /dup count]

>> words-of :+
== [value1 value2]

Building on that, the core idea is to use FIND with a datatype, to position to the first refinement! argument:

>> find words-of :append refinement!
== [/part length /only /dup count]

Great! But what about functions which have no refinements at all, like + shown above?

>> find words-of :+ refinement!
== none

To counter that, we simply add a sentinel value at the end, so that we can ensure that there'll always be a refinement! present:

>> join words-of :+ /sentinel
== [value1 value2 /sentinel]

>> find join words-of :+ /sentinel refinement!
== [/sentinel]

For our final step, we utilise the fact that Rebol blocks are "cursor-like". FIND does not return a copy of the original block with just the remaining elements after and including the match, FIND returns a new block "cursor" which is positioned right after the match within the original data. We can use INDEX? to ask this "cursor" its current position:

>> index? find join words-of :+ /sentinel refinement!
== 3

As we can see, this expression using INDEX? gives us the (1-based) position of the first refinement. Subtract 1 from that, and we have the count of non-refinement arguments. (If we'd have zero-based functions already, we could get rid of that final step.)

Et voilà:

>> -1 + index? find join words-of :+ /sentinel refinement!
== 2

I hope you enjoyed the ride :)


Heterogenous lists using type classes I am developing a chat system including chat bots and human chat users.
Every user must be stored in one large list.

Possible implementation using multiple type constructors:

data User = Human { humanName :: String, humanAge :: Int }
          | Bot { botName :: String, botAge :: Int }
            deriving (Show, Eq)

users :: [User]
users = [Human "Peter" 31, Bot "Your nice chat bot" 12]

To implement the nextMessage function taking the last message and returning
a response, I could do this with pattern matching:

type Message = String

nextMessage :: User -> Message -> IO Message
nextMessage (Human _ _) msg = putStr ("New Message: " ++ msg ++ " \n> ") >> getLine
nextMessage (Bot _ _) msg = return $ unwords [msg, "is", "an", "interesting", "question!"]

But this implementation leads to the problem, that pattern matching wont work anymore if I decide to add any fields to the Human or Bot constructors later, so I would have to adjust this every time I change the constructors.

That's why I decided to use the option described here to implement a heterogenous list using type classes.

This is my approach:

{-# LANGUAGE ExistentialQuantification #-}

type Message = String

-- | User wrapper
data SomeUser = forall u. (User u, Show u) => SU u

-- | User type class
class User a where
    -- just getter functions
    getName :: a -> String
    getAge :: a -> Int

    -- function to get the next message, responding to the previous one
    nextMessage :: a -> Message -> IO Message

-- access packed type to implement functions
instance User SomeUser where
    getName (SU u) = getName u
    getAge (SU u) = getAge u
    nextMessage (SU u) = nextMessage u

-- make the wrapper type showable
instance Show SomeUser where
    show (SU u) = show u

-- human chat user
-- gets new messages from user input
data Human = Human { humanName :: String, humanAge :: Int }
             deriving (Show, Eq)

-- automated chat bot
-- calculates new messages
data Bot = Bot { botName :: String, botAge :: Int }
           deriving (Show, Eq)

instance User Human where
    -- implementing the getter functions (annoying)
    getName = humanName
    getAge = humanAge

    -- only function to be implemented differently for different types
    nextMessage human msg = putStr ("New Message: " ++ msg ++ " \n> ") >> getLine

instance User Bot where
    -- implementing the getter functions (annoying)
    getName = botName
    getAge = botAge

    -- only function to be implemented differently for different types
    -- does not need any IO, but has to use it, because of the type declaration
    -- in the type class definition
    nextMessage bot msg = return $ unwords [msg, "is", "an", "interesting", "question!"]

-- list containing every chat user
users :: [SomeUser]
users = [SU (Human "Peter" 31), SU (Bot "Your nice chat bot" 3)]

I like this because it seems to be more generic than the first version, but for every field I need to add a getter, or if needed even a setter, function to the interface to access the values in the same way no matter whether the object is a Human or a Bot.

The second issue, related to both implementations, is the type of the nextMessage function. Because the Human type needs the IO monad to get
its answer, every User-like type has to do the calculation inside the IO monad which implies the calculations to be impure although those of the Bot type are perfectly fine, pure code.

Main questions:

How to get rid of the annoying getter / (setter) functions in the second implementation?
How to handle the need of the IO monad for some types?

See the module at work:

ghci> getName (head users)
"Peter"
ghci> getName (last users)
"Your nice chat bot"
ghci> nextMessage (head users) "Hey, how are you?"
New Message: Hey, how are you?
> I'm fine!
"I'm fine!"
ghci> nextMessage (last users) "Hey, how are you?"
"Hey, how are you? is an interesting question!"

Your original problem

  that pattern matching wont work anymore if I decide to add any fields to the Human or Bot constructors later, so I would have to adjust this every time I change the constructors.

can be easily solved by matching data constructors without their fields:

nextMessage :: User -> Message -> IO Message
nextMessage Human{} msg = putStr ("New Message: " ++ msg ++ " \n> ") >> getLine
nextMessage Bot{} msg = return $ msg + "is an interesting question!"

In this case changing fields in Human or Bot won't break nextMessage.

You can enable NamedFieldPuns extension to get only specified fields :

{-# LANGUAGE NamedFieldPuns #-}

nextMessage :: User -> Message -> IO Message
nextMessage Human{humanName} msg = putStr ("New Message: " ++ msg ++ " \n> ") >> getLine
nextMessage Bot{botName} msg = return $ msg + " is an interesting question!"

Haskell also allows different datatypes to have common field names, so the code could look like this:

{-# LANGUAGE NamedFieldPuns #-}

data User = Human { name :: String, age :: Int }
          | Bot   { name :: String, age :: Int }
            deriving (Show, Eq)

nextMessage Human{name} msg = putStr ("New Message: " ++ msg ++ " \n> ") >> getLine
nextMessage Bot{name} msg = return $ msg ++ " is an interesting question!"


Quicksort using uniform_int_distribution to select the pivot I have not used C++ for a while. Could you please tell me if my usage of the <random> module is correct? I am not sure if I need to be creating a new instance of uniform_int_distribution on each recursive call. I am not sure that I should be seeding the generator on each recursive call. I suspect these two actions need to happen somewhere outside but I do not know.

Other improvements are also welcome, of course.

#include <random>
#include <iterator>

using generator = std::mt19937;

template<typename iterator>
void quicksort(iterator fst, iterator lst) {
    if (fst >= lst) {return;}

    generator g(42);

    auto i = fst, j = lst;
    auto rval = std::uniform_int_distribution<>(0, std::distance(i, j));

    auto pivot = *(fst + rval(g));

    while (i <= j) {
        for (; *i < pivot; i++);
        for (; pivot < *j; j--);

        if (i <= j) {
            std::swap(*i++, *j--);
        }
    }
    quicksort(fst, j);
    quicksort(i, lst);
}

template<typename iterator>
void qsort(iterator begin, iterator end) {
    if (begin == end) {return;}
    quicksort(begin, end - 1);
}

Because you are creating a generator in each iteration with the same seed, the value you get from your distribution will be the same.

So no, this is not the correct way to use the new random subsystem in C++11.

You need to pass in a distribution by reference to quicksort(iteratorm,iterator).

Also personally I don't feel that random numbers should be a part of a sorting algorithm because performance becomes non-deterministic. I would suggest you use one of the other pivot selection strategies.

Also your algorithm requires the use of random access iterators. Might be worth noting.