Three very similar functions operating on XML files I have 3 functions which are annoyingly similar, but I can't figure out a way to refactor them without making the code unreadable.

The three functions deal with xml files (.resx), the first function reads all files in a directory and "collects" all strings inside a data.value tag, adds them to a set and returns that set, for example:

<data name="AccessibilityTitleResource1.Text" xml:space="preserve">
<value>Pages Accessibility</value>

The second function reads these same files, and swaps the values read previously with a translation which is inside a dict called definicoes (definicoes[english] = translation), with the previous example, then saves the file. So from our previous example definicoes["Pages Accessibility"] = "Acessibilidade de Páginas". The xml file would become:

<data name="AccessibilityTitleResource1.Text" xml:space="preserve">
<value>Acessibilidade de Páginas</value>

The third function simply checks if the xml file is fully translatable, so it checks all the values <value> tags inside the <data> ones against the ones in the dict, if there is any value not present on the dict it returns False.

Here are the three functions:

def ler_todos(caminho) -> set:
    definicoes = set()
    for file in os.listdir(caminho):
        if file.endswith(".ascx.resx") or file.endswith(".aspx.resx"):
            tree = ET.parse(caminho + file)
            root = tree.getroot()
            for child in root.iter('data'):
                if child.attrib.get('namespace') == 'preserve':
                    node = child[0]
                    if node.tag == "value" and node.text != None and not node.text in definicoes:
                        definicoes.add(node.text)
    return definicoes

def escrever_arquivo(definicoes, caminho):
    if caminho.endswith(".pt.resx"):
        tree = ET.parse(caminho)
        root = tree.getroot()
        for child in root.iter('data'):
            if child.attrib.get('namespace') == 'preserve':
                node = child[0]
                if node.tag == "value" and node.text != None and node.text in definicoes:
                    node.text = definicoes[node.text]
        tree.write(caminho, encoding="UTF-8")
        anexar_cabecalho(caminho)

def existe_traducao(definicoes, caminho):
    if caminho.endswith(".resx"):
        tree = ET.parse(caminho)
        root = tree.getroot()
        for child in root.iter('data'):
            if child.attrib.get('namespace') == 'preserve':
                node = child[0]
                if node.tag == "value" and node.text != None and len(node.text) > 0 and node.text not in definicoes:
                    return False
        return True

One of the possible ways to improve on following the DRY principle would be to extract the XML tree iteration part which is the same in all 3 functions. Something like this generator:

def values(tree):
    root = tree.getroot()
    for child in root.iter('data'):
        if child.attrib.get('namespace') == 'preserve':
            node = child[0]
            if node.tag == "value" and node.text is not None:
                yield node

Then, you can use it this way:

tree = ET.parse(caminho)
for node in values(tree):
    # do something with node


Object instantiation with multiple parameters I'm trying to create a little block presenter for Laravel which displays blocks of content in certain positions. I have 2 classes, BlockFactory and Block. The BlockFactory contains all the blocks and creates them whereas the Block class is exactly just that: a simple block.

As can be seen in the code below, when I'm creating a new Block from within the BlockFactory, I have to pass all these parameters to the create method. Which then get passed onto the instantiation of the Block class and finally passed onto the constructor.

This seems very unwieldy to me. Any ideas for a smarter way of doing this?

Also: not sure if it's smart to create a new Block object inside create method. I was trying to think in terms of dependency injection but I couldn't figure it out.

// create and save new block
Block::create('name', 'content', 'position', 0, [])->add();

// BlockFactory, bound to the Block facade (bad naming i know)
class BlockFactory
{

    protected $blocks = [];
    protected $currentBlock;

    public function create($title, $content, $position, $order = 0, $parameters = [])
    {
        $this->currentBlock = new Block($title, $content, $position, $order, $parameters); // this part is bugging me somehow...
        return $this;
    }

    public function add()
    {
        $this->blocks[] = $this->currentBlock;
    }

    public function render()
    {
        return $this->blocks;
    }

}

// The actual block class
class Block
{

    protected $title;
    protected $content;
    protected $position;
    protected $order;
    protected $parameters;

    public function __construct($name, $content, $position, $order = 0, $parameters = [])
    {
        $this->title = $name;
        $this->content = $content;
        $this->position = $position;
        $this->order = $order;
        $this->parameters = $parameters;
    }
}

The problem might be the semantic reference you are using as a pattern. You are creating a class as a "factory", but the actual Factory patter is related only to creating objects, and it seems that what you mean is actually the "Repository" pattern, where you can created, manage and retrieve objects.

And your case you would create a BlockRepository that it would be injected in the controller to be the interface between the controller and the actual blocks you need to create and retieve.

You can find related references on the links below:

http://heera.it/laravel-repository-pattern
https://stackoverflow.com/questions/21805939/laravel-using-the-repository-pattern


Trello list scraper that extracts all numbers from boards and logs the sum I wrote a script that pulls all the cards in my food expenses list that are separated by months. I then extract all the numbers and log the sum to a file. Later I plan on graphing my monthly expenses and the names.

#!/usr/bin/python

from trollop import TrelloConnection
import logging
# Lots of issues with Python3. Lots of unicode, string errors, Just switched to
# py2. should try to use dicts for {name: cost} and to  practice using dicts

# TODO: Data Visualization, Error/exception handling, appending to log for running total
# TODO: clean up code, get feedback on reddit, maybe use args for spec params
# TODO: set up cron job to run every month

api_key = '####' # TRELLO_API_KEY
token = '####'
# idBoard': '577b17583e5d17ee55b20e44',
# idList': '577b17583e5d17ee55b20e45',
# Set up basic logging
logging.basicConfig(format='%(levelname)s %(message)s',
    level=logging.INFO, filename='trello_expenses.log', filemode='w')

# Establish connection
conn = TrelloConnection(api_key, token)
months = ['January', 'February', 'July', 'August', \
          'September','October','November', 'December']
# Get expenses board via ID

logging.info("Iterating through boards...")

# for board in conn.me.boards:
#     # logging.info("id: {}".format(each._id))
#     # print u"board name: {0}, board id {1}" . format( board.name, board._id )
#     logging.info (u"board name: {0}, board id {1}".\
#                     format( board.name, board._id ))

# for each_card in conn.me.boards[2].cards:
#     print u"{0},{1},{2}".format(each_card._id, each_card.url,each_card.desc)

def main():
    total = 0.0
    costs = list()
    names = list()

    # get first index i.e the list im currently working on
    board = conn.get_board('BE89pW61')
    board_lists = board.lists
    current = board_lists[0]
    # Get all lists  names and ids in board
    for entry in board_lists:

        if 'august' in entry.name.lower():
            # print u'List name: {0} list id: {1}'.format (entry.name, entry._id)

            for cards in entry.cards:
                name, cost =  cards.name.split('-')
                costs.append(float (cost) )
                # print cards.name
            # print costs
    total = sum (costs)
    print 'total is {}'.format( total )

if __name__=='__main__':
    main()

It is good to see that you are using the if __name__=='__main__': guard. However, I think you are doing a bit too much in your main and I would make a get_total_month function like this:

...
def get_total_month(month, board_lists):
    costs = 0
    month = month.lower()
    for entry in board_lists:
        if month in entry.name.lower():
            for card in entry.cards:
                costs += float(card.name.split('-')[1])
    return costs

def main():
    board = conn.get_board('BE89pW61')
    total = get_total_month('august', board.lists)
    print 'total is {}'.format(total)

This way it will be easier to incorporate for example something like:

totals_per month = [get_total_month(month, boards.lists) for month in months]

which is then easy to plot e.g. with matplotlib:

import matplotlib.pyplot as plt
...

totals = [get_total_month(month, boards.lists) for month in months]
months = range(len(months))

plt.plot(months, totals)
plt.show()

Adding the actual names of the month (and maybe also their year) is a bit more involved, but you can have a look at the examples here and here.

Regarding whitespace, try to stick to one convention, preferentially the one set by pythons offical style-guie, PEP8. This means in particular not writing sum (total) or float( entry ), but sum(total) and float(entry).


HttpClient Wrapper supports ignoring invalid/self-signed certs I'm using the following as a wrapper for the HtptClient because it doesn't implement an interface -- which makes mocking more difficult. This logic also supports invalid and self-signed certs.

public class HttpClientWrapper : IHttpClient
{
    private readonly HttpClient _client;

    public Uri BaseAddress
    {
        get
        {
            return _client.BaseAddress;
        }

        set
        {
            _client.BaseAddress = value;
        }
    }

    public HttpRequestHeaders DefaultRequestHeaders
    {
        get
        {
            return _client.DefaultRequestHeaders;
        }
    }

    public HttpClientWrapper()
    {
        bool ignoreCertificateErrors = ConfigurationManager.AppSettings["IgnoreCertificateErrors"].ToBool();
        string environmentName = ConfigurationManager.AppSettings["EnvironmentName"];

        _client = new HttpClient();

        // If ignoreCertificateErrors config value is true,
        // the setting ignores all Certificate validation errors in AppDomain,
        // in every environment but PRODUCTION.
        if (ignoreCertErrors && environmentName.ToUpper() != "PRODUCTION")
        {
            ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };
        }
    }

    public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, String userOrProcessName)
    {
        IUnityContainer container = UnityCommon.GetContainer();
        ILogService logService = container.Resolve<ILogService>();

        logService.Log(ApplicationLogTypes.Debug, JsonConvert.SerializeObject(request), userOrProcessName);

        return _client.SendAsync(request);
    }

    #region IDisposable Support
    private bool disposedValue = false; // To detect redundant calls

    protected virtual void Dispose(bool disposing)
    {
        if (!disposedValue)
        {
            if (disposing && _client != null)
            {
                _client.Dispose();
            }

            disposedValue = true;
        }
    }

    public void Dispose()
    {
        Dispose(true);
    }
    #endregion
}

using the following for interface:

public interface IHttpClient : IDisposable
{
    Uri BaseAddress { get; set; }

    HttpRequestHeaders DefaultRequestHeaders { get; }

    Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, String userOrProcessName);
}

Any suggestions?

bool ignoreCertificateErrors = ConfigurationManager.AppSettings["IgnoreCertificateErrors"].ToBool();
string environmentName = ConfigurationManager.AppSettings["EnvironmentName"];

These two lines I think shouldn't be there because with them, the wrapper is tigthly coupled to the app.config.

I see three possibilities how you can decouple the wrapper:

make both values constructor parameters if they shouldn't change
make them properties if they can
use the UnityCommon service locator that you use in the SendAsync method and let it resolve some ConfigProvider from which you can get both settings.

environmentName.ToUpper() != "PRODUCTION"

Usually in situations like this it's safer to use a  case insensitive comparison

environmentName.Equals("PRODUCTION", StringComparison.OrdinalIgnoreCase)


Calculate probability of word occurence I am pretty new in Python and I am not sure if I did everything right in my program. May someone to check it, please. Thank you!

First of all, I have a text file, for example, abc.txt.
I have to create a dictionary and for this, I have to split the
sentences into a list of words and convert each word to lowercase.
Then I have to assign a counter to each word in my dictionary and
increment it every time when I meet this word in sentence one more
time.
Finally, I have to divide the word count by sum of all counts (prob = count(word) / sum(count of all words in dict) and write this probability into new file called x.txt.

Here is what I got so far:

import sys
filename = 'abc.txt'
new_dict ={}
total_count = 0
with open(filename, "r") as fp:
    for line in fp:
        words = line.split()

        for word in words:
            word = word.lower()

        if not word in new_dict:
             new_dict[word] = 1
        else:
            new_dict[word] +=1
            total_count +=1
probab = new_dict[word]/ total_count
print(probab)
fp.close()
output_file = 'x.txt'
with open(output_file, "w") as fs:
           fs.write(str(probab))
fs.close()

and my output is 0.0006865437426441742

I apologize for updating my answer multiple times. I shouldn't upload my answer when I'm tired.
I'm going to ignore the running time complexity to avoid confusing you.
If you want to know, this will run in O(n^2) time. This can be simplified to O(n log n) time using a tree, but using and explaining a tree might confuse a beginner.
The simplest way to do and undertand this is as follows:
    # you don't need "import os" in this case.
    new_dict = {}

    # This is to open the file to get the count of all words:
    filename = 'abc.txt'
    with open(filename, "r") as fp:
        for line in fp:
            # For this to work, make sure the words don't end with any punctuation.
            # If any words end with a punctuation, take a look into re.sub()
            words = line.split()

            for word in words:
                word = word.lower()

                if word not in new_dict:
                    new_dict[word] = 1
                else:
                    new_dict[word] += 1

    # This is to calculate the count of all words:
    total_words = sum(new_dict.values())

    # This is used to get the probability of each word.
    output_file = 'x.txt'
    with open(output_file, "w") as fs:
        # The dictionary is set as: {dictionary_name[key] : value}
        for key, value in sorted(new_dict.items()):
            probability = value / total_words
            fs.write(key + ": " + str(probability) + "\n")

Using for x_variable in collection_variable

When you are using for x_variable in collection_variable, you need to make sure any code using the x_variable resides inside of the for each loop. In this case, I pushed anything that uses word to make sure the word variable is accessible because you are calling it from inside the for word in words iterator.

When to call file.close()

When opening files with the with open(...) function, you don't need to explicitly close it. You can just leave the with open code block and the garbage collector will know you are done with that file. However, if you do open(...) without the with operator, then yeah, you need to call fs.close()

How sorted(variable) works

With simple data types like string, int, float, tuple, list, dictionary, they already include comparison functions, so you can use sorted(variable) to sort them. However, if you have your own data type or object, you need to define the comparison functions for the sorted() to work. Read more about sorted(variable) on Python docs
Hope this helps :)


Listings template for on-line auctions I am working on an HTML / JavaScript listings template for online auctions.  There are two sets of tabbed navigation features in the listing that are driven by JavaScript as well as a dynamic image gallery. Everything appears to be functioning very nicely. The template is fiddled here. (complete project repository here). However, I have one BIG issue I need help resolving and a couple of smaller items I'd like feedback on.

BIG Issue:

This is the showstopper.  I am NOT a JavaScript programmer yet, so I don't know how involved of a question this is. Because the template will be used on Ebay, I am restricted to what can be done with JavaScript.  The tabs module that I am using (which I forked from here) uses the replace() function in two places:

removeClass: function (element, className) {
    element.className = element.className.replace(new RegExp("(^|\\s)" + className + "(\\s|$)"), "$1");
    if (element.className === " ") {
        element.className = "";
    }
},

and here:

getStyle: function (element, property) {
    if (element.style[property]) {
        return element.style[property];
    }
    if (element.currentStyle) {    // IE.
        return element.currentStyle[property];
    }
    property = property.replace(/([A-Z])/g, "-$1").toLowerCase();    // Turns propertyName into property-name.
    var style = document.defaultView.getComputedStyle(element, "");
    if (style) {
        return style.getPropertyValue(property);
    }
},

Unfortunately, the replace() function is one of the Ebay BANNED JavaScript functions.  Is there a replacement or work-around for the replace()?  I did find a couple of ideas over on SO here, but I am not sure how to implement them.

Questions:

I feel like the CSS is a mess.  Is there a tool available that could help streamline CSS and help identify redundant, useless or otherwise inefficient CSS (besides good old fashioned experience, which I am working on)? For instance, I find myself putting a margin:0; or padding:0 quite often.
Cross browser compatibility.  I am making use of the border-radius style in a couple of places.  In the CSS, I have added the prefixes -moz and -webkit for these features.  However, looking over at caniuse.com, it looks like support for this feature is pretty high (over 90%).  Is it necessary to include these prefixes in the CSS?
The image gallery is using the onclick() function to pass name attributess to the preview area whenever a thumbnail is clicked.  Because the name attribute is obsolete, this method is preventing the file from validating to HTML 5.  Is there a different way this could be done using id attributes instead? I tried simply replacing name with id but that didn't work. Alternatively, I do have a script I can use, but it also requires solving the BIG issue above.

Big issue: you can create your own replace function as something like a recursive array iterator. It should be quite easy, but there might be some performance drawbacks if used on long strings, but the code above is not the case. Also you probably better off drop the regexp, unless search() function is allowed.
As of css there are reset stylesheets/boilerplates to start with and there are also preprocessors like sass or less, to help you create a more structured stylesheet.
Cross browser compatibility is always depending on your target audience, but generally speaking it's enough if you support the last 2 version of all major browsers and try to keep it at least "minimally functional" for the rest if possible.


Hello, First Name Last Name I've been working on learning React, and so like most people do when they get started on something new. I created an arbitrary simple project to get a bit more familiar with the concepts.

It's so simple that it shouldn't really require any explanation. Here's the React code.

var TestInput = React.createClass({
    propTypes: {
        id: React.PropTypes.string.isRequired,
        label: React.PropTypes.string.isRequired,
        placeholder: React.PropTypes.string
    },
    getDefaultProps: function() {
        return {
            type: 'text'
        }
    },
    render: function() {

        var {label, id} = this.props;
        var other = _.omit(this.props, 'label');

        return (

                {label}
                <input className="form-control" {...other}/>

        );
    }
});

var TestOutput = React.createClass({
    propTypes: {
        name: React.PropTypes.string.isRequired
    },
    render: function() {
        return (

Hello {this.props.name}!

        );
    }
});

var TestBox = React.createClass({
    getInitialState: function() {
        return {
            firstName: '',
            lastName: ''
        }
    },
    handleInput: function(event) {
        var newState = {};
        newState[event.target.id] = event.target.value;
        this.setState(newState);
    },
    createName: function() {
        var nameParts = [];

        if (this.state.firstName.length == 0 && this.state.lastName.length != 0) {
            nameParts.push('Mr.');
        }
        if (this.state.firstName.length != 0) {
            nameParts.push(this.state.firstName);
        }
        if (this.state.lastName.length != 0) {
            nameParts.push(this.state.lastName);
        }
        if (nameParts.length == 0) {
            nameParts.push('Anonymous');
        }

        return nameParts.join(' ');
    },
    render: function() {
        return (

                <TestInput id="firstName" label="First Name" placeholder="John" onKeyUp={this.handleInput}/>
                <TestInput id="lastName" label="Last Name" placeholder="Smith" onKeyUp={this.handleInput}/>
                <TestOutput name={this.createName()} />

        );
    }
});

React.render(
    <TestBox/>,
    document.getElementById('container')
);

It works, but it seems like I had to do way too much to get it to work. Not that it was difficult to write, but it just seems like a lot of data transfer and boilerplate to do very little. Maybe this is just because my example is simple and contrived? I'm just thinking that with jQuery I could write a single event handler which works on both input's keyUp and update the H3's text based on the current values.

What am I doing wrong? Or am I doing it right?

React has a small amount of boilerplate you need to repeat in order to keep your components well documented, thus the usage of propTypes which is one of great things to validate data.

There is nothing wrong with how your components are structured, as react "ways" were followed.

Only thing I can observe and which can be improved is to use strict comparisons, eg: === instead of == and so on. Why?

  The identity operator returns true if the operands are strictly equal (see above) with no type conversion.

More information at MDN

As a bonus if your project uses Babel then you can use const and let for defining variables.

Also you could transform:

var {label, id} = this.props;
var other = _.omit(this.props, 'label');

To:

const {label, id, ...other} = this.props;

You can notice that we have eliminated the use of lodash/underscore.

You can find more about Babel at their homepage.


Read a number with commas and decimal dots not using "builtin" support I recently had a job interview and messed up on writing an algorithm which, realistically I wouldn't have any trouble with if I wasn't under pressure. I was annoyed and wrote the solution up when I got home in like 5 minutes. My conclusion as to why this happened: I am a bit rusty at writing algorithms so drawing a blank when pressure is applied, therefore I am practicing several questions to get my mind ticking again.

I ran into this question;

"Write a program that reads a number with commas and decimal dots (such as “23,419.34”) and then prints a
number that is half of it. Do not use Double.parseDouble()."

Constraints:

Integer.parseInt only (Assumed)
No use of Arrays/Lists etc.

Here is my solution;

private static void textToNum() {
    System.out.println("Enter Number nnn,xxx,yyy.zz");
    String text = sc.nextLine();

    double res = 0.0;
    StringBuilder bd = new StringBuilder();

    for (int i = 0; i < text.length(); i++) {
        if (Character.isDigit(text.charAt(i))) {
            System.out.println("CA: " + text.charAt(i));
            bd.append(text.substring(i, i+1));
            if (i == text.length() - 1) {
                double d = Math.pow(10, bd.length());
                double temp = Integer.parseInt(bd.toString()) / d;
                res += (temp / 2);
            }
        }
        else if (text.charAt(i) == '.') {
            res = Integer.parseInt(bd.toString()) / 2.0;
            System.out.println("1Res: " + res);
            bd = new StringBuilder();
        }
    }
    System.out.println("Res: " + res);
}

While this solves the problem, I couldn't help thinking there must be a cleaner solution that I was missing.

Any suggestions?

Was intended as a joke, but should actually work for very large numbers because it only looks at the digits one by one. It uses some knowledge about digit character code points (48..58) O:)

int carry = 0;
boolean dot = false;
for (int i = 0; i < text.length(); i++) {
  char c = text.charAt(i);
  if (c >= '0' && c <= '9') {
    System.out.print((char) (c / 2 + 24 + carry))
    carry = (c & 1) * 5;
  } else {
    System.out.print(c);
    dot |= c == '.';
  }
}
if (carry != 0) {
  System.out.print(dot ? "5" : ".5");
}

More reasonable (one could do the division inline here, too) -- and no library calls at all (except for printing the result).

double result = 0;
double decimal = 0;
for (int i = 0; i < text.length(); i++) {
  char c = text.charAt(i);
  if (c >= '0' && c <= '9') {
    if (decimal != 0) {
      result += (c - '0') * decimal;
      decimal /= 10;
    } else {
      result = result * 10 + (c - '0');
    }
  } else if (c == '.') {
    decimal = 0.1;
  }
}
System.out.println(result / 2);


Better way to set number of threads used by NumPy Background

When NumPy is linked against multithreaded implementations of BLAS (like MKL or OpenBLAS), the computationally intensive parts of a program run on multiple cores (sometimes all cores) automatically.

This is bad when:

you are sharing resources
you know of a better way to parallelize your program.

In these cases it is reasonable to restrict the number of threads used by MKL/OpenBLAS to 1, and parallelize your program manually.

My solution below involves loading the libraries at runtime and calling the corresponding C functions from Python.

Questions

Are there any best/better practices in solving this problem?
What are the pitfalls of my approach?
Please comment on code quality in general.

Example of use

import numpy

# this uses however many threads MKL/OpenBLAS uses
result = numpy.linalg.svd(matrix)

# this uses one thread
with single_threaded(numpy):
    result = numpy.linalg.svd(matrix)

Implementation

Imports and definitions

import subprocess
import re
import sys
import os
import glob
import warnings
import ctypes

MKL = 'mkl'
OPENBLAS = 'openblas'

Class BLAS, abstracting a BLAS library with methods to get and set the number of threads:

class BLAS:
    def __init__(self, cdll, kind):

        if kind not in (MKL, OPENBLAS):
            raise ValueError(f'kind must be {MKL} or {OPENBLAS}, got {kind} instead.')

        self.kind = kind
        self.cdll = cdll

        if kind == MKL:
            self.get_n_threads = cdll.MKL_Get_Max_Threads
            self.set_n_threads = cdll.MKL_Set_Num_Threads
        else:
            self.get_n_threads = cdll.openblas_get_num_threads
            self.set_n_threads = cdll.openblas_set_num_threads

Function get_blas, returning a BLAS object given an imported NumPy module.

def get_blas(numpy_module):

    LDD = 'ldd'
    LDD_PATTERN = r'^\t(?P<lib>.*{}.*) => (?P<path>.*) \(0x.*$'

    NUMPY_PATH = os.path.join(numpy_module.__path__[0], 'core')
    MULTIARRAY_PATH = glob.glob(os.path.join(NUMPY_PATH, 'multiarray.*so'))[0]

    ldd_result = subprocess.run(
        args=[LDD, MULTIARRAY_PATH],
        check=True,
        stdout=subprocess.PIPE,
        universal_newlines=True
    )

    output = ldd_result.stdout

    if MKL in output:
        kind = MKL
    elif OPENBLAS in output:
        kind = OPENBLAS
    else:
        return None

    pattern = LDD_PATTERN.format(kind)
    match = re.search(pattern, output, flags=re.MULTILINE)

    if match:
        lib = ctypes.CDLL(match.groupdict()['path'])
        return BLAS(lib, kind)
    else:
        return None

Context manager single_threaded, that takes an imported NumPy module, sets number of threads to 1 on enter, resets to previous value on exit.

class single_threaded:
    def __init__(self, numpy_module):
        self.blas = get_blas(numpy_module)

    def __enter__(self):
        if self.blas is not None:
            self.old_n_threads = self.blas.get_n_threads()
            self.blas.set_n_threads(1)
        else:
            warnings.warn(
                'No MKL/OpenBLAS found, assuming NumPy is single-threaded.'
            )

    def __exit__(self, *args):
        if self.blas is not None:
            self.blas.set_n_threads(self.old_n_threads)
            if self.blas.get_n_threads() != self.old_n_threads:
                message = (
                    f'Failed to reset {self.blas.kind} '
                    f'to {self.old_n_threads} threads (previous value).'
                )
                raise RuntimeError(message)

What are the pitfalls of my approach?

Calling the functions setting the maximum number of threads to use change a global state inside the respective libraries which makes the decorator not thread safe. When you use the numpy functions from several different threads, the threads not using your decorator might also get a single threaded implementation called.

AFAIK there is no good way to solve this. OpenBLAS FAQ even states that you should disable threads in OpenBLAS if your application uses threads itself.


Initialize C array at definition depending on runtime input Best way to initialize rr and cc depending on cycle number to pre-defined values in r and c.

int func(int cycle, int index, int i) {
  char *rr;
  char *cc;

  if (cycle % 2 == 0) {
      char r[4] = { 0, -1, 0, 1 };
      char c[4] = { 1, 0, -1, 0 };
      rr = r;
      cc = c;
  }
  else {
      char r[4] = { -1, -1, 1, 1 };
      char c[4] = { 1, -1, -1, 1 };
      rr = r;
      cc = c;
  }

  return(index + rr[i] + cc[i]);
}

This code is not correct. The r[4] and c[4] arrays you have are stored on the stack, which means that as soon as they go out of scope the data is no longer valid and you get undefined behavior because rr and cc now point to unused (and possibly overwritten) areas on the stack.

If you wanted to do this correctly, you could ensure that the arrays are kept alive either by storing on the heap, static memory, or high enough on the stack that they will live as long as rr and cc are used.

After reading it again, I realized I made a mistake. While the temporary r and c arrays are initialized in their own scopes that are immediately exited, the arrays themselves live on the same stack with the rest of the function. Still, it's definitely a bad habit to have use variables after they've left the scope even if they're technically still on the stack, so I would make the same changes anyway.

For example, you could do:

// Somewhere in a high enough scope (possibly the current one) or static
char r_1[4] = { 0, -1, 0, 1 };
char c_1[4] = { 1, 0, -1, 0 };

char r_2[4] = { -1, -1, 1, 1 };
char c_2[4] = { 1, -1, -1, 1 };

// In the current scope
char *rr;
char *cc;

if (cycle % 2 == 0) {
  rr = r_1;
  cc = c_1;
}
else {
  rr = r_2;
  cc = c_2;
}


Pair elements of two lists by condition without reusing elements I have two lists that I want to join on a condition. Unlike in a relational algebra join, if more than one element of each list can be matched, only one should be selected, and shouldn't then be reused.
Also, if any of the elements of the first list don't match any of the second list, the process should fail.

Example:

# inputs
list1 = [{'amount': 124, 'name': 'john'},
         {'amount': 456, 'name': 'jack'},
         {'amount': 456, 'name': 'jill'},
         {'amount': 666, 'name': 'manuel'}]
list2 = [{'amount': 124, 'color': 'red'},
         {'amount': 456, 'color': 'yellow'},
         {'amount': 456, 'color': 'on fire'},
         {'amount': 666, 'color': 'purple'}]
keyfunc = lambda e: e['amount']

# expected result
[({'amount': 124, 'name': 'john'}, {'amount': 124, 'color': 'red'}),
 ({'amount': 456, 'name': 'jack'}, {'amount': 456, 'color': 'yellow'}),
 ({'amount': 456, 'name': 'jill'}, {'amount': 456, 'color': 'on fire'}),
 ({'amount': 666, 'name': 'manuel'}, {'amount': 666, 'color': 'purple'})]

I've written a working implementation in Python, but it seems clunky, unclear and inefficient:

def match(al, bl, key):
    bl = list(bl)
    for a in al:
        found = False
        for i, b in enumerate(bl):
            if key(a) == key(b):
                found = True
                yield (a, b)
                del bl[i]
                break
        if not found:
            raise Exception("Not found: %s" % a)

result = list(match(list1, list2, key=keyfunc)

Theory

It might not be clear at first, but you're basically describing a bipartite graph.

You're interested in finding the maximum matching and if it's perfect.

NetworkX is a great Python library for graphs, and the maximum_matching function is already implemented. It uses the Hopcroft-Karp algorithm and runs in \$O(n^{2.5})\$ where \$n\$ is the number of nodes.

You only have to preprocess your lists into a graph and let networkx do its job.

Code

Here's a slightly modified version of a previous answer on Stack Overflow:

import networkx as nx
import matplotlib.pyplot as plt

def has_a_perfect_match(list1, list2):
    if len(list1) != len(list2):
        return False

    g = nx.Graph()

    l = [('l', d['name'],  d['amount']) for d in list1]
    r = [('r', d['color'], d['amount']) for d in list2]

    g.add_nodes_from(l, bipartite=0)
    g.add_nodes_from(r, bipartite=1)

    edges = [(a,b) for a in l for b in r if a[2] == b[2]]
    g.add_edges_from(edges)

    pos = {}
    pos.update((node, (1, index)) for index, node in enumerate(l))
    pos.update((node, (2, index)) for index, node in enumerate(r))

    m = nx.bipartite.maximum_matching(g, l)
    colors = ['blue' if m.get(a) == b else 'gray' for a,b in edges]

    nx.draw_networkx(g,
                     pos=pos,
                     arrows=False,
                     labels = {n:"%s\n%d" % (n[1], n[2]) for n in g.nodes()},
                     edge_color=colors)
    plt.axis('off')
    plt.show()

    return len(m) // 2 == len(list1)

As a bonus, it displays a diagram with the graph and maximum matching:

list1 = [{'amount': 124, 'name': 'john'},
         {'amount': 456, 'name': 'jack'},
         {'amount': 456, 'name': 'jill'},
         {'amount': 666, 'name': 'manuel'}]
list2 = [{'amount': 124, 'color': 'red'},
         {'amount': 456, 'color': 'yellow'},
         {'amount': 456, 'color': 'on fire'},
         {'amount': 666, 'color': 'purple'}]

print(has_a_perfect_match(list1, list2))
# True

list1 = [{'amount': 124, 'name': 'john'},
         {'amount': 456, 'name': 'jack'},
         {'amount': 457, 'name': 'jill'},
         {'amount': 666, 'name': 'manuel'}]
list2 = [{'amount': 124, 'color': 'red'},
         {'amount': 458, 'color': 'yellow'},
         {'amount': 456, 'color': 'on fire'},
         {'amount': 666, 'color': 'purple'}]

print(has_a_perfect_match(list1, list2))
# False

Notes

The desired matching is in m and has a slightly different format than what you mentioned:

{('l', 'jack', 456): ('r', 'yellow', 456), ('l', 'jill', 456): ('r', 'on fire', 456), ('l', 'john', 124): ('r', 'red', 124), ('l', 'manuel', 666): ('r', 'purple', 666), ('r', 'red', 124): ('l', 'john', 124), ('r', 'yellow', 456): ('l', 'jack', 456), ('r', 'purple', 666): ('l', 'manuel', 666), ('r', 'on fire', 456): ('l', 'jill', 456)}

It does have enough information, though.

Note that the edge generation isn't optimal (it's \$O(n^{2})\$ and could be \$O(n)\$ with dicts) but it's concise and still faster than the matching algorithm. Feel free to modify it!

Optimization

@Peilonrayz' answer has a better performance because your problem is easier than the general matching problem : there are no connections between nodes with distinct ids, so a greedy algorithm works fine.

Actually, it's possible to check in 2 lines if the lists match. With a Counter,  you just need to check if the distribution (e.g. Counter({124: 1, 456: 2, 666: 1})) is the same for both lists:

from collections import Counter
Counter(map(key, list1)) == Counter(map(key, list2))
# True


Handle redirect manually I built a class which uses an HttpClient instance for downloading the content of a page. This is working pretty well but I'm not really satisfied about my implementation of redirect.

In fact, I handle the redirect manually as you can see:

/// <summary>
/// Web request handler
/// </summary>
public class NetworkHelper
{
    /// <summary>
    /// Store the url.
    /// </summary>
    private static Uri _storedUrl = null;

    /// <summary>
    /// Store the html data.
    /// </summary>
    private static string _storedData = "";

    public static string GetHtml(Uri url)
    {
        if (url == _storedUrl)
            return _storedData;
        else
            _storedUrl = url;

        HttpWebRequest webReq = (HttpWebRequest)WebRequest.Create(url);

        try
        {
            HttpClientHandler handler = new HttpClientHandler();
            handler.AutomaticDecompression = DecompressionMethods.Deflate | DecompressionMethods.GZip;

            HttpClient httpClient = new HttpClient(handler);

            HttpRequestMessage request = new HttpRequestMessage
            {
                RequestUri = url,
                Method = HttpMethod.Get
            };

            HttpResponseMessage response = httpClient.SendAsync(request).Result;

            int statusCode = (int)response.StatusCode;

            if (statusCode >= 300 && statusCode <= 399)
            {
                Uri redirectUri = response.Headers.Location;

                if (!redirectUri.IsAbsoluteUri)
                {
                    redirectUri = new Uri(request.RequestUri.GetLeftPart(UriPartial.Authority) + redirectUri);
                }

                return GetHtml(redirectUri);
            }

            using (WebClient wClient = new WebClient())
            {
                wClient.Encoding = System.Text.Encoding.UTF8;
                _storedData = wClient.DownloadString(url);
            }

            return _storedData;
        }
        catch (WebException)
        {
            throw;
        }
    }
}

How can I improve my implementation for handling the redirect?

Your code looks solid, however here are a few minor points.

Catching/throwing exceptions

You should only catch exceptions when you are going to handle them. There's no point in catching an exception when you're only going to (re)throw it. However if you were to use logging and still 'bubble up' the exception, you could go with this:

try
{
    //Your webrequest/response code...
}
catch (Exception ex)
{
    //DO YOUR LOGGING HERE

    throw;
}

But as of now I would remove the try/catch from the method and handle exceptions outside of it:

try
{
    string theHtml = NetworkHelper.GetHtml("http://example.com");
}
catch (WebException ex)
{
   //Handle the exception
}

Stored url and data

As of now, your code doesn't allow the same url twice in a row; it'll just return the stored data. This raises 2 questions/issues with me:

What if I want to refresh the result?
I can bypass the check easily but in a hacky way:

Pass url1
Pass url2 to change the _storedUrl variable
Call the method again with url1

I think better would be to change the signature of the method and add a boolean (optional) parameter to indicate if you want to refresh the result:

public static string GetHtml(Uri url, bool refresh = false)
{
    if (url == _storedUrl && !refresh)
        return _storedData;
    else
        _storedUrl = url;

    //Rest of the logic...
 }

Now you will only return the _storedData if you have the same url and don't want to refresh.

Hope this helps!


Passphrase generator script Below script is used to generate passphrases for gpg (or anything really).

Do you see any blatant security mistakes in it?

I wrote it by myself, because I couldn't find any passphrase generator that doesn't use external services and uses /dev/urandom.

Note: I run it with ./generator.bash 4 > x && vim x so stdout is not printed to the terminal, but I'm not sure if this is neccessary. I'm thinking that somewhere terminal history could be stored.

#!/bin/bash
# Usage: ./passphrase_gen.bash 4 > x && vim x
# We redirect to file and print via vim so no history is saved anywhere.
# Repeat after you get passphrase you like. Memorize it/write it down
# and remove x afterwards.
set -eo pipefail

words=/usr/share/dict/words

# https://serverfault.com/a/214620/216850
sudo rngd -r /dev/urandom

n=$1

# print $n randomly chosen words from $words file
for i in $(seq 1 $n); do
  # https://unix.stackexchange.com/a/268960
  random=$(od -vAn -N4 -tu4 < /dev/urandom)

  lines=$(cat $words | wc -l)

  line=$((random % lines))
  awk "NR==$line" $words
done

Gist link: https://gist.github.com/jan-swiecki/9974501047c79bad12a3c87cfe846cd6

Note: I run it with ./generator.bash 4 > x && vim x so stdout is not printed to the terminal, but I'm not sure if this is neccessary. I'm thinking that somewhere terminal history could be stored.

I don't think this is necessary.
If anything, you might forget to remove x,
which could be a security risk.

Avoid seq

seq is not a standard tool, and a native alternative exists:

for ((i = 0; i < n; i++)); do ...; done

Extract constants out of loops

This statement is executed in each iteration of the loop:

lines=$(cat $words | wc -l)

But the content of the $words file is likely constant,
so it's safe to compute this once, before entering the loop.

Unnecessary cat

The cat command is unnecessary with wc, it's better to redirect input:

lines=$(wc -l < "$words")

Double-quote variables in command line arguments

Although the content of $words doesn't contain spaces,
as a good practice it's good to double-quote it when using as program arguments, for example instead of:

awk "NR==$line" $words

Write:

awk "NR==$line" "$words"

Arithmetic context

Instead of this:

line=$((random % lines))

You can write simpler:

((line = random % lines))


Keep ABC size down for conditional string concatenation in Ruby This method takes a comment object and displays it correctly depending on what columns are available. EVENT_TEXT and REASON_TEXT are hashes that map properties to more human readable strings, like so

EVENT_TEXT = { edit_item: 'edited an item' }
REASON_TEXT = { pickup_date: 'a pickup date change' }

The method that calls this one prepends this text with a username, so the text output expected is something along the lines of edited an item due to a pickup date change: the supplier is not ready yet.

  def comment_text(comment)
    text = ''
    if comment.event.present?
      text += EVENT_TEXT[comment.event.to_sym]
      if comment.reason.present?
        text += ' due to ' + REASON_TEXT[comment.reason.to_sym]
      end
    end
    text + ': ' + comment.text if comment.text.present?
  end

Which has a an Assignement Branch Condition size of 17, higher than Rubocop's default of 15. Is there a better way to concatenate strings conditionally in Ruby? I really don't feel like this method is doing too much.

Should I just increase Rubocop's threshhold? But even if I did, how would I handle a scenario like this with 10 conditions?

Perhaps just extracting the hash lookups would help:

def comment_event_string(comment)
  EVENT_TEXT[comment.event.to_sym]
end

def comment_reason_string(comment)
  " due to #{REASON_TEXT[comment.reason.to_sym]}"
end

def comment_text_string(comment)
  ": #{comment.text}"
end

def comment_text(comment)
  text = ''
  if comment.event.present?
    text += comment_event_string(comment)
    text += comment_reason_string(comment) if comment.reason.present?
  end
  text + comment_text_string(comment) if comment.text.present?
end

Passing comment around seems to be to be something of a code smell though, and I wonder if this code would be better placed inside the Comment class.

private def event_string
  EVENT_TEXT[event.to_sym]
end

private def reason_string
  " due to #{REASON_TEXTreason.to_sym]}"
end

private def text_string
  ": #{text}"
end

def nice_text(comment)
  nice_text = ''
  if event.present?
    nice_text += event_string
    nice_text += reason_string if reason.present?
  end
  nice_text + text_string if text.present?
end


Does this loop unrolling make sense? I am coding a Hanning filter which is a recursive relation:

y[i] = 0.25*(x[i] + 2*x[i-1] + x[i-2])

And here is my code were I attempt to unroll 3 times:

public void HanningFilter(float[] array) {
        int len = array.length;
        int i;
        for (i = 2; i < len-3; i+=3) {
            array[i] = 0.25f*(array[i] + 2*array[i-1] + array[i-2]);
            array[i+1] = 0.25f*(array[i+1] + 2*array[i] + array[i-1]);
            array[i+2] = 0.25f*(array[i+2] + 2*array[i+1] + array[i]);
        }
        //Clean-up at the end
        for (int j = i; j < len; j++) {
            array[i] = 0.25f*(array[i] + 2*array[i-1] + array[i-2]);
        }
    }

Would this help at all with an OoO superscalar processor? Is 3 the right amount of times to unroll? Thanks

No, rule of thumbs for this case:

Never guess optimizations, use a profiler
You're not smarter then the JIT compiler

Loop unrolling only makes sense if you profiled the code and found the unrolled version to be faster then the normal version.

I guess you're coming from a C background, so please don't try to bring C-habits to Java. Java is different, always go for readability first.

public void HanningFilter(float[] array) {

The Java naming conventions dictate that function names are lowerCamelCase.

Also the name array is a bad name, more appropriate would be a name like data or input.

int len = array.length;
int i;
for (i = 2; i < len-3; i+=3) {

What you're doing here is absolutely unnecessary. Java allows declarations of variables inside the for and there's no need to define a variable for the condition.

for (int idx = 2; idx < data.length; idx++) {

Naming loop variables idx or counter is not very famous, I prefer it to i.


Improving time complexity of finding the longest palindrome in Python The Longest Palindromic Substring challenge from InterviewBit:

Given a string S, find the longest palindromic substring in S.

where a "substring" must be contiguous, and in case of ties the first such substring should be returned.

Example:
Input : "aaaabaaa"
Output : "aaabaaa"

My code:
class Solution:
    # @param A : string
    # @return a strings
    def clean_string(self, string):
        result = []
        str_list = string.split()
        for word in str_list:
            result.append(''.join(ch for ch in word if ch.isalnum()))
        return "".join(result).lower()

    def is_palindrome(self, string):
        if len(string)==1:
            return True
        if string[::-1]==string:
            return True
        return False

    def longestPalindrome(self, A):
        if len(A)==1:
            return A
        cleaned = self.clean_string(A)
        start_index = 0
        max = -1
        longest = None
        while (start_index < len(cleaned)-1):
            end_index = start_index
            while (end_index < len(cleaned)):
                end_index +=1
                if self.is_palindrome(cleaned[start_index:end_index]):
                    if len(cleaned[start_index:end_index]) > max:
                        longest = cleaned[start_index:end_index]
                        max = end_index-start_index

            start_index +=1
        return longest

s = Solution()
print(s.longestPalindrome("abbcccaaadaaakl"))

My code passes all the tests except for the time limit:

Please provide some insight as to how to modify the code to pass the time limit.

[EDIT] I believe the time-out occurs if you are not fast enough typing your code in the website where this challenge is posted. I believe it is not about optimizing code to run faster, but how quickly you can come up with working code. The following is still true in regards to Code Review:

I might not have all improvements, but here are some I would do

from re import sub

you could rewrite this function:

def clean_string(self, string):
    return sub(r'[^a-zA-Z0-9]', '', string)

and rewrite this function:

def is_palindrome(self, string):
    return string[::-1]==string

Then the first if statement in def longestPalindrome(self, A) would change too:

def longestPalindrome(self, A):
    if self.is_palindrome(A):
        return A

No need for this:

    if len(A)==1:
        return A

You can replace the while loops with for loops, something like this:

def longestPalindrome(self, A):
    if self.is_palindrome(A):
        return A
    cleaned = self.clean_string(A)
    for l in range(len(cleaned)-1,0,-1):  ## l = lenght to check high to low
        for i in range(0, len(cleaned)-l+1):  ## i is position in A to check
            if self.is_palindrome(A[i:l+i]):
                return A[i:l+i]
    return None

Those changes would be faster then your original code, but I did not do any scientific tests, so the obligatory: "YMMV"

[EDIT]

In fact you can shorten the code further:

from re import sub

class Solution:

    def longest_palindrome(self, a):
        if a[::-1] == a:
            return a
        a = sub(r'[^a-zA-Z0-9]', '', a)
        for l in range(len(a)-1, 0, -1):
            for i in range(0, len(a)-l+1):
                if a[i:l+i][::-1] == a[i:l+i]:
                    return a[i:l+i]
        return None

s = Solution()
print(s.longest_palindrome("abbcccaaadaaakl"))

and you really don't need a class here:

from re import sub

def lp(a):
    if a[::-1] == a:
        return a
    a = sub(r'[^a-zA-Z0-9]', '', a)
    for l in range(len(a)-1, 0, -1):
        for i in range(0, len(a)-l+1):
            if a[i:l+i][::-1] == a[i:l+i]:
                return a[i:l+i]
    return None

print(lp("abbcccaaadaaakl"))


Sending HTML-formatted mail in Java This is my code for sending mail in HTML format:

import java.util.Properties;
import javax.mail.Authenticator;
import javax.mail.Message;
import javax.mail.Message.RecipientType;
import javax.mail.MessagingException;
import javax.mail.PasswordAuthentication;
import javax.mail.Session;
import javax.mail.Transport;
import javax.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage;
public class SendMail {
    public void sendMail(String m_from,String m_to,String m_subject,String m_body){
      try {
          Session m_Session;
          Message m_simpleMessage;
          InternetAddress m_fromAddress;
          InternetAddress m_toAddress;
          Properties m_properties;

          m_properties     = new Properties();
          m_properties.put("mail.smtp.host", "smtp.gmail.com");
          m_properties.put("mail.smtp.socketFactory.port", "465");
          m_properties.put("mail.smtp.socketFactory.class",
                                     "javax.net.ssl.SSLSocketFactory");
          m_properties.put("mail.smtp.auth", "true");
          m_properties.put("mail.smtp.port", "465");

          m_Session=Session.getDefaultInstance(m_properties,new Authenticator() {
               protected PasswordAuthentication getPasswordAuthentication() {
                    return new PasswordAuthentication("xxxxx","yyyyy"); // username and the password
               }
          });

          m_simpleMessage  =   new MimeMessage(m_Session);
          m_fromAddress    =   new InternetAddress(m_from);
          m_toAddress      =   new InternetAddress(m_to);
          m_simpleMessage.setFrom(m_fromAddress);
          m_simpleMessage.setRecipient(RecipientType.TO, m_toAddress);
          m_simpleMessage.setSubject(m_subject);

          m_simpleMessage.setContent(m_body, "text/html");
          //m_simpleMessage.setContent(m_body,"text/plain");

          Transport.send(m_simpleMessage);
        } catch (MessagingException ex) {
            ex.printStackTrace();
        }
  }

  public static void main(String[] args) {
      SendMail send_mail    =   new SendMail();
      String empName = "Antony Raj S";
      String title ="Hi !"+empName+"";
      send_mail.sendMail("anto@gmail.com", "anthony@slingmedia.com", "Please apply for leave for the following dates", title+"
by
HR");
    }
}

This works fine, but is there any other better way to do the same or how can I make the same code still better one? I get all the values like from address, SMTP host, port from the contex.xml file which is there in conf folder of Tomcat, so please do suggest anything better.

Try to minimize the scope of local variables. It's not necessary to declare them at the beginning of the method, declare them where they are first used.
See Effective Java, Second Edition, Item 45: Minimize the scope of local variables. (Google for "minimize the scope of local variables", it's on Google Books too.))

The m_ prefix is unnecessary and uncommon in the Java world. See Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions

The sendMail method should validate its input parameters. Does it make sense if the form, to etc. is null or empty string? If not, check it and throw a NullPointerException or an IllegalArgumentException. (Effective Java, Second Edition, Item 38: Check parameters for validity)


Click a link to delete targeted content using Ajax I used a glyphicon as a link to delete a comment:

    delete

I send the request using Ajax.

retrieve the comment_url
get the grand-grand-parent element to hide
send request to views.py and delete the target comment

$(document).ready(function () {
    $("body").on("click",".delCommentLink",function (e) {
        e.preventDefault();
        var comment_url = $(e.target).parent().attr("href");
        var $commentEle = $(e.target).closest(".comment");
        if (window.confirm("Delete this comment?")) {
            $.ajax({
                type: "get",
                url: comment_url,
                success: function (data) {
                    var ret = JSON.parse(data);
                    if (ret['status'] == 1) {
                        $commentEle.hide();
                    } else {
                        alert(ret['msg']);
                };
            },//success
        });//ajax
        } else {
            e.preventDefault()
        }
    });//click event
})

Click a link to delete it. My code seems like it has too many lines for such a routine task.
How can I complete it elegantly?

I only see two things to "reduce", regarding the line amount.

The if statement in the success callback could be replaced by a ternary operator.
The last e.preventDefault() is redundant, you can squarely remove the whole else part.

$(document).ready(function () {
  $("body").on("click",".delCommentLink",function (e) {
    e.preventDefault();
    var comment_url = $(e.target).parent().attr("href");
    var $commentEle = $(e.target).closest(".comment");
    if (window.confirm("Delete this comment?")) {
      $.ajax({
        type: "get",
        url: comment_url,
        success: function (data) {
          var ret = JSON.parse(data);
          (ret['status'] == 1) ? $commentEle.hide() : alert(ret['msg']);
        },//success
      });//ajax
    }
  });//click event
});

That is 6 lines less...


Java Tree Implementation I wrote a simple tree data type in Java consisting of nodes which are themselves trees, and where each tree has its own list of children (not a binary tree). I plan on eventually using it in a minimax algorithm where I keep track of future game states in a board game by holding different board states in nodes and holding moves that lead to each state as edges in the tree. Here's my code so far — any suggestions (whether it be conceptual or performance related, or other features I should add) are appreciated:

public class Tree<V, E> {
   private V data;
   //here the edgeVal is the edge leading from the node's parent to the node itself,
   //not from the node to any of its children
   private E edgeVal;
   private Tree<V, E> parent;
   private ArrayList<Tree<V, E>> children;
   public Tree(V data, Tree<V, E> parent, E edgeVal) {
      this.data = data;
      this.edgeVal = edgeVal;
      this.parent = parent;
      this.children = new ArrayList<Tree<V, E>>();
   }
   public void addChild(V childData, E edgeVal) {
      Tree<V, E> child = new Tree<V, E>(childData, this, edgeVal);
      this.children.add(child);
   }
   public Tree<V, E> getParent() {
      return this.parent;
   }
   public ArrayList<Tree<V, E>> getChildren() {
      return this.children;
   }
   public V getData() {
      return this.data;
   }
   public E getEdgeVal() {
      return this.edgeVal;
   }
   public boolean isRoot() {
      return this.parent == null;
   }
   public boolean isLeaf() {
      return this.children.size() == 0;
   }
   public String toString() {
      //assuming type of data has a nicely defined toString() method
      return this.data.toString();
   }
}

Welcome to Code Review!

isLeaf compares the Size of children, a solution that is better readable is to use isEmpty() instead of compare the size of children.
The tree avoid a serialization that others may support, add the serializable-interface to keep serialization support.
edgeVal and data have no setter and to add final would help to avoid sideeffects whenever the code changes.
To have a ArrayList as a list of children has the problem that

The ArrayList can contain null
The ArrayList can contain one element twice (i do not think its a case you like to have)

You better use some kind of set, a LinkedHashSet in example.
You can use getChildren() to have read-write-access to the children, if you do oscar.getChildren().add(oscar); your loop is perfect but not a Tree. Use the unmodifyable-functions of the class Collections to return a readonly-collection.
The class is named Tree that can contains a list of Tree. Even if a Tree can be a parasite to other Trees i propose to name the Class TreeNode.
toString should describe the node, not the data inside.
Am i the only one who write javadoc?


Colorizing Excel cells with a date range filter In a table there is a number of column (chambre) and a number of row (alveole). If these matches in a list in an other sheet then it checks if it the date is between two dates on that second sheets. If it matches those 3 things, then it puts the sheet

My code works if I add DoEvents in every For, else Excel and VBA stops responding. But it takes around 5 minutes and I need to to this for 4 tables.

Is it my code that is not efficient? Do you have any ideas how to make it better?

Sub alveoles()

    Application.ScreenUpdating = True
    Dim moment As Date
    'variables Photo
    Dim four As Integer
    Dim chambre As Integer
    Dim alveole As Integer
    Dim aaa As Integer
    Dim ccc As Integer

    'variables Gems
    Dim ligne As Integer
    Dim enfournement As Date
    Dim defournement As Date

    Dim lastrow As Integer

    'set date and lastrow
    moment = Worksheets("Photo").Range("O2").Value
    lastrow = Worksheets("Gems").Range("A" & Rows.Count).End(xlUp).Row
    four = 1
    'Four1 1 row
    For ligne = 2 To lastrow
    DoEvents

            For chambre = 1 To 38
            DoEvents
                For alveole = 1 To 6
                 DoEvents

                enfournement = Worksheets("Gems").Range("D" & ligne).Value
                defournement = Worksheets("Gems").Range("E" & ligne).Value

                If four = Worksheets("Gems").Range("A" & ligne).Value And _
                   chambre = Worksheets("Gems").Range("B" & ligne).Value And _
                   alveole = Worksheets("Gems").Range("C" & ligne).Value And _
                    moment > enfournement And _
                    moment < defournement Then

                                aaa = alveole + 5
                                ccc = chambre + 2

                            Worksheets("Photo").Cells(aaa, ccc).Interior.ColorIndex = 1 'NOIR

                End If
                Next alveole
            Next chambre
Next ligne

End Sub

Let's think about what you're doing for a second.

For ligne = 2 To lastrow
    DoEvents

    For chambre = 1 To 38
    DoEvents
        For alveole = 1 To 6

6 * 38 = 228 iterations for every row. Wow. Yes. There's no wonder you had to give the OS time to operate.

All of this just to set the interior color of a cell. Which is a problem as well. What if the data changes? Well.... it's going to remain the same, forever as far as I can tell. There's nothing here to revert it. There's a better way. Use some conditional formatting to do this. No VBA needed. Just craft a formula that returns a True/False condition.

I can't claim this one is perfect or tested, but it should get you started.

=AND(GEMS!$A1 = 1, AND(GEMS!$B1 = COLUMN(), AND(GEMS!$C1 = ROW(), AND(GEMS!$D1 < PHOTO!$O$2, AND(GEMS!$E1 > PHOTO!$O$2)))))


Creating a EventHandler class for buttons with different actions I have the following code at the moment:

public void initialiseButtons() {
    this.saveNewButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent actionEvent) {
            savePatientData(actionEvent);
        }
    });
    this.clearFieldsButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent actionEvent) {
            TextFieldHelpers.clearInputFields(dataInputFields);

        }
    });
    this.cancelNewPatientButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent actionEvent) {
            clearWorkspace();
        }
    });
    this.saveChangesButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent actionEvent) {
            savePatientData(actionEvent);
        }
    });
    this.deletePatientButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent actionEvent) {
            deleteExistingPatient();
        }
    });
    this.deselectPatientButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent actionEvent) {
            patientListView.getSelectionModel().clearSelection();
        }
    });
    addNewButton.setOnAction(new EventHandler<ActionEvent>() {
        @Override
        public void handle(ActionEvent event) {
            setupViewForEditingOrAddingPatientAction(1);
        }
    });
}

As you can see I have repeated the creation of a EventHandler numerous amount of times. Can someone guide me on how I can minimise this or is this the only way?

Assuming that you have Java 8 available:

this.saveNewButton.setOnAction(new EventHandler<ActionEvent>() {
    @Override
    public void handle(ActionEvent actionEvent) {
        savePatientData(actionEvent);
    }
});

Can become, using a lambda:

this.saveNewButton.setOnAction(actionEvent -> savePatientData(actionEvent));

Or, for that particular event handler:

this.saveNewButton.setOnAction(this::savePatientData);

Example of another EventHandler

this.deletePatientButton.setOnAction(new EventHandler<ActionEvent>() {
    @Override
    public void handle(ActionEvent actionEvent) {
        deleteExistingPatient();
    }
});

Can become:

this.deletePatientButton.setOnAction(event -> deleteExistingPatient());

If you do not have Java 8 available:

Sorry, you're pretty much screwed. :(

Essentially, there is not that much code duplication as you might think, as each EventHandler is different.

The only way possible to solve it in this case would be to have all the buttons use the same EventHandler and perform a whole lot of checks such as:

if (event.getSource() == this.deletePatientButton) {
    ...
}

Side note:

If you are using JavaFX 8, you have the possibility to define an onAction in your FXML for your buttons, this will be associated with a method of that name in your controller.

See also http://code.makery.ch/blog/javafx-8-event-handling-examples/


Scraping titles and links from a site using python I've written a script which when I run parses the link of the item from the left sided menu,then it tracks down each individual links to it's main page. Some links spread through pagination some end in a single page. Whatever the case is, the crawler scrapes name and href of each item. While doing so I came across some duplicates which eventually I shook off using got_already function. The site contains 251 videos and my crawler is able to parse them all. Here is what I have done:

import requests
from lxml import html

got_already=[]
Page_link="http://www.wiseowl.co.uk/videos/"
Blink="http://www.wiseowl.co.uk"

def startpoint(page):
    req=requests.Session()
    response = req.get(page)
    tree = html.fromstring(response.text)
    titles = tree.xpath("//ul[@class='woMenuList']//li[@class='woMenuItem']/a/@href")
    for title in titles:
        if "author" not in title and "year" not in title:
            GrabbingData(Blink + title)
            midpoint(Blink + title)

def midpoint(links):
    req=requests.Session()
    response = req.get(links)
    tree = html.fromstring(response.text)
    for links in tree.xpath("//a[@class='woPagingItem']/@href"):
        GrabbingData(Blink + links)

def GrabbingData(url):
    req=requests.Session()
    response = req.get(url)
    tree = html.fromstring(response.text)
    for item in tree.xpath("//p[@class='woVideoListDefaultSeriesTitle']"):
        title = item.xpath('.//a/text()')[0]
        link = item.xpath('.//a/@href')[0]
        if title not in got_already:
            got_already.append(title)
            got_already.append(link)
            print(title,link)

startpoint(Page_link)

There are a couple of things you can improve. I'll start by mentioning some PEP8 related observations:

variables / methods should be lower_cased (instead of GrabbingData you should write grabbing_data;
for readability, you should have two newlines between your methods;
use spaces around operators like =, + etc;
after each , you should put a space;
constants are usually uppercased so page_link should be PAGE_LINK and blink should be BLINK;
add docstrings to your functions;
It's good practice to split the words using _ (e.g: start_point instead of startpoint);
it's also recommended to have one newline between builtin modules and 3rd party modules.

Now, about the code

it's not necessary to create a new session in all of your functions. It's preferred to create only one session for the same website as it can give a boost of speed to your scrapper. More, the session object allows you to persist certain parameters across requests (if needed);
you only use got_already in your last function, so define it there. Or better yet, use a Set()
instead of if "author" not in title and "year" not in title you can use all()
PAGE_LINK is irrelevant and confusing. You can build your full video links using just BLINK.
last but not least, I'd use beautifulsoup for html parsing: but, here I'd like to add that The downside of using this parser is that it is much slower than the HTML parser of lxml. So if performance matters, you might want to consider using soupparser only as a fallback for certain cases.

That said, if I were you, I'd have built the scrapper like this:
from requests import Session
from bs4 import BeautifulSoup as bs

def get_html_source(_session, url, min_pag, max_pag):
    """Yield the source html for each set of videos"""

    for i in range(min_pag, max_pag):
        html_source = _session.get('{}/default-{}.htm'.format(url, i)).text
        yield bs(html_source, 'html.parser')

def parse_html(soup, url):
    """
    Process the html and get all the titles and the
    video urls that match the condition
    """
    for a in soup.find_all('p', attrs={'class', 'woVideoListDefaultSeriesTitle'}):
        video_title = a.find('a').text
        video_url = a.find('a', href=True)['href']

        if "author" not in video_title and "year" not in video_title:
            print('Title: {} | Url: {}{}'.format(video_title, url, video_url))

def main():
    _session = Session()
    url = 'http://www.wiseowl.co.uk/videos'

    for soup in get_html_source(_session, url, 1, 14):
        parse_html(soup, url)

if __name__ == '__main__':
    main()

Things that I did differently:
Intro
If you were paying attention, the website adds \url\video\default-N.htm at the end of the url depending on the set of pages you click in the bottom of the page. There's a total of 13 urls.
Coding around
In my first function, I've created a function which will go through each page and will yield the source html for each one.
In the second function, I've just used BeautifulSoup to get the title and the urls of the videos that you're interested in. I've also cut off A LOT of the logic you had because it wasn't needed anymore. I've also printed the data a bit different than you did. You should know that printing huge amounts of data has a big impact on performance. I'd recommend logging into a file.
The third function is the one that will be called in our safeguard if __name__ == '__main__'


How to create a 'tee' command in Ruby for a Unix like pipeline? I came across the question about creating memory-efficient Ruby pipe class with lazy evaluation. Some code is given to effectively create a pipeline of commands using lazy enumerators. I have been messing round with it and have implemented a tee command shown below. I feel like there is a better way to do it though.

class Pipe

  class Tee

    def initialize(pipe)
      @pipe = pipe
      @buffer = []
      @pipe.source = self.to_enum
    end

    def <<(item)
      @buffer << item
      # puts "Added '#{item}' to buffer: #{@buffer.inspect}"
    end

    def to_enum
      Enumerator.new do |yielder|
        item = @buffer.shift
        yielder << item if item
      end.lazy
    end

  end

  attr_writer :source

  def initialize
    @commands = []
    @source = nil
  end

  def add(command, opts = {})
    @commands << [command, opts]
    self
  end

  def run
    enum = @source

    @commands.each do |command, options|
      enum = method(command).call(enum, options)
    end

    enum.each {}

    enum
  end

  def cat(enum, options)
    Enumerator.new do |yielder|
      enum.map { |line| yielder << line } if enum

      options[:input].tap do |ios|
        ios.each { |line| yielder << line }
      end
    end.lazy
  end

  def out(enum, options)
    Enumerator.new do |yielder|
      enum.each do |line|
        puts line
        yielder << line
      end
    end.lazy
  end

  def cut(enum, options)
    Enumerator.new do |yielder|
      enum.each do |line|
        fields = line.chomp.split(%r{#{options[:delimiter]}})

        yielder << fields[options[:field]]
      end
    end.lazy
  end

  def upcase(enum, options)
    Enumerator.new do |yielder|
      enum.each do |line|
        yielder << line.upcase
      end
    end
  end

  def tee(enum, options)

    teed = Tee.new(options.fetch(:other))
    Enumerator.new do |yielder|
      enum.each do |line|
        yielder << line
        teed << line
      end
    end
  end

  def grep(enum, options)
    Enumerator.new do |yielder|
      enum.each do |line|
        yielder << line if line.match(options[:pattern])
      end
    end.lazy
  end

end

another_pipe = Pipe.new
another_pipe.add(:grep, pattern: /testing/i)
another_pipe.add(:out)

pipe = Pipe.new
pipe.add(:cat, :input => StringIO.new("testing\na new\nline"))
# pipe.add(:grep, pattern: /line/)
pipe.add(:tee, :other => another_pipe)
pipe.add(:upcase)
pipe.add(:out)
pipe.run

puts "================================================="
another_pipe.run

Here's the output:

TESTING
A NEW
LINE
=================================================
testing

Does anyone see a smarter way of doing this?

I know it's an old question, but it's an interesting one.

First, a couple small things:

In Pipe#run:

enum = method(command).call(enum, options)

Calling method has a lot of overhead, and send(command, enum, options) does the same thing.
Also in Pipe#run:

enum.each {}

The purpose of this is to cause the final object to be enumerated. We can do the same thing cheaper (even an empty block takes CPU cycles) with enum.to_a, which is also aliased as enum.force.

Those things aside, the first thing I noticed is that there's a lot of repetition. grep and upcase, for example, only differ on one line. We can extract the shared code out into another method, which I'll call enum_lazy:

def upcase(enum, options)
  enum_lazy(enum) do |line, yielder|
    yielder << line.upcase
  end
end

def grep(enum, options)
  enum_lazy(enum) do |line, yielder|
    yielder << line if line.match(options[:pattern])
  end
end

private
def enum_lazy(enum, &block)
  Enumerator.new do |yielder|
    enum.each do |line|
      yield(line, yielder)
    end
  end.lazy
end

...but this made me realize that some of these methods don't need the yielder pattern at all. For example, upcase takes one element at a time, performs an operation on it, and yields one value each time—it's just a lazy map, and this is equivalent:

def upcase(enum, options)
  enum.lazy.map(&:upcase)
end

Since we called lazy, this will return an Enumerator::Lazy as expected. We can simplify cut and out in the same way:

def out(enum, options)
  enum.lazy.map do |line|
    puts line
    line
  end
end

def cut(enum, options)
  enum.lazy.map do |line|
    fields = line.chomp.split(options[:delimiter])
    fields[ options[:field] ]
  end
end

grep is different, because we only don't want output for every input line. As it turns out, though, grep is just a lazy select:

def grep(enum, options)
  enum.lazy.select {|line| line =~ options[:pattern] }
end

That just leaves cat. I have a strong inkling that there's a similar optimization we could do for cat, but I haven't yet been able to actually figure it out. Perhaps someone else can help figure it out.

Here's the final code:

class Pipe
  class Tee
    # (no changes here)
  end

  attr_writer :source

  def initialize
    @commands = []
    @source = nil
  end

  def add(command, opts = {})
    @commands << [command, opts]
    self
  end

  def run
    enum = @source

    @commands.each do |command, options|
      enum = send(command, enum, options)
    end

    enum.force
    enum
  end

  def cat(enum, options)
    Enumerator.new do |yielder|
      enum.map { |line| yielder << line } if enum

      options[:input].tap do |ios|
        ios.each { |line| yielder << line }
      end
    end.lazy
  end

  def out(enum, options)
    enum.lazy.map do |line|
      puts line
      line
    end
  end

  def cut(enum, options)
    enum.lazy.map do |line|
      fields = line.chomp.split(/#{options[:delimiter]}/)
      fields[ options[:field] ]
    end
  end

  def upcase(enum, options)
    enum.lazy.map(&:upcase)
  end

  def tee(enum, options)
    teed = Tee.new(options.fetch(:other))

    enum_lazy(enum) do |line, yielder|
      yielder << line
      teed << line
    end
  end

  def grep(enum, options)
    enum.lazy.select {|line| line =~ options[:pattern] }
  end

  private
  def enum_lazy(enum, &block)
    Enumerator.new do |yielder|
      enum.each do |line|
        yield(line, yielder)
      end
    end.lazy
  end
end


Math equation generator program I am building this program in Python to generate 10 random arithmetic questions of either multiplication, addition or subtraction and then saves users scores to a .txt file. I just wanted some help condensing the code as I'm really stuck as of what to do with it.

How can I combine the function (e.g. multiplication, addition and subtraction) into one?

import random
import time
import sys

ans = 0   #variable to hold question answer
question = 0   #question number
user_score = 0  #user's score
userInput = int()   #where user enters the answer
lastName = str()   #holds last name
firstName = str()   #holds first name
form = str()    #holds user's form

def function(score,name):   #writes user's information to a .txt file
    sumOfStudent = (name + ' scored ' + str(user_score))
    classNameTxt = (className, '.txt.')
    f = open(className, 'a')
    f.write(sumOfStudent + form + '\n')
    f.close()

def multiplication():   #creates a multiplication question
    global ans
    numberOne, numberTwo  = random.randint(0,20), random.randint(0,20)
    print("What is" , numberOne , "*" , numberTwo)
    ans = (numberOne * numberTwo)

def subtraction():   #creates a subtraction question
    global ans
    numberOne, numberTwo  = random.randint(0,20), random.randint(0,20)
    print("What is" , numberOne , "-" , numberTwo)
    ans = (numberOne - numberTwo)

def addition():   #creates a addition question
    global ans
    numberOne, numberTwo  = random.randint(0,20), random.randint(0,20)
    print("What is" , numberOne , "+" , numberTwo)
    ans = (numberOne + numberTwo)

operation = [multiplication,subtraction,addition]   #holds all of the opperators
randOperation = random.choice(operation)    #chooses a random operator

lastName = input("Please enter your surname: ").title()
firstName = input("Please enter your first name: ").title()
className = input("Please enter your form: ").title()
print()

def main():   #main game loop - ask questions and checks it against answer, stops are a give amount of questions
    question = 0
    user_score = 0
    randOperation = random.choice(operation)

    while True:
        try:
            randOperation()
            randOperation = random.choice(operation)
            if question >= 10:
                break
            userInput = int(input("Enter the answer: "))
            if userInput == ans:
                print("Correct!" + "\n")
                user_score += 1
                question += 1
            else:
                print("Incorrect!" + "\n")
                question += 1
        except ValueError:
            print("I'm sorry that's invalid")
            question += 1

main()    #initializes the function

print(firstName, lastName , "you scored" , user_score , "out of 10")   #shows the user's score and name

user_name = firstName + ' ' + lastName
function(user_score,user_name)

def endMenu():
    while True:
        try:
            options = int(input('''Press '1' to view users' scores,
            press '2' to restart the test,
            press '3' to exit the game,

            Enter option here: '''))
        except ValueError:
            print("I'm sorry that was invalid...")

        if options == 3:  #exits the game...
            sys.exit()

        elif options == 2:   #starts the game loop again because it's in a function
            main()

        elif options == 1:   #displays everything on the .txt file
            f = open('userScore.txt', 'r')
            print(f.read())
            print()
            endMenu()

        else:
            print("Sorry, I don't understand. Please try again...")
            print()
            endMenu()

endMenu()

Besides the obvious code repetition that you mentioned, a few salient issues deserve mentioning:

Your main() function is not really the main code of your program.  Rather, you have some free-floating code outside of any function, and the main function is actually called endMenu(), which is surprising.
The endMenu() function is improperly recursive.  The while True loop should suffice.
You use global variables a lot.  While firstName and lastName are somewhat excusable (they are entered once by the user, and never change subsequently), you really shouldn't use globals for transient state like ans, question, etc.
function is the least-informative name possible for a function.

To address those issues, as well as the code repetition you mentioned, I'd write the program like this:

import operator
import random

OPERATIONS = [
    ('+', operator.add),
    ('-', operator.sub),
    ('*', operator.mul),
]

def random_question(binary_operations, operand_range):
    """Generate a pair consisting of a random question (as a string)
    and its answer (as a number)"""
    op_sym, op_func = random.choice(binary_operations)
    n1 = random.randint(min(operand_range), max(operand_range))
    n2 = random.randint(min(operand_range), max(operand_range))
    question = '{} {} {}'.format(n1, op_sym, n2)
    answer = op_func(n1, n2)
    return question, answer

def quiz(number_of_questions):
    """Ask the specified number of questions, and return the number of correct
    answers."""
    score = 0
    for _ in range(number_of_questions):
        question, answer = random_question(OPERATIONS, range(0, 21))
        print('What is {}'.format(question))
        try:
            user_input = float(input("Enter the answer: "))
        except ValueError:
            print("I'm sorry that's invalid")
        else:
            if answer == user_input:
                print("Correct!\n")
                score += 1
            else:
                print("Incorrect!\n")
    return score

def identify_user():
    # TODO, as an exercise for you
    pass

def display_score(first_name, last_name, class_name):
    # TODO, as an exercise for you
    pass

def menu():
    # TODO, as an exercise for you
    pass

def main():
    first_name, last_name, class_name = identify_user()
    while True:
        menu_choice = menu()
        if menu_choice == 1:        # Display score
            display_score(first_name, last_name, class_name)

        elif menu_choice == 2:      # Run quiz
            QUESTIONS = 10
            score = quiz(QUESTIONS)
            print('{first_name} {last_name}, you scored {score} out of {QUESTIONS}'.format(**locals()))

        elif menu_choice == 3:      # Exit
            break

        else:
            print("Sorry, I don't understand. Please try again...")
            print()

if __name__ == '__main__':
    main()

Observe how there are no global variables: each function accepts parameters and returns specific output values.


Automated batch and python skeleton script file creation The code below uses a single command line 'filename' argument to create a batch and python skeleton script file in the cwd. The python file is then automatically opened in IDLE for convenience.

As a beginner making a significant amount of python files for worked educational examples this could save me 15 minutes per day.

Example usage for the below code which is named "make.py":

Cd in command line to the desired wd
Input script name("make") and the desired name of the files to be created minus any filename extension(in this case "test") as the single argument

Batch file created("test.bat"):

@py.exe C:\Users\Dave\Desktop\2016Coding\AutomateBoring\18-GUIAutomation\test.py %*
@pause

Python file("test.py") created in cwd and which is opened automatically in IDLE:

#! python3

#

import logging

logging.basicConfig(level=logging.DEBUG, format="%(asctime)s - " +\
            "%(levelname)s - %(message)s")

def main():
    pass

if __name__ == "__main__":
    main()

Code:

#! python3

# Takes a command line filename and creates a batch file and .py file
# named "filename" in the cwd. The python file contains a basic script
# framework. In additon, the python file will be automatically opened.

import argparse
import os
import subprocess

def handle_input():
    """Parses user commandline input"""

    parser = argparse.ArgumentParser()
    parser.add_argument("filename", type=str,
                        help="The name of the files to create minus any" +\
                        "extensions.")
    args = parser.parse_args()
    return args.filename

def make_batch(filename: str, cwd: str):
    """Makes a batch file name "filename" in "cwd"""

    filepath = os.path.join(cwd, filename + ".bat")
    with open(filepath, "w") as f:
        f.write("@py.exe " + os.path.join(cwd, filename + ".py ") +
                "%*\n@pause")
    print("\nCreated batch file: {}".format(filepath))

def make_py(filepath: str):
    """Makes a .py file in cwd written with a basic skelton script"""

    # *****I realize it's a simpler task to copy a template from a .
    # py file created for the purpose and write that to the new .py*****

    shebang = "#! python3\n\n"
    comment = "#\n\n"
    imports = "import logging\n\n"
    logging = "logging.basicConfig(level=logging.DEBUG, format=\"%(asctime)s - " \
              "\" +\\\n\t\t    \"%(levelname)s - %(message)s\")\n\n"
    functions = "def main():\n\tpass\n\n"
    main = "if __name__ == \"__main__\":\n\tmain()"

    with open(filepath, "w") as f:
        f.write(shebang + comment + imports + logging + functions + main)
    print("Created .py file: {}".format(filepath))

def open_py_idle(filename: str):
    """Opens a .py file in IDLE using subprocess"""

    idle_path = r"C:\Users\Dave\AppData\Local\Programs\Python\Python35" +\
                r"-32\Lib\idlelib\idle.bat"
    subprocess.run([idle_path, filename], shell=True)

def main():
    """Creates .bat and .py file in cwd. Auto opens .py file"""

    filename = handle_input()
    cwd = os.getcwd()
    python_file = os.path.join(cwd, filename + ".py")

    # ensure an existing python file is not overwritten
    if not os.path.exists(python_file):
        # make .bat file named "filename" in cwd
        make_batch(filename, cwd)
        # make .py file
        make_py(python_file)
        # open .py file in idle
        open_py_idle(python_file)
    else:
        print("\n.py file already exists: {}".format(python_file))

    print("\n*****Done*****\n")

if __name__ == "__main__":
    main()

This is rather good code: no blatant style issues, doctrings for functions and some comments (albeit some are unnecessary). Well done.

Now for the critics:

traditional shebang would rather look like #!/usr/bin/python3 or #!/usr/bin/env python3;
the comments you put between the shebang and the imports should be the module docstring instead;
use 4 spaces per indentation level rather than tabs;
since you are creating files in the current working directory, you can check for file existence with the filename only; thus you can do it early and generate an argparse error instead of "rolling your own" ;
you could make the path to idle an optional parameter of the CLI instead of hardcoding your path to it;
I like to use the file argument of the print function to visualize the layout of the output file better. But for the python template, using a multiline string should be an improvement:

#!/usr/bin/env python3

"""Takes a command line filename and creates a batch file and .py file
named "filename" in the cwd. The python file contains a basic script
framework. In additon, the python file will be automatically opened.
"""

import argparse
import os
import subprocess

PYTHON_TEMPLATE = '''\
#!/usr/bin/env python3

"""DOCSTRING"""

import logging

logging.basicConfig(
    level=logging.DEBUG,
    format="%(asctime)s - %(levelname)s - %(message)s")

def main():
    pass

if __name__ == "__main__" :
    main()'''

def handle_input():
    """Parses user commandline input"""

    parser = argparse.ArgumentParser()
    parser.add_argument(
            'filename',
            help='The name of the files to '
                 'create minus any extensions.')
    parser.add_argument(
            '-i', '--idle-path',
            help='The path to the IDLE executable',
            default=r'C:\Users\Dave\AppData\Local\Programs'
                    r'\Python\Python35-32\Lib\idlelib\idle.bat')
    args = parser.parse_args()

    filename = args.filename
    # ensure an existing python file is not overwritten
    if os.path.exists(filename+'.py'):
        parser.error('file already exist: {}.py'.format(filename))
    return filename, args.idle_path

def make_batch(filename: str, cwd: str):
    """Makes a batch file name "filename" in "cwd"""

    filepath = os.path.join(cwd, filename + ".bat")
    with open(filepath, "w") as f:
        print('@py.exe', os.path.join(cwd, filename+'.py'), '%*', file=f)
        print('@pause', file=f)
    print('Created batch file:', filepath)

def make_py(filepath: str):
    """Makes a .py file in cwd written with a basic skelton script"""

    # *****I realize it's a simpler task to copy a template from a .
    # py file created for the purpose and write that to the new .py*****

    with open(filepath, "w") as f:
        print(PYTHON_TEMPLATE, file=f)
    print('Created .py file:', filepath)

def open_py_idle(filename: str, idle_path: str):
    """Opens a .py file in IDLE using subprocess"""

    subprocess.run([idle_path, filename], shell=True)

def main():
    """Creates .bat and .py file in cwd. Auto opens .py file"""

    filename, idle_path = handle_input()
    cwd = os.getcwd()
    python_file = os.path.join(cwd, filename + '.py')

    make_batch(filename, cwd)
    make_py(python_file)
    open_py_idle(python_file, idle_path)

    print('\n*****Done*****\n')

if __name__ == "__main__":
    main()


Using ref for value types I chanced upon this code written by the Solution Architect for an MS CRM project and I am lost for words. Am I going crazy or is this code OK?

string returnedOptionSetStringValue=string.Empty;
int returnedInt = 0;

Utils.RetrieveOptionSetLabelOrValue(CRMAccess.xrmService, Contact.EntityLogicalName, "new_status", optionSetValue.Value, string.Empty, ref returnedOptionSetStringValue, ref returnedInt, CRMAccess.tracerService);

The method within the Utils class is as follows

public static void RetrieveOptionSetLabelOrValue(IOrganizationService CrmWebService, string EntityName, string AttributeName, int OptionSetValue, string optionSetText, ref string returnedText, ref int returnedNumber, ITracingService tracerService)
{
    string returnLabel = string.Empty;
    tracerService.Trace("starting in function ");
    OptionMetadataCollection optionsSetLabels = null;

    tracerService.Trace("in retrieve option set label with values:" + OptionSetValue + " and text " + optionSetText);

    optionsSetLabels = RetrieveOptionSetMetaDataCollection(ref CrmWebService, EntityName, AttributeName);

    foreach (OptionMetadata optionMetdaData in optionsSetLabels)
    {
        tracerService.Trace("now in loop with  " + optionMetdaData.Label.UserLocalizedLabel.Label + " and " + optionMetdaData.Value.Value);
        //we have number we need text from optionset
        if (OptionSetValue != 0)
        {
            if (optionMetdaData.Value == OptionSetValue)
            {
                returnedText = optionMetdaData.Label.UserLocalizedLabel.Label;
                break;
            }
        }
        //we have text we need number from optionset
        else if (optionSetText != String.Empty)
        {

            if (optionMetdaData.Label.UserLocalizedLabel.Label == optionSetText)
            {
                returnedNumber = optionMetdaData.Value.Value;
                break;
            }

        }
    }
}

To answer your question, to me this is an illegitimate use of ref on a value type parameter.

Here's why:

1) Legit usages of ref are when you need to keep track the value of ref parameter or when passing the value is expensive. e.g. when using recursion to traverse a tree and you need to keep track of its depth.

Node<T> FindNode(T root, int nodeValue, ref int depth);

2) The code by the Solution Architect looks confusing and weird because it should be using out parameters instead (not to mention that it smells and looks like it deserves some refactoring).

Unlike ref params, out params need not be initialized prior being passed; which is why returnedOptionSetStringValue and returnedInt are being initialized.

For further reading:
http://msdn.microsoft.com/en-us/library/t3c3bfhx(v=vs.80).aspx
https://stackoverflow.com/questions/1516876/when-to-use-ref-vs-out


Remove matched Rows from two Datatable Here I have two DataTables:

dtTotalDateRangeExcludeSundays
dtAbsentsLeavesHolidaysWorks

Each has 10k+ values, and I need to delete matched rows.

foreach (DataRow rw in dtTotalDateRangeExcludeSundays.Select())
 {
    DateTime dateFromRange=Convert.ToDateTime(rw[0].ToString());
    string strPrnt = dateFromRange.ToShortDateString();
    foreach (DataRow row in dtAbsentsLeavesHolidaysWorks.Select())
    {
      DateTime getDate = Convert.ToDateTime(row[0].ToString());
      string strchild = getDate.ToShortDateString();
       if (strPrnt == strchild)
       {
          rw.Delete();
        }
       }
 }

I want to know if any better alternative suggestion, because the foreach loop takes time when bulk data is there.  I am not good with Linq, so I want to know if any Linq tricks work.

Assuming that you load the data into your DataTable instances from database, it's better to issue a direct DELETE FROM ... query over database rather than iterate on a client.

As a side note - it is an awful practice to compare dates by comparing strings. If your table has a DateTime column it's much better to cast the value to DateTime and compare typed dates like that:

DateTime dateFromRange = ((DateTime)rw[0]).Date;
....

    DateTime getDate = ((DateTime)row[0]).Date;
    if (dateFromRange == getDate)
....


Reading n chars from stream to string I need to read n chars from a binary file into a string. Currently, what I do is:

static string Read(istream &stream, uint32_t count)
{
    auto bytes = unique_ptr<char[]>(new char[count]);

    stream.read(bytes.get(), count);

    return string(bytes.get(), count);
}

I found the way I deal with the array of chars quite messy. If I used new and delete[] directly, it would make the code messy in another way (I would need to add a local variable for the result). And I'm trying to avoid delete as a general rule.

Is there a clear way to write this code? The fact that it uses twice as much memory as it needs is probably not a big deal, but fixing that would be nice too.

Why not:

static string Read(istream &stream, uint32_t count)
{
    std::string result(count, ' ');
    stream.read(&result[0], count);

    return result;
}

Though not strictly C++03 compatible that is easily validated. One of the reasons the committee found it easy to add the new constraint in C++11 was that no implementation did not use contiguous memory (Random Access Iterators are a hint).

But a C++03 strict implementation would be:

static string Read(istream &stream, uint32_t count)
{
    std::vector<char> result(count);  // Because vector is guranteed to be contiguous in C++03
    stream.read(&result[0], count);

    return std::string(&result[0], &result[count]);
}


Clockwise spiral traversal of 2D input matrix I found this interview question online at Pramp:

  Given a 2D array (matrix) inputMatrix of integers, create a function spiralCopy that copies inputMatrix’s values into a 1D array in a spiral order, clockwise. Your function then should return that array. Analyze the time and space complexities of your solution.

  Example:

inputMatrix = [
    [1, 2, 3, 4, 5],
    [6, 7, 8, 9, 10],
    [11, 12, 13, 14, 15],
    [16, 17, 18, 19, 20]
]

  output:

[1, 2, 3, 4, 5, 10, 15, 20, 19, 18, 17, 16, 11, 6, 7, 8, 9, 14, 13, 12]

  Constraints:

  1 ≤ inputMatrix[0].length ≤ 100
  1 ≤ inputMatrix.length ≤ 100

My solution

def spiral_copy(inputMatrix):
    output = []

    top_row = 0
    bottom_row = len(inputMatrix) - 1
    left_col = 0
    right_col = len(inputMatrix[0]) - 1

    while top_row <= bottom_row and left_col <= right_col:

        for i in range(left_col, right_col + 1):
            output.append(inputMatrix[top_row][i])
        top_row += 1

        for i in range(top_row, bottom_row + 1):
            output.append(inputMatrix[i][right_col])
        right_col -= 1

        if top_row > bottom_row: break

        for i in range(right_col, left_col - 1, -1):
            output.append(inputMatrix[bottom_row][i])
        bottom_row -= 1

        if left_col > right_col: break

        for i in range(bottom_row, top_row - 1, -1):
            output.append(inputMatrix[i][left_col])
        left_col += 1

    return output

Passed these Test cases:

Input
[[1,2],[3,4]]
Expected Result
[1, 2, 4, 3]
Input
[[1,2],[3,4]
Expected Result
[1, 2, 4, 3]

Input
[[1,2,3,4],[5,6,7,8],[9,10,11,12]]
Expected Result
[1, 2, 3, 4, 8, 12, 11, 10, 9, 5, 6, 7]

input
[[1,2],[3,4]
Expected Result
[1, 2, 3, 4, 5, 6, 12, 18, 17, 16, 15, 14, 13, 7, 8, 9, 10, 11]

Input
[[1,0],[0,1]]
Expected Result
[1, 0, 1, 0]

Input
[[1,2,3],[4,5,6],[7,8,9]]
Expected Result
[1, 2, 3, 6, 9, 8, 7, 4, 5]

You’re being a bit verbose, adding elements one at a time in a loop to the output array.  You can use .extend() to add several elements at once, and use a slice to extract the required elements from the inputMatrix rows:

output.extend( inputMatrix[top_row][left_col:right_col+1] )

Unfortunately, you can’t slice a column (without using numpy), but you can still use list comprehension:

output.extend( inputMatrix[row][right_col] for row in range(top_row, bottom_row+1) )

Adding the elements at the bottom of the spiral would require a slice using a negative step, but left_col-1 will initially evaluate to -1 so inputMatrix[bottom_row][right_col:left_col-1:-1] doesn’t work as desired.  You’d need to special case left_col == 0 and use the slice [right_col:None:-1] or [right_col::-1].  Alternately, you could use list comprehension.

Going up the left side is exactly like going down the right side: use list comprehension.

output.extend( inputMatrix[row][left_col] for row in range(bottom_row, top_row-1, -1) )

Notice I used row, instead of i, as the loop index.  I feel this makes the code a little clearer.

An alternative method of doing the spiral copy would be to separate the index generation from the copy.

output = [ inputMatrix[r][c] for r, c in spiral(len(inputMaxtrix), len(inputMatrix[0])) ]

where spiral(rows, cols) is a generator function for the spiral coordinates.  Writing the spiral generator left as an exercise to the student.