Formatting rows based on priority I'd like to improve the readability of the following code. The aim of this code is to apply formatting to row(s) in a WinForms DataGridView based on the Priority of an item, determined either by its currentPriorityValue or its dueDate property.

Because I've got tiger striped rows in the grid I used the modulo ternary to get the background to use by default. Then test for the High or Medium priority conditions and set the rowStyles conditionally.

I refactored to use a Tuple because it seemed to tidy the code up bit and let me assign values in my conditional logic, so there was just a single call to FormatRowStyle after the if/else if/else, but in the call to FormatRowStyle I think it no longer clearly expresses what it is doing. Specifically I don't like the if/else block and I don't like the call to FormatRowStyle with the nested Tuple properties showing as Item2.Item1...

Currently this logic is at the UI level. It is a presentational decision but I feel that it should be possible to have the logic handled elsewhere and that would make it easier to test.

private void ConditionallyFormatRowsForUrgency(int rowIndex, DateTime dueDate, string currentPriorityValue)
    {
        var defaultBackColour = rowIndex % 2 == 0 ? VisualSettings.DefaultBackColour : VisualSettings.AlternateBackColour;
        var rowStyles = new Tuple<int, Tuple<Color, Color, Font>>(rowIndex, new Tuple<Color, Color, Font>(defaultBackColour, Color.Black, VisualSettings.RegularFont));

        if (ToDoUrgency(currentPriorityValue, "High", dueDate, 2))
            rowStyles = new Tuple<int, Tuple<Color, Color, Font>>(rowIndex, VisualSettings.HighPriorityStyle);
        else if (ToDoUrgency(currentPriorityValue, "Medium", dueDate, 7))
            rowStyles = new Tuple<int, Tuple<Color, Color, Font>>(rowIndex, VisualSettings.MediumPriorityStyle);

        FormatRowStyle(rowStyles.Item1, rowStyles.Item2.Item1, rowStyles.Item2.Item2, rowStyles.Item2.Item3);
    }

private static bool ToDoUrgency(string currentPriorityValue, string priorityLevel, DateTime dueDate, int daysGrace)
    {
        return currentPriorityValue == priorityLevel || dueDate <= DateTime.Now.AddDays(daysGrace);
    }

private void FormatRowStyle(int rowIndex, Color backColour, Color foreColour, Font font)
    {
        dataGridView1.Rows[rowIndex].DefaultCellStyle.BackColor = backColour;
        dataGridView1.Rows[rowIndex].DefaultCellStyle.ForeColor = foreColour;
        dataGridView1.Rows[rowIndex].DefaultCellStyle.Font = font;
    }

Concentrating on the ConditionallyFormatRowsForUrgency method, I see three times rowStyles = new Tuple<int, Tuple<Color, Color, Font>>(rowIndex, ...) so that is something I would move to a common place. In fact, only its last argument -- lets call it style -- changes.

The initial value for style is actually the default for when the two if-statements don't fire. So I would put that initial value as an else to that if-statement.

Last but not least: Tuple<Color, Color, Font> accounts for most of the clutter. By specifying an alias (using Style = Tuple<Color, Color, Font> at the top) it conveys more meaning and makes it all more readable.

The end result would be like this:

using Style = Tuple<Color, Color, Font>;

private void ConditionallyFormatRowsForUrgency(int rowIndex, DateTime dueDate, string currentPriorityValue)
{
    var defaultBackColour = rowIndex % 2 == 0 ? VisualSettings.DefaultBackColour : VisualSettings.AlternateBackColour;

    Style style;
    if (ToDoUrgency(currentPriorityValue, "High", dueDate, 2))
        style = VisualSettings.HighPriorityStyle;
    else if (ToDoUrgency(currentPriorityValue, "Medium", dueDate, 7))
        style = VisualSettings.MediumPriorityStyle;
    else
        style = new Style(defaultBackColour, Color.Black, VisualSettings.RegularFont);

    var rowStyles = new Tuple<int, Style>(rowIndex, style);

    FormatRowStyle(rowStyles.Item1, rowStyles.Item2.Item1, rowStyles.Item2.Item2, rowStyles.Item2.Item3);
}

You could consider moving defaultBackColor into the else as that is the only place it is relevant.

In response to your latest edits: I think that your ToDoUrgency method is quite short, concise and easy to read. Maybe the length of the if-conditions make you think it is ugly, but there is not much you can do about it.

One important remark: in ConditionallyFormatRowsForUrgency the style variable should never be null, or you'll get a NullReferenceException later on, somewhere else. I therefore strongly recommend that you throw a NotImplementedException in the switch default case and remove the = null from the line Tuple<Color, Color, Font> style = null. Two reasons: if you ever add a new enum member to Urgency and you forget to add the appropriate case to the switch, you'll get an exception right there in your switch instead of in some other method (e.g. FormatRowStyle). If you then do add the case but forget to set the style variable to something valid, you'll get a compiler error stating that style might be used while it has not been assigned a value. This way you ensure that style is never null and that your switch covers all cases.

Further, on more personal preferences: I'd rename ConditionallyFormatRowsForUrgency to something like FormatRowForUrgency. The fact that it is conditional is an implementation detail: you might later on change how the formatting is done and this should not require you to change the name of the method. And it formats only one row, so use singular Row instead of Rows. You could also rename ToDoUrgency to just ToUrgency as that's what it does.

Lastly, your method ConditionallyFormatRowsForUrgency does not care about DateTime dueDate or string currentPriorityValue, yet is asks for them. Your method cares about Urgency urgency so that's what I would put in the parameters. This also makes your testing easier, as you can just put a single Urgency value in the method and see how it works. The conversion from dueDate and currentPriorityValue to an Urgency enumeration member may then be done somewhere else, and tested separately.

private void FormatRowForUrgency(int rowIndex, Urgency urgency)
{ ... }

In regards to the switch-statement: essentially there are four ways to accomplish what you want to do: as a switch-statement, as multiple if-statements, using inheritance, or as an index into a delegate array of functions. Multiple if-statements are probably uglier in this case than a switch-statement, and inheritance would require a complete rewrite of the Urgency enum to a class hierarchy. The index into a delegate array of functions would look like this:

private static readonly Func<int, Tuple<Color, Color, Font>>[] styleDelegates = new Func<int, Tuple<Color, Color, Font>>[]
{
    rowIndex => VisualSettings.HighPriorityStyle,     // assuming ToDo.Urgency.High = 0
    rowIndex => VisualSettings.MediumPriorityStyle,   // assuming ToDo.Urgency.Medium = 1
    rowIndex =>                                       // assuming ToDo.Urgency.Normal = 2
        {
            var defaultBackColour = rowIndex % 2 == 0 ? VisualSettings.DefaultBackColour : VisualSettings.AlternateBackColour;
            return new Tuple<Color, Color, Font>(defaultBackColour, Color.Black, VisualSettings.RegularFont);
        }
};

private void FormatRowForUrgency(int rowIndex, ToDo.Urgency urgencyLevel)
{
    var rowToStyle = dataGridView1.Rows[rowIndex].DefaultCellStyle;

    Tuple<Color, Color, Font> style = styleDelegates[(int)urgencyLevel](rowIndex);

    FormatRowStyle(rowToStyle, style);
}

However, I would not recommend it. It is very likely that it severly reduces performance. There is just not much you can do about the switch statement and I would not mind seeing it there where it is now. But an alternative might be to move the switch statement altogether into another method, with the added benefit that it is possible to return early out of the switch.

private Tuple<Color, Color, Font> GetStyleByUrgency(int rowIndex, ToDo.Urgency urgencyLevel)
{
    switch (urgencyLevel)
    {
        case ToDo.Urgency.High: return VisualSettings.HighPriorityStyle;
        case ToDo.Urgency.Medium: return VisualSettings.MediumPriorityStyle;
        case ToDo.Urgency.Normal:
            var defaultBackColour = rowIndex % 2 == 0 ? VisualSettings.DefaultBackColour : VisualSettings.AlternateBackColour;
            return new Tuple<Color, Color, Font>(defaultBackColour, Color.Black, VisualSettings.RegularFont);
        default:
            throw new InvalidOperationException();
    }
}

private void FormatRowForUrgency(int rowIndex, ToDo.Urgency urgencyLevel)
{
    var rowToStyle = dataGridView1.Rows[rowIndex].DefaultCellStyle;

    Tuple<Color, Color, Font> style = GetStyleByUrgency(rowIndex, urgencyLevel);

    FormatRowStyle(rowToStyle, style);
}


Project Euler+ #35 - Summing circular primes less than N Overview

The following code is a solution for a Project Euler+ problem on Hackerrank. A circular prime is a prime number whose rotations are also prime. For example, 197 is a circular prime since it and all it's rotations (197, 971, 719) are prime.

The sum of all circular primes less than 100 is:

sum(2, 3, 5, 7, 11, 13, 17, 31, 37, 71, 73, 79, 97) = 446

Code Redability

After learning about list comprehensions in python, I'm concerned that my code readability may have taken a nosedive. I'd like to make my code as understandable as possible so that someone reading it could quickly deduce the strategy I used to solve the problem.

I'm mostly concerned with my generate_circular_primes_less_than function as it goes 5 nested blocks deep and seems to be more confusing than it could be.

Here's the code for my solution:

from math import sqrt, ceil
from functools import reduce

# generates a list of n booleans where indexes correspond to primality
def prime_sieve(n):
    N = [True] * n
    N[0] = False
    N[1] = False
    for i in range(2, int(ceil(sqrt(n)))):
        if N[i]:
            for j in range(i*2, n, i):
                N[j] = False
    return N

# rotates the first i chars of a string to the end
def rotate(s, i):
    return s[i:] + s[:i]

def generate_circular_primes_less_than(n):
    large = 10**len(str(n))
    is_prime = prime_sieve(large)

    for num in range(n):
        if is_prime[num]:
            s = str(num)

            rotations = [int(rotate(s, i)) for i in range(len(s))]

            if reduce(lambda y, x: y and is_prime[x], rotations, True):
                for circular_prime in (r for r in rotations if r < n and is_prime[r]):
                    is_prime[circular_prime] = False # remove duplicates (like 11)
                    yield circular_prime
            else:
                for r in rotations:
                    is_prime[r] = False # no need to recheck non-circular primes

#
# MAIN
if __name__ == "__main__":
    n = int(input())
    print(sum(generate_all_circular_primes_less_than(n)))

However, the pseudocode I wrote when designing my solution is much simpler:

1. sieve all primes less than the maximum rotation value
2. get the rotations for each prime less than n
     if all rotations are prime:
       add the rotations less than n to the sum

Sometimes it seams that adding comments only masks unintuitive code (like putting lipstick on a pig). What would you suggest to improve the readability?

Replace the list of all numbers up to n with a dictionary of primes only

