Navigation properties with sql/dapper I want to have the same experiance with slq/dapper for my navigation properties just like in Entity Framework.
But I'm not sure which sql query i should use.
As you see, every single Resource entity calls a "select query" for retrieving it's related Language.
Does it effect the performance? Maybe i should use single "join query"?

public class Resource  {
    public int Id { get; set; }
    public string Name { get; set; }
    public int LanguageId { get; set; }
    public Language Language = {
         return connection.Query<Language>("select * from Language where Id = @Id", new { Id = LanguageId }).First();
    }
}

public class Language {
    public intId { get; set; }
    public string Name { get; set; }
    public List<Resource> Resources => {
         return connection.Query<Resource>("select * from Resource where LanguageId = @LanguageId", new { LanguageId = Id }).ToList();
    }
}

Your entities have a dependency to a connection object that's somehow available to them. If this is one instance you may experience technical impediments when reading entities and their nested entities in one statement. But that's not the most important thing.

The most important objection is that these "active" navigation properties will always give rise to the n + 1 problem: for each item you pull from the database by 1 query, you will trigger n queries to get their related data. That will certainly affect performance and it depends on the amount of data whether that's serious.

This effect is aggravated by the fact that the data aren't stored into the parent entities: each time a navigation property is accessed the query is executed.

There are more things to consider when it comes to reproducing Entity-Framework's (EF) behavior regarding navigation properties. EF loads entities into a context, which implements Identity Map: i.e. each database record will be represented by exactly one C# object. The benefits of this are hard to reproduce:

If one entity is modified, the changes are reflected wherever the entity is referenced. Also, changes are unambiguous: there are no "equal" entities around having old values. (You're probably getting read-only data, so this may not affect you).
Ability to perform operations that are based on referential equality. For example, in your case, grouping Resources by Language will produce groups having 1 item, because the language object aren't equal (sure, can be evaded by using LanguageId, but still, it's a gotcha).
EF can perform relationship fixup when it loads entities: loading Resources and Languages separately into one context will auto-populate the navigation properties. Your code won't ever do that.

Side note: Dapper's Multi Mapping Feature won't alleviate the first two points because it also creates separate object instances for "equal" entities.

So all in all, I wouldn't do this. I would get the data separately and perform some sort of "relationship fixup" manually.


Service status checker... Is there a more efficient way? I've been working on a website status checker.. However this array that I use has around 30 sites within it, I've noticed it's taking around 6-7 seconds to load it.

I was wondering if there's any kind of way of doing below, but more efficiently.

I have thought of doing the below in a cron, and fetching the stored results from a database.

<?php

function Visit($url){
   $agent = "S3 (SF Service status)";
   $ch=curl_init();
   curl_setopt ($ch, CURLOPT_URL,$url );
   curl_setopt($ch, CURLOPT_USERAGENT, $agent);
   curl_setopt ($ch, CURLOPT_RETURNTRANSFER, 1);
   curl_setopt ($ch,CURLOPT_VERBOSE,false);
   curl_setopt($ch, CURLOPT_TIMEOUT, 5);
   curl_setopt($ch,CURLOPT_SSL_VERIFYPEER, FALSE);
   curl_setopt($ch,CURLOPT_SSLVERSION,3);
   curl_setopt($ch,CURLOPT_SSL_VERIFYHOST, FALSE);
   $page=curl_exec($ch);
   #curl_error($ch);
   #
   #http header response code
   global $httpcode;
   $httpcode = curl_getinfo($ch, CURLINFO_HTTP_CODE);

   curl_close($ch);
   if($httpcode>=200 && $httpcode<=308) return true;
   else return false;
}

$sites = array(
"Name of site" => "URL",
);
?>

    <title>Service Status</title>
    <link href="static/css/main/main.css" rel="stylesheet" type="css/stylesheet">

service status

        <?php
        foreach ($sites as $key => $value) {
            echo '
';
            if (Visit($value)) {
                echo "$key ($value)	Available ($httpcode)	";
            } else {
                echo "$key ($value)	Unavailable ($httpcode)	";
            }
            echo '
';
        }
        ?>

Any recommendations on how parts of this could be done better would be greatly appreciated :)

PHP is not suitable for multi-threading. Though there are some extensions that offer ways of multithreading, they all are, basically, hacks IMO. The best way forward in your case would be: to use a tool that is async by nature, and update the url's as you go along.
Thankfully, such a thing exists, and it goes by the name JavaScript (AJAX calls), here's what I'd do:

foreach ($sites as $key => $value)
{
    echo '
',$key, ' (', $value, ')	',
    '
';
}

This will present the client with a complete overview of urls (since that's what you say they are), each row showing a gif (optional, of course) of one of those circular loading things This one, for example.
On the page, have script that goes something like this:

window.addEventListener('load',function l()
{
    var pending = document.querySelectorAll('td.pending'),i=0,
    callNext = function(elem)
    {
        var urlMatch = elem.parentNode.cells[0].textContent.match(/\(([^)]+)/)[1],
        xhr = new XMLHttpRequest;
        xhr.open('POST','ajax.php', true);
        xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
        xhr.setRequestHeader('Content-type', 'application/x-www-form-urlencoded');
        xhr.onreadystatechange = callback;
        xhr.send('url=' + encodeURI(urlMatch));
    },
    callback = function()
    {
        if (this.readyState === 4 && this.status === 200)
        {
            var resp = JSON.parse(this.responseText);
            pending[i].setAttribute('class','error');//default error
            if (resp.success)
            {
                pending[i].setAttribute('class','success');
            }
            pending[i].replaceChild(
                document.createTextElement(resp.cellText),
                pending[i].firstChild
            );
            if (++i < pending.length)
            {
                callNext(pending[i]);
            }
        }
    };
    callNext(pending[i]);
    window.removeEventListener('load',l,false);
},false);

Then, the ajax.php script, could just be the Visit function (BTW: please follow the PHP-FIG-coding standards as much as possible, and lower-case that function name). Instead of $url just use:

$url = isset($_POST['url']) && filter_var($_POST['url'], FILTER_VALIDATE_URL) ? $_POST['url'] : false;
if (!$url)
{
    echo json_encode(array('cellText' => 'invalid URL'));
    exit;
}
//do curl request
echo json_encode(array('success' => true, 'cellText' => 'valid'));//or invalid, depending on curl results

This way, the client gets to see the page quite quickly, and sees it's being updated once every 6~7 seconds, depending on how long the curl request takes to complete.
In addition to that, look into caching extensions, such as APC, and set the cache options for the cUrl requests, too. This usually requires some trial and error to find the right balance...


Removing all stopwords from a list of words What is the fastest Pythonic way to remove all stopwords from a list of words in a document? Right now I am using a list comprehension that contains a for loop.

from nltk.corpus import stopwords

''' Push stopwords to a list '''
stop = stopwords.words('english')
Document = ' Some huge text .......................... '
''' Tokenize the doc '''
words = nltk.word_tokenize(Document)
''' Comparing two lists '''
stopwordsfree_words = [word for word in words if word not in stop]

Is there a faster way to do this?

If stop is a list containing \$s\$ stopwords, and words is a list containing \$w\$ words, then the loop in the list comprehension will be \$O(w s)\$, since it basically has to iterate over both lists in a nested loop.

However, if you make the stopwords into a set…

stop = set(stopwords.words('english'))

… then each lookup can be done in \$O(1)\$ time.  You would get \$O(w)\$ running time just by changing the data structure like that.

Another minor issue is that by convention, Document should be lowercase, because it is a variable rather than a class.


Ruby CSV reading performance problems I (ruby beginner) have a performance issue with the following ruby code. The idea is to aggregate a CSV into a smaller, better-to-handle file and calculating the average of some values. Basically the same code in groovy runs about 30 times faster. And I can't believe that Ruby is that slow! ;-)

Some background:
The file consists of lines like this (it is an output file of a JMeter performance test):

timeStamp|elapsed|label|responseCode|responseMessage|threadName|dataType|success|failureMessage|bytes|Latency
2013-05-17_16:30:11.261|4779|Session_Cookie|200|OK|Thread-Gruppe 1-1|text|true||21647|4739

All values of e.g. a certain minute are selected by looking at the first 16 characters of the timestamp:

2013-05-17_16:30:11.261 => 2013-05-17_16:30

I wanted to collect values in buckets/slices which are represented by the shortened timestamp and the label (third column). The value of the second column ("elapsed") is summed up.

require 'csv'
require_relative 'slice'

# Parameters: <precision> <input file> [<output file>]
precision = ARGV[0].to_i
input_file = ARGV[1]
output_file = ARGV[2]

time_slices = Hash.new

CSV.foreach(input_file, {:col_sep => '|', :headers => :first_line}) do |row|
  current_time_slice = row['timeStamp'][0, precision]

  if time_slices[current_time_slice] == nil
    time_slices[current_time_slice] = Hash.new
  end

  if time_slices[current_time_slice][row['label']]
    time_slices[current_time_slice][row['label']].put_line(row)
  else
    new_slice = Slice.new(current_time_slice, row['label'])
    new_slice.put_line(row)
    time_slices[current_time_slice][row['label']] = new_slice
  end
end

out = File.new(output_file, 'a')
out.puts 'time|label|elapsed_average'
time_slices.values.each do |time_slice|
  time_slice.values.each do |slice|
    out.puts slice.aggregated_row
  end
end

The slice class looks like this:

class Slice
  attr_accessor :slice_timestamp, :slice_label, :lines, :sum, :count

  def initialize(slice_timestamp, slice_label)
    @slice_timestamp = slice_timestamp
    @slice_label = slice_label
    @count = 0
    @sum = 0
  end

  def put_line(line)
    @sum = @sum + line[1].to_i
    @count = @count + 1
  end

  def average
    @sum / @count
  end

  def aggregated_row
    @slice_timestamp + '|' + @slice_label + '|' + average.to_s
  end
end

I think that I chose a quite straightforward and non-optimized approach, but still - the same approach is much faster in Groovy. What can be the reason for that?

if time_slices[current_time_slice] == nil
  time_slices[current_time_slice] = Hash.new
end

It's more idomatic to write

time_slices[current_time_slice] ||= {}

In general don't use Hash.new if you can just write {}. You can rid of that statement alltogether though if you declare the time_slices variable with

time_slices = Hash.new { |h, k| h[k] = {} }

instead of time_slices = Hash.new.

if time_slices[current_time_slice][row['label']]
  time_slices[current_time_slice][row['label']].put_line(row)
else
  new_slice = Slice.new(current_time_slice, row['label'])
  new_slice.put_line(row)
  time_slices[current_time_slice][row['label']] = new_slice
end

Also

time_slices[current_time_slice][row['label']] ||= Slice.new ...

is more idiomatic here. But there is more to say here:

Don't forget that Ruby is an interpreted language in principle (although the byte compiler of Ruby 1.9 relativizes that), so don't repeat complex references like time_slices[current_time_slice][row['label']]. But that's not only an performance but even more so an readability issue.
Why does a Slice instance has to know the current_time_slice and label? That's completely redundant, since that information is already in the Hash. Get rid of that.

Considering this I would write the loop like this:

time_slices = Hash.new { |h, k| h[k] = Hash.new { |h, k| h[k] = Slice.new } }

CSV.foreach(input_file, col_sep: '|', headers: :first_line) do |row|
  current_time_slice = row['timeStamp'][0, precision]
  time_slices[current_time_slice][row['label']].put_line(row)
end

Of course you then have to modify your output loop a little

time_slices.each do |slice_timestamp, time_slice|
  time_slice.each do |label, slice|
    out.puts [slice_timestamp, label, slice.average].join('|')
  end
end

So you don't need the Slice#aggregated_row method anymore. But that's a good idea anyway: e.g. why does Slice has to know about your pipe separator? That's not a good separation of concerns.

Note that I avoid the + operator here. That would create a new String instance with every + call. Alternatively you can also try <<, but be aware that this modifies the leftmost string (but oftentimes that doesn't matter).

In contrast there is no penalty in using + in groovy normally, because modern JVMs are using StringBuilder under the hood for it when they see fit. But in Ruby you still should be aware of its implications.

attr_accessor :slice_timestamp, :slice_label, :lines, :sum, :count

Why are you declaring these accessors? They are never used.


array of struct read from file in C I want to read csv file in to array of structs with NULL ending.

test.txt:

11,345,1,k
12,34.23,3,v
13,13.4,5,x
14,15.7,6,c

My code:

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

struct csv_line {
  int value1;
  float value3;
  int value2;
  char char1;
};

struct csv_line *line_pointer = NULL;

int main(void) {
    FILE *csvFile = NULL;
    csvFile = fopen("test.txt", "r");
    if (csvFile == NULL)
      exit(1);

    size_t i = 0;
    while (1) {
    line_pointer = (struct csv_line *)realloc(line_pointer, sizeof(struct csv_line) * (i + 1));
    if (fscanf(csvFile, "%d,%f,%d,%c",
            &(line_pointer + i)->value1,
            &(line_pointer + i)->value3,
            &(line_pointer + i)->value2,
            &(line_pointer + i)->char1) != EOF) {
      i++;
    } else{
        (line_pointer + i)->value1 = 0;
        (line_pointer + i)->value3 = 0.0;
        (line_pointer + i)->value2 = 0;
        (line_pointer + i)->char1 = '\0';
        break;
     }
    }
    fclose(csvFile);
    int k = 0;
    while('\0' != (line_pointer + k)->char1) {
        printf("%d, %.2f, %d, %c",
                (line_pointer + k)->value1,
                (line_pointer + k)->value3,
                (line_pointer + k)->value2,
                (line_pointer + k)->char1);
        printf("\n");
        k++;
    }

    free(line_pointer);

    return EXIT_SUCCESS;
}

Questons:

Is it possible to move fscanf condition inside while-loop condition instead of endless (1) ?
How to set "end" of array with single NULL like "line_pointer[i] = NULL" in else statement?

user3629249's answer is good. The only thing I have to add:

struct csv_line *line_pointer = NULL;

// ...

&(line_pointer + i)->value1

is awkward. Why go through the pointer addition acrobatics? Why not just

&line_pointer[i].value1


Countdown to multiple update times each day I'm creating an update function to show my users when the next update will be available. I'm using the jQuery countdown plugin to show the time. What is a more efficient way to write this?

today = new Date();
todayDay = today.getDate();
todayYear = today.getFullYear();
todayMonth = today.getMonth();
todayMonth ++;
var countTo = "";
// $('#countdown').countdown('12:00:00');
var morningUpdate = new Date();
morningUpdate.setHours(10,0,0,0);

var middayUpdate = new Date();
middayUpdate.setHours(14,0,0,0);

var afternoonUpdate = new Date();
afternoonUpdate.setHours(17,0,0,0);

if(today < afternoonUpdate) {
 countTo = "17:00:00";
}

if(today < middayUpdate) {
    countTo = "14:00:00";
}

if(today < morningUpdate) {
    countTo = "10:00:00";
}

todayString = todayYear + "/" + todayMonth + "/" + todayDay;
console.log(todayString);
var countDownTo = todayString + " " + countTo;
console.log(countDownTo);
console.log("2015/5/13 12:00:00");

$('#clock').countdown(countDownTo, function(event) {
    var totalHours = event.offset.totalDays * 24 + event.offset.hours;
    $(this).html(event.strftime(totalHours + ' hr %M min %S sec'));
});

You don't need to create Date object for each day time case (morning/midday/afternoon) since you don't actually use any date component except hour. So you only need to extract current hour from today and define range it belong to.

var today = new Date();
var hour = today.getHours();
hour = hour < 10? 10 : hour < 14? 14 : hour < 17? 17 : 0;
var countDownTo = [today.getFullYear(), today.getMonth()+1, today.getDate()].join('/')+ (hour? hour+':00:00' : '');
...


Sliding tiles solver in Python I've implemented the sliding blocks puzzle in Python to solve it using different algorithms.
I'd like to know if the class "Sliding_blocks" is nicely designed or if I am missing
concepts of OOP.

I think that the indices of the blocks are a bit obfuscated, because I mix tuples,
lists and arrays (arrays are better to add but then I have to convert them to tuples
at some point).

I use the A* algorithm with the number of misplaced blocks as heuristic function.
It's quite slow and I think's it's because I have to create many copies of objects.

I'm more concerned with best practices than speed.

The code is:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
from __future__ import division
import numpy as np
from copy import copy, deepcopy

class Sliding_blocks():

    def __init__(self):
        self.size = 4
        self.block = self.generate_block()

    def generate_block(self):
        """Goal state"""

        block = np.arange(1,self.size**2)
        block.resize(self.size,self.size)
        return block

    def move(self, piece):
        """Moves the piece with index "piece" to free place, if possible"""

        if list(piece) in self.find_moves():
            self.block[tuple( self.find_free() )] = self.block[tuple(piece)]
            self.block[tuple(piece)] = 0
            return "success"
        else:
            return "error"

    def find_free(self):
        """Returns array of indices of the free cell"""

        free_position = np.where(self.block == 0)
        free_position = np.array(free_position).flatten()
        return free_position

    def find_moves(self):
        """Returns list of allowed indices to move"""

        from itertools import product
        free_position = self.find_free()
        return [list(free_position+i) for i in [[0,1],[1,0],[-1,0],[0,-1]] if tuple(i+free_position) in product(range(self.size),repeat=2)]

    def shuffle(self):
        steps = 30
        for i in xrange(steps):
            self.rand_move()

    def rand_move(self):
        from random import choice

        self.move(choice(self.find_moves()))

    #The following functions are used to find the solution

    def isWin(self):
        return (self.block == self.generate_block()).all()

    def total_misplaced(self):
        return np.sum( self.block != self.generate_block() )

def tree_search():#
    Game = Sliding_blocks()
    Game.shuffle()

    frontier = [[Game]]
    explored = []

    while 1:
        if frontier==[]: return "Error"

        path, frontier = remove_choice(frontier)
        endnode = path[-1]
        explored.append(endnode)
        if endnode.isWin(): return path

        #Iterate over all possible actions at endnode
        for action in allactions(endnode):
            if not action in explored and not action in frontier or action.isWin():
                pathtem=copy(path)
                pathtem.append(action)
                frontier.append(pathtem)

def allactions(obj):
    possible = obj.find_moves()
    actions = []
    for i in range(len(possible)):
        actions.append(deepcopy(obj))
        actions[i].move(possible[i])
    return actions

#A*
def a_star(frontier):
    #Calculates the cost (lenght + number misplaced cells)
    #of all paths in frontier, returns the frontier
    #without the least expensive path and also returns that path

    lengths = [f[-1].total_misplaced()+cost(f) for f in frontier]
    shortest=[i for i,l in enumerate(lengths) if l<=min(lengths)]
    return frontier.pop(shortest[0]), frontier

def cost(path): return len(path)

if __name__ == "__main__":

    remove_choice = a_star
    sol = tree_search()
    for s in sol:
        print s.block
        print "\n"

Whether the code is well designed or not -- is hard to tell without using it. But I have several points:

It's good that you have small functions/methods which do one thing and do well. It's good that you wrote docstrings and comments.
shortest=[i for i,l in enumerate(lengths) if l<=min(lengths)] -- here you are calculating min(lengths) on each loop cycle. Calculate this and put into a variable.
Comply with PEP-8. def cost(path): return len(path) -> have two different lines. np.arange(1,self.size**2) use one space after commas. Game = Sliding_blocks() -- only classes should named CamelCased -> game = SlidingBlocks(). Use pylint to see all warnings.
Use enumerate:

for i in range(len(possible)):
    actions.append(deepcopy(obj))
    actions[i].move(possible[i])

Will become:

    for i, _possible in enumerate(possible):
        action = deepcopy(obj)
        actions.append(action)
        action.move(_possible)

Move the imports to the top of the file

def rand_move(self):
    from random import choice

Import module instead of objects from it.

from itertools import product
...
from random import choice

Will become:

import copy
import itertools
import random

Using constants? Name them uppercase and make global. Is it a setting? Pass it as an argument with a default value or store as a class attribute.

def shuffle(self):
    steps = 30
    for i in xrange(steps):
        self.rand_move()

Fail early.

    if list(piece) in self.find_moves():
        self.block[tuple( self.find_free() )] = self.block[tuple(piece)]
        self.block[tuple(piece)] = 0
        return "success"
    else:
        return "error"

This can be rewritten as

        if list(piece) not in self.find_moves():
            return "error"
        ...
        return "success"

This will also reduce indentation.

return "error" -- this is a hardcoded string. Use a constant.

Indeed, using tuples and lists is hard to read and is slow

if list(piece) in self.find_moves():
    self.block[tuple( self.find_free() )] = self.block[tuple(piece)]
    self.block[tuple(piece)] = 0

You should rethink this.


Filter fragments manager How could I simplify the onCreate() method? Maybe via compact and properly-named methods?

public class FilterActivity extends FragmentActivity {
    private Bundle args;
    private FilterFragment filterFragment;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_3_filter);

        if (savedInstanceState == null) {
            args = getIntent().getExtras();
        } else {
            args = savedInstanceState.getBundle(Keys.args);
        }

        FragmentManager fragmentManager = getSupportFragmentManager();
        FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
        filterFragment = new FilterFragment();
        filterFragment.setArguments(args);
        if (savedInstanceState == null) {
            fragmentTransaction.add(R.id.containerOfFilterFragment, filterFragment);
        } else {
            fragmentTransaction.replace(R.id.containerOfFilterFragment, filterFragment);
        }
        fragmentTransaction.commit();

        filterFragment = (FilterFragment)
                getSupportFragmentManager().findFragmentById(R.id.containerOfFilterFragment);
    }

    @Override
    protected void onSaveInstanceState(Bundle outState) {
        super.onSaveInstanceState(outState);
        outState.putBundle(Keys.args, args);
    }
}

Is this method really a problem? Why are you worried about it? There are some small changes you can make, but they do not have much of an impact on the performance of the routine (if it is slow...).

About the only things I can think of improving are:

you check whether the savedInstanceState is null twice, and those two checks can be merged in to one...
you create a new FilterFragment instance, and you either add it to (if it is not there), or replace it (if there already was one). Thus, you have the filterFragement, and you do not need to findFragmentById(...) since you should be finding the fragment you just put in there....

Here's your method with the lines commented out that I don't think you need, and also two new lines marked with //**ADDED**

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_3_filter);

    //if (savedInstanceState == null) {
    //    args = getIntent().getExtras();
    //} else {
    //    args = savedInstanceState.getBundle(Keys.args);
    //}

    FragmentManager fragmentManager = getSupportFragmentManager();
    FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
    filterFragment = new FilterFragment();
    //filterFragment.setArguments(args);
    if (savedInstanceState == null) {
        filterFragment.setArguments(getIntent().getExtras());  //**ADDED**
        fragmentTransaction.add(R.id.containerOfFilterFragment, filterFragment);
    } else {
        filterFragment.setArguments(savedInstanceState.getBundle(Keys.args));  //**ADDED**
        fragmentTransaction.replace(R.id.containerOfFilterFragment, filterFragment);
    }
    fragmentTransaction.commit();

    //filterFragment = (FilterFragment)
    //        getSupportFragmentManager().findFragmentById(R.id.containerOfFilterFragment);
}


Current user as a class-level property in Objective-C and Swift I was recently tasked with architecting a user/profile object in Objective-C and wanted the ability to access a static instance from the class level, similar in style to the way Parse manages their current user (a static variable on a class, not a singleton).  With Parse, I can call a class method to return the current user object if a user is logged in, or nil if there is no current user session with [PFUser currentUser].

I architected my class like so:

User.h

@interface User : NSObject

/// Returns the current user if logged in, or nil if logged out
+ (User *)currentUser;

/// Sets the current user, call this on login
+ (void)setCurrentUser:(NSDictionary *)userInfo;

/// Removes the current user, call this on logout
+ (void)removeCurrentUser;

// Getters

/// Returns the user's name if logged in, nil if logged out
@property (nonatomic, readonly, strong) NSString *name;

@end

User.m

@interface User ()

@property (nonatomic, readwrite, strong) NSString *name;
@property (nonatomic, strong) NSString *address;

@end

@implementation User

#pragma mark - Static Variables

static User *currentUser;

#pragma mark - Static Object Setters and Getters

+ (User *)currentUser
{
    return currentUser;
}

+ (void)setCurrentUser:(NSDictionary *)userInfo
{
    currentUser = [[User alloc] initWithDictionary:userInfo];
}

+ (void)removeCurrentUser
{
    currentUser = nil;
}

#pragma init

- (instancetype)initWithDictionary:(NSDictionary *)userInfo
{
    self = [super init];

    if (self) {
        NSString *name = userInfo[@"name"];
        NSString *address = userInfo[@"address"];

        if (!name || !address ) { return nil; }

        _name = name;
        _address = address;
    }

    return self;
}

@end

This works well.  I have a single static User instance which can be non-nil if a user is logged in, and nil if the user is logged out.  I can access this object from anywhere with [User currentUser] without a singleton.

Now as an additional challenge I'm trying to translate this type of pattern into Swift.  Here is my fist pass:

public class User {

    // Private Properties
    private var name: String?
    private var address: String?

    // Private instance of User
    private static var user: User?

    // Class Functions

    public class func currentUser() -> User? {
        return self.user
    }

    public class func setCurrentUser(userInfo: [String : AnyObject]) {
        self.user = User(dictionary: userInfo)
    }

    public class func removeCurrentUser() {
        self.user = nil
    }

    // Instance methods

    public func getName() -> String? {
        return self.name
    }

    // Private

    private convenience init?(dictionary: [String : AnyObject]) {
        self.init()

        guard let name = dictionary["name"] as? String else { return nil }
        guard let address = dictionary["address"] as? String else { return nil }

        self.name = name
        self.address = address
    }
}

This works but doesn't feel "Swifty" enough.

Questions:

Is there a way to expose a public getter on a private property (sort of like a property redeclaration through a class extension in Objective-C)?  Or do I need to write an additional accessor.

Does declaring my private User object as private static ensure that it can only be accessed on the class level and not on the instance level?  Is this variable really static like it would be in C or Objective-C?

Can a User setter and getter be added/modified to remove the need for setCurrentUser and removeCurrentUser?

I am open to optimizations in both my Swift and Objective-C code.

(I am referring to the Swift code only.)

You can declare a property as

public private(set) var name: String?

to make it public read-only, but internally read-write. That makes
the additional accessor getName() obsolete.

If the property is only assigned in the init method and never
changed later, you can simply make it constant:

public let name: String?

Since the initializer ensures that both name and address are assigned
a value, you don't have to declare them as optionals.

The currentUser(), setCurrentUser(), removeCurrentUser()
methods can be replaced by a static property

public static var currentUser: User?

if you make the initializer public. static means class final,
i.e. it is a property of the type (and not an instance) and cannot
be overridden in subclasses.

The two guard statements can be combined into one.

The class then looks like this:

public class User {

    public static var currentUser: User?

    public let name: String
    public let address: String

    public init?(dictionary: [String : AnyObject]) {

        guard let name = dictionary["name"] as? String,
            let address = dictionary["address"] as? String
        else { return nil }

        self.name = name
        self.address = address
    }
}

The "current user" can now be set, retrieved, and unset like this:

User.currentUser = User(dictionary: ...)
if let user = User.currentUser {
    // ...
}
User.currentUser = nil

If you change the initializer to

public init?(dictionary: [String : Any])

then you can pass a native Swift dictionary without casting, e.g.

User.currentUser = User(dictionary: ["name": "Joe", "address": "San Francisco"])


JavaScript Blackjack I am currently learning JavaScript and the project I have been working on is creating a basic Blackjack game. Some code was provided for me, with the additional instructions:

"How would you edit the above code to PRINT what you and the Dealer have? How would you edit the code to let you know if you beat the dealer or not? How would you edit the code to let you know who busted? Hit 21? Also, would you hit once? Twice? What code would you put in if you hit either way? Test and run your code in an IDE of your choice."

The code I completed does work, but I believe there is probably a smarter way to write it. My code is as follows:

/* Created by Michael on 5/18/2016. */
// ——- —— —
// INPUTS: Two Integers
// OUTPUT: an Integer
// EFFECT: Returns a random integer between two inputs
function randint( min, max) {
    return Math.floor( Math.random() * (max - min + 1) + min);
}

// INPUTS: none
// OUTPUT: none
// EFFECT: "Player gets a Card"
 function hitMe() {
    console.log(" Hit me." );
handValue += randint( 1,11);
console.log(" New Hand: " + handValue)
}

// These are the cards you have
// your HandValue is an Integer
var handValue = 0;

// These are the cards the Dealer has
// the DealerValue is an Integer
var dealerValue = 0;

// here are your first two cards
handValue += randint( 1,11);
handValue += randint( 1,11);

// here are the Dealers cards
dealerValue += randint( 1,11);
dealerValue += randint( 1,11);

// the Dealer always hits once
dealerValue += randint( 1,11);
console.log("Dealer hits, do you hit or call?");

console.log("hit or call?");

if (handValue < 18)
{
    hitMe()
}
    if (handValue < 18) {
        hitMe()
    }
        if (handValue > 21) {
        console.log("Player Busts! Dealer has " + dealerValue + ".")
    } else if (handValue == 21) {
        console.log("Player hit 21! Dealer has " + dealerValue + ".")
    } else if (dealerValue == 21) {
        console.log("Dealer hit 21! Player has " + handValue + ".")
    } else if (handValue > dealerValue && handValue < 22) {
        console.log("Player wins with " + handValue + "! Dealer had " +              dealerValue + ".")
} else if (dealerValue > 21) {
    console.log("Dealer Busts with " + dealerValue + "! Player has " + handValue + ".")
} else if (dealerValue > handValue && dealerValue < 22) {
    console.log("Dealer wins with " + dealerValue + "! Player had " +   handValue + ".")
}

// Do you hit once? Twice? Or not? Insert your line of code below

// ————————-

Any recommendations for a more efficient way to write this game?

Better user experience

JavaScript is primarily for web browsers with some exceptions (like node.js). Unless your using node, you won't be able to get user input using the console. Instead of using console.log("New Hand: " + handValue) use:

alert("New Hand: " + handValue);

and to get user input you can use:

var userResponse = prompt("Dealer hits, do you HIT or CALL?");

Check out this JSFiddle to see alert and prompt in action.

You could create a much more interactive user experience using HTML, but since you're quite new to JavaScript, alert and prompt should be fine for now.

Abstracting duplicate code - Create a function to get cards

Currently, you use a randomInt function to hit the dealer and user with a new card. A random int from 1 to 11 sort of works, but it's statistically inaccurate with a deck of cards. As you get better and better at JavaScript you may eventually refactor want to edit your code to more closely models a deck of cards:

// Here is some code that more closely models a deck of cards
var cards = [];
for (var i = 1; i < 11; i++) { // for cards 1-10
  cards = cards.concat([i, i, i, i])
}
cards = cards.concat((new Array(12)).fill(10));

function drawCard() {
  return cards.splice(randInt(0, cards.length), 1);
}

The only problem is that your code is not prepared for such a refactor. You'll need to change all your calls to randInt that are intended to return cards to the new function. To prepare for this refactor, you can abstract the way you get cards into a function:

function drawCard() {
  return randInt(1, 11); // for now
}

// here are your first two cards
handValue += drawCard();
handValue += drawCard();

// here are the Dealers cards
dealerValue += drawCard();
dealerValue += drawCard();

Now any changes to your drawCard function will be changed throughout all your code.

Keeping track of cards given to dealer and user

You could keep track of the cards dealt to players by using arrays. Use one array for each player:

var userCards = [];
var dealerCards = [];

function hit(array) {
  array.push(drawCard());
}

function printHand(message, cards) {
  alert(message + "\n" + cards.join(", ");
}

//usage
hit(userCards);
hit(userCards);
hit(dealerCards);
hit(dealerCards);

printHand("You have:", userCards);
printHand("Dealer has:", dealerCards);

To get the sum of a hand, you can use a for loop:

function getTotal(cards) {
   var sum = 0;

  for (var i = 0; i < cards.length; i++) {
    sum += cards[i];
  }

  return sum;
}


Modifying stress markers in Cyrillic words This is used to change a Cyrillic word with a stress marker into the same Cyrillic word but with the stress marker separately represented as a number. As I want to return two values I pass two parameters by reference. I assume there is at most only one stress marker per word.

What could I improve here?

function seperateStress($word,&$cleanWord,&$stressLetter){
    $stressLetter=-1;
    $cleanWord="";
    for($i=0;$i<strlen($word);$i++){

        if($word[$i]=="&" ){
            //echo $i."
";
            $stressLetter=$i;

            $i=$i+5;
        }else{
            $cleanWord .=$word[$i];
            //echo $word[$i].":".$i."
";
        }
    }

    //return array($cleansedWord,$stressedLetter);
}

//Sample Test run
$stressWord= "велосипе́ды";
$noStressWord="";
$stressLetter=-1;

seperateStress($stressWord,$noStressWord,$stressLetter);
echo $noStressWord."::".$stressLetter."
";

Normally I would look at the PHP manual and make sure you aren't recreating a function that already exists, but I have no experience with non ASCII characters so i don't feel comfortable with the terminology to look this up. I would suggest that you make sure of this though. This sounds like something that might already exist somewhere. As to your function, there are a few things that jumped out at me.

First are those references. They are a powerful tool, but I would be careful using them. They are relatively difficult to spot and not everyone knows what they do. Sometimes its better just to use plain return statements, but that is something you will have to decide based on the scope of this project. If its something that isn't likely to be used by others, then this is fine, otherwise you might think about changing it.

When incrementing a variable by more than one, a more elegant way of doing so is by using the following syntax. By the way, where did the five come from? Unless cyrillic always has three digit entities, then you run the risk of missing some of the escape sequence. For example nbsp is a four digit entity. I would use strpos() or something similar to find the position of the semicolon instead.

$i += 5;

$stressLetter and $cleanWord definitions are redundant. You set them to the same values before the function was called. The only reason this would be necessary is if you revert back to using a return value instead of references, and then you should remove those parameters from the parameter list and from the procedural code before the function call.


Recursive function to merge 3D objects I have several 3D items in listOri. For this example:

listOri has A,B,C,D,E.

A overlaps with C.
B overlaps with D.
D overlaps with E.

I have a recursive function which accepts listOri, check if each item overlaps with each other, and generates a final listNew which has AC, BDE.

Iteration 1:
Loop through each item in listOri, generates listNew containing AC,B,D,E

Iteration 2:
Loop through AC,B,D,E in listNew, generates (new) listNew containing, AC,BD,E

Iteration 3: and so on.

Here is the snippet code which check if each 3D object in a list overlaps, and produces a new list recursively.

 Private Function SimplifyModel2(ByVal listOri As List(Of Mesh3D)) As List(Of Mesh3D)
    Dim listNew As New List(Of Mesh3D)(listOri)
    Dim indexOut, indexIn, indexInner, PercentProgressCurrent As Integer
    Dim currentMeshOutter, currentMeshInner As Mesh3D
    Dim isExitForCalled As Boolean = False

    totInnerLoops = totInnerLoops + 1   ' increment total number of inner loops

    For indexOut = 0 To (listOri.Count - 1)
        currentMeshOutter = listOri(indexOut)

        indexInner = indexOut + 1
        For indexIn = indexInner To (listOri.Count - indexInner)
            currentMeshInner = listOri(indexIn)

            If Is3DOverlap(currentMeshInner, currentMeshOutter) = True Then
                currentMeshOutter.CombineMerge(currentMeshInner)
                listNew.Remove(currentMeshInner)
                listNew.Remove(currentMeshOutter)
                listNew.Insert(0, currentMeshOutter)

                listNew = SimplifyModel2(listNew) ' recursively call the function
                isExitForCalled = True
                Exit For
            End If
        Next

        If isExitForCalled = True Then
            Exit For
        End If
    Next

    indLoopExit = indLoopExit + 1

    Return listNew
End Function

The function works well with listOri with very few items.
However, when there are thousands of 3D items in listOri, the functions takes very long time to produce the listNew.

How do I increase the speed of the recursive function?
Is there another way to write an algorithm which performs the same task above?

Let me know if you need any information.

Thank you.

I don't think you need recursion. The items in your first loop always "eat" the other ahead of them. Loop all items, for each item check if it can merge into an item ahead of it, if yes, merge and remove the item that was ahead.

Now VB does store the "To" part of a for loop in memory, so you'll need to convert your For into Do loop instead.

If you want your main list to be untouched, you could clone it. But don't forget that it seems each instance will be affected in the two list if you do that.

Private Sub SimplifyModel2(ByVal listOri As List(Of Mesh3D))
    Dim indexOut, indexIn As Integer
    Dim currentMeshOutter, currentMeshInner As Mesh3D

    indexOut = 0

    Do While indexOut < istOri.Count
        currentMeshOutter = listOri(indexOut)
        indexIn = indexOut + 1

        Do While indexIn < istOri.Count
            currentMeshInner = listOri(indexIn)

            If Is3DOverlap(currentMeshInner, currentMeshOutter) Then
                currentMeshOutter.CombineMerge(currentMeshInner)
                istOri.Remove(currentMeshInner)
            Else
                indexIn += 1
            End If
        Loop

        indexOut += 1
    Loop

End Sub


Becoming root from Python In order to avoid the user having to explicitly prefix a script with sudo or su --command, I wrote the following:

import sys
import os

if os.getuid():
    root = "/usr/bin/sudo"
    if not os.path.exists("/usr/bin/sudo"):
        root = "/bin/su --command"
    command = "{} {}".format(root, sys.argv[0])
    command = command.split()
    retcode = subprocess.call(command)
    if retcode:
        print("something wrong happened")
else:
    action_that_needs_admin_rights()

It feels like a hack to me, so am looking forward to better approaches.

The last time I checked (which admittedly has been a while) all major Linux distributions except Ubuntu have a default setup in which sudo is installed, but not configured to be able to start arbitrary applications. So on those your script will fail.

Apart from that I think it's a bad idea to use split like this. This will break if the python file (or the path to it if it was invoked with a path) contains spaces. I'd do it like this instead:

if sudo:
    root = ["/usr/bin/sudo"]
else:
    root = ["/bin/su", "--command"]
command = root + [ sys.argv[0] ]

A further problem is that you're requiring the script to be marked as executable. I think it'd be a better idea to use sys.executable to get the python interpreter and invoke that.


Extract title with regex I'm quite new to Go and I feel like this code could be smaller and cleaner.

I would love any suggestions and/or hints about mistakes and conventional go things!

func getBookTitle(client *http.Client) {
    rsp, err := client.Get(bookSite)
    if err != nil {
        panic(err)
    }
    html, _ := ioutil.ReadAll(rsp.Body)

    //Get div with title
    regTitle := regexp.MustCompile("(
)[\n+\\s]*(
)[a-zA-Z–\\-\n\\s:]*(
)[\n+\\sdd]*(
)")
    //remove linebreaks regex
    regFormatTitle := regexp.MustCompile("[\r\n]*")
    //apply regex
    title := regFormatTitle.ReplaceAllString(string(regTitle.Find(html)),"")
    //Remove remove html tags and remove whitespaces
    title =  strings.TrimSpace(title[strings.Index(title,"
")+len("
"):strings.Index(title,"
")])

    fmt.Printf("Book title:%s\n",title)

    rsp.Body.Close()
}

Go practices

It's recommended to close things using defer as early as possible,
so that you won't forget them later.
For example you have:

html, _ := ioutil.ReadAll(rsp.Body)

// some code

defer rsp.Body.Close()

Using defer, you should write like this instead:

html, _ := ioutil.ReadAll(rsp.Body)
defer rsp.Body.Close()

// some code

Another thing, it's recommended to not ignore errors.
This example ignores the err value returned by ioutil.ReadAll,
and that may lead to crashes later in the program.

Single responsibility principle

It's recommended for functions to do just one thing.
The getBookTitle function does do many things:

Download a web page from bookSite (a free variable defined elsewhere)
Read content from a stream
Extract title from an HTML string

The name implies only the 3rd task. It would be better if it did only that,
and the other tasks should be performed in other functions.

Strange regex

This regex looks strange:

(
)[\n+\\s]*(
)[a-zA-Z–\\-\n\\s:]*(
)[\n+\\sdd]*(
)

For several reasons:

[\n+\\s] is odd. \\s already includes \n. You could write simpler as [+\\s]. But are you really expecting a literal + between the starting
 and
? I seriously doubt it. In which case the expression could be reduced to just \\s.
[\n+\\sdd] is similarly odd. Again, the \n is unnecessary. Two d are also unnecessary, and if there are literal d between
 and
, that would be invalid HTML.
Why the groupings with (...)? Those parentheses don't serve any purpose in this program.

All in all, it seems the regex is mistakenly over-complicated,
this simpler regex is probably good enough and much more clear:

\\s*
[a-zA-Z–\\s:]*
\\s*


Fixing starting times command by command and globally I have a lot of Command objects. Each command has lots of Task objects. Tasks and commands are related. Each command wants to know which task belongs to it. Where each task wants to know which command it belongs to.
class Command:

    def __init__(self, date):
        self.deadline = date
        self.tasks = []

    def add_task(self, task:Task):
        """
        add a task to this command
        """
        self.tasks.append(task)
        return task

    def has_task(self, task):
        return task in self.tasks

    def remove_task(self, task):
        """
        Remove the task from this command.  After calling this,
        the task is orphaned and the reference to the task should be discarded.
        """
        self.tasks.remove(task)

class Task:
    def __init__(self, command:Command, duration:int):
        self.duration = duration
        self._command = command

    @property
    def command(self):
        """
        Getter for self._command with sanity checking that
        the task is not orphaned.
        """
        if not self._command.has_task(self):
            raise ValueError("orphaned task")
        return self._command

class Problem:
    def __init__(self, commands:Dict[str,Command], tasks:Dict[str,Task]):
        self._commands = commands
        self._tasks = tasks

In the Problem object I feel there is some redundancy in the data between Commands and Tasks.
I fix starting times in two ways:

Fix the start time of tasks command by command.
This is why is a commands attribute in Problem.
Merge them and then fix the starting times.
This is why there is tasks attribute in Problem.

I need both ways to fix starting times. However, fixing the starting time of a task always depends on the deadline of the command to which it belongs.
I'm unsure if my design is good. Sometimes I need to go through the tasks command by command. Sometimes I need to consider all the tasks for the command they belong.
Is there a design pattern I can use?

Maintaining bidirectional relationships
Rather than this check:
    if not self._command.has_task(self):
        raise ValueError("orphaned task")

it would be better to mark the task as orphaned by nulling out its command. In other words,
def remove_task(self, task):
    """
    Remove the task from this command.  After calling this,
    the task is orphaned and the reference to the task should be discarded.
    """
    task.command = None
    self.tasks.remove(task)

Having command as a simple, "public", possibly-None member on Task is simpler. At most, I would give this a type hint of Optional[Command] in the constructor.
Types
Don't call date date; that's a commonly used class in a built-in library.
deadline should probably be of type datetime (not date), and duration should be of type timedelta, not int.


Support multi functionality in server.js i've node app that having the main function server.js(entry point)

Following is the server.js file code

var childModule = require("./controller/childModule");
var server = http.createServer(app);
var proxy= require('./controller/proxy');
var proc = require('./controller/runIntProcess');
var extLoader = require("./controller/extLoader");
....

var server = module.exports = {};

server.run = function () {
    proc.pre(function () {
        server.listen(app.get('port'), function (err) {
            if (err) {
                console.error(err);
            } else {
                console.log('application Listening on port: ' + app.get('port'));
            }
            proxy.web(server);
        });
    })
};

server.addNewFunctionlity= function(oDir){
    extLoader.load(oDir.folder);
};

module.exports = function () {
    server.run();
}();

server.childModule = childModule;

The code do this:

when user call to the server he need to write the following code

var myModule = require("myModule");
myModule.addNewFunctionlity({folderPath:__dirname + '/files'});
myModule.addNewFunctionlity({folderPath:__dirname + '/files2'});

The Code is working! but I've several questions :-)

Does I export the childModule OK?
Does the usage of my API is make sense?
Does the var server = module.exports = {} is OK?

The code is running a node app which can be cousume by CMD (call to the server.js) without any args or using it by code.
in addition I need to expose some API's like childModule

I wouldn't pass file folders/files to a fn like that, I'd rather pass the functionality subject like:

myModule.addNewFunctionlity('geoMapping');
myModule.addNewFunctionlity('cookieService');

then refactor you fn nomenclature, maybe

myModule.addNewFunctionlity('geo.geoMapping');
myModule.addNewFunctionlity('frontend.cookieService');

If you think you really need to setup file/folder structure, then apply maybe a setupPath() fn:

myModule.setupPath({ folderPath:__dirname + '/files2'} );

so that in 5-7 years you and others will understand at a glance what
addNewFunctionlity() and setupPath() is about :)


Calculating the total of each column and display in footer of gridview in DataTable I've used this code to get the sum of the values of each column and display in footer. Here, the advantage is that I don't have to look for the column name, just get the count of total number of rows and columns and then iterate over each row and column.

Please suggest how I can make this code more efficient or if there is any way to make this code block more effective.

#region column sum and show in footer
//loop all the rows in the datatable
int[] RowTotals = new int[dt_.Columns.Count];
foreach (DataRow row in dt_.Rows)
{
    if(dt_.Rows.IndexOf(row) !=0)
    {
        //loop all the columns in the row
        for (int i = 1; i < dt_.Columns.Count; i++)
        {
            //add the values for each cell to the total

            RowTotals[i] += Convert.ToInt32(row[dt_.Columns[i].ColumnName]);
        }
    }

}

//loop all the columns again to set the values in the footer

for (int i = 1; i < dt_.Columns.Count; i++)
{

    GridView1.FooterRow.Cells[i].Text = string.Format("{0:N2}", RowTotals[i]);
}
///////
#endregion

#region column sum and show in footer

Before you start optimizing this block you should move it into a separate method. The region clearly indicates that it's a part of something bigger that is doing too much. I find it's pointless to try to optimize this as long as it's a region of something else.

Currently your method not only calculates the RowTotals but it also updates a grid-view. This should not be done by the same method because you cannot test whether the totals are calculated correctly. The only way to validate it is to look at the grid. This is very unreliable.

Basically it should look like this: you have two worker methods

int[] CalculateRowTotals(DataTable dataTable) { ... }

void UpdateTotalsGridView(int[] rowTotals) { ... }

and one method that combined the results of both:

void DisplayRowTotals(..)
{
    var rowTotals = CalcualteRowTotals(..);
    UpdateTotalsGridView(rowTotals);
}

About the code itself...

if(dt_.Rows.IndexOf(row) !=0)

It is not clear why you are doing this. A helper variable or method would be nice so that you can give it a meaning like I don't know isHeader or something that makes sense.


Is context.clearRect() necessary? My application is a bit slow. I believe this is because .clearRect is one of the more intensive events in my application so far. However I also draw an image on top of the screen (as background) at the top of each loop, like this:

function redraw(){
  ctx.save();
  ctx.clearRect(0,0,canvas.clientWidth,canvas.clientHeight);
  ctx.drawImage(background,0,0,canvas.clientWidth,canvas.clientHeight);

  var i=0,l=objects.length;
  while(i<l){
    objects[i].redraw();
    i++
  }
  ctx.restore();
}

I'd like to improve the speed of this code. Would removing the .clearRect call be a good way to optimize performance? Or will this clog up memory?

The way it's phrased, this isn't really a Code Review question. Still, I'll try to treat it as one.

But first, to answer your question: Yes, I would imagine you could just remove the call to clearRect as long as the background image you're drawing is entirely opaque.

(incidentally, you should just be able to try it out and see quicker than asking question on a Stack Exchange site: Just comment out the line and see what happens.)

However, in code review terms, I have to wonder why you're drawing the image on every update. If the background image doesn't constantly change, you could just consider keeping the clearRect call, but setting the background image as a regular CSS background on the canvas element. Then you don't need to worry about drawing the background yourself; it'll just be there, behind the canvas element.

Also, what's with the while loop? You've just written a complicated equivalent to a for-loop. Given its purpose here, a for loop would by far be the most straightforward thing to use:

for(var i = 0, l = objects.length ; i < l ; i++) {
  objects[i].redraw();
}

While you're at it, please give your code some room to breathe. Apart from indentation, I don't see a single space anywhere in your code. It's all squished together, which makes it unnecessarilyhardtoread.

And you're missing a semi-colon in the while loop. JS will still run your code, but I see no reason to rely on JS's automatic semi-colon insertion. Especially not on the one line that must work to avoid an infinite loop.


A basic multi-dimensional array I have been lurking on the C++ side of stack-overflow only long enough to know there are a lot of beginners and intermediate programmers baffled by multi-dimensional arrays. I've seen a lot of monstrosities : three-star programmers, cache-unfriendly implementations, vectors of vectors of vectors ..., etc.

I thought to provide them with a basic multi-dimensional array:

open for extensions,
fit for production and experimentation,
working without allocations,
and cache-friendly.

Before I do so, I'd like some advice from the community.

  What can I improve in the provided features of this class?
  What new feature could one add, and why would it be useful?

#include <array>
#include <numeric>

namespace ysc
{
namespace _details
{
    template<class InputIt, class OutputIt>
    OutputIt partial_product(InputIt first, InputIt last, OutputIt output)
    { *output++ = 1; return partial_sum(first, last, output, std::multiplies<>{}); }

    // cache-friendly:
    // neighbor objects within the right-most coordinate are neighbors in memory
    template<class TDim, class TCoord>
    auto coordinates_to_index(TDim const& dimensions, TCoord const& coords)
    {
        std::array<std::size_t, dimensions.size()> dimension_product;
        using std::crbegin, std::crend, std::prev;
        partial_product(crbegin(dimensions), prev(crend(dimensions)), begin(dimension_product));
        return std::inner_product(cbegin(dimension_product), cend(dimension_product), crbegin(coords), 0);
    }
}

constexpr struct matrix_zero_t {} matrix_zero;

template<class T, std::size_t... Dimensions>
class matrix
{
template<class, std::size_t...> friend class matrix;

public:
    static constexpr std::size_t order      = sizeof...(Dimensions);
    static constexpr std::array  dimensions = { Dimensions... };

private:
    std::array<T, (Dimensions * ...)> _data;

public:
    friend void swap(matrix& lhs, matrix& rhs)
    {
        using std::swap;
        swap(lhs._data, rhs._data);
    }

public:
    matrix()                          = default;
    matrix(matrix&& other)            = default;
    matrix& operator=(matrix&& other) = default;

    matrix(matrix_zero_t) : _data({}) {}

    template<class U>
    matrix(matrix<U, Dimensions...> const& other) { std::copy(cbegin(other._data), cend(other._data), begin(_data)); }

    template<class U>
    matrix& operator=(matrix<U, Dimensions...> const& other)
    {
        matrix o{other};
        swap(*this, o);
        return *this;
    }

public:
    template<class... Args>
    T const& operator()(Args... coordinates) const
    { return _data[_details::coordinates_to_index(dimensions, std::array{coordinates...})]; }

    template<class... Args>
    T& operator()(Args... coordinates)
    { return _data[_details::coordinates_to_index(dimensions, std::array{coordinates...})]; }
};
} // namespace ysc

Usage demo: http://coliru.stacked-crooked.com/a/1652451c78275436

This is a C++17 implementation; this itself is not set in stone.

Here are some suggestions:

coordinates_to_index should return std::size_t instead of int because int may not be able to hold the required values.

return std::inner_product(cbegin(dimension_product), cend(dimension_product),
                          crbegin(coords), std::size_t{0}); // instead of int

You don't need to compute the strides and store them. You can use Horner's rule directly.

std::size_t result = 0;
for (std::size_t i = 0; i < N; ++i)
    result = result * dimensions[i] + coords[i];
return result;

Your calculation of the total size fails to consider the case where order == 0. Instead of

(Dimensions * ...)                   // ill-formed if Dimensions is empty

Use

(Dimensions * ... * std::size_t{1})

Moreover, I would suggest exposing the total size, along with the order and dimensions. Like this:

static constexpr std::size_t size = (Dimensions * ... * std::size_t(1));

And use it in place of ad-hoc computations further in code.
Your matrix is not Copyable. You declare move operations but not copy operations (the template doesn't make a difference), and the compiler synthesizes deleted copy operations which take precedence over the copying template. You should either default the copy operations explicitly (and maybe destructor as well), or omit the redundant default declarations directly.
Why use :_data({}) when :_data{} is more readable and less verbose?
Consider adding out-of-range detection for operator() in some form.

Update

Currently, the converting copy constructor first default-initializes the elements and then copy assign from the initializer matrix, which is kinda surprising. It is more reasonable to initialize directly (with some template hacks):

namespace detail {
    template <class T, std::size_t N, class U, std::size_t... Is>
    std::array<T, N> construct(const std::array<U, N>& arr, std::index_sequence<Is...>)
    {
        return std::array<T, N>{static_cast<T>(arr[Is])...};
    }
}

template <class T, std::size_t... Dimensions>
class matrix {
    // ...
public:
    // ...
    template <class U>
    matrix(matrix<U, Dimensions...> const& other)
        : _data{detail::construct<T, N>(other._data, std::make_index_sequence<size>{})}
    {
    }
    // ...
};

The above implementation requires access to relies on access to _data of another matrix specialization, which can be granted with a friend declaration or an access function. It also relies on the C++17 guaranteed copy elision.

Now, the converting copy assignment operator cannot delegate to the converting copy constructor:

template <class U>
matrix& operator=(const matrix<U, Dimensions...>& other)
{
    std::copy_n(_data.begin(), size, other._data.begin());
    return *this;
}

It makes sense to constrain the converting copy constructor and the converting copy assignment operator with SFINAE. Also, they should be made conditionally explicit and conditionally noexcept:

template <class U, class = std::enable_if_t<
    std::is_constructible_v<T, const U&> && std::is_convertible_v<const U&, T>
>> matrix(matrix<U, Dimensions...> const& other)
    noexcept(std::is_nothrow_constructible_v<T, const U&>);

template <class U, class = std::enable_if_t<
    std::is_constructible_v<T, const U&> && !std::is_convertible_v<const U&, T>
>> explicit matrix(matrix<U, Dimensions...> const& other)
    noexcept(std::is_nothrow_constructible_v<T, const U&>);

Similarly for the converting copy assignment operator.
Consider support converting move construction and converting move assignment.
The class can be modified to support constexpr.


Fuzzy matching dict keys What do you think of this idea?  It's supposed to be a dict that makes a reasonable guess at the key you were trying to access for getters and deleters.  Setters should be untouched.

Similar to how tab completion in bash works, if some command you are typing only has one reasonable completion then pressing tab will fill it out for you.

The logic should be like this, if the query_key is not in the dict but there is only one dict_key satisfying the expression query_key in dict_key then we can assume that is the key.  For string keys, additionally, it should be case-insensitive.

class FriendlyDict(dict):

    def _guess_key_or_die(self, q):
        possibilities = []
        for key in self:
            try:
                if q in key or q.lower() in key.lower():
                    possibilities.append(key)
            except (TypeError, AttributeError):
                pass
        if len(possibilities) == 1:
            return possibilities[0]
        err = KeyError(q)
        err.data = possibilities
        raise err

    def __getitem__(self, key):
        try:
            return super(FriendlyDict, self).__getitem__(key)
        except KeyError:
            guess = self._guess_key_or_die(key)
            return super(FriendlyDict, self).__getitem__(guess)

    def __delitem__(self, key):
        try:
            return super(FriendlyDict, self).__delitem__(key)
        except KeyError:
            guess = self._guess_key_or_die(key)
            return super(FriendlyDict, self).__delitem__(guess)

    def __contains__(self, key):
        if super(FriendlyDict, self).__contains__(key):
            return True
        try:
            self._guess_key_or_die(key)
        except KeyError:
            return False
        else:
            return True

Are there any more __dunder__ methods which really should be overridden?
Are there any gotchas I may have missed which might make the class behave in weird/unpredictable ways?
Are there any other nice features that could be added which are consistent with the general idea of this?
Is it better as a subclass or would it be possible to make it a mixin to use with other types of mappings, e.g. OrderedDict?

A few other methods I would suggest implementing:

get;
setdefault;
has_key (if supporting 2.x); and
__repr__/__str__.

There is an edge case on hashable collections (tuple, frozenset):

>>> d = FriendlyDict()
>>> d[(1, 2)] = "foo"
>>> d[1]
'foo'
>>> d[frozenset(("bar", "baz"))] = 3
>>> d["bar"]
3

Also equality comparisons might be a problem - should two FriendlyDicts be considered equal if all (most?) of their keys are "close enough" and have the same values?

In terms of additional features, consider whether numerical keys should be matched with some tolerance - dictionaries not handling floats particularly well could be one reason to use this class. Also, a missing letter within a string key might be something to pick up (e.g. d["parameer"]).


Sort multidimensional array based on another one containing the desired order I have an array like this:
$array = [
    ['id' => 1, 'content' => 'value 1'],
    ['id' => 2, 'content' => 'value 2'],
    ['id' => 3, 'content' => 'value 3'],
];

I have another array containing the actual order of ids in which $array should be sorted :
$order = [3, 1, 2];

Which means, the result should be this:
$sorted = [
    0 => ['id' => 3, 'content' => 'value 3'],
    1 => ['id' => 1, 'content' => 'value 1'],
    2 => ['id' => 2, 'content' => 'value 2'],
];

The final requirement is, that the array should be printed using foreach:
foreach($sorted as $value) {
    print $value['content'];
}

I came up with this solution:
function orderArray(array $array, array $order) {
    $sorted = [];

    foreach($array as $value) {
        $id = $value['id'];
        $index = array_search($id, $order);
        $sorted[$index] = $value;
    }

    ksort($sorted);

    return $sorted;
}

This works fine. However, I wonder whether this is efficent, especially as I have to call ksort to make the array be printed correctly by foreach.
Can this be optimized? Maybe there's also a more elegant solution to the problem.

It's a given, that all ids and only those are in $order - so no need to test for that.

I think this would be the cleanest solution. Similar to @YourCommonSense answer, but slightly inverted as ordered index is already there - only values missing.

$sorted = array_flip($order);
foreach ($array as $value) {
    $id = $value['id'];
    $sorted[$id] = $value;
}


Implementation of the Selection Sort Algorithm I have tried to implement the Selection Sort Algorithm in two different ways, and I was just wondering if that is considered to be a valid implementation of that algorithm. Would that be also considered an efficient implementation of the algorithm, or are there different and more efficient ways?

Here are my implementations:

Implementation Attempt #1

def selection_sort(L: List[int]) -> List[int]:
    for i in range(len(L) - 1):
        j = i
        smallest_num = L[i]
        while j < len(L) - 1:
            if smallest_num > L[j + 1]:
                smallest_num = L[j + 1]
            j += 1
        L[L.index(smallest_num)], L[i] = L[i], L[L.index(smallest_num)]
    return L

Implementation Attempt #2

def selection_sort(L: List[int]):
    for i in range(len(L) - 1):
        j = i
        while j < len(L) - 1:
            swap_min(L, i)
            j += 1
    return L

def swap_min(L: List[int], n: int) -> None:
    smallet_num = min(L[n:])
    sm_indx = L.index(smallet_num)
    L[n], L[sm_indx] = L[sm_indx], L[n]

The use of L.index(smallest_num) makes neither of these implementations valid. Using the input list [1, 0, 1, 0, 1] as an example, both of your implementations give [1, 1, 0, 0, 1] as their result, and this is clearly not sorted correctly.

The reason your algorithm doesn't work on this input is that it contains duplicate elements, so the .index method doesn't always return the index you want it to; as the documentation says, the .index method returns the index of the first occurrence of the given value. That will not always be the index of the occurrence you want to find; sometimes, that index is in the "already-sorted" part of the list, so you end up swapping things back there where they don't belong.

A correct implementation of selection sort should search for the index of the minimum element in just the unsorted part of the list.


Simple boolean gate with two TriState inputs As part of a code review I came across this isDummy(a,b)-Method and the question, whether it is more readable as a conjunction, or a disjunction:

Original snippet:

public static boolean isDummy(TriState a, TriState b) {
    return (a.is(ZERO) || b.is(ZERO)) && (a.is(NULL) || b.is(NULL));
}

Suggested change:

public static boolean isDummy(TriState a, TriState b) {
    return a.is(ZERO) && b.is(NULL) || a.is(NULL) && b.is(ZERO);
}

Both of the above return true if one TriState is ZERO and the other is NULL.

Both do exactly the same; however, the latter imho clearly states, that just two possible combinations are a dummy.
The first one could - without knowing that ZERO is not a VALUE and TriState#is(TriState) does an equality check - possibly suggest, that four possible combinations are dummy values.

As you can see in TriState's constructor the author's intention was to implement a helper mechanism to check Doubles for its "valueness". Infinity and NaN aren't taken into account:

enum TriState {
    NULL, ZERO, VALUE;

    public static TriState of(Double number) {
        if (number == null) {
            return TriState.NULL;
        } else if (number == 0d) {
            return TriState.ZERO;
        } else {
            return VALUE;
        }
    }

    private boolean is(TriState other) {
        return this == other;
    }

    /**
    * Tests if this object is {@link #VALUE} and the given TriState is not {@link #VALUE}.
    *
    * @param other the TriState to test against
    * @return true if this object is {@link #VALUE} AND the given TriState is not {@link #VALUE}. False otherwise.
    */
    public boolean andNot(TriState other) {
        return this.is(VALUE) && !other.is(VALUE);
    }

    /**
    * Compares two TriState objects to find out if exactly one is ZERO and exactly one is NULL. This method is commutative.
    *
    * @param a a TriState instance to check
    * @param b a TriState instance to check
    * @return true, if one of the given TriState objects is {@link #ZERO} while the other is {@link #NULL}. False otherwise.
    */
    public static boolean isDummy(TriState a, TriState b) {
        return (a.is(ZERO) || b.is(ZERO)) && (a.is(NULL) || b.is(NULL));
    }
}

While the mere existence of that Enum instead of just using Double instances directly is a whole different matter, I would be interested if there are any known best practices regarding that boolean expression.

Which of the both is easier to understand?

The readability can be improved by using explanatory variables, (of course changing the variable names as they make sense to you).

public static boolean isDummy(TriState a, TriState b) {
    boolean oneOfTheInputsIsZero = a.is(ZERO) || b.is(ZERO);
    boolean oneOfTheInputsIsNull = a.is(NULL) || b.is(NULL);
    return oneOfTheInputsIsZero && oneOfTheInputsIsNull;
}

First one makes it obvious that the function is commutative, in the second one you have to check the LHS and RHS are indeed the mirror image of each other.

Although the two functions may evaluate the same thing they don't say the same thing.
First one says "There is at least one zero and there is at least one null."
Second one says "There is one zero and the other one is null." (which matches the javadoc)

But the comments sometimes also lie. To check the veracity of the comment consider how would you write the function if it took more parameters.

They still say what they say before but now they don't evaluate the same coincidentally.

public static boolean isDummy(TriState a, TriState b, TriState c) {
    return (a.is(ZERO) || b.is(ZERO) || c.is(ZERO))
        && (a.is(NULL) || b.is(NULL) || c.is(NULL));
}

public static boolean isDummy(TriState a, TriState b, TriState c) {
    return a.is(ZERO) && b.is(NULL) && c.is(NULL)
        || a.is(NULL) && b.is(ZERO) && c.is(NULL)
        || a.is(NULL) && b.is(NULL) && c.is(ZERO);
}


Fastest path on a snakes and ladders board I am not looking for a detailed review of this code as I am aware it is very inefficient. The main reason for posting this is to see if there is a better way to achieve my goal (I am certain there must be)

I have created code to find the fewest number of dice rolls that would permit someone to move from the first to last square on a snakes and ladders board. The player wins if they land on or go beyond the final square on the board. They start off the board at position -1.

My approach uses recursion so is OK for small boards. The moment the board reaches a size of 30+ it takes far too long to generate the solution.

Is there a better way to solve this problem

"""Module works out fastest way to traverse a snakes and ladders board"""
def roll_dice(position, roll_number, number_squares, snakes, ladders, list_moves=[]):
    """Roll the dice and then work out if the player can climb a ladder / has won"""
    if position in ladders:
        position = ladders[position]
    if position >= number_squares - 1:
        list_moves.append(roll_number)
        return
    for i in range(1, 7): #For each position roll the dice 6 times
        if position + i in snakes:
            continue  # Forbid a dice-roll that lands on a snake
        roll_dice(position + i, roll_number + 1, number_squares, snakes, ladders)
    return list_moves

def minimum_moves(number_squares, snakes={}, ladders={}):
    """Returns the minimum number of moves starting from position 0 for a board of size n
    snakes and ladders are both dictionaries containing the starting point as the key
    and end point as the value"""
    # Initialise board
    # The player starts off the board at position -1
    list_moves = roll_dice(-1, 0, number_squares, snakes, ladders)
    print(f"Board is traversable in {min(list_moves)} moves")

if __name__ == "__main__":
    NUMBER_SQUARES = 25
    SNAKES = {21:0, 19:9, 14: 2, 18:5}
    LADDERS = {2: 21, 4:9, 10:20, 17:23}
    minimum_moves(NUMBER_SQUARES, SNAKES, LADDERS)

Don't use mutable default arguments. If you need to default to a list then default to None and then change to an empty list.

Take the following example code:

>>> def example_list_builder(value, list_=[]):
    list_.append(value)
    return list_

>>> example_list_builder(1)
[1]
>>> example_list_builder(2)
[1, 2]

This makes list_ a really poorly defined singleton variable, that can't be accessed globally.

Your code looks like it's something like \$O(6^n)\$ where \$n\$ is the size of the board.

You can instead make it \$O(l^2)\$ where \$l\$ is LADDERS and SNAKES.

To improve your code, you should remove the simulation and instead only work with the LADDERS and SNAKES variables.

For each snake and ladder you should traverse it, and then traverse the snake or ladder after this. You should note the distance, there are two ways to do this:

Least tiles visited.
Least turns taken.

For the latter, you'll have to check for collisions with other snakes and ladders.

You should take snakes into account, as the fastest path for the following is 4 turns:

NUMBER_SQUARES = 1000
LADDERS = {1: 502, 501: 999}
SNAKES = {503: 500}


Tornado chat with SockJS and Redis I have Tornado chat with export to Redis. Other scripts will export messages from Redis to MongoDB in the time. It works, but how is it written?

def tornadoredis_client():
    client = tornadoredis.Client(*configuration.REDIS_CONFIG)
    client.connect()
    return client

class PlansqTornadoChat(SockJSConnection):
    users = dict()
    timezones = dict()

    def on_open(self, info):
        self.authenticated = False
        self.timezone = None
        self.client = tornadoredis_client()

    @tornado.gen.engine
    def on_message(self, msg):
        data = proto.json_decode(msg)

        if data['type'] == 'auth':
            if bool(data.get('sid', None)):
                try:
                    int(data['sid'])  # check that sid is number, not word
                    self.user_sid = data['sid']
                    self.users[self.user_sid] = self
                    self.user_timezone = yield tornado.gen.Task(self.client.hget, 'timezones', self.user_sid)
                    self.timezones[self.user_sid] = self.user_timezone
                    self.authenticated = True
                    self.send(proto.json_encode({"body": "You was authorized", 'type': 'alert'}))
                except Exception, e:
                    logging.info("bad user's sid: " + str(data['sid']))
            else:
                self.send(proto.json_encode({"body": "You did not authorize", 'type': 'alert'}))
                self.close()

        elif data['type'] == 'message':
            if all([x in data for x in ['type', 'recipient', 'body', 'sender']]) \
                    and data['sender'] == self.user_sid:
                recipient = data['recipient']

                try:
                    int(recipient)  # check that sid is number, not word
                except Exception, e:
                    logging.info('sid of recipient is not number')
                    return

                self.thread = "_".join([str(x) for x in sorted([int(self.user_sid), int(recipient)])])

                # Save to Redis
                data_to_send = {
                    'thread_id': self.thread,
                    'sender': self.user_sid,
                    'recipient': recipient,
                    'body': data['body'],
                    'datetime': (datetime.now() - datetime(1970, 1, 1)).total_seconds()
                }

                yield tornado.gen.Task(self.client.sadd, "thread:" + self.thread, proto.json_encode(data_to_send))
                yield tornado.gen.Task(self.client.sadd, 'new_threads:all', "thread:" + self.thread)

                # Send to Socket
                data_to_send['type'] = 'message'

                # if user is online
                if recipient in self.users:
                    data_to_send['datetime'] = utc_to_localtime(data_to_send['datetime'], self.timezones[recipient])
                    self.users[recipient].send(proto.json_encode(data_to_send))

                data_to_send['datetime'] = utc_to_localtime(data_to_send['datetime'], self.user_timezone)
                self.send(proto.json_encode(data_to_send))

            else:
                logging.info('Bad request: ' + str(data))
        else:
            logging.info('Bad request: ' + str(data))

    def on_close(self):
        del self.users[self.user_sid]
        del self.timezones[self.user_sid]
        self.client.disconnect()

My configuration in configuration of main application (Flask + MongoDB). There only Redis configuration (host and port).

The rest of the code:

if __name__ == '__main__':
    logging.getLogger().setLevel(logging.DEBUG)
    logging.getLogger().setLevel(logging.INFO)

    EchoRouter = SockJSRouter(PlansqTornadoChat, '/socket')

    app = web.Application(EchoRouter.urls)
    app.listen(configuration.TORNADO_PORT)
    ioloop.IOLoop.instance().start()

I have some notes on Pythonic style.

People usually instantiate an empty dict with {}, instead of actually calling dict(). Usually, the only reason you need to call dict is if you're passing some variables to create your dictionary.

There's no need to explicitly cast a value to a boolean, Python can implicitly run a boolean test on any type. So you can just do this:

if data.get('sid', None):

I don't know exactly what type data is, but assuming it's a dictionary, get will default to returning None, so there's no need to make it an explicit parameter of get.

You have a long block inside a try followed by a blanket except Exception, e. There's a few problems here. First, your try block should be as narrow as possible. You can use return to prematurely end functions from the except block if you need to. Speaking of that except block, you should ensure that it's only catching errors you specifically expect. In this case, presumably you want to catch ValueErrors from a bad data['sid'] string, as well as catching KeyError for user_sids that don't show up in the users dictionary. Blanket exceptions will just mask errors you're not expecting, instead of handling the ones you know can occur in normal execution.

Lastly, don't use except Exception, e syntax, it will lead to trouble and was removed in Python 3. It causes ambiguities when you want to catch multiple exception types, like except KeyError, ValueError, e:. How is Python supposed to interpret that? This is why you should use except (KeyError, ValueError) as e: or except (KeyError, ValueError):, as you don't have to store the result in a variable if you're not using it.


JSON backup to backend server if connection issues I have a program that includes sending data to our backend servers upon completion.

Due to the occasional connection issue at either side I have coded into the main script a JSON dump as a backup in case we are unable to connect to the BE server.

The JSON file is simply a Key/Value mirror of the SQL database fields in the table in question.

I will run this program in the background which will periodically check the folder. It will open the JSON and insert / update any files that are in there and remove them if completed.

from json import load
from time import sleep
from os.path import join
from os import remove
from glob import glob

from sql_tunnel_control import (
    SqlTunnelConnect,
    SSH_KEY,
    sql_ip,
    sql_hostname,
    sql_username,
    sql_password,
    sql_main_database,
    sql_port,
    ssh_host,
    ssh_port,
    ssh_user,
    ssh_password,
    table
)

production = SqlTunnelConnect(
    SSH_KEY,
    None,
    sql_hostname,
    sql_username,
    sql_password,
    table,
    sql_port,
    ssh_host,
    ssh_port,
    ssh_user,
    None,
)

def load_be_data(f):
    """ Save data to json file and allow another program to iterate over that folder until all data is safely in the backend server"""
    try:
        with open(join(f), "r") as j:
            return load(j)
    except Exception as e:
        print(e)
        # pass

def get_files():
    f = glob("backend_data/*.json")
    return f

def send_to_be(f):
    """ takes in a file and attempts to send  to the server.. Returns False if it fails.. Deletes the file if it succeeds """
    print(f)

    if f["query_type"] == "insert":
        create_new_component = """INSERT INTO components .... """
        try:
            production.run_query(create_new_component)
            return True
        except:
            return False
    elif f["query_type"] == "update":
        update_component = """UPDATE components SET ..."""
        try:
            production.run_query(update_component)
            return True
        except:
            return False
    else:
        return False  # problem with query type

def persist_dump():
    to_send = get_files()
    while len(to_send) > 0:
        for f in to_send:
            r = send_to_be(load_be_data(f))
            if r:
                remove(f)
                sleep(1)
            else:
                sleep(300)  # 5 minutes rest.
    # all files completed.. sleep for a bit.. then see whats what//
    sleep(1800)
    persist_dump()

persist_dump()

load_be_data

Parameter name is not very descriptive. I'd say that 'file_path' would be better.
Assuming that there is some end user, it would be better to add some descriptive messages for different kinds of exceptions instead of printing the message. Also, it is a good practice to note where the exception was caught. For example, 'Exception with message XYZ was raised in load_the_data'.

get_files

I don't think there is any reason to create a local variable.
Just do :

def get_files():
    return glob("backend_data/*.json")

send_to_be

Again, parameter name is not descriptive.
Is there any reason why do you want to return boolean value instead of raising the error? The 'most pythonic' way is to ask for forgiveness than permission EAFP. In my opinion, what should happen here is that the function should raise an exception when something bad happens or continue when everything is okay. This would help you because you wouldn't need to handle errors separately.

Try this in persist_dump:

try:
    send_to_be(load_be_data(f))
    sleep(300)
except (some exception) as ex:
    print("Exception thrown in ...")
    remove(f)
    sleep(1)

Both "insert" and "update" should be a global variables (so you can possibly reuse them somewhere else).
If create_new_component and update_component are not calculated inside the method, it would be good to make them global variables.

persist_dump

I don't think that it is a good idea to call persist_dump recursively. Assuming that you script will run for a while you are using more memory than you need to. Additionally, at some point in the future you might get RuntimeError: maximum recursion depth exceeded.


Making an atomic transaction out of several operations I'm looking for a design pattern or suggestions that can help refactor my code into something a bit less repetitive. I have a method that has several sequential steps (10-15) that if any of them fail must record detail about the failure to a log and rollback all previous portions of the transaction. The example is below.

I have thought about taking each try catch and turning it into an individual method but then I have to pass around my undo stack (among several other variables) and I still have to conditionally abort the method and process the undo stack. I've briefly looked into the Momento and Command patterns but both seemed to grow the line count by a fair margin.

EnumResult TransactionThatRollsBack()
{
        var undoLog = new Stack<Action>();
        try
        {
            MoveInventory(inv, src, dest);
            undoLog.Push(() => MoveInventory(inv,dest,src));
        }
        catch (Exception ex)
        {
            RecordError("InventoryError", ex, inv, src, dest);
            Undo(undoLog);
            return EnumResult.FailedInventoryMove;
        }

        try
        {
            NotifyReportingOfMove(inv, user);
            undoLog.Push(() => NotifyReportingOfUnMove(inv, user));
        }
        catch (Exception ex)
        {
            RecordError("NotifyReportingOfMove", ex, inv, user);
            Undo(undoLog);
            return EnumResult.FailedReportingMove;
        }

        try
        {
            AddUserToSweepstakes(user);
            undoLog.Push(() => RemoevUserFromSweepstakes(user));

        }
        catch (Exception ex)
        {
            RecordError("AddUserToSweepstakes", ex, user);
            Undo(undoLog);
            return EnumResult.FailedSweepstakesAdd;
        }

        ...
        return EnumResult.Success
}

You can remove repeated code if you record all your actions for executions and then execute them all at once with some generic exception handling. The idea is below.

Sorry for some possible lapses, didn't write in C# for quite some time. However I hope that you'll find the whole idea to be helpful.

  EnumResult TransactionThatRollsBack()
    {
         var undoLog = new SmartStack();
         undoLog.Push(() => MoveInventory(inv,dest,src), EnumResult.1);
         undoLog.Push(() => NotifyReportingOfUnMove(inv, user), EnumResult.2);
         undoLog.Push(() => RemoevUserFromSweepstakes(user), EnumResult.3);

         undoLog.execute();
    }

    class SmartStack {
       SortedDictionary<Action, EnumResult> actions;

       void push(Action a, EnumResult r) {
         actions.add(a, r);
       }

       void execute() {
         Stack<Action> completedActions = new Stack<Action>();
         foreach(KeyValuePair<Action, EnumResult> pair in actions) {
             try {
               pair.Key.Invoke();
               completedActions.push(pair.Key);
             }
             catch(Exception ex) {
               for (Action a in completedActions) {
                   Undo(a);
               }
               return pair.Value;
             }
         }
       }
    }


Quadratic equation solver in Ruby I wrote a simple quadratic equation solver with Ruby to help me solve quadratic equations. Please tell me what I did right or wrong, and what I can do to improve it.

#!/usr/bin/ruby

if ARGV.length != 3
        STDERR << "Usage: #{$0} <a> <b> <c>\n"
        exit 1
end

a = ARGV[0].to_i
b = ARGV[1].to_i
c = ARGV[2].to_i

disc = b**2 - 4*a*c
d_str = (disc >= 0) ? (Math.sqrt(disc) % 1 == 0) ? Math.sqrt(disc).to_i : "√#{disc}" : "√#{disc}"

s = "#{-b} ± #{d_str}"
d = "#{2*a}"
puts "\e[4m#{s}\e[0m"
#puts "─"*(s.length)
puts d.center s.length

What you did right is solve the problem.
What you did "wrong" really depends on a number of factors, so I'll respond with what you can do to improve it, with an example that implements those improvements.  Note, many improvements will make the code longer.
Improvements

Use the shebang #!/usr/bin/env ruby.  It allows the environment to have ruby in a different directory than /usr/bin/ruby.
Use $stderr.puts "Usage: #{$0} <a> <b> <c>".  $stderr is a global variable and can be changed.  It allows standard error to be redirected, say to a file or a logger.  It would be restored by $stderr = STDERR.
Ruby indentation standard is 2 spaces and no hard tabs.
Use disc_str to indicate it is a String version of disc.
Use disc_sqrt_str to indicate it is a String version of disc_sqrt.
Use a variable to reduce wasted calculations; sqrt is not a simple calculation.
Nested ? operators can be hard to read and can lead to mistakes.
Use numerator and denominator to convey intention, instead of s and d.
Allow for the case when discriminant is zero.
Use parentheses for arguments when method calls are arguments.  Thus instead of f g h x do f g(h(x)) or even better f(g(h(x))).
Use discriminant because it is a Mathematics domain term.  A non-mathematics literate developer will be blocked from maintaining it because Googling "disc" won't help them.
Eliminate common factors.
Eliminate fraction if denominator is 1.
Define methods for greater flexibility.
Include some tests to test each pathway.
Use appropriate comments and self-commenting code by using good names.
Use Rubocop to provide similar automated improvements.
Use the colorize gem in an optional way, since it is not actually necessary.
Check if it is available by using ''.respond_to?(:underline), which will mean that it is more flexible because another method could be added to String which might do a similar thing or even something rather different; CSS, LaTeX, ASCII, ... .
Use Unicode codes instead of the characters directly.  Some editors may not display Unicode characters properly and could change the characters to something unexpected.

#!/usr/bin/env ruby
#
#   quadratic   - Output quadratic solutions as a fraction
#
#     Luis Esteban    9 August 2020
#       review of code of Dmitry Kudriavtsev

# Output rational solutions to the quadratic equation
#   For a.x^2 + b.x + c = 0
#
#   x = (-b ± √d) / 2a
#   d = b^2 - 4.a.c       (discriminant)
#

begin
  require 'colorize'
rescue LoadError
end

ALLOW_IMAGINARY = true

def solve_quadratic(a, b, c)
  discriminant = b**2 - 4*a*c
  denominator  = 2 * a

  if discriminant > 0
    discriminant_sqrt = Math.sqrt(discriminant)
    if discriminant_sqrt % 1 == 0
      b, discriminant, denominator = simplify(b, discriminant_sqrt.to_i, denominator)
      numerator = [-b, " \u00b1 ", discriminant]
    else
      b, discriminant, denominator = simplify_with_sqrt(b, discriminant, denominator)
      numerator = [-b, " \u00b1 \u221a", discriminant]
    end
  elsif discriminant == 0
    solution    = Rational(-b, denominator)
    numerator   = [solution.numerator]
    denominator = solution.denominator
  else
    if ALLOW_IMAGINARY
      discriminant_sqrt = Math.sqrt(-discriminant)
      discriminant_sqrt = discriminant_sqrt.to_i if discriminant_sqrt % 1 == 0
      if discriminant_sqrt % 1 == 0
        b, discriminant, denominator = simplify(b, discriminant_sqrt.to_i, denominator)
        numerator = [-b, " \u00b1 i \u2a2f ", discriminant]
      else
        b, discriminant, denominator = simplify_with_sqrt(b, discriminant, denominator)
        numerator = [-b, " \u00b1 i \u2a2f \u221a", -discriminant]
      end
    else
      numerator = []
    end
  end

  [numerator, denominator]
end

def simplify(b, discriminant, denominator)
  gcd = [b, discriminant, denominator].inject(&:gcd)
  gcd = -gcd unless denominator.positive?

  [b, discriminant, denominator].map{|n| n / gcd }
end

def simplify_with_sqrt(b, discriminant, denominator)
  gcd = [b**2, discriminant, denominator**2].inject(&:gcd)
  gcd_sqrt = Math.sqrt(gcd).round
  if denominator.negative?
    gcd      = -gcd
    gcd_sqrt = -gcd_sqrt
  end

  [
    b            / gcd_sqrt,
    discriminant / gcd,
    denominator  / gcd_sqrt
  ]
end

def display_fraction(numerator, denominator)
  size        = numerator.size
  numerator   = numerator.join
  denominator = denominator.to_s
  width       = [numerator, denominator].map(&:length).max + 2

  if size > 0
    if denominator != "1"
      if ''.respond_to?(:underline)
        puts "#{numerator}".center(width).underline
        puts denominator.center(width)
      else
        puts numerator.center(width)
        puts "─" * width
        puts denominator.center(width)
      end
    else
      puts numerator
    end
  else
    puts "No solutions"
  end
end

if ARGV.length != 3
  $stderr.puts "Usage: #{$0} <a> <b> <c>"
  exit 1
else
  a, b, c = ARGV.map(&:to_i)
  puts "solution(s) to #{a}x\u00b2 + #{b}x + #{c} = 0"

  display_fraction(*solve_quadratic(a,b,c))
end

# Testing
#  [
#    [1, 1, -12],    # Integer discriminant
#    [-1, 1, 12],    # Make denominator positive
#    [1, 2, 0],      # Don't show denominator if 1
#    [4, 4, 1],      # Don't show discriminant if 0
#    [1, 2, -17],    # Don't show denominator if 1 with surd
#    [2, 4, 2],      # Don't show denominator if 1 and  discriminant if 0
#    [1, 1, -13],    # Show surd
#    [1, 2, 17],     # Imaginary solution with integer discriminant no denominator
#    [1, 2, 16],     # Imaginary solution with correct cancellation of surd
#    [2, 0, 2],      # Imaginary solution with surd, hide denominator
#    [2, 8, 16],     # Imaginary solution with integer discriminant
#  ].each do |a,b,c|
#    puts "For: a = #{a.inspect}, b = #{b.inspect}, c = #{c.inspect}"
#    puts
#    display_fraction(*solve_quadratic(a,b,c))
#    puts
#  end


A concurrency-safe stack structure This is a teaching sample intended to illustrate use of mutability and list consing in F#.  The structure is intended to be a thread-safe stack structure, which multiple threads can push to/pop from concurrently.  Concerns:

Will this actually be thread safe?
Are the locks necessary?
Is there an immutable way to do the same thing?  I don't think one can use the traditional immutable method here of having the Push/Pop return a new stack instance because different clients will see divergent views of the stack.

The code looks like this:

type ConcurrentStack<'T>() =
    let mutable _stack : List<'T> = []

    member this.Push value =
      lock _stack (fun () ->
         _stack <- value :: _stack)

    member this.TryPop() =
      lock _stack (fun () ->
         match _stack with
         | result :: newStack ->
            _stack <- newStack
            result |> Some
         | [] -> None
      )

Will this actually be thread safe?

Yes, I think it will.  All access to the _stack field is protected by a lock, so I do not see why this wouldn't be thread safe.

A nice coincidence of using immutable lists is that you can also nicely implement IEnumerable<'T> or even just return the list of all elements currently in the stack - because the data type is immutable, it can be safely shared with other threads (this means, you won't get into the usual troubles with "Collection was modified; enumeration operation cannot continue" that you'd get in C#.

  Are the locks necessary?

I think so. Without the lock, one thread could start evaluating Push by evaluating value::_stack, then another thread could modify _stack and the original thread would then overwrite _stack with the old value of value::_stack (and so the effect of the other thread would be lost!)

  Is there an immutable way to do the same thing? I don't think one can use the traditional immutable method here of having the Push/Pop return a new stack instance because different clients will see divergent views of the stack.

If the idea is to have multiple threads sharing the state, then they'll need some way of sharing & mutating the state. Another approach in F# would be to use an agent and keep the current _stack as a parameter of the asynchronous loop - in that case, you're not using mutation in the F# code you write (but there is hidden mutation and synchronization going on in the agent).

Logically, the agent-based version is quite similar to the one you have using locks - it might be a bit nicer for other reasons (getting locks right is hard, agents do not make it possible to do the same kinds of errors), but that really depends on what you want to illustrate!