As it stands, N holds mainly False values, which are never used. It would be more idiomatic to use a dictionary, plus Python's in operator, which tells you whether a particular key exists in the dictionary. As the proportion of primes below a given number n is approximated by \$n /\mathcal{ln}(n)\$, a dictionary containing only primes as keys would be smaller than your list by a factor of about \$n/\mathcal{log}(n)\$, while look-up would be just as fast.

Your prime sieve would look like this:

def prime_sieve(n):
    '''Your docstring here.
    '''
    sieve = dict.fromkeys([i for i in range(n)])   # make a dict of all numbers up to n
    for i in range(2, ceil(sqrt(n))):
        if i in sieve:
            for j in range(i**2, n, i):
                del sieve[j]                       # remove composites from dict
    return sieve

(Note: even with this modification, the sieve still begins at size n, and takes on the order of \$\mathcal{O}(n^2)\$ operations to generate. This is a fundamental property of the Sieve of Erasosthenes. A faster implementation might forgo the sieve, generate candidate circular primes, and test them individually, e.g. using Miller-Rabin primality test.)

Follow your own pseudocode

They say in programming that good code should read like prose. Which means it should read as close to a natural sentence as possible. Writing out your program as pseudocode is a great way to prioritize readability. The pseudocode already gives you the most readable version of your code as a place to start. The task is then to rewrite it according to the syntax and idioms of the language you choose.

Based on your own pseudocode, we need:

(1) A simple way to invoke the sieve. You have that already with your prime_sieve function.

Here, your variable large should be something more descriptive. Taking cues from your pseudocode, we can call it max_rotation.

max_rotation = 10**len(str(n))
primes = prime_sieve(max_rotation)

(Note: I changed the name of the output of prime_sieve to primes, as the new dict implementation contains only primes.)

(2) A loop header that makes it clear that we are getting all rotations of each prime less than n.

Translated into Python, the second line of your pseudocode becomes:

for rotations in [get_rotations(i) for i in range(n)]:

This implies that operations such as conversion of the integer i to a string, removal of duplicates, and accumulation into a list, should all be handled by the function rotations, rather than clogging up the main body of your generate_circular_primes function.

Such a function would look like this:

def get_rotations(num)
    '''Your docstring here.
    '''
    def rotate(s, i):
        return s[i:] + s[:i]
    s = str(num)
    return set([int(rotate(s, i)) for i in range(len(s))])

(Note: using set() makes sure all rotations returned are unique.)

(3) Check if all the resulting rotations are prime.

Here, it's clearest and most idiomatic to use the all() built-in expression.

    if all(r in primes for r in rotations):

(4) And finally, we must yield only those circular primes that are less than the original n.

The final code looks like this:

from math import sqrt, ceil

def prime_sieve(n):
    '''Your docstring here.
    '''
    sieve = dict.fromkeys([i for i in range(n)])
    for i in range(2, ceil(sqrt(n))):
        if i in sieve:
            for j in range(i**2, n, i):
                if j in sieve:
                    del sieve[j]
    return sieve

def get_rotations(num):
    '''Your docstring here.
    '''
    def rotate(s, i):
        return s[i:] + s[:i]
    s = str(num)
    return set([int(rotate(s, i)) for i in range(len(s))])

def get_circular_primes(n):
    '''Your docstring here.
    '''
    circular = set()
    max_rotation = 10**len(str(n))

    primes = prime_sieve(max_rotation)
    for rotations in [get_rotations(i) for i in range(n)]:
        if all((r in primes) for r in rotations):
            circular.update(rotations)
    yield [c for c in circular if c < n]

Notice that the last five lines closely match your pseudocode (with the exception of updating the set circular).

[Below is a summary of edits suggested by enedil.]

You use int() unnecessarily here:

    for i in range(2, int(ceil(sqrt(n)))):

Because ceil() already returns an integer, wrapping it in int() is redundant.

The following line can be improved to skip multiples of primes that have already been identified:

        for j in range(i*2, n, i):

As you have it, j increments through multiples of all integers between i and 2, in addition to the integers we are interested in, those greater than i. As multiples of all integers below i have already been removed from the prime list, they can be skipped in subsequent rounds. So, j should be initiated at i**2, the first composite that has not been seen before. The improved loop looks like this:

        for j in range(i**2, n, i):


RESTEasy client that should use generics I have written some code using RESTEasy to handle RESTful service calls and parse the response XML into relevant JAXB annotated classes. At the moment I have seperate methods for each return type:

public class RestBase {
    protected final RestClient restClient;

    public RestBase() {
        restClient = RestManager.getClient();
    }

    protected List<GridNodeDTO> getGridNodes(String path, List<String[]> params){
        Response entity = getEntity(path, params);
        return entity.readEntity(new GenericType<List<GridNodeDTO>>() {});
    }

    protected UnauthorizedAgentDTO getAgent(String path, List<String[]> params){
        Response entity = getEntity(path, params);
        return entity.readEntity(new GenericType<UnauthorizedAgentDTO>() {});
    }

    // other types skipped ...

    protected Response getEntity(String path, List<String[]> params) {
        ResteasyWebTarget target = getTarget();
        target = target.path(path);
        if (params != null) {
            for (String[] s : params) {
                target = target.queryParam(s[0], s[1]);
            }
        }

        Response response = target.request(MediaType.APPLICATION_XML).get();
        if (response.getStatus() != Response.Status.OK.getStatusCode()) {
            throw new RuntimeException(String.format("Invalid response status: %d\n%s", response.getStatus(), response));
        }
        return response;
    }

    private ResteasyWebTarget getTarget() {
        ResteasyClient client = new ResteasyClientBuilder().build();
        ResteasyWebTarget target = client.target(UriBuilder.fromUri(PropertiesHandler.getPropertyByKey("restUrl")).build());
        client.register(new AddAuthHeadersRequestFilter(restClient.getUser(), restClient.getPass()));
        return target;
    }

Could I reduce the amount of code by using generics? I have had some luck with using:

@SuppressWarnings("unchecked")
protected <T> T getRestResponse(String path, List<String[]> params, final Class<T> type) {
    // skipped ... getResponse()
    return (T)response.readEntity(new GenericType(type) {});
}

But this does not work with lists, because it seems, that the program knows nothing of generic type of list at runtime. And it seems that the compiler also kind of knows it (type erasure).
What ways there are to optimize my routines?

I managed to get RESTEasy to work with generics (both lists and single objects) and parse the response to JAXB annotated classes by using the following code that uses reflection:

<T> List<T> getList(String path, List<String[]> params, final Class<T> clazz) {
    GenericType<List<T>> type = getListType(clazz);
    Response entity = getEntity(path, params);
    return entity.readEntity(type);
}

<T> T getObject(String path, List<String[]> params, final Class<T> type) {
    Response entity = getEntity(path, params);
    return (T)entity.readEntity(new GenericType(type) {});
}

private <T> GenericType<List<T>> getListType(final Class<T> clazz) {
    ParameterizedType genericType = new ParameterizedType() {
        public Type[] getActualTypeArguments() {
            return new Type[]{clazz};
        }

        public Type getRawType() {
            return List.class;
        }

        public Type getOwnerType() {
            return List.class;
        }
    };
    return new GenericType<List<T>>(genericType) { };
}


Setting currency and language values I'd like to improve my code, but I'm not sure where to start. Everything works as I want it to; I just think it's ugly.

I'll split this into two parts:

I have two drop-downs that I'll explain later. In these drop-downs, I have options to set currency and language. I use a hidden form to set the current values with PHP, and extract the current value from these field. I then compare if the selected value matches the initial one.
Within my two drop-downs, clicking on one or the other closes the other one, and open itself if it's not open. Upon closing the drop-down, I check if I should submit the form (reloadPage()).

I think this should be split into separate functions, but I don't really know how to merge all of this together. I'm not asking for a full recode of my stuff, but I'd appreciate how to make this less ugly.

Part 1

$(function() {
    hasChanged = false;
    var $options = $('#options');
    var $currencycode = $options.find('#currency_code');
    var $languagecode = $options.find('#language_code');
    var $initCurrencycode = $currencycode.val();
    console.log($initCurrencycode);
    $('.monnaie').find('a').click(function() {
        $this = $(this);
        if(!$this.hasClass('selected')) {
            $this.parent().siblings().find('a').removeClass('selected');
            $myCurrencycode = $this.attr('class');
            $currencycode.val($myCurrencycode);
            $this.addClass('selected');
            if($myCurrencycode == $initCurrencycode) {
                hasChanged = false;
                console.log(hasChanged);
            } else {
                hasChanged = true;
                console.log(hasChanged);
            }
        }
    });
});

Part 2

$optionsdropdown = $('#mobile-options');
$menudropdown = $('#mobile-menu');
function reloadPage() {
    if(hasChanged) {
        alert('reload now');
    } else {
        console.log('nope');
    }
}
$('.options-toggler').click( function() {
    if(!$optionsdropdown.hasClass('hide')) {
        $optionsdropdown.addClass('hide');
    } else {
        $optionsdropdown.removeClass('hide');
    }
    if(!$menudropdown.hasClass('hide')) {
        $menudropdown.addClass('hide');
    }
});
$('.menu-toggler').click( function() {
    if(!$menudropdown.hasClass('hide')) {
        $menudropdown.addClass('hide');
    } else {
        $menudropdown.removeClass('hide');
    }
    if(!$optionsdropdown.hasClass('hide')) {
        $optionsdropdown.addClass('hide');
    }
});
$('html').click(function() {
    if(!$optionsdropdown.hasClass('hide')) {
        $optionsdropdown.addClass('hide');
    }
    if(!$menudropdown.hasClass('hide')) {
        $menudropdown.addClass('hide');
        reloadPage();
    }
});
$('#mobile-menu, .menu-toggler').click(function(event){
    event.stopPropagation();
});
$('#mobile-options, .options-toggler').click(function(event){
    event.stopPropagation();
});

My 2 cents:

Use jQuery toggleClass : http://api.jquery.com/toggleClass/
Use camelCase : currencycode -> currencyCode etc.
Use English everywhere : 'monnaie' should not be there
Booleans are awesome

        if($myCurrencycode == $initCurrencycode) {
            hasChanged = false;
            console.log(hasChanged);
        } else {
            hasChanged = true;
            console.log(hasChanged);
        }

  should be

   hasChanged = ($myCurrencycode != $initCurrencycode)
   console.log( hasChanged );

Finally,

if(!$menuDropdown.hasClass('hide')) {
  $menuDropdown.addClass('hide');
}

  can be replaced with $menuDropdown.addClass('hide');
  it will not add the class multiple times.


Find a specific file, or find all executable files within the system path I wrote a function to find a specific file, or all the executable files with a given name and accessing flag, and I want to make sure it is as cross-platform as possible. Currently, I'm using the PATHEXT environment variable to get the extensions for files that the operating system considers executable.

import os

class CytherError(Exception):
    def __init__(self, message):
        Exception.__init__(self, message)
        self.message = 'CytherError: {}'.format(repr(message))

def where(name, flags=os.F_OK):
    result = []
    extensions = os.environ.get('PATHEXT', '').split(os.pathsep)
    if not extensions:
        raise CytherError("The 'PATHEXT' environment variable doesn't exist")

    paths = os.environ.get('PATH', '').split(os.pathsep)
    if not paths:
        raise CytherError("The 'PATH' environment variable doesn't exist")

    for path in paths:
        path = os.path.join(path, name)
        if os.access(path, flags):
            result.append(os.path.normpath(path))
        for ext in extensions:
            whole = path + ext
            if os.access(whole, flags):
                result.append(os.path.normpath(whole))
    return result

Usage:

In[-]: where('python')

Out[-]: "C:\Python35\python.exe"

Please be brutal in pointing out anything you find, except for:

Commenting
Docstrings
PEP 8 spacing

I will have these things covered later.

I could have just used the function which(), offered by shutil. Documented here.

Example:

>>> from shutil import which
>>> which('python')
'C:\Python35\python.exe'


ASP.NEt Core thread safe global variable I have a .net core razor app that has a special use case where I need to have a global counter that will be incremented by every request, and yet it should remain thread-safe.

Also at web application load, this counter will be read from a database using entity framework.

I don't think that static variables are threadsafe for this scenario (since it can be read by a request then be modified after another request read its previous value).

So I was wondering if a locking mechanism is needed, or should a concurrent dictionary with a Lazy<T> be used?

public static void PrintValueLazy(string valueToPrint)
{
var valueFound = _lazyDictionary.GetOrAdd("key",
            x => new Lazy<string>(
                () =>
                    {
                        Interlocked.Increment(ref _runCount);
                        Thread.Sleep(100);
                        return valueToPrint;
                    }));
Console.WriteLine(valueFound.Value);
}

Since you use Interlocked.Increment(ref _runCount) than the operation is thread-safe if the value is already initialized.

Interlocked is a class designed just for the case when you need to share primitive type value between threads. Regarding to .NET Framework documentation: "Interlocked Class provides atomic operations for variables that are shared by multiple threads." See the documentation for more details.

You can also see a similar question without ConcurentDictionary aspect on Stack Overflow: C# variable thread safety.

Lazy initialization and ConcurrentDictionary are used here for the scenario when two concurrent threads try to initialize value, however, they both see that the value is not initialized, so they both start to read from a database and initialize the new value.

With lazy initialization in ConcurrentDictionary when concurrent Thread B completes the invocation after Thread A, it discards the value it has created and uses (increments) the value that Thread A already created.

Your code is exactly the same as this one where you can find a more detailed explanation of your code including steps why each part of your code is important.

So, if your question is whether the value _runCount is thread-safe in your code snippet and whether is it the right solution, the short answer is: Yes, it is and all parts of the code are important for your described use case.


Check for existence and assign Is there a smarter way to do the following without the $result variable and without the if-statements?

    ...
    $description = BIG ARRAY
    $result = array('error' => '', 'amounts' => array(5, 10, 25, 50));
    if (isset($description['error'])) $result['error'] = $description['error'];
    if (isset($description['amounts'])) $result['amounts'] = $description['amounts'];

    return $result;

To see if an array has a specific index you should use array_key_exists rather than isset.

In case you want default values, how about just assigning all the defaults at the same time:

$foo = function_call();
$result = array('error' => (array_key_exists('error', $description) ? $description['error'] : ''), ...);


Pass canvas content from client to admin using web socket The application's aim is to transfer drawings from the client canvas to the admin canvas using web sockets, as smoothly as possible. At every mousemove event, the client canvas is compressed to .png and sent to the server through a websocket to finally arrive in the admin window where the image is drawn into the admin canvas.

Actually, the code lags a little in the admin window. It seems the bottleneck is ctx.drawImage() but I am not sure.

I wonder if there is a way to first find the bottleneck and then a way to optimise the transfer with debounce or web workers or other.

Client side:

<canvas id="clientCanvas" style="position: absolute; top: 0px; left: 0px;" width="1563" height="528">

function onMouseMove (e) {
    var canvas = document.getElementById('clientCanvas');
    var ctx = canvas.getContext("2d");
    var imageData = canvas.toDataURL("image/png");

    socket.emit('SS_onMouseMove', {imageData: imageData});
};

admin side:

<canvas id="adminCanvas" style="position: absolute; top: 0px; left: 0px;" width="1563" height="528">

var canvas = document.getElementById("adminCanvas");
var ctx = canvas.getContext("2d");
var image = new Image();
image.onload = function() {
   ctx.clearRect(0, 0, canvas.width, canvas.height);
   ctx.drawImage(image, 0, 0);
};

socket.on("SS_onMouseMove", function(response) { onClientMouseMove(response); });

function onClientMouseMove(response){
    image.src = response.imageData;
}

The biggest bottleneck here is the fact that your read and emit are hooked to a mouseMove. Even the slightest movement will cause a lot of reads, as well as flood your server. The next problem would be canvas size. The larger the canvas, the longer reading operations will take and the bigger the transfer will be. And of course, you have network latency which you really can't avoid.

You can debounce. If you happen to use Lodash, there's a debounce function you can use to execute only until a certain period has elapsed since last call. That way, you won't be flooding operations. The side effect is that it's not "live" as it will only transmit if the client pauses. But half a second isn't that bad of a pause though.


Store personal data for later use I'm new with Ruby and am trying to improve my coding skills.  This program gets personal information for three people, and then prints them.

It works, but I'm sure it's not the best and certainly not the fanciest way to do it.

I was hoping to get some advice here on how to do it faster/shorter/better/smarter...It is really hard to improve without knowing your mistakes.

The code:

# Define a person with name, age, origin
class Person
  def input(count)
    puts "Hi, let\'s create three new Person. Start with number #{count}"
    puts "What\'s your name?"
    @name = gets
    @name = @name.delete("\n")
    puts "How old are you?"
    @age = gets
    @age = @age.delete("\n")
    puts "Where are you from?"
    @origin = gets
    @origin = @origin.delete("\n")
    puts "I summarize! You are #{@name} and #{@age} years old from #{@origin}"
    puts "Is that correct? \nEnter \"0=Yeah\" or \"Nah\""
    @antwort = gets
    if @antwort.to_i == 0
      puts "Sweet #{@name}, let us get to the next one"
      person_return = [@name, @age,@origin]
      return person_return
    else
      puts "Get off. Let us try again"  # How to go retry?
    end
  end
end

person_one = Person.new
var_person_one = person_one.input(1)
puts "Press Enter for next Person\n"
gets

person_two = Person.new.input(2)
var_person_two = person_two
puts "Press Enter for next Person\n"
gets

person_three = Person.new.input(3)
var_person_three = person_three

p var_person_one
p var_person_two
p var_person_three

Output:

Hi, let's create three new Person. Start with number 1
What's your name?
Kevin
How old are you?
29
Where are you from?
Hamburg
I summarize! You are Kevin and 29 years old from Hamburg
Is that correct?
Enter "0=Yeah" or "Nah"
0
Sweet Kevin, let us get to the next one
Press Enter for next Person

Hi, let's create three new Person. Start with number 2
What's your name?
Steven
How old are you?
22
Where are you from?
London
I summarize! You are Steven and 22 years old from London
Is that correct?
Enter "0=Yeah" or "Nah"
0
Sweet Steven, let us get to the next one
Press Enter for next Person

Hi, let's create three new Person. Start with number 3
What's your name?
Mike
How old are you?
33
Where are you from?
Tampa,FL
I summarize! You are Mike and 33 years old from Tampa,FL
Is that correct?
Enter "0=Yeah" or "Nah"
1
Get off. Let us try again
["Kevin", "29", "Hamburg"]
["Steven", "22", "London"]
nil

I have a few suggestions. Some high-level and then I'll go through your code.

My first suggestion would be to focus on readability. That means:

good use of spacing
descriptive names for modules, classes, methods, variables, everything
think about the different concepts involved
extract into simple modules, classes and methods
start small
read the documentation (here), particularly string, hash, array

For example, look at a subsection of your code with a little spacing:

class Person
  def input(count)
    puts "Hi, let\'s create three new Person. Start with number #{count}"

    puts "What\'s your name?"
    @name = gets
    @name = @name.delete("\n")

    puts "How old are you?"
    @age = gets
    @age = @age.delete("\n")

    puts "Where are you from?"
    @origin = gets
    @origin = @origin.delete("\n")
  end
end

I would argue you can clearly see the structure of the 3 questions, and all it took was 3 blank lines in the right place.

Next up is proper naming. You look good for the most part. You have @name, @age, @origin.. you have @antwort. From what I understand that is German for answer. Up to you if you want to mix German and English, but be aware of the native language of other people who will be reading your code. I would do the safe thing and use @answer instead.

Avoid redundancy in naming. For example, you have return person_return. Instead, just use return person. Instead of var_person_one just use person_one (note: this is still not descriptive enough, but better).

Now let's have a look at the different concepts involved, turn them into classes.

You have a Person.. but the question and answer format doesn't really suit the person itself. Think about what you could do if you introduced a Question class and an Answer class.

You have name, age and origin as answers to your questions about person, then you build up an array of those instance variables and call that array a person, but those are attributes of the Person class, so add them to it:

class Person
  attr_accessor :age, :name, :origin
end

You also have this concept of a QA session being interacted with through the console. So, you could create a QuestionAnswerSession.

To not complicate things too much you could get quite far by using just a Person and QuestionAnswerSession and forget the separate Question and Answer classes.

The qa session is responsible for outputting questions to the console and listening for answers, then using those answers to create Person objects.

My next suggestion would be to start small. Just introduce a couple of attributes and questions at first and forget about the multiple persons. Try to figure out the overall structure. It will be fewer lines and will be easier to take in and allow to focus on the design.

Let's say we have a Person with a name and age attribute, and we have corresponding questions. From that we want to assign name and age to a Person and, finally, output the Person to the console.

Let's create the person:

class Person
  attr_accessor :name, :age
end

# usage
# Person.new(name: 'Dave', age: 20)
# or
# person = Person.new
# person.name = 'Dave'
# person.age = 20

Now let's build the QA session.

class QuestionAnswerSession
  def start
    puts "Hi, let's create a new Person"

    person = Person.new

    puts "What's your name?"
    person.name = gets.strip

    puts "What's your age?"
    person.age = gets.strip

    puts person
  end
end

qa = QuestionAnswerSession.new
qa.start

Few minor things first. Notice I've removed What\'s which is now simply What's. This is an escape character and is only required if the enclosing quote is the same. Since you are using double quotes you do not need to escape it.

Also note I have replaced @age.delete("\n") with String#strip (docs here). This will remove the newline character for you.

The qa session itself is fairly simple. You instantiate it first, then you call the start method which outputs a question and starts listening for input. It then builds up attributes for a person object and outputs the final result.

I would add one small aspect to this to give you an idea on how to expand the behaviour. Let us say we wanted the person to be able to speak for itself. When we do puts person this is what we get:

#<Person:0x0000000002572518 @name="Dave", @age="20">

Not very nice. If you wanted to output something cleaner, you could add it to the qa session class (puts "Hi, my name is #{person.name}"), but you could also add a simple method to the Person class such as:

class Person
  def introduce_self
    "Hi, my name is #{name} and I'm #{age}"
  end
end

Then in the qa session, instead of puts person you could do puts person.introduce_self.

Extracting different responsibilities into simple, descriptive methods is a great way to keep things manageable, rather than dumping everything into the same method.

Though there are other ways you could improve your code and it could be designed with numerous approaches, I'd urge you to focus on these important aspects first as outlined above, then tackle multiple Person objects later.

Hope that helps! If you have any questions leave a comment.


Make a directory and change to it A basic shell function to create a directory and change to it goes like this:

mkcd () { mkdir "$1" && cd "$1"; }

This works well in many cases but breaks in unusual cases (e.g. if the argument begins with -).

I'm writing a more sophisticated version. This version calls mkdir -p to create parent directories if needed and just change to the directory if it already exists. It has these design goals:

Work in any POSIX compliant shell.
Cope with any file name.
If the shell has logical directory tracking, where foo/.. is the current directory even if foo is a symbolic link to a directory, then the function must follow that logical tracking: it must act as if the cd builtin was called and magically created the target directory.
If a directory is created, it is guaranteed that the function changes into it, as long as there is no race condition (another process moving a parent directory, changing relevant permissions, …).

Here is my best current effort. Does it meet the goals above? Are there situations where the behavior is surprising?

mkcd () {
  case "$1" in
    */..|*/../) cd -- "$1";; # that doesn't make any sense unless the directory already exists
    /*/../*) (cd "${1%/../*}/.." && mkdir -p "./${1##*/../}") && cd -- "$1";;
    /*) mkdir -p "$1" && cd "$1";;
    */../*) (cd "./${1%/../*}/.." && mkdir -p "./${1##*/../}") && cd "./$1";;
    ../*) (cd .. && mkdir -p "${1#.}") && cd "$1";;
    *) mkdir -p "./$1" && cd "./$1";;
  esac
}

Some error messages may be confusing like:

$ mkcd /foo/../bar
mkcd:cd:3: no such file or directory: /foo/..
$ mkcd /bin/../bar
mkdir: cannot create directory `./bar': Permission denied

Probably not much you can do about that.


Shorten Python/Maya Window UI code I'd like to know any ways of shortening this UI window for Maya, at the moment, because I couldn't find very detailed documentation and not very skilled with python, I have just built this UI with the knowledge that I have already, I know it's not the best way of doing it (using empty text elements to fill gaps etc)

Can someone provide me with an example on how to shorten this code or structure it properly?

import maya.cmds as cmds
class randomScatter():
    def __init__(self):
        if cmds.window("randSelection", exists=1): cmds.deleteUI("randSelection")
        cmds.window("randSelection", t="Scatter Objects - Shannon Hochkins", w=405, sizeable=0)
        cmds.rowColumnLayout(nc=3,cal=[(1,"right")], cw=[(1,100),(2,200),(3,105)])
        cmds.text(l="Prefix ")
        cmds.textField("prefix")
        cmds.text(l="")
        cmds.text(l="Instanced object(s) ")
        cmds.textField("sourceObj", editable=0)
        cmds.button("sourceButton", l="Select")
        cmds.text(l="Target ")
        cmds.textField("targetObj", editable=0)
        cmds.button("targetButton", l="Select")
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.text(l='Copy Options ')
        cmds.radioButtonGrp('copyOptions', labelArray2=['Duplicate', 'Instance'], numberOfRadioButtons=2, sl=2)
        cmds.text(l="")
        cmds.text(l="Rotation Options ")
        cmds.radioButtonGrp('orientation', labelArray2=['Follow Normals', 'Keep original'], numberOfRadioButtons=2, sl=1)
        cmds.text(l="")
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.text(l="")
        cmds.checkBox('copyCheck', l='Stick copies to target mesh.')
        cmds.text(l="")
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.text('maxNumber', l="Copies ")
        cmds.intFieldGrp("totalCopies", nf=1, v1=10, cw1=100)
        cmds.text(l="")
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.separator( height=20, style='in')
        cmds.text(l='')
        cmds.text(l=" From                          To", align='left')
        cmds.text(l='')
        cmds.text(l="Random Rotation")
        cmds.floatFieldGrp("rotValues", nf=2, v1=0, v2=0, cw2=[100,100])
        cmds.text(l="")
        cmds.text(l="Random Scale")
        cmds.floatFieldGrp("scaleValues", nf=2, v1=0, v2=0, cw2=[100,100])
        cmds.text(l="")
        cmds.setParent("..")
        cmds.rowColumnLayout(w=405)
        cmds.separator( height=20, style='in')
        cmds.text("Progress", height=20)
        cmds.progressBar('buildGridProgBar', maxValue=100, width=250)
        cmds.separator( height=20, style='in')
        cmds.button("excute", l="Scatter Objects",  w=403, al="right")
        cmds.showWindow("randSelection")
randomScatter()

(1) You define random_scatter as a class but then call it like a function, which works but is confusing. With your programme as it currently stands, there is no reason for it to be a class, so better simply to define random_scatter as a function (use def random_scatter(): at the top).

(2) You can remove repetition and make the code more readable using functions and loops, e.g. write a function

def insert_seperators(number):
    for _ in range(number):
        cmds.separator(height=20, style='in')

then you can replace

    cmds.separator( height=20, style='in')
    cmds.separator( height=20, style='in')
    cmds.separator( height=20, style='in')

with

    insert_separators(3)

(3) If you have other functions doing similar things with cmds then try to see what they have in common and put that in a single function, which accepts parameters according to what changes.


script- loop through files and directories I have written the script, which unzips a zip file (passed as argument to the bash), loops through directory and files, then perform different actions based on directory or file. It is working fine.

However I would like to know some things:

Is it possible to do this without the unzip command? I somehow feel like that step can be avoided and we can directly use the zip file to perform the task.
I have used three temporary files/directories(tmp.txt - to store the directory and file lines, out.xml - for the curl command, DIREC - for finding the files and directories). I have removed those three files at the end of the script. Is there a way to optimize this? may be using variables instead of files.

Here is the bash script, with comments:

#!/bin/bash
#The below line unzips the zip file as passed as argument
unzip $1
length=${#1}
const=4
count=$((length-const))
DIREC=$(echo $1 | cut -b 1-$count)
OUTPUTFIL=out.xml
#The below line will find the directory, which was created by the unzip command. Creation of tmp.txt here.
find $DIREC -exec echo {} \; >tmp.txt
exec<tmp.txt
   while read line
   do
if [ -d "${line}" ] ; then
    echo "$line is a directory";
else
    if [ -f "${line}" ]; then
BASE=$(base64 $line);
        echo "${line} is a file";
{
    echo "<inpu>${BASE}</inpu>"
} > ${OUTPUTFIL}
curl -u admin:secret --data-binary @out.xml http://dp1-l2.com; echo
    else
        echo "${line} is not valid";
        exit 1
    fi
fi
   done
rm -f tmp.txt
rm -rf $DIREC
rm -f out.xml

There's no documentation. Even a comment at the top of the script summarizing what it does would be better than nothing.
There's no error handling. What if the user forgets the argument?
This script will break if the argument $1 contains a space. You need to write "$1" in several places.
You don't need to put ; at the end of lines like this:

echo "$line is a directory";

This code is really unclear:

length=${#1}
const=4
count=$((length-const))
DIREC=$(echo $1 | cut -b 1-$count)

First, the variable names are poorly chosen: length of what? why const? count of what? Second, this assumes that $1 ends with .zip and will go wrong if the file has any other name. Third, Bash has a feature ${parameter%pattern}  for removing a suffix from a string, so you can remove the extension from $1 like this:

DIREC=${1%.*}

But fourth and most importantly, how do you know that the ZIP file will unpack into a directory with the same name as the ZIP file? In the example below I create a ZIP file called b.zip that unzips into the directory a:

$ mkdir a
$ touch a/a
$ zip b a/a
  adding: a/a (stored 0%)
$ rm -r a
$ unzip b
Archive:  b.zip
 extracting: a/a

What you should do instead (that is, if you must unzip $1) is to use the -d option to unzip to extract it to a known place.

$ unzip -d b b
Archive:  b.zip
 extracting: b/a/a

Why is OUTPUTFIL misspelled? Why not spell it OUTPUT_FILE?
It's a bad idea to put temporary files like $OUTPUTFIL in the current directory. There might already be a file called out.xml in the current directory and then you would end up overwriting it. You should use the mktemp program to create a unique temporary file name or directory, for example like this:

OUTPUT_FILE=$(mktemp -t unzip-XXXXXX.xml)

These lines have several problems:

find $DIREC -exec echo {} \; >tmp.txt
exec<tmp.txt
while read line

First, this will go wrong if $DIREC contains a space: you need to use quotes.
Second, this will go wrong if $DIREC starts with a - (it will be interpreted as an option): you need to use the -f option to find.
Third, there's no need to run a separate echo command for each file: find has a -print option.
Fourth, you don't need to write the output to a temporary file in order to read it using while read line; you can pipe the output of find into the while loop. Like this:

find -f "$DIREC" -- -print | while read line; do

Inside your while loop you use [ -f ... ] to ensure that you only process plain files (not directories). But you could use the -type f argument to find to ensure that it only outputs plain files:

find -f "$DIREC" -- -type f -print | while read line; do

This line will go wrong if $line contains a space or starts with a -:

BASE=$(base64 $line);

You should write:

BASE=$(base64 -i "$line")

It's not a good idea to read the output of the base64 program into a variable like this: the output might be very large and cause Bash to use large amounts of memory. It is better to let base64 write its output directly to your output file, so that it does not have to be stored in a variable in Bash. Instead of:

BASE=$(base64 $line);
{
    echo "<inpu>${BASE}</inpu>"
} > ${OUTPUTFIL}

you should write:

{
    echo "<inpu>"
    base64 -i "$line"
    echo "</inpu>"
} > "${OUTPUT_FILE}"

However, there is no need for an OUTPUT_FILE at all here, because curl accepts the filename - meaning "standard input", so you can write:

{
    echo "<inpu>"
    base64 -i "$line"
    echo "</inpu>"
} | curl -u admin:secret --data-binary @- http://dp1-l2.com

<inpu> looks like a typo to me. Are you sure you don't mean <input>? If <inpu> is correct, you should probably add a comment to explain why.
It would be better to put configuration settings like admin:secret and http://dp1-l2.com in named variables at the top of the file. (It might be even better to accept them as command-line arguments.)
The way you have written this code means that each individual file in the archive gets uploaded as a separate XML file to the server. Is this really what you want? It seems more likely to me that you would want to upload the whole archive as one XML file. Otherwise, what's the point of using XML?
As you suspected, there is in fact no need to unzip the file, because you can use zipinfo -1 to get a listing of the contents, and then use the -p option to unzip to extract a file to standard output.

Putting all this together, I would write the script line this:

#!/bin/bash
# upload-zip.sh -- encode contents of ZIP archive as base64
# strings in an XML document, and upload it.

# Destination and credentials for upload.
UPLOAD_URL=http://dp1-l2.com
USER=admin
PASSWORD=secret

if [ "$#" != 1 ]; then
    echo "Usage: $0 zipfile"
    exit 1
fi
ZIPFILE=$1

zipinfo -1 -- "$ZIPFILE" | while read FILE; do
    echo "<input>"
    unzip -p -- "$ZIPFILE" "$FILE" | base64
    echo "</input>"
done | curl --user "$USER:$PASSWORD" --data-binary @- --url "$UPLOAD_URL"


SQL-style join on arrays using ECMA6 I have a code snippet which performs an SQL-style join on objects in two arrays based on a specific field (in the code below, .id).

let preprocessed_array = [
  {id: 1,category:'a'},
  {id: 2,category:'b'},
  {id: 3,category:'b'}
]

let new_array = [
  {id: 4},
  {id: 1,category:'b'},
  {id: 5}
]

 for(let i=0;i< preprocessed_array.length;i++)
 {
   for(let j=0;j<new_array.length;j++)
   {
     if( preprocessed_array[i].id===new_array[j].id)
     {
       new_array[j]=preprocessed_array[i]
     }
   }
 }

 console.log(new_array)

Although this snippet is straightforward, I don't like the imperative style.  Is there a more declarative way to do this just with javascript / ECMA6 arrays?

Build an index and use it to map the second array:

const index = new Map(source.map(el => [el.id, el]));
const result = picks.map(el => (index.get(el.id) || el));

The larger the arrays, the more effective the index becomes.
If this code is a bottleneck, it can be rewritten to build the index on-the-go (memoization).

P.S. source stands for preprocessed_array and picks for new_array in the original code.


Linked List reversal Recursive version. The only interesting function:

        List& List::reverse()
        {
            if (empty())
            {   return *this;
            }
            int val = head();
            //
            // The fun bit.
            // See if you can figure this bit out.
            return pop().reverse().append(val);
        }

The rest:

#include <iostream>
#include <memory>
struct List
{
    struct Node
    {
        int     val;
        std::unique_ptr<Node>   next;
        Node(){}
        Node(int v, Node* oldTail)
            : val(v)
            , next(nullptr)
        {
            oldTail->next.reset(this);
        }
        friend std::ostream& operator<<(std::ostream& s, Node const& data)
        {
            return s << data.val << " ";
        }
    };
    Node    sentinal;
    Node*   tail;
    public:
        List()
            : tail(&sentinal)
        {}
        bool empty() const
        {
            return &sentinal == tail;
        }
        int& head()
        {
            // Pre-Condition not empty
            return sentinal.next->val;
        }
        List& append(int val)
        {
            tail = new Node(val, tail);
            return *this;
        }
        List& pop()
        {
            // Pre-Condition not empty
            sentinal.next.reset(sentinal.next->next.release());
            tail = sentinal.next.get() ? tail : &sentinal;
            return *this;
        }
        List& reverse();
        friend std::ostream& operator<<(std::ostream& s, List const& data)
        {
            for(Node* loop = data.sentinal.next.get(); loop != nullptr; loop=loop->next.get())
            {   s << *loop;
            }
            return s << "\n";
        }

};

See if you can figure this bit out.

Is this meant to be a Programming Puzzle?

You don't call append until after you finish calling reverse; reverse is recursive; so all the append calls will be called after all the reverse calls: and that explains why reverse will eventually terminate i.e. why the list will eventually be empty.

From a code-review point of view:

I fear that a recursive implementation might exceed your finite maximum stack size if the list is long.
You're deleting and creating new Node objects; you might instead be able to do it in a way that reuses existing Node instances.

Also, it should be spelled "sentinel".


Python - Quiz Game with Tkinter I have made a little Quiz Game in Python. This was my first experience in programming Gui's in Python with Tkinter. In general, I do not have much knowledge about graphic programming and have acquired the necessary knowledge through skimming internet articles and try and error. Accordingly, the code looks like sciolism and semi-professional structuring. That should be improved. I will not be able to avoid a good basic tutorial on graphic programming, I think. However, if you have tips for me how to improve the style I would be grateful to hear them.

from tkinter import Tk, Frame, Label, Button
from time import sleep

class Question:
    def __init__(self, question, answers, correctLetter):
        self.question = question
        self.answers = answers
        self.correctLetter = correctLetter

    def check(self, letter, view):
        global right
        if(letter == self.correctLetter):
            label = Label(view, text="Right!")
            right += 1
        else:
            label = Label(view, text="Wrong!")
        label.pack()
        view.after(1000, lambda *args: self.unpackView(view))

    def getView(self, window):
        view = Frame(window)
        Label(view, text=self.question).pack()
        Button(view, text=self.answers[0], command=lambda *args: self.check("A", view)).pack()
        Button(view, text=self.answers[1], command=lambda *args: self.check("B", view)).pack()
        Button(view, text=self.answers[2], command=lambda *args: self.check("C", view)).pack()
        Button(view, text=self.answers[3], command=lambda *args: self.check("D", view)).pack()
        return view

    def unpackView(self, view):
        view.pack_forget()
        askQuestion()

def askQuestion():
    global questions, window, index, button, right, number_of_questions
    if(len(questions) == index + 1):
        Label(window, text="Thank you for answering the questions. " + str(right) + " of " + str(number_of_questions) + " questions answered right").pack()
        return
    button.pack_forget()
    index += 1
    questions[index].getView(window).pack()

questions = []
file = open("questions.txt", "r")
line = file.readline()
while(line != ""):
    questionString = line
    answers = []
    for i in range (4):
        answers.append(file.readline())

    correctLetter = file.readline()
    correctLetter = correctLetter[:-1]
    questions.append(Question(questionString, answers, correctLetter))
    line = file.readline()
file.close()
index = -1
right = 0
number_of_questions = len(questions)

window = Tk()
button = Button(window, text="Start", command=askQuestion)
button.pack()
window.mainloop()

questions.txt (this file has to be in the same folder)

Where is Germany?
In Europe
In Great Britain
In Asia
In the USA
A
Where is England?
In Asia
On an island
In the USA
In China
B
What is the etymological meaning of antibiotics?
For Life
For Health
Against Live
Against Diseases
C
"Portability is for people who can not write new programs." From whom does the quote come from?
Bill Gates
Hillary Clinton
Richard Stallman
Linus Torvalds
D
Who is the richest man in the world?
Jeff Bezos
Bill Gates
Donald Trump
Me
AX

Use a single import

Instead of importing each individual class, I recommend importing the whole module with a short name. You can then use the short name as a prefix for all of your commands. It makes your code a bit more self-documenting, especially if you mix both tkinter and ttk in the same application.

For example:

import tkinter as tk
...
window = tk.Tk()
button = tk.Button(window, text="Start", command=askQuestion)

Separate widget creation from layout

In my experience, the layout of a GUI changes a lot more than the actual widgets, especially early in development. By separating widget creation from widget layout, it makes debugging much easier. It also makes it much easier to visualize the layout just by looking at the code.

Instead of this:

Label(view, text=self.question).pack()
Button(view, text=self.answers[0], command=lambda *args: self.check("A", view)).pack()
Button(view, text=self.answers[1], command=lambda *args: self.check("B", view)).pack()
Button(view, text=self.answers[2], command=lambda *args: self.check("C", view)).pack()
Button(view, text=self.answers[3], command=lambda *args: self.check("D", view)).pack()

... I recommend this:

label = Label(view, text=self.question)
button_a = Button(view, text=self.answers[0], command=lambda *args: self.check("A", view))
button_b = Button(view, text=self.answers[1], command=lambda *args: self.check("B", view))
button_c = Button(view, text=self.answers[2], command=lambda *args: self.check("C", view))
button_d = Button(view, text=self.answers[3], command=lambda *args: self.check("D", view))

label.pack()
button_a.pack()
button_b.pack()
button_c.pack()
button_d.pack()

Explicitly set side for pack

I recommend always explicitly setting the side for pack. Even though the default will likely never change, explicitly setting the side makes the code less ambiguous:

label.pack(side="top")
button_a.pack(side="top")
button_b.pack(side="top")
button_c.pack(side="top")
button_d.pack(side="top")


Searching within multiple objects of the bucket I have millions of files in the Google cloud storage's bucket. I want to search within files with a .index extension and retrieve the contents.

This is currently what I am doing, but the time required for overall process is large. Is there any better and faster way to do this?

class storage():
    ...
    self.indexFile = []
    self.indexFileIndex = []
    ...

    def get_content(self,param1,param2):
        c_n = []
        for value, index in zip(param1, param2):
            object_contents = StringIO.StringIO()
            srcObjURI = boto.storage_uri(value, self.storage)
            srcObjURI.get_key().get_file(object_contents)
            c_n.append(object_contents.getvalue())
            object_contents.close()
        return c_n

    def get_PATHs(self):
        paths=[]
        #pts=open("paths.txt","w")
        pts_log=open("paths_log.txt","a")
        pts_log.write("-"*20+time.ctime()+"-"*20+"\n")
        indexFileContents = self.get_content(self.indexFile, self.indexFileIndex)
        for c,d in zip(indexFileContents,self.indexFile):
            regx = r"(.*)\/" + r"(.*?)\|"
            patternPathList = re.compile(regx)
            for match in patternPathList.finditer(c):
                p=match.group(1).strip() + "/"+ match.group(2).strip()
                tst_exst=""
                if p in paths:
                    tst_exst="Already exist !"
                else:
                    tst_exst="Added to PATHs list"
                    paths.append(p)
                    #pts.write(p)
                    #pts.write("\n")
                pts_log.write("FROM : %s --> %s %s"%(d,p,tst_exst))
                pts_log.write("\n")
        #pts.close()
        pts_log.close()
        return paths

The files that I am trying to search varies from 200KB to 1MB and sometimes has Unicode characters.

A few remarks to add to @JoeWallis already excellent answer:

If you want speed, consider using the module cStringIO instead of StringIO. Since we are missing your import statements, I can't tell whether you have a try .. catch to conditionally import it or not. You should really post the whole code.
There are very few occasions when you want to keep commented out code. Generally speaking, the best thing to do is to remove dead code and let source control software remind you of what the old code was like.
Instead of zip, consider using itertools.izip which uses lazy evaluation to zip the iterables. In your case, it does not change many things since you never break early (in case of error maybe?) but in general, it saves more memory and sometimes avoids computing unused values.
You don't need the parenthesis in class storage():, unless you explicitly inherit from a class. Since you don't inherit from anything, you can simply drop them so that your code looks cleaner.


Memory management for small allocations This code is for a program that needs to add thousands of strings to memory while it's running and these strings will be used until the end of execution. So the memory for these strings will be freed only in the end.

First I tested calling malloc for every new string and have used Valgrind to benchmark total execution cost for 200 hundred thousand strings. Then I wrote these memory handling functions that call malloc once and hand out small chunks of memory for every string. The overall execution cost for the entire program went down over 20%.

This is my first attempt at doing this, so I know this code is probably bad. I would like to know exactly what is wrong with it, so I won't make the same mistakes again.

//The actual numbers will be smaller after DEBUG
#define BIG_BLOCK       1073741824  //  1GiB ..........
#define MEDIUM_BLOCK     524288000  //500MiB .....
#define SMALL_BLOCK      104857600  //100MiB .

//Linked-list containing the required data for each malloc'ed block
typedef struct {
   void   *mem_pool;   //starting address for this block
   void   *next;       //next node
   size_t *position;   //current position
   size_t *available;  //how much memory was allocated?
} memory_t;

//First link
static memory_t *root = 0;

//handles malloc and returns a new memory_t node, it allocates
//more memory on the first call than on subsequent calls
static void *increase_memory(void)
{
    void        *result    = 0;
    memory_t    *new_block = 0;
    static int  firstCall  = 0;
    static const size_t default_sizes[] = {BIG_BLOCK, MEDIUM_BLOCK, SMALL_BLOCK, 0};

    for(size_t i = firstCall; default_sizes[i] != 0; ++i)
    {
        if((result = malloc(default_sizes[i])))
        {
            new_block = result;
            new_block->mem_pool = new_block;                                    //starting address for this block
            new_block->next = 0;                                                //link to next node
            new_block->position = result + sizeof(memory_t);                    //space for size_t *position
            *new_block->position = sizeof(memory_t) + sizeof(size_t) * 2;       //remove used addresses from pool (position, available)
            new_block->available = result + sizeof(memory_t) + sizeof(size_t);  //allocate space for size_t *available
            *new_block->available = default_sizes[i];                           //how much was allocated
            break;
        }
    }
    firstCall = 1; //first call happened, let's malloc smaller blocks
    return result;
}

//mallocs the first block, called by main()
int init_mem_pool(void)
{
    static int initialized = 0;
    if(!initialized++) root = increase_memory(); //protection against multiple calls

    return (root) ? 1 : 0;
    //Should I change the line above?
    //I believe it doesn't need to be this way but GCC compiler warns when the following are used:
    //return root;
    //return (int)root;
    //What should be done here?
}

//hands out memory chunks
void *get_memory(const size_t size)
{
    memory_t *current = root;
    void *temp = 0;

    while(current)
    {
        //there's enough memory in the current node
        if((*current->position + size) < *current->available) //Brackets added for clarity, not sure if it's bad style...
        {
            temp = current->mem_pool + *current->position;
            *current->position += size;
            break;
        }

        //not enough memory, go to next node
        else if(current->next) current = current->next;

        //all nodes are full, create another one
        else current = current->next = increase_memory();
    }
    return temp;
}

//free all nodes
static void recursive_free(const memory_t * const node)
{
    if(node->next) recursive_free(node->next);
    free(node->mem_pool);
}

//called by main(), free all nodes
void clean_mem(void)
{
    //is there anything to free?
    if(root)recursive_free(root);
}

I know it's kind of standard to do this in C land but you should spell things like this if((result = malloc(default_sizes[i]))) out as:

result = malloc(default_sizes[i]);
if (result != NULL)
{
}

Yes it's one line of code more but the intent is much more obvious at a first glance. As suggested in the comments you might want to invert the condition and continue. Reduces the level of nesting:

result = malloc(default_sizes[i]);
if (result == NULL) { continue; }

Technically it's better to write if (NULL == result) because if you accidentally write = instead of == then the compiler will throw an error instead of compiling buggy code - I just can't seem to get myself into that habit for some reason. Probably because it reads a bit clunkier that way (imho).
It's perfectly legal for a struct to contain a pointer to itself like this:

typedef struct memory_ {
    void           *mem_pool;   //starting address for this block
    struct memory_ *next;       //next node
    size_t         *position;   //current position
    size_t         *available;  //how much memory was allocated?
} memory_t;

The return type of increase_memory should be memory_t * because that's what it returns. You can also get rid of result in there and use new_block in its place.
In init_mem_pool you asked some questions on the return value. I guess you want it to indicate whether the initialization was successful or not - which is if root is NULL or not so you can just write this:

return root != NULL;


Slice chunking in Go I have a slice with ~2.1 million log message strings in it that I am parsing for regular expression matches. For any matches, I add it to a local slice and then return said slice.

func Search(re *regexp.Regexp, logs []string) []string {
    var lwg sync.WaitGroup
    var lresults = make([]string, 0)
    NumCPU := runtime.NumCPU() * 2
    var divided = make([][]string, 0)

    // Splits the log lines into sub slices based
    // on the number of CPUs available.
    ChunkSize := len(logs) / NumCPU
    if ChunkSize == 0 {
        ChunkSize = 0
    }
    if ChunkSize > 200000 {
        log.Warnf("There are %s lines of logs available. Searching may take awhile.", humanize.Comma(int64(len(logs))))
    }

    log.Debug("distributing the logs evenly.")
    for i := 0; i < len(logs); i += ChunkSize {
        end := i + ChunkSize
        if end > len(logs) {
            end = len(logs)
        }
        divided = append(divided, logs[i:end])
    }

    log.Debugf("searching through all logs for '%s'", regex)
    for ii := range divided {
        lwg.Add(2)
        total := len(divided[ii])
        mid := total / 2
        array := divided[ii]
        first := array[:mid]
        second := array[mid:]
        go func(s []string) {
            for _, xx := range s {
                if re.MatchString(xx) {
                    locker.Lock()
                    lresults = append(lresults, xx)
                    locker.Unlock()
                }
            }
            lwg.Done()
        }(first)
        go func(s []string) {
            for _, xx := range s {
                if re.MatchString(xx) {
                    locker.Lock()
                    lresults = append(lresults, xx)
                    locker.Unlock()
                }
            }
            lwg.Done()
        }(second)
    }
    lwg.Wait()
    log.Debugf("found %s matches.", humanize.Comma(int64(len(lresults))))

    return lresults
}

Is there a better/more efficient way to do this in Go? I know I could use channels, I'm just not sure how to implement it without hitting deadlocks (I'm still a beginner Go programmer).

It's hard to review code that doesn't work per se, but I'll try my hand at a style review anyways.

func Search(re *regexp.Regexp, logs []string) []string

Nice method signature! But, exported function Search should have comment or be unexported. I trust there are docstrings in your code, right? That's valuable stuff, don't omit it here!

var lwg sync.WaitGroup
var lresults = make([]string, 0)
NumCPU := runtime.NumCPU() * 2
var divided = make([][]string, 0)

Personally, I find this kinda hard to read. You should do this instead:

var (
  lwg        sync.WaitGroup
  divided  = make([][]string, 0)
  lresults = make([]string, 0)
  NumCPU   = runtime.NumCPU() * 2
)

So much nicer.
That's the handiwork of my IDE. The Go language distribution comes with a lot of code quality tools for readability and static analysis, and you should really use them often. (Every save, perhaps?)

// Splits the log lines into sub slices based
// on the number of CPUs available.

Yay, comments! /* multiline comments are more readable, though */.

ChunkSize := len(logs) / NumCPU
if ChunkSize == 0 {
    ChunkSize = 0
}

First of all, ChunkSize would ideally be declared in the var () block up there, because it seems like there are no other uses for NumCPU. Go is fastest when you are smart and efficient with your memory, so don't leave variables lying around for no reason.

Second, I find this pointless. Yes, I, too, cry at Go's lack of ternary operator, but it's missing for a reason. That being said, I don't understand why you are setting a variable to what it is... ?

if ChunkSize > 200000 {
    log.Warnf("There are %s lines of logs available. Searching may take awhile.", humanize.Comma(int64(len(logs))))
}

2e5 is more readable and more easily changeable later. Moreover, I don't understand the coersion to int64 here. Does humanize really only like int64? That seems lame.

for ii := range divided {
    lwg.Add(2)
    total := len(divided[ii])
    mid := total / 2
    array := divided[ii]
    first := array[:mid]
    second := array[mid:]
    go func(s []string) {
        for _, xx := range s {
            if re.MatchString(xx) {
                locker.Lock()
                lresults = append(lresults, xx)
                locker.Unlock()
            }
        }
        lwg.Done()
    }(first)
    go func(s []string) {
        for _, xx := range s {
            if re.MatchString(xx) {
                locker.Lock()
                lresults = append(lresults, xx)
                locker.Unlock()
            }
        }
        lwg.Done()
    }(second)
}

Whoa. Alright:

array := divided[ii] is pointlessly cryptic, instead use for e, i := range divided {} and get the element and index that way -- it's cleaner.
Naming. I understand it's hard; I struggle with it too: every coder does. But please, for the love of Go, either use i, index or int_theIndexOfTheFreakinFullyQualifiedStringObjectWeAreIteratingOn. ii is hard to pronounce, not rememberable... I hate it.
As far as I can tell, these variables total, mid, first, second have exactly one use: right here in this loop. Remember what I said about Go being faster when you keep the work for the GC down to a minimum. I get it improves readability, but at a point, that stuff should just be inlined to keep the cache happy.
I see zero difference between these go funcs, because they're identical. Ideally, you would assign it to a local var and call it on first and second.
I can't run your code right now, which means I can't make it testable and I can't ask go race to take a look at it. What I can tell you, from a glance, is that you're accessing the same resources through two goroutines at once. The locker, assuming it does what it says on the tin, is a good thought, but you're still locking them at the same time and it's a recipe for disaster in the form of race conditions. See here, here, here, and the other results from the first page of google results on "avoid race condition golang".

return lresults

This could be a bare return if you put (lresults []string) in the signature, which would in my opinion improve readability both of the code and any generated docs.


Functional translation of dice simulation I have tried my hands at "functionalising" a toy problem I had: find the expected number of throws of a six sided die until all sides have been seen (the answer is 14.7)

My starting point is my imperative solution

import scala.collection.mutable.SortedSet
import scala.util.Random

def dice(nsim:Int) : Double = {
    val nsides = 6
    val r = Random
    val throws = SortedSet[Int]()
    var nthrows = 0
    var res = 0.0

    for (i <- 1 to nsim) {
        throws.clear
        nthrows = 0
        while ( throws.size != nsides) {
            nthrows += 1
            throws += r.nextInt(nsides) + 1
        }
        res += nthrows
    }
    res / nsim
}

dice(10000) // ~14.7

I initialise an empty set, and keep adding my thrown dice until i have seen all sides (the set length equals six), while keeping track of how many die I have thrown. The expected number is just the sum of all these throws divided by the number of simulations (repetitions) I perform: approx 14.7

And here is my functional attempt (almost half a day went into this, including plenty of googling, outofmemory errors etc) - and I am not sure if this is considered good.

def throwdie(nsides:Int) : Int = {
    val r = Random
    r.nextInt(nsides) + 1
}

def nthrows(seen:SortedSet[Int], count:Int, nsides:Int) : Int = {
    if (seen.size == nsides)
        return count
    return nthrows(seen + throwdie(nsides), count + 1, nsides)
}

def fdice(nsim:Int, nsides:Int) : Double = {
    val runs = Iterator.fill(nsim)(SortedSet[Int]())
    runs.map( x=> nthrows(x, 0, nsides) ).reduceLeft(_+_) * 1.0 / nsim
}

fdice(10000, 6) // ~14.7

explanation

So I start with a function throwdie that just returns the outcome of rolling a single die.

Next I have tried to rewrite the while loop using recursion. I came up with nthrows which takes an empty set, a zero count, and the number of die sides, and returns the number of throws until all sides have been seen for one "simulation". I use a mutable data structure here, which as I understand is a nono in functional programming, but it is all I came up with.

Next I run the simulation in fdice where I fill an Iterator with nsim instances of the empty SortedSet, and apply nthrows to all of these and sum the result.

Let me first present my solution and then comment below: Let me point the code is more verbose than needed, in order to be more descriptive.

import scala.annotation.tailrec
import scala.collection.SortedSet
import scala.util.Random

//=========Neat reusable methods=============

//Infinite iterator of the results of a dice throw
def diceResultIterator(sidesCount: Int): Iterator[Int] = {

  case class Dice(rnd: Random, sidesCount: Int, upside: Int)
  //Gets a dice, returns a dice, functional "mutation"! For more, search State monad
  def roll(initDice: Dice) = {
    val newSide = initDice.rnd.nextInt(initDice.sidesCount) + 1
    Dice(initDice.rnd, initDice.sidesCount, newSide)
  }
  val someDice = Dice(new Random, sidesCount, 1)
  val diceIterator = Iterator.iterate(someDice)(roll)
  diceIterator.map(_.upside)
}

//Infinite iterator of the results of the given function
def simulateManyTimes[T](f: => T) = Iterator.iterate(f)(_ => f)

//A usual general function to calculate and average
def average(values: Iterable[Int]): Option[Double] = {
  val (total, len) = values.foldLeft((0, 0)) { case ((sum, cnt), value) => (sum + value, cnt + 1) }
  if (len > 0) Some(total.toDouble / len) else None
}
def averageResultsOfIntSimulations(f: => Int, repeat: Int) = {
  average(simulateManyTimes(f).take(repeat).toIterable)
}
//===================Problem specific part======================

//Similar to OP method, just encapsulated the recursion
def countThrowsUntilAllSidesSeen(diceIter: Iterator[Int], sidesCount: Int): Int = {
  @tailrec
  def recursiveHelper(seen: SortedSet[Int], count: Int): Int = {
    if (seen.size == sidesCount)
      count
    else
      recursiveHelper(seen + diceIter.next(), count + 1)
  }
  recursiveHelper(SortedSet[Int](), 0)
}
def simulationWrapper(sides: Int, simRepeat: Int) = {
  averageResultsOfIntSimulations(countThrowsUntilAllSidesSeen(diceResultIterator(sides), sides), simRepeat).get
}

simulationWrapper(6, 1000000)

Generally, creating a new Randomfor each throw is a bad practice since it may produce less random results than expected. My proposed solution reuses the same Random in a functional way for each simulation.

I am in favor of the school that says code should be self documenting, and your second method tries to fit too many things in one place, adding obscurity.

val runs = Iterator.fill(nsim)(SortedSet[Int]())
runs.map( x=> nthrows(x, 0, nsides) ).reduceLeft(_+_) * 1.0 / nsim

For example there is no good reason why you would fill an iterator with SortedSets then use then with map, instead of just runs.map( _ => nthrows(SortedSet[Int](), 0, nsides)) Counter-intuitive code is bad code. In production, the dev next to you would certainly tap your shoulder to ask you to explain your code, generally not too happy that he had to do so.

As general advice, tune the code to your audience, like with all forms of communication. For example, the diceResultIterator in my solution is purposefully verbose, to make the concept clearer. When you are comfortable with it, you can see that it is rather trivially refactored to ~half the lines. Scala has a trap to chain operations into obscurity, be mindful about this ;)

When you see recursiveHelper inside countThrowsUntilAllSidesSeenwith arguments seen: SortedSet[Int], count: Int, it's purpose is already quite clear, so the most important piece of information was to say that it is there just to encapsulate recursion.

Lastly, I added explicit return type is the functions that I felt it added clarity. Think of them like training wheels: Just remove them when they feel useless.


How can I optimize this database to array structure code? I need to take data from a MySQL database and message it into a format expected by the front end of an application - I can not change the front end as other services provide it data in this same format.

The database is structured as follows:

id   type     value     label         optgroup
1    car      ix5       Ford Taurus   Ford
2    car      ix6       Ford Focus    Ford
3    car      ix9       Cobalt        Chevy
4    planet   ix8       Earth         Default

The output from this code must do the following: for types with optgroups, records must be categorized by optgroup; if there is only one optgroup, then it should be ignored. The real data has hundreds to thousands of rows per type. The finally array output from this data would be:

$data = [
    'car' => [
        'chevy' =>  [ 'ix9' => 'Cobalt' ],
        'ford'  =>  [ 'ix5' => 'Ford Taurus', 'ix6' => 'Ford Focus' ]
    ],
    'planet' => [ 'ix8' => 'earth' ]
];

The code I have doing this currently works, but is a bit slow, and I am looking for a possible improvement.  Here's the functioning code, where $STH->result() is the database result as an array of rows:

protected function _format($STH)
{
    $data = [];

    foreach ($STH->result() as $row)
    {
        if ( ! $row->optgroup)
            $data[ $row->type ][ $row->value ] = $row->label;
        else
            $data[ $row->type ][ $row->optgroup ][ $row->value ] = $row->label;
    }

    // selects with a single optgroup can have that optgroup removed
    foreach ($data as $menutype => $optorkey)
    {
        if (is_array($optorkey) && count($optorkey) == 1)
            $data[$menutype] = current($optorkey);
    }

    return $data;
}

EDIT

The original query generating the data is very simple, as follows:

SELECT type, value, label, optgroup FROM ####.options ORDER BY type, optgroup, label ASC

The data in this table is updated frequently by automated processes.

Non-optimizing suggestions

Your SQL is not very clear. What kind of object is acceptable for the type column, and how do you manage to put items into that column? How specific or general should the terms be in that column? What is the value column? label, and even optgroup? To me, these are extremely confusing. Is optgroup a group of options, or a group of outputs? This isn't official, but it's a good read and it might help you.

If each value of value (naming confusion right there!) starts with "ix", then why is it there? I can't even tell what that column means because the Earth comes between two cars! Anyways, if each entry starts with that prefix, consider adding it in your PHP.

Optimizing...

I don't see to much you can do. Your query is already quite minimal, but I'm not an expert there.


Tokenizing a shell command I want to handle input like the following

ls -al | awk '{print $1}'

Now to parse it I couldn't tokenize at whitespace because of the quotations and I had to make a special case for awk instead. The code below successfully parses the command-line input. Is there a better way?

int handleToken(int awk, char *token, char *params[100], int i) {
    while (token != NULL) {
        if (awk == 1) {
            params[i++] = token;
            token = strtok(NULL, " ");
            awk = 0;
            continue;
        }
        if (strcmp(token, "awk") == 0) {
            params[i++] = token;
            awk = 1;
            token = strtok(NULL, "\'");
            continue;
        }
        params[i++] = token;
        token = strtok(NULL, " ");
    }
    params[i] = NULL;
    return i;
}

When you say

  I want to handle input like the following [...]

I take you to mean that you want to tokenize a language similar to the one recognized by the standard POSIX shell (since you tagged [posix] and presented an example), or maybe even that exact language.

You posited in comments that perhaps you needed a parser generator such as yacc or bison for this job.  Although these tools can indeed do such a job, code for such a subsystem -- a "lexical analyzer" or "scanner" -- is more often generated via a different kind of code generator; lex and its GNU variant flex are the canonical tools for this purpose.  These two particular tools allow you to describe your tokens (and separators) via regular expressions, and from such descriptions they generate C code for a table-based DFA that processes the language character by character and splits it into tokens.  This is certainly one plausible way you could proceed.

On the other hand, depending on the language you want to recognize, it's not necessarily unreasonable to write your own lexical analyzer from scratch, or in favorable cases to use an available function such as strtok() to do the job.  This affords the possibility of a better-tuned implementation than can be produced via a general-purpose tool (or not), it does not incorporate an additional language into the project, and the source might even be smaller.

You ask,

  Is there a better way?

and the answer is certainly "yes".  Other than broad generalities such as I have given so far, however, code review is not really a good platform for what essentially boils down to designing a complete replacement for your present code.  From here out, therefore, I focus on the code you actually presented:

The name of your function is poorly chosen, as it does not describe its behavior very well.  For what it seems meant to do, "tokenize", "find_tokens", "scan", "analyze", or a similar name would be more fitting.
The name of the token parameter seems poorly chosen.  Tokens (or in shell terminology, "words") are the function's output, not a good description of its likely input.  Maybe this parameter should be a "line", or such.
On the other hand, such code would be more flexible if it could read directly from the input, instead of being presented a pre-read string.
That your function accepts parameters awk and i suggests that it is intended to be used multiple times, incrementally, to build up one params array, but if so then it has no mechanism to return its present value of awk to the caller, which it seems would be necessary.
You have no protection against overrunning the bounds of your params array.  It would probably be better to pass a double pointer, and let the function (re)allocate space as needed.
When you identify tokens / words, you should consider making copies instead of using pointers into the original string (the latter being what strtok() provides) as there are all kinds of unpleasant surprises that could result from using the space in the original string as storage for the tokenized words of the command.
And of course, I presume you know that strtok() consumes / modifies / destroys the input string.
Calling out awk as a special case is untenable for parsing shell input, as you cannot enumerate all the possible commands, nor know what form of argument or arguments to expect.  Even awk itself does not necessarily take a single-quoted (or even double-quoted) argument.  (Consider the perfectly viable command awk {print}.)
The standard shell does not organize input into bare words and quoted strings.  Single- and double-quoted subsequences can be concatenated with other quoted or unquoted sequences to form a single token.  For example, these mean the same thing to the shell: 'one shell word', one' 'shell" "word, and o"ne"' shel'l" "wo'r'"d"''.
If your shell language is simpler, so that you don't need to worry about internal quotes, then you probably still should not use the preceding token to predict quoting status.  Instead, look at the first character of each substring itself to see whether it is a single or double quote.
The shell recognizes escape sequences inside double-quoted strings; this requires a smarter scanner than strtok() to sort out.
Although the standard shell does not split words at quotes, there are several characters other than whitespace where it does split them (when they appear unquoted): |  & ; ( ) < >.  So for example, the shell will interpret the input ls -al|awk '{print $1}' exactly the same as it does your example command, even though there is no whitespace around the |.


generating stochastic coloured noise for many oscillators I have a system of n oscillators which oscillate with time. To each of these oscillators I am adding a fluctuating coloured noise term with a different seed for each oscillator. Here is my code for the generation of this noise:
#include <math.h>
#include <stdio.h>
#include <stddef.h>
#include <stdlib.h>
#include <iostream>
#include <random>

using namespace std;

double dt = 0.01;

static normal_distribution<double> n_dist(0 , sqrt(dt));
static random_device rd;
static default_random_engine generator(rd());

static double dW ( double dt ) {
    return n_dist(generator);
}

void exp_noise(double gamma, int steps, double c_sigma, float X[])
{
    X[0] = 1;

    for (int i = 0; i < steps; i++ ){
        X[i+1] = (1 - gamma) * X[i] + c_sigma * sqrt(2*gamma) * dW(1);
    }
}

int main()
{
    int n = 256;
    constexpr int steps = 1000;

    float X[n*steps];
    for( int i=0 ; i<n ; ++i ){
        float X_i[steps];
        memset(X_i, 0.0, sizeof X_i);
        exp_noise(0.4, steps, 1, X_i);
        for( int j=0 ; j<steps ; ++j ){cout << X_i[j] << '\n';}
        memcpy( X + i*steps, X_i, sizeof( X_i ) );
    }

    cout << "total vector is" << '\n';
    for( int i=0 ; i<n*steps ; ++i ){cout << X[i] << '\n';}

    return 0;
}

Essentially my code is producing something similar to an array of noise for each oscillator, and then appending all of those arrays together. The intention is to apply to above code to my oscillators in the following way.
for (int h = 0; h < steps; ++h){
    for( int i=0 ; i<n ; ++i ){
        oscillator[i] = stoch_runge_kutta( oscillator[i], X[ h + i*steps ], ... )
    }
}

This code seems to work well for a small number of steps. This code unfortunately breaks down however for values of n * steps > 2500000 with a stack overflow error. So my first question would be whether the code can be made more stable to the value of n*steps with some minor changes, or whether I need to change everything by using vectors instead? I would also be interested in knowing whether the code would be faster or more elegant if I completely redesigned the code in some other way, for example by not creating an array of arrays, and doing something else instead? How would these things be done? It's not essential, but it's preferable to have the noise as an array so that it can be reused to for other things.
Any help would be greatly appreciated.

#include <math.h>
#include <stdio.h>
#include <stddef.h>
#include <stdlib.h>
#include <iostream>
#include <random>

using namespace std;

Please don't put all the standard identifiers into the global namespace.  Use the C++ versions of the libraries, and qualified names.  If you really must drag some identifiers in (as is often required for argument-dependent lookup), be specific and keep their scope small.
And try not to mix C standard I/O and C++ streams if possible.
#include <cmath>
#include <cstddef>
#include <cstdlib>
#include <iostream>
#include <random>


Vectorize pairwise edit distance computation How can I vectorize this function? It takes an array of array of tuples as input and computes the distance between elements using Levenshtein's method. I found some simpler examples but they weren't of much use for me since I couldn't apply the same logic for my problem. Any help with making this code more efficient will be appreciated!

def sim_mat(sequence):
    sim_mat = np.empty(sequence.shape[0], sequence.shape[0])
    for i in range(sequence.shape[0]):
        for j in range(i, sequence.shape[0]):
            distance = levenshtein(sequence[i], sequence[j])
            sim_mat [i, j] = sim_mat [j, i] = distance
    return sim_mat

EDIT: Here is the levenshtein function that we use to compute the distance:

def levenshtein(seq1, seq2):
    m = len(seq1)
    n = len(seq2)
    dp = [[0]*(n + 1)]*(m + 1))
    for i in range(1, m + 1):
        dp[i][0] = get_sequence_length(seq1[:i])
    for j in range(1, n + 1):
        dp[0][j] = get_sequence_length(seq2[:j])
    for i in range(m + 1):
        for j in range(n + 1):
            if seq1[i - 1] == seq2[j - 1]:
                dp[i][j] = dp[i - 1][j - 1]
            else:
                dp[i][j] = min(dp[i][j - 1] + len(seq2[j - 1]),  # Insert
                               dp[i - 1][j] + len(seq1[i - 1]),  # Remove
                               dp[i - 1][j - 1] + max(len(sets_difference(seq1[i - 1], seq2[j - 1])),
                                                      len(sets_difference(seq2[j - 1], seq1[i - 1]))))
    return dp[m][n]

and get_sequence_length:

def get_sequence_length(sequence):
    return sum(map(len, sequence))

EDIT2: sets_difference

def sets_difference(set1, set2):
    return list(set(set1).difference(set2))

An example sequence would look like this:

[[('a', 'b', 'c'), ('c', 'd')], [('b', 'd', 'f')]]

I do not think vectorisation to buy you anything "in the upper levels".
My guess is the statement little use due to data dependencies is true even more than for
parallel computing in general.
With SciPy/NumPy I'm out of my depth; drawing on Syncrossus levenshtein.py.
I'd need a better motivation for the distance function - I guess the minimum requirement being triangle inequality
Shot from the hip using set.symmetric_difference() as food for thought:

import numpy as np

def replacement_cost(s1, s2):
    return len(set(s1).symmetric_difference(s2))

def edit_distance(seq1, seq2):
    """ return edit distance between two list-like sequences of sequences
        Args:
            seq1, seq2: subscriptable objects compatible with len()
                containing objects comparable with the `==` operator.
    """
    m = len(seq1)
    n = len(seq2)
    dp = np.zeros(shape=(m, n))
    dp[0, :] = range(0, n)
    dp[:, 0] = range(0, m)

    for i in range(1, m):
        for j in range(1, n):
            cost = 0 if seq1[i] == seq2[j] \
                     else replacement_cost(seq1[i], seq2[j])
            dp[i][j] = min(dp[i][j - 1] + len(seq2[j - 1]),  # Insert
                           dp[i - 1][j] + len(seq1[i - 1]),  # Remove
                           dp[i - 1][j - 1] + cost)          # Replace
    return dp[m-1][n-1]


Binary search as a generic algorithm I am upgrading my C++11 knowledge and repeating some essential algorithm. Here is binary search only in terms of iterators.

#include <iostream>

#include <vector>
#include <iterator>

using namespace std;

template<typename t_iter>
t_iter binarySearch(t_iter begin, t_iter end, typename iterator_traits<t_iter>::value_type q)
{
    typedef iterator_traits<t_iter>::value_type t_elem;
    size_t len = end - begin;

    if (len < 2)
    {
        if (*begin == q)
        {
            return begin;
        }

        return end;
    }

    t_iter middleElem = begin + len / 2;

    if (*middleElem < q)
    {
        return binarySearch(middleElem, end, q);
    }

    return binarySearch(begin, middleElem, q);
}

int main()
{
    int tmp[] = {1, 2, 4, 5, 8, 10, 20, 35, 41, 42, 100};
    size_t len = sizeof(tmp)/sizeof(tmp[0]);
    vector<int> a(tmp, tmp + len);

    int q = 35;

    auto it = binarySearch(a.begin(), a.end(), q);

    cout << distance(a.begin(), it) << endl;
}

Iterators may not be random access iterators which means the length calculation and the advance by half won't compile.

For this there is the distance and advance functions available which use the random access if it is available but fall back to counting and repeated incrementing when it isn't.

You never use the t_elem type def.

In the recursion step both branches are equally likely and equally important, that is best signified and made more readable by using an else branch rather than letting it fall through.

you can also implement as a loop to minimize recursion overhead (micro-optimization admittedly because automatic tail-call optimization would do the same).

template<typename t_iter>
t_iter binarySearch(t_iter begin, t_iter end, typename iterator_traits<t_iter>::value_type q)
{
    auto len = std::distance(begin, end);

    while(len>1)
    {
        t_iter middleElem = begin;
        std::advance(middleElem, len / 2);

        if (*middleElem < q)
        {
            begin = middleElem;
        }
        else
        {
            end = middleElem;
        }
        len = std::distance(begin, end);
    }

    if (*begin == q)
    {
        return begin;
    }
    return end;

}


Optimize code usage and repeated code I have a method that I use nested foreach and the same variable call multiple times. I read a lot of optimizing code but I'm pretty sure I can do this with less code and best practices:

   public async Task<IEnumerable<IssueViewModel>> Get(IssueSelectModel model)

        {
            async Task<IEnumerable<IssueViewModel>> DoGet()

            {

                var issueList = await _bugTrackerRepository.Get(model);

                var userList = await _userService.Get(new UserSelectModel { RowsPerPage = 1000 });

                foreach (var item in issueList.IssueList)
                {

                    item.AssigneeName = userList.Items.FirstOrDefault(x => x.UserId == item.AssigneeId)?.UserName;

                    item.CreatedByName = userList.Items.FirstOrDefault(x => x.UserId == item.CreatedBy)?.UserName;

                    item.ModifiedByName = userList.Items.FirstOrDefault(x => x.UserId == item.ModifiedBy)?.UserName;

                    item.ClosedByName = userList.Items.FirstOrDefault(x => x.UserId == item.ClosedBy)?.UserName;

                    foreach(var comment in item.CommentList)
                    {
                        comment.CreatedByName = userList.Items.FirstOrDefault(x => x.UserId == comment.CreatedBy)?.UserName;

                        foreach(var tag in comment.TagList)
                        {
                            tag.UserByName = userList.Items.FirstOrDefault(x => x.UserId == tag.UserId)?.UserName;
                        }
                    }
                }
                return issueList.IssueList;

            }

            return await DoGet();

        }

Is it possible to use userList only one time?
Is there an alternative of multiple foreach statement?

Regards

Is it possible to use userList only one time?

Every time you call a FirstOrDefault, you are doing an O(n) search through the user list. You should consider keeping a dictionary of user IDs to user objects. Without knowing what kind of frameworks you're using, I can't say whether this is a job for your ORM or whether you have to do it yourself. With a dictionary, user lookup will be much faster.

So short answer, no; but you shouldn't be using a list.

  Is there an alternative of multiple foreach statement?

Not really.

Other things I see that could use some love:

Overuse of var

I find var to be useful in contexts where it's visually obvious what type a variable will take, including during instantiation statements. However, that is not the case with any of the instances of var in your code; I have no idea what's going into those variables. Unless the type is some kind of monstrous generic, just use the type.

Data model issues

You have some kind of user object, and some kind of item object with multiple references to user names. Are you sure it's a good idea to refer to the names, and not the user objects? The answer to this might be that you need the names in this case because the object is shortly destined for some kind of serialization, but if that isn't the case, it's probably a better idea to keep object references instead of string references.

In other words, why store comment.CreatedByName when you already have comment.CreatedBy?