Stocks of iPods for various countries A company sells iPods online. There are 100 stocks maintained at
  Argentina and Brazil. A single iPod costs $100 in Brazil and $50 in
  Argentina. The cost of exporting stocks from one country to the other
  will cost $400 per 10 blocks. The transportation is always done in the
  multiples of 10. Calculate the minimum cost for purchasing the given
  no of units. Assume that for each transaction, the available stock in
  each country is maintained to be 100.

  Input and output format:

  The country from which the order is being placed: No of units
  Minimum costs:No of stock left in Brazil: No of stock left at Argentina

  Sample input and output:

Brazil:5

500:95:100

Brazil:50

4500:100:50

class Country:
     shp=40
     def country_details(self,numberofitems,sellingprice):
            self.sp=sellingprice
            self.no=numberofitems
     def display(self):
            print self.sp
            print self.no
            print self.shp

Brazil=Country()
Brazil.country_details(100,100)
Brazil.display()

Argentina=Country()
Argentina.country_details(100,50)
Argentina.display()

print "Enter the country from which you order from"
x=raw_input()
if x=="Brazil":
    c=1
else:
    c=2

print "Enter the amount of products needed"
y=raw_input()
y=int(y)

def profit(y):
    if c==1:
        if y%10==0:
            P=orderfromArgentinafb(y);
        else:
            P=orderfromBrazil(y);
    if c==2:
        P=orderfromArgentina(y);

    return P

def calculate(y):
    if (y<101):
        Pr=profit(y)
        return Pr
    else:
        Pr=profit(y-(y%100))+orderfromBrazil(y-100)
        return Pr

def orderfromBrazil(y):
    Cost=y*Brazil.sp
    if c==2:
        Cost=Cost+y*Brazil.shp
    Brazil.no=Brazil.no-y
    return Cost

def orderfromArgentina(y):
    Cost=y*Argentina.sp
    Argentina.no=Argentina.no-y
    return Cost

def orderfromArgentinafb(y):
    Cost=y*Argentina.sp+(y*Argentina.shp)
    Argentina.no=Argentina.no-y
    if Argentina.no <0:
        Argentina.no=0
    return Cost

Pr=calculate(y)
print Pr

print  "%d:%d:%d" %(Pr,Brazil.no,Argentina.no)

You can write

if raw_input("Enter the country from which you order from") == "Brazil":
    c = 1
else:
    c = 2

instead of

print "Enter the country from which you order from"
x=raw_input()
if x=="Brazil":
    c=1
else:
    c=2

as you are not using x anywhere. I less variable and raw_input has optional string so why not use that?

In function profit instead of using

if y%10==0:
    P=orderfromArgentinafb(y);
else:
    P=orderfromBrazil(y);

you can use

if y % 10:
    return orderfromBrazil(y)
else:
    return orderfromArgentinafb(y)

You don't need the variable P when you are just returning it.
Why are there semicolons at the end? This isn't C
Notice the changing the order of return. It gives the same result but with one less comparison.

Same thing can be done in function calculate.

Use if...elif... in case you are just doing one of the things. Using as it is leads to more comparisons than are needed.

In both the functions orderfromArgentina and orderfromArgentinafb a simple reordering of statements can lead to removal of additional variable Cost.

You need to read a style guide. Check for official tutorials of Python. They contain brief style guide. That is also an optimization. It doesn't effect memory or time complexity but it leads to better maintenance


Reverse Polish Notation shell interpreter with one variable in C I wrote an RPN shell stack-based interpreter. It supports following operators: +, -, *, /, %, ^. Right now it only works with positive numbers. It features also printing out the top stack element by entering new line character and popping out the result with = operator. I decided to also add one variable available to the user, it is the x variable, which can be used in two states. X (capital X) is used for reading the value of the variable and x (lowercase x) is used for writing the value to the variable.

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

#define CRESET   "\033[0m"
#define CYELLOW  "\033[33m"
#define CRED     "\033[31m"

#define MAXOP 100
#define NUMBER '0'
#define MAXSTACK 100
#define BUFSIZE 100

int getop(char []);
void push(double);
double top(void);
double pop(void);
int getch(void);
void ungetch(int);

int sp = 0;
double stack[MAXSTACK];
char buf[BUFSIZE];
int bufp = 0;
double x = 0;

main () {
    int type;
    double _op;
    char s[MAXOP];

    while ((type = getop(s)) != EOF) {
        switch (type) {
            case NUMBER:
                push(atof(s));
                break;
            case '+':
                push(pop() + pop());
                break;
            case '*':
                push(pop() * pop());
                break;
            case '-':
                _op = pop();
                push(pop() - _op);
                break;
            case '^':
                _op = pop();
                push(pow(pop(), _op));
                break;
            case '/':
                _op = pop();
                if (_op != 0.0)
                    push(pop() / _op);
                else
                    printf(CRED "ERROR: Cannot divide by 0\n" CRESET);
                break;
            case '%':
                _op = pop();
                if (_op != 0.0)
                    push(fmod(pop(), _op));
                else
                    printf(CRED "ERROR: Cannot divide by 0\n" CRESET);
                break;
            case 'x':
                x = pop();
                break;
            case 'X':
                push(x);
                break;
            case '\n':
                printf(CYELLOW "Top: %.8g\n" CRESET, top());
                break;
            case '=':
                printf(CYELLOW "Result: %.8g\n" CRESET, pop());
                break;
            default:
                printf(CRED "ERROR: Unknown command %s\n" CRESET, s);
                break;
        }

    }

}

void push(double f) {
    if (sp < MAXSTACK)
        stack[sp++] = f;
    else
        printf(CRED "ERROR: Stack is full, cannot push %g\n" CRESET, f);
}

double top(void) {
    if (sp > 0)
        return stack[sp - 1];
    else {
        return 0.0;
    }

}

double pop(void) {
    if (sp > 0)
        return stack[--sp];
    else {
        printf(CRED "ERROR: Stack is empty\n" CRESET);
        return 0.0;
    }

}

int getop(char s[]) {
    int i, c;

    while ((s[0] = c = getch()) == ' ' || c == '\t')
        ;

    s[1] = '\0';

    if (!isdigit(c) && c != '.')
        return c;
    i = 0;
    if (isdigit(c))
        while (isdigit(s[++i] = c = getch()))
            ;
    if (c == '.')
        while (isdigit(s[++i] = c = getch()))
            ;
    s[i] = '\0';
    if (c != EOF)
        ungetch(c);
    return NUMBER;

}

int getch(void) {
    return (bufp > 0) ? buf[--bufp] : getchar();
}

void ungetch(int c) {
    if (bufp >= BUFSIZE)
        printf(CRED "UNGETCH: Too many characters\n" CRESET);

  else
        buf[bufp++] = c;

}

Samples

Input:

> 2 2 + =

Output:

Result: 4
Top: 0

Input:

> 3 x
> X 5 +

Output:

Top: 8

Input (x = 3):

> X 3 ^ 2 +

Output:

Top: 29

Could you review my code? Are there any issues I missed regarding the user input?

Here are some ideas for improving your program.

Use the correct prototype for main

Many compilers will default to an int return type when the function return type is undeclared, but it's not a good idea to omit it, especially with main.  Instead of this:

main () {

write this:

int main() {

Eliminate global variables where practical

The code declares and uses 5 global variables.  Global variables obfuscate the actual dependencies within code and make maintainance and understanding of the code that much more difficult.  It also makes the code harder to reuse.  For all of these reasons, it's generally far preferable to eliminate global variables and to instead pass pointers to them.  That way the linkage is explicit and may be altered more easily if needed.  In this case, I'd suggest gathering everything together into a structure within main's scope and then passing that structure to each of the functions.  For example, it might be defined like this:

typedef struct stack_s {
    int sp;
    int bufp;
    double x;
    double stack[MAXSTACK];
    char buf[BUFSIZE];
} StackMachine;

And then within main it could be defined like this:

StackMachine sm = { .sp = 0, .bufp = 0 };

Then push could be reimplemented like this:

void push(StackMachine *sm, double f) {
    if (sm->sp < MAXSTACK)
        sm->stack[sm->sp++] = f;
    else
        printf(CRED "ERROR: Stack is full, cannot push %g\n" CRESET, f);
}

Be consistent with the user interface

When the user uses an empty line to see the top of the stack, it reports 0 instead of STACK EMPTY when the stack is empty.  This is inconsistent with the other operations.


TamperMonkey Script to replace the YouTube favicon with the channel logo I felt that having a plain red favicon for every youtube tab is a little boring. So, I wrote a TamperMonkey Script with the new ES6 promises that replaces the favicon with the channel's logo. Link to the script here.

The script works by first extracting the channel logo's URL from the window.ytInitialData object. Then, a mutation listener is attached to the title, and whenever the title of the page changes, the script makes a GET request using the window.location.href and then extracts the logo's URL from the responseText.

Since this is my first time writing a userscript, I wanted to confirm that I am following best practices and not breaking anything on the site. Also, is there a better way of achieving the same result?

Thank you for your time.

My code:

// ==UserScript==
// @name        YouTube Favicon to Channel logo
// @version     1.1
// @description Changes the YouTube Favicon to the Channel logo
// @author      Kartik Soneji
// @updateURL   https://openuserjs.org/meta/KartikSoneji/YouTube_Favicon_to_Channel_logo.meta.js
// @match       https://www.youtube.com/*
// @exclude     https://www.youtube.com/
// @exclude     https://www.youtube.com/tv*
// @exclude     https://www.youtube.com/embed/*
// @exclude     https://www.youtube.com/live_chat*
// @grant       none
// @run-at      document-end
// @license     MIT
// ==/UserScript==

(function(){
    'use strict';

    const urlRegex = /https:\/\/yt3.ggpht.com\/a[/-]{1,2}[A-Za-z\d-_]+/;

    function isInIframe(){
        try{
            return window.self !== window.top;
        }
        catch(e){
            return true;
        }
    }
    if(isInIframe()) return;

    console.log("Started");

    roundImageToDataUrl(JSON.stringify(window.ytInitialData).match(urlRegex)[0]).then(setFavicon);

    function run(){
        getChannelLogo(window.location.href).then(roundImageToDataUrl).then(setFavicon);
    }

    async function getChannelLogo(url){
        return new Promise((resolve, reject) => {
            let xhr = new XMLHttpRequest();
            xhr.open("GET", url);
            xhr.onload = () =>{
                let u = xhr.responseText.match(urlRegex)[0];
                console.log(u);
                resolve(u);
            }
            xhr.send();
        });
    }

    let observer = new MutationObserver(run);

    function registerUrlChangeListener(){
        let e = document.querySelector("title");
        if(e){
            //observer.observe(e, {attributes: true});
            observer.observe(document.querySelector("title"), {attributes: true, characterData: true, childList: true});
            return;
        }

        setTimeout(registerUrlChangeListener, 250);
    }
    registerUrlChangeListener();

    async function roundImageToDataUrl(url){
        return new Promise((resolve, reject) => {
            if(url.src) url = url.src;

            let img = document.createElement("img");
            img.crossOrigin = "anonymous";
            img.src = url;
            img.style = "position: absolute; top: -100000px; left: -100000px;";

            img.onload = function(){
                let canvas = document.createElement("canvas"), g = canvas.getContext("2d");

                canvas.height = img.naturalHeight;
                canvas.width = img.naturalWidth;

                g.beginPath();
                g.arc(canvas.width/2, canvas.height/2, canvas.width/2, 0, Math.PI * 2);
                g.clip();

                g.drawImage(img, 0, 0);

                img.parentElement.removeChild(img);

                resolve(canvas.toDataURL());
            }

            document.body.appendChild(img);
        });
    }

    function setFavicon(url){
        let a = document.querySelectorAll("link[rel *= 'icon']");

        if(a.length == 0){
            let link = document.createElement('link');
            link.type = 'image/x-icon';
            link.rel = 'icon';
            document.head.appendChild(link);
            a = [link];
        }

        for(let i of a) i.href = url;
    }
})();

img.style = "position: absolute; top: -100000px; left: -100000px;";
document.body.appendChild(img);

img.parentElement.removeChild(img);

This seems weird.  The canvas can be drawn from an unattached image and these lines can be removed.

  if(url.src) url = url.src;

  img.src = url;

A more idiomatic phrasing is img.src = url.src || url.

 .querySelectorAll("link[rel *= 'icon']");

Unless you have something specific in mind, I'd use [rel='icon'] here.  It's faster, and not clear that matching an exotic future variant like <link rel="BiscottiConfig"> is going to be desirable.

let

Except for the final loop, every use of let in your script can be replaced by const.  It's a good habit, even if the aesthetics are inferior to let or var.

const urlRegex = /https:\/\/yt3.ggpht.com\/a[/-]{1,2}[A-Za-z\d-_]+/;

roundImageToDataUrl(JSON.stringify(window.ytInitialData).match(urlRegex)[0]).then(setFavicon);

This looks like a misstep.  Youtube avatars have id=img; why not just roundImageToDataUrl( document.getElementById("img").src )?

Dealing with future changes to the host page is a constant challenge with userscripts.  There are many things that have to remain unchanged for this to work (the image host, the URL format, the name, content and encoding of ytInitialData) and I'll predict that some of them won't.  Selecting the avatar itself with semantic markup is likely to be less brittle, and easier to fix if the markup changes (assuming there will always be an avatar and it will always have some semantics attached to it).


Best way to handle elements of a private array This feels like a simple question, but I'm wondering what the best way would be to manage the elements of a private, fixed-size array.

Context: I'm making a simple particle engine, and my Emitter class will smoothly interpolate between up to 8 colors per Particle, depending on the Particle's Life value. I have that part implemented and it works great. However, my question is more concerning the Color[] array itself.

Suppose I have the following code. What I would like to do is be able to alter the 8 values in a simple and logical way:

public sealed class Emitter
{
    private readonly Color[] _colors = new Color[8];
}

One option would be to return the whole array, but according to CA1819, a property should not return an array, since it warns the array is mutable unless a copy is made. However, this is the behavior I want, so it might make sense to do it:

public Color[] Colors
{
    get { return _colors; }
}

Another option would be to do a simple get/set and pass a desired index as a parameter; this has the benefit of checking for success, but is more cumbersome to use because of the out parameter. In addition, there's nothing that explicitly states the min/max values of the color array, which means I'd have to have a const byte MaxColors = 8; or the like:

public bool GetColor(int id, out Color result)
{
    if (id >= 0 && id < 8)
    {
        // Valid index, return the color at that position
        result = _colors[id];
        return true;
    }
    // Invalid index, return the default color (R:0, G:0, B:0, A:0)
    result = default(Color);
    return false;
}

public void SetColor(int id, Color value)
{
    if (id >= 0 && id < 8)
    {
        _colors[id] = value;
    }
}

public void SetColor(param Color[] values)
{
    // Set all 8 colors, using default values if not enough arguments are supplied
    if(values != null)
        for(int i = 0; i < 8; i++)
            if (i > values.Length)
                _colors[i] = default(Color);
            else
                _colors[i] = values[i];
}

Finally, I could do the brute force approach and make a property for each color value, and I probably don't need to explain why that's a bad idea:

public Color Color1
{
    get { return _colors[0]; }
    set { _colors[0] = value; }
}
...
public Color Color8
{
    get { return _colors[7]; }
    set { _colors[7] = value; }
}

So... yeah. Based on my needs, which approach seems to make the most sense? I'm leaning towards the getter/setter pattern. Is there another way I haven't considered that would be desireable in this situation?

If you expose it via a public property then it's not a "private" array any more:it's part of the public interface.

If resetting its elements is something that you want clients to do, I'd choose the first option (because it's simplest).

Or, these ...

public void SetColor(int id, Color value) { ... }
public bool GetColor(int id, out Color result) { ... }

... could perhaps be done using indexers ...

public Color this[int id]
{
    get { ... }
    set { ... }
}

... but that's not very different from exposing the array (and there can be only one indexer of a given signature, which doesn't work if there's more than one array to be exposed).

Here's a blog post on the topic: http://blogs.msdn.com/b/ericlippert/archive/2008/09/22/arrays-considered-somewhat-harmful.aspx

Another possibility (I've just thought of it, haven't tested/researched this idea) might be to pass the array into the constructor:

public sealed class Emitter
{
    private readonly Color[] _colors;
    public Emitter(Color[] colors) { _colors = colors; }
}

The reason excuse for that is that so that whoever constructs Emitter has the array, can keep that array, and can (evilly) modify that array after it has used it to construct Emitter.

Color[] colors = new Color[8];
Emitter emitter = new Emitter(colors);
colors[0] = Color.Red; // changes constents of array inside the Emitter

The official recommendation is to use a List<Color> instead of a Color[] array, and return IList<Color> ... conceptually similar IMO, but you won't get the compiler and FxCop warnings.

public sealed class Emitter
{
    private readonly List<Color> _colors;
    public IList<Color> Colors { get { return _colors; } }
}

As @svick noted in a comment below, because arrays implement the IList<T> interface, you can also do this:

public sealed class Emitter
{
    private readonly Color[] _colors;
    public IList<Color> Colors { get { return _colors; } }
}

IList<T> supports the API you want (i.e. an indexer property). Using IList<T> supresses the compiler warning. It may be less intuitive to the client/user though (who may think, incorrectly, that the IList<T> would allow them to change its size).


Getting initial values of non-primitive types from another Javascript file? I have a file where I keep some initial values for some form fields base on field type. Some of them are primitive values, like '' or 0 or false but some of them are non-primitive reference values, like [] or {}. For example:

INITIAL_VALUES.js

const INITIAL_VALUES= {
  SINGLE_CHOICE: '',
  MULTIPLE_CHOICE: [],
  OBJECT: {},
  BOOLEAN: false,
  NUMBER: 0,
  STRING: ''
};

export default INITIAL_VALUES;

The fact is that I have to be careful when using non-primitive initial values, because since they come as reference, I might change them, and get them a second time with some residual value, instead of being empty.

I've tought about some options to deal with this:

OPTION #1

Make the INITIAL_VALUES.js export a function that returns the new values object and call that function inside my component.

Example:

getInitialValues.js

function getInitialValues() {
  return({
    SINGLE_CHOICE: '',
    MULTIPLE_CHOICE: [],
    OBJECT: {},
    // and so on...
  });
}

export default getInitialValues;

SomeComponent.js

import getInitialValues from './getInitialValues'

function SomeComponent() {
  const INITIAL_VALUES = getInitialValues();
}

OPTION #2

Keep the initial values as an object and everytime I use them, I need to check if it's a non-primitive and create a new one (instead of using the same reference from INITIAL_VALUES).

Example:

SomeComponent.js

myVariable = INITIAL_VALUES[fieldType];

if (Array.isArray(myVariable)) {
  myVariable = [...myVariable];                   // CREATE A NEW ARRAY
}
else if (typeof(myVariable)  === 'object') {
  myVariable = {...myVariable};                   // CREATE A NEW OBJECT
}

OPTION #3

Use only non-primitive values. And set null instead of empty arrays [] and empty objects {}.

Example:

INITIAL_VALUES.js

const INITIAL_VALUES= {
  SINGLE_CHOICE: '',
  MULTIPLE_CHOICE: null,
  OBJECT: null,
  BOOLEAN: false,
  NUMBER: 0,
  STRING: ''
};

But doing so, whenever I try to populate those fields, I'll need to check if the object/array exists, and create it if it doesn't.

Example

SomeComponent.js

function pushValueToArrayField(newValue) {
  setState((prevState) => {
    const aux = {...prevState};
    if (aux.someArrayField === null) {
      aux.someArrayField = [];
    }
    aux.someArrayField.push(newValue);
    return aux;
  });
}

What is my best option here?

So here's what I've ended up doing:

INITIAL_VALUES.js

function GET_FEATURE_TYPES_INITIAL_VALUES() {
  const FEATURE_TYPE_INITIAL_VALUES = {
    SINGLE_CHOICE: '',
    MULTIPLE_CHOICE: [],
    BOOLEAN: false,
    NUMBER: 0,
    STRING: ''
  };
  return FEATURE_TYPE_INITIAL_VALUES;
}

function GET_FILTER_TYPES_INITIAL_VALUES() {
  const FILTER_TYPE_INITIAL_VALUES = {
    NUMBER_RANGE_ALLOW_MULTIPLE: [],
    NUMBER_ACUM_RANGE_ALLOW_SINGLE: [],
    SINGLE_CHOICE_ALLOW_SINGLE: [],
    SINGLE_CHOICE_ALLOW_MULTIPLE: [],
    MULTIPLE_CHOICE_ALLOW_MULTIPLE: [],
    BOOLEAN_ALLOW_MULTIPLE: [],
    STRING_ALLOW_MULTIPLE: []
  };
  return FILTER_TYPE_INITIAL_VALUES;
}

function GET_FIELD_TYPES_INITIAL_VALUES() {
  const FIELD_TYPE_INITIAL_VALUES = {
    STRING_FIELD: '',
    BOOLEAN_FIELD: '',
    NUMBER_FIELD: 0,
    ARRAY_STRINGS_FIELD: [],
    ARRAY_OBJECTS_FIELD: [],
    OBJECT_FIELD: {}
  };
  return FIELD_TYPE_INITIAL_VALUES;
}

export {
  GET_FEATURE_TYPES_INITIAL_VALUES,
  GET_FILTER_TYPES_INITIAL_VALUES,
  GET_FIELD_TYPES_INITIAL_VALUES
};

SomeComponent.js

import { GET_FEATURE_TYPES_INITIAL_VALUES } from '@constants/INITIAL_VALUES';

function SomeComponent() {
  const FEATURE_TYPES_INITIAL_VALUES = GET_FEATURE_TYPES_INITIAL_VALUES();

  // FRESH NEW INITIAL_VALUES AVAILABLE ON EVERY RENDER
}


Ruby class that validates a string I have this class that needs to do three things:

Determine if a string is 8 characters or longer
Determine if a string has a number in it
Determine if a string has a capital letter in it

NB: I don't want to use regex. Is there a better way of doing this?

class ValidateString
  def initialize(string)
    @string = string

    if valid
      puts @string
    end
  end

  def valid
    if has_number? and has_upper? and has_length?
      return @string
    end
  end

  def has_number?
    @string.count("0-9") > 0
  end

  def has_upper?
    @string =~ /[A-Z]/
  end

  def has_length?
    @string.length >= 8
  end
end

ValidateString is strange name for a class. StringValidator would be more apt.
Don't make your constructor print anything. Your class exists to validate a string, not print things to stdout. Leave that to some other piece of code.
valid should be named valid? and return a boolean. And you can omit the return keyword.
Add attr_reader :string, and replace your @string with just string
8 is a magic number, and should be replaced with constant (e.g. MIN_STRING_LENGTH)
Presumably, this exists to check for simple passwords. However, you're not checking that the string contains both upper- and lowercase letters. An all-uppercase string with a number would pass validation. For passwords, you'd want to enforce that it's mixed casing.

I'd just write a method:

def valid_string?(string)
  return false unless string.length >= 8
  return false unless string =~ /[0-9]/
  return false unless string =~ /[[:upper:]]/ # allows non-Latin letters
  return false unless string =~ /[[:lower:]]/ # ditto
  true
end

Of course, you'd be better off using something like zxcvbn to check password entropy. Your rules allow for the good ol' Password1 string just fine.


Counting subnumbers divisible by a given prime Assume we are given a number and a prime p. The goal is to count all subnumbers of number that are divisible by p. A subnumber of number is formed of any number of consecutive digits in the decimal representation of number. Here is my solution:
number = input()
p = int(input())
print([int(number[i : j]) % p for i in range(len(number)) for j in range(i + 1, len(number) + 1)].count(0))

Can this solution be made faster?
Here is the original source of the problem.

Let \$N_{i,j}\$ be the subnumber of \$N\$ from the \$i\$th digit (excluded) to the \$j\$th digit (included). Here, the first digit is the least significant one.
Let  \$R_i = N \bmod 10^i \$. We have \$R_j-R_i=N_{i,j} * 10^i, 0\leq i < j \leq D(N)\$, where \$D(N)\$ is the number of digits in \$N\$. There are two cases.

If \$p \nmid 10\$, then
$$p \mid N_{i,j} \Leftrightarrow p\mid N_{i,j} * 10^i \Leftrightarrow p\mid R_j-R_i \Leftrightarrow R_j \equiv R_i \pmod{p}$$

In this case, we can compute \$R_i\bmod p\$ for all \$i=0,\ldots,D(N)\$, group them by values, count the number of pairs in each group and then take the sum. (Note: the implementation can simply count the size of each group \$|G|\$ using itertools.Counter and compute the number of pairs \$C_{|G|}^2\$)

If \$p\mid 10\$, we have either \$p = 2\$ or \$p = 5\$. In this case,
$$p \mid N_{i,j} \Leftrightarrow p \mid N_{i+1} $$
Here, \$N_{i+1}\$ is the \$(i+1)\$th digit of \$N\$. In other words, if \$p \mid N_{i+1} \$ holds, \$p \mid N_{i,j}\$ holds for all \$j\$ s.t. \$i<j\leq D(N)\$. It is easy to show that the final result is
$$\sum_{0\leq i < D(N),\,p\,\mid\, N_{i+1}}(D(N)-i)$$

The time complexity of this algorithm is \$\Theta(D(N))=\Theta(\log N)\$, which is better than the \$\Theta(D(N)^4)\$ solution in OP (see comments for explanation). Note that the runtime of the actual program depends on your implementation.
You may try to implement this approach yourself.


Activating items in some randomized arrays I've got a list with about 120 items in IE and I have some items I want to activate. I compare, build and randomize some Arrays on the fly. The for loop in the run() function is really slow, and i tried to add a setTimeout(), like i've read somewhere on stackoverflow.

It does not seem to improve the performance, and if the requested list of items to be activated is too large, IE starts to give the slow script error.

Can i optimize this to something that'll work?

for(index = 0; index < 120; index++){
            wrapper.tempSlot.push(index);
}
Array.prototype.shuffle = function shuffle(){
    var tempSlot;
    var randomNumber;
    for(var i =0; i != this.length; i++){
        randomNumber = Math.floor(Math.random() * this.length);
        tempSlot = this[i];
        this[i] = this[randomNumber];
        this[randomNumber] = tempSlot;
    }
}

Array.prototype.compare = function(testArr) {
    if (this.length != testArr.length) return false;

    for (var i = 0; i < testArr.length; i++) {
        if (this[i].compare) {
            if (!this[i].compare(testArr[i])) return false;
        }
        if (this[i] !== testArr[i]) return false;
    }
    return true;
}

function checkAgainstEl(val){
    val = $($item.select).eq(val).data('personid')
    return val in oc([array, with, to, be, activated, numbers]);
}

var index = 0;
var length = wall.globals.highlight.selection.length;
var run = function(){
    for(;index < length; index++){

        wrapper.tempSlot.shuffle();
        var pop     = wrapper.tempSlot.pop();
        var value   = wall.globals.highlight.selection[index];
        var newItem = $('#wallItem' + value);

        wrapper.objectsStr = wrapper.objectsStr + '#wallItem' + value + ', ';

        while(checkAgainstEl(pop)){
            wrapper.tempSlot.shuffle()
            pop = wrapper.tempSlot.pop();
        }

        beforeItem  = $($item.select).eq(pop);

        newItem.insertBefore(beforeItem);
        $($container.select).append(beforeItem);

        if (index + 1 < length && index % 5 == 0) {
                setTimeout(run, 25);
            }
    }
}
run();

Interacting with the DOM is one of the slowest operations in JavaScript.  I'd suggest selecting all your elements before the loop, then referring to the cached elements inside.
On the same token, do not perform any DOM manipulation in a loop if you can help it.  Since it doesn't look like you can, another approach is to detach all DOM elements you're going to rearrange before the loop, perform the manipulation inside the loop, then re-attach them to the DOM at the end.
A reverse while loop would be faster than the for.
If you're using a version of jQuery prior to 1.6, the .data() method carries a lot of overhead.  In checkAgainstEl(), use $.data($($item.select).get(val), 'personId') instead.
It looks like you're re-wrapping jQuery objects in new jQuery objects unnecessarily, and in multiple places.  Like in #4.  I'm assuming variables prefixed with a $ are jQuery objects; if they are, no need to wrap them in $() again.

Your biggest problem is the DOM interaction inside a loop.  Cut that out any way you can and you'll be golden.


git Create Tree from Arbitrary Directory (works in bare repository) I made a script for a project I'm working on to create a tree object from a filesystem directory, without any working tree operations (can be run in a bare repository). The tree can be inserted into (a) commit(s) by using an index filter that invokes git read-tree --prefix=</...>.

#!/bin/bash
# createSubtreeRec <path>
# Creates a subtree with all the contents of path
# does not need to be in a work tree.
# Tree and related objects are added to the repository (unreachable)
# recursive.
# Prints the line that should be used to include the built tree in a parent
#   in the form used by git mktree on stdout. This might be a blob or tree.
#   <mode> <type> <hash>    <name>
#         ^space ^space    ^tab
function createSubtreeRec () {
    local path="$1"
    local hash=""
    local type=""
    local mode=""
    local treeData=""
    #echo "" >&2
    if [ -d "$path" ]; then
        for fullpath in $path/*; do
            # edited from my original post to simplify the code
            treeData="$(printf '%s\n%s' "$treeData" "$(createSubtreeRec "$fullpath")")"
        done
        # ignore empty dirs
        if [ "$treeData" ]; then
            # remove leading \n
            treeData="${treeData:1}"
            #echo "$treeData" >&2
            hash="$(echo "$treeData" | git mktree)"
            printf '040000 tree %s\t%s' $hash "$(basename "$path")"
        fi
    elif [ -f "$path" ]; then
        hash=$(git hash-object "$path" -w)
        type=blob
        mode=100644
        if [ -x "$path" ]; then
            mode=100755
        fi
        printf '%s blob %s\t%s' $mode $hash "$(basename "$path")"
    fi
    #echo "" >&2
}

# createSubtree <workdir> <path>
# Creates a subtree starting in <workdir> with the names in <path> as recursive trees,
# Use to get a tree that does not contain <workdir> but contains all of <path>
#   and all contents beneath it.
# Prints the top tree's hash on standard output.
function createSubtree () {
    local workdir="$1"
    local path="$2"
    # Get tree from
    if [ "$path" != "" ]; then
        local entry=$(createSubtreeRec "$workdir/$path")
    else
        local entry=$(createSubtreeRec "$workdir")
    fi
    if [ "$entry" ]; then
        while [ "$path" ] && [ "$path" != "." ] ; do
            local hash=$(echo "$entry" | git mktree)
            path="$(dirname "$path")"
            local entry="$(printf '%s %s %s\t%s' '040000' tree $hash $(basename "$path"))"
        done
        printf '%s\n' $hash
    fi
}

createSubtreeRec outputs the full tree data line because it can sometimes output blobs.

This code is a significant part of my runtime, so any performance improving observations would be appreciated.

Part of me just wants this available online as it would have helped me out, and I can't find any git builtin commands for this.

Consider portable shell
Do we really need Bash for this?  If we can manage without local variables and string indexing, then we could use a leaner, more portable shell instead.  Certainly, there's no benefit using the non-standard function keyword - just omit it.
We probably do want local in the recursive function, but it's good to be clear that we've made the choice for a good reason.
Add some error checking options
set -eu -o pipefail

Use more quoting
We don't want $path or $fullpath to be split:
    for fullpath in "$path"/*; do
        f=$(basename "$fullpath")

There's some more needed later:
            treeData=$(printf '%s\n%s %s %s\t%s' "$treeData" $mode $type $hash "$f")
        elif [ -d "$path/$f" ]; then
            # recurse
            treeData=$(printf '%s\n%s' "$treeData" "$(createSubtreeRec "$fullpath")")

Shellcheck can't know that $hash will always be a single token, so it recommends more quoting than is actually necessary.


Numbers of length N and value less than K I am solving interview question from here.

Problem : Given a set of digits (A) in sorted order, find how many numbers of length B are possible whose value is less than number C.
Constraints: 1 ≤ B ≤ 9, 0 ≤ C ≤ 1e9 & 0 ≤ A[i] ≤ 9
Input: A = [ 0 1 5], B=  1  , C = 2 ; Output:  2 (0 and 1 are possible)
Input: A = 0 1 2 5  , B =  2  , C = 21 ; Output: 5 (10, 11, 12, 15, 20 are possible)

This is my approach
from itertools import product
from itertools import ifilter
def solve(A, B, C):
    if A == [] or B > len(str(C)):
        return 0

    elif B < len(str(C)):
        #constraint is B
        if B == 1:
            new_list = A
            return len(new_list)
        else:
            new_list = list((product((''.join(str(i)for i in A)),repeat = B)))
            b = [''.join(num) for num in new_list]
            c = list(ifilter(lambda  x: x[0]!='0'  , b))
            return len(c)

    elif B == len(str(C)):
        #constraint is C
        if B == 1:
            new_list = [i  for i in A if i< C]
            return len(new_list)
        else:
            new_list = list((product((''.join(str(i)for i in A)),repeat = B)))
            b = [''.join(num) for num in new_list]
            c = list(ifilter(lambda  x: x[0]!='0' and int(x) < C   , b))
            return len(c)

Test cases:
assert solve([2],5,51345) == 1
assert solve([],1,1) == 0
assert solve([ 2, 3, 5, 6, 7, 9 ],5,42950) == 2592
assert solve([0],1,5) == 1
assert solve([0,1,2,5],1,123) == 4
assert solve([0,1,5],1,2) == 2
assert solve([ 3 ],5, 26110) == 0
assert solve([0,1,2,5],2,21) == 5

How can I optimize this code in terms of memory usage?

Optimise memory usage

You could optimise memory usage by not converting your iterators into list and by avoiding non-required steps (like join).

Changing a few others details (formatting, adding tests, etc), you'd get something like:

from itertools import product
from itertools import ifilter

def solve(A, B, C):
    c_len = len(str(C))
    if A == [] or B > c_len:
        return 0
    elif B < c_len:
        # Constraint is B
        if B == 1:
            return len(A)
        else:
            candidates = product((str(i) for i in A), repeat = B)
            return sum(x[0] != '0' for x in candidates)
    else:
        assert B == c_len
        # Constraint is C
        if B == 1:
            return sum(i < C for i in A)
        else:
            candidates = product((str(i) for i in A), repeat = B)
            return sum(x[0] != '0' and int(''.join(x)) < C for x in candidates)

assert solve([2],5,51345) == 1
assert solve([],1,1) == 0
assert solve([2, 3, 5, 6, 7, 9],4,42950) == 1296
assert solve([2, 3, 5, 6, 7, 9],5,42950) == 2592
assert solve([0],1,5) == 1
assert solve([0,1,2,5],1,123) == 4
assert solve([0,1,5],1,2) == 2
assert solve([3],5, 26110) == 0
assert solve([0,1,2,5],2,21) == 5

Another algorithm

I'm pretty sure the whole thing can be optimised further by not generating the various numbers to count them but just using mathematical tricks to get the solution with no counting.

The easiest case to handle is B < c_len:

elif B < c_len:
    # All combinations of B elements are valid
    return len(set(A)) ** B

Actually, as mentionned by Maarten Fabré, this does not handle 0s perfectly. The code below is updated to handle it better.

The last case is trickier. We can try to use recursion to solve smaller versions of the problem. I didn't manage to make this work properly...

from itertools import product, ifilter, dropwhile, product, takewhile
import timeit

def solve_naive(A, B, C):
    A = set(str(A))
    mini = 10 ** (B-1)
    maxi = min(10 * mini, C)
    cand = [str(i) for i in (['0'] if B == 1 else []) + range(mini, maxi)]
    valid = [i for i in cand if all(c in A for c in i)]
    return len(valid)

def solve_op(A, B, C):
    # print(A, B, C)
    c_len = len(str(C))
    if A == [] or B > c_len:
        return 0
    elif B < c_len:
        # Constraint is B
        if B == 1:
            return len(A)
        else:
            candidates = product((str(i) for i in A), repeat = B)
            return sum(x[0] != '0' for x in candidates)
    else:
        assert B == c_len
        # Constraint is C
        if B == 1:
            return sum(i < C for i in A)
        else:
            candidates = product((str(i) for i in A), repeat = B)
            return sum(x[0] != '0' and int(''.join(x)) < C for x in candidates)

def solve_maarten(A, B, C):
    if A == [] or B > len(str(C)):
        return 0
    c_tuple = tuple(map(int, str(C)))
    combinations = product(A, repeat=B)
    if B != 1:
        combinations = dropwhile(lambda x: x[0] == 0, combinations)
    if B == len(c_tuple):
        combinations = takewhile(lambda x: x < c_tuple, combinations)
    combinations = list(combinations)
    return sum(1 for _ in combinations)

def solve(A, B, C):
    c_str = str(C)
    c_len = len(c_str)
    if A == [] or B > c_len:
        return 0
    if B < c_len:
        a_len = len(set(A))
        if B == 1:
            return a_len
        non_0_len = a_len - (0 in A)
        return non_0_len * (a_len ** (B-1))
    assert B == c_len    # Constraint is C
    head, tail = int(c_str[0]), c_str[1:]
    nb_first_dig_cand = sum(i < head for i in A)
    if not tail or not nb_first_dig_cand:
        return nb_first_dig_cand
    if head in A:  # TODO: This case is not handled properly...
        # It should involve ret and solve(A, B-1, int(tail)) or something like that
        return solve_maarten(A, B, C)
    solve_c = solve(A, B-1, C)
    ret = nb_first_dig_cand * solve_c
    return ret

tests = [
    ([2], 4, 51345, 1),
    ([2], 5, 51345, 1),
    ([], 1, 1, 0),
    ([2, 3, 5, 6, 7, 9], 4, 42950, 1296),
    ([2, 3, 5, 6, 7, 9], 5, 42950, 2592),
    ([0], 1, 5, 1),
    ([0, 1, 2, 5], 1, 123, 4),
    ([0, 1, 5], 1, 2, 2),
    ([3], 5, 26110, 0),
    ([0, 1, 2, 5], 1, 21, 4),
    ([0, 1, 2, 5], 2, 21, 5),
    ([0, 1, 2, 5], 2, 201, 12),
    ([0, 1, 2, 5], 3, 2010, 48),
    ([0, 1, 2, 5], 4, 20108, 192),
    ([0, 1, 2, 5], 5, 201089, 768),
    ([0, 1, 2, 3, 4, 5, 7, 8], 5, 201089, 28672),
    ([0, 1, 2, 3, 4, 5, 7, 8], 6, 201089, 33344),
    ([0, 1, 2, 3, 4, 5, 7, 8, 9], 6, 200000, 59049),
    ([0, 1, 2, 3, 4, 5, 7, 8, 9], 6, 999999, 472391),
    ([1, 2, 3, 4, 5, 7, 8, 9], 6, 200000, 32768),
    ([1, 2, 3, 4, 5, 7, 8, 9], 6, 999999, 262143),
]
funcs = [solve, solve_op, solve_maarten, solve_naive]

for func in funcs:
    start = timeit.default_timer()
    for (A, B, C, exp) in tests:
        ret = func(A, B, C)
        if ret != exp:
            print "%s(%s, %d, %d): ret=%d, exp:%d" % (func.__name__, str(A), B, C, ret, exp)
    end = timeit.default_timer()
    print("Time for %s: %f" % (func.__name__, end - start))

def solve2(A, B, C):
    c_str = str(C)
    c_len = len(c_str)
    if A == [] or B > c_len:
        return 0
    if B < c_len:
        a_len = len(set(A))
        if B == 1:
            return a_len
        non_0_len = a_len - (0 in A)
        return non_0_len * (a_len ** (B-1))
    assert B == c_len    # Constraint is C
    head, last_dig = divmod(C, 10)
    nb_last_dig_cand = sum(i < last_dig for i in A)
    if head == 0:
        return nb_last_dig_cand
    ret = solve_naive(A, B-1, head - 1) * len(A)
    ret_dummy = solve_naive(A, B, C)
    print(ret - ret_dummy, A, B, C)
    return ret_dummy


Architecture to cache data I cache the data and use local database in my Windows Phone app. The algorithm is very simple:

Get data from DB and show in UI
Get data from a web service and show in UI
Update data in DB from the web service

Some data need to be saved in DB, and some do not need to.

For these purposes, I have the following classes.

GYDataAccessLayer is an entry point. This class sets and changes the data source.

Example:

GYDataAccessLayer da = new GYDataAccessLayer();
da.GetInfo((result, error) =>
 {
    if (error != null) { return; } // handling exception
    GYUser user = result;
 });

public class GYDataAccessLayer
{
    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYDataSource dataSource = new GYDataSource(new GYLocalData());
        dataSource.GetUserInfo(act, id);

        dataSource.SetDataSource(new GYWebData());
        dataSource.GetUserInfo(act, id);
    }
}

public class GYDataSource
{
    IGYDataAccess _gyDataAccess;

    public GYDataSource(IGYDataAccess gyDataAccess)
    {
        _gyDataAccess = gyDataAccess;
    }

    public void SetDataSource(IGYDataAccess gyDataAccess)
    {
        _gyDataAccess = gyDataAccess;
    }

    public void GetInfo(Action<GYUser, Exception> act, string uid)
    {
        _gyDataAccess.GetUserInfo(act, uid);
    }
}

I have a two classes to work with Local and Web data source. GYWebData gets data from web and calls update method in GYLocalData.

public interface IGYDataAccess
    {
        void GetUserInfo(Action<GYUser, Exception> act, string id);
    }

public class GYWebData : IGYDataAccess
{
    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYUserAPI.GetInfo(id, (result, error) =>
            {
                if (error != null)
                {
                    act.Invoke(null, error);
                    return;
                }

                act.Invoke(result.Result, null);
                GYLocalData gyLocalData = new GYLocalData();
                gyLocalData.UpdateUserInfo(result.Result);

            });
    }
}

 public class GYLocalData : IGYDataAccess
{
    private const string ConnectionString = @"isostore:/Cache.sdf";
    CacheDataContext DataBase;
    CacheDataContextProfiles DataBaseProfiles;

    private void CheckDataBase()
    {
        using (DataBase = new CacheDataContext(ConnectionString))
        {
            if (!DataBase.DatabaseExists())
            {
                DataBase.CreateDatabase();
            }
        }
    }

    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        CheckDataBase();

        using (DataBaseProfiles = new CacheDataContextProfiles(ConnectionString))
        {
            try
            {
                var user = DataBaseProfiles.GetInfo(id);
                if (user != null)
                {
                    act.Invoke(user, null);
                }
            }
            catch (Exception ex)
            {

                act.Invoke(null, ex);
            }
        }
    }

   public void UpdateUserInfo(GYUser user)
    {
        CheckDataBase();

        using (DataBaseProfiles = new CacheDataContextProfiles(ConnectionString))
        {
            DataBaseProfiles.UpdatePersonProfile(user);
        }
    }
 }

With this architecture it is inconvenient to work and hard to support. You have to write a lot of code and be very attentive (eg do not forget to call the update data method in the DB). Are there good patterns for such problems? How can I improve this code?

Let's go over the basics first!

The indentation is quite a mess. But I suppose that might be related to the SE formatting. Otherwise, you should make your indentation coherent. Stuff like :

public interface IGYDataAccess
    {
        void GetUserInfo(Action<GYUser, Exception> act, string id);
    }

shouldn't be in production code, it looks bad and gives headaches to maintainer (that could be you!)

Why are your classes prefixed of GY? Maybe there's a good reason, but I'm pretty sure your names would make just as much sense without that prefix. The pattern you're using for caching uses the Repository design pattern. It abstracts the implementation to access to data. So that pattern name should be found in your naming! IRepository, LocalDataRepository, WebDataRepository. Some might argue that it's useless, but I think it's good to see the pattern's name in the class name, so we know what we're dealing with.

Multi-line lambdas are... okay, I guess. But I think in your case they should be in a method. A lambda is an anonymous function, something that doesn't matter that much in your code. But that code is important, don't let it crawl in the shadows, that code deserves a method. (The lambda in GetUserInfo)

Don't keep your connection string as a const in your code, that's a very bad practice. It should be in a configuration file, and you should get it using the ConfigurationManager class (In System.Configuration).

private members should be camelCased, not PascalCased, so DataBase -> dataBase (In GYLocalData). Also, you should specify the private accessor, so I know it wasn't forgotten or something like that. See, if it's not there. I don't know your clear intent. Maybe you wanted this field to be public but you forgot because you were out of coffee. I'll never know because you didn't specify it.

I want to give a little warning that might be unnecessary. The CheckDatabase method isn't thread safe. That means if your GYLocalData object can be called from multiple threads at once, you might create the database twice. And that's a costly operation. That's bad.

You should only use var when you can figure the Type by reading the code. In the example below, I can't figure it out, so it's harder to know what's happening and even harder to review! :)

var user = DataBaseProfiles.GetInfo(id);

You could use some Dependancy Injection in your GYWebData class. You want to store everything that is took from the web API in a another storage. So the GYWebdata class should receive in the constructor a GYLocalData instance where you would store the information :

public class GYWebData : IGYDataAccess
{
    private GYLocalData storage;

    public GYWebData(GYLocalData storage)
    {
        this.storage = storage;
    }

    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYUserAPI.GetInfo(id, (result, error) =>
            {
                if (error != null)
                {
                    act.Invoke(null, error);
                    return;
                }

                act.Invoke(result.Result, null);
                storage.UpdateUserInfo(result.Result);
            });
    }
}

Now this isn't exactly Dependency Injection because I give a concrete class as parameter. You would need another interface, IDataUpdater, maybe :

public interface IDataUpdater
{
   void UpdateUserInfo(GYUser user);
}

The LocalData class should implement that interface, and then :

public class GYWebData : IGYDataAccess
{
    private IDataUpdater updater;

    public GYWebData(IDataUpdater updater)
    {
        this.updater = updater;
    }

    public void GetUserInfo(Action<GYUser, Exception> act, string id)
    {
        GYUserAPI.GetInfo(id, (result, error) =>
            {
                if (error != null)
                {
                    act.Invoke(null, error);
                    return;
                }

                act.Invoke(result.Result, null);
                updater.UpdateUserInfo(result.Result);
            });
    }
}

Boom, you now have zero dependency between the local storage and the web storage.

Finally, looking at your workflow, you could use the Proxy design pattern to help a bit. The Proxy would be used to get info from the web and cache it in the local storage. The proxy would be the only class you use for storage. I don't want to give it all to you, because you'll learn better by yourself! :) But know that the Proxy is a good case for this!


K&R 1-20 Converting tabs to spaces using idiomatic C I'm learning C with K&R 2nd Ed.  I just completed exercise 1-20 (which is the detab program).  I was hoping to get some feedback on my work.  I want to make sure that I am taking a good C approach and that my knowledge in other languages isn't bleeding in.

/*
Exercise 1-20 in K&R 2nd Edition
detab: clarified here: http://stackoverflow.com/questions/7178201/kr-exercise-1-20-need-some-clarification
Written by Z. Bornheimer (provided as is without warranty).
*/

#include <stdio.h>

#define MAXLEN 10000
#define TABSTOP 4

int detab(char c, char str[], int i);

/* calls detab with appropriate data */
main()
{
    int i = 0;
    char c, str[MAXLEN];
    while ((c = getchar()) != EOF)
       i = detab(c, str, i);
    printf("%s\n", str);
    return 0;
}

/* replaces tabs w/ spaces in accordance to TABSTOP */
int detab(char c, char str[], int i)
{
    if (c == '\t')
        do
            str[i++] = ' ';
        while ((i % TABSTOP) != 0);
    else
        str[i++] = c;
    return i;
}

I don't think this program is correct, according to my interpretation of tab stops.  One problem is that the column count should reset to 0 after every carriage return or newline character encountered.

Your program is vulnerable to buffer overflow.  This may be acceptable for a beginner, as long as you acknowledge the problem, but this code should never be put into production use.  (You may think that your buffers are generously sized, but a malicious attacker will scoff at whatever limit you choose.  There is no substitute for proper bounds checking.)  One simple strategy you could use is to minimize the use of a buffer by printing output on every call to detab() — your buffer would not need to be much larger than TABSTOP bytes.

In your detab() function, you want to treat i as an in/out parameter (i.e., the function alters the parameter and passes it back to the caller).  It is customary to accomplish this in C by passing a pointer to i, like this:

void func(int *in_out_param) {
    while (0 != *in_out_param % 4) {
        (*in_out_param)++;
    }
}

void caller() {
    int i = 2;
    func(&i);
    printf("%d\n", *i);  /* prints 4 */
}

Your variables are cryptically named.  While short variable names are acceptable and even encouraged as, say, dummy variables for iteration where their purpose is obvious, they should not be used as a matter of habit.  In particular, it is important to give descriptive names to function parameters, since they help prevent accidental misinterpretation by the function's users.  I would suggest this function interface:

/**
 * Converts input character c at column col into a string, with the output
 * placed in buf.  If c is a tab character, it is expanded into the appropriate
 * number of spaces.  The buffer size should be at least one more than tabwidth.
 */
void detab(unsigned int tabwidth, char c, unsigned int *col, char *buf, size_t bufsize)

For best performance, avoid reading one character at a time using getchar().  For this application, I recommend fgets(), which reads one line at a time (or up to the buffer size or until end-of-file, whichever is shortest).  Admittedly, that does complicate the solution quite a bit.


Improving the speed of creation for three Perlin Noise Maps in Python? I am interested in learning how I can improve the speed of the code in this pygame file. I iterate over 6400 * 1800 * 3 or 34,560,000 elements of various numpy arrays here to apply noise values to them. The noise library I'm using can be found on GitHub here.

I am calling static variables from a class called ST here. ST.MAP_WIDTH = 6400 and ST.MAP_HEIGHT = 1800. All other ST attributes called here are assigned in the code. They are the noise-maps I'm making.

from __future__ import division
from singleton import ST
import numpy as np
import noise
import timeit
import random
import math

def __noise(noise_x, noise_y, octaves=1, persistence=0.5, lacunarity=2.0):
    """
    Generates and returns a noise value.

    :param noise_x: The noise value of x
    :param noise_y: The noise value of y
    :return: numpy.float32
    """

    value = noise.pnoise2(noise_x, noise_y,
                          octaves, persistence, lacunarity,
                          random.randint(1, 9999))

    return np.float32(value)

def __elevation_mapper(noise_x, noise_y):
    """
    Finds and returns the elevation noise for the given noise_x and
    noise_y parameters.

    :param noise_x: noise_x = x / ST.MAP_WIDTH - randomizer
    :param noise_y: noise_y = y / ST.MAP_HEIGHT - randomizer
    :return: float
    """
    return __noise(noise_x, noise_y,  8, 0.9)

def __climate_mapper(y, noise_x, noise_y):
    """
    Finds and returns the climate noise for the given noise_x and
    noise_y parameters.

    :param noise_x: noise_x = x / ST.MAP_WIDTH - randomizer
    :param noise_y: noise_y = y / ST.MAP_HEIGHT - randomizer
    :return: float
    """
    # find distance from bottom of map and normalize to range [0, 1]
    distance = math.sqrt((y - (ST.MAP_HEIGHT >> 1))**2) / ST.MAP_HEIGHT

    value = __noise(noise_x, noise_y,  8, 0.7)

    return (1 + value - distance) / 2

def __rainfall_mapper(noise_x, noise_y):
    """
    Finds and returns the rainfall noise for the given noise_x and
    noise_y parameters.

    :param noise_x: noise_x = x / ST.MAP_WIDTH - randomizer
    :param noise_y: noise_y = y / ST.MAP_HEIGHT - randomizer
    :return: float
    """
    return __noise(noise_x, noise_y,  4, 0.65, 2.5)

def create_map_arr():
    """
    This function creates the elevation, climate, and rainfall noise maps,
    normalizes them to the range [0, 1], and then assigns them to their
    appropriate attributes in the singleton ST.
    """

    start = timeit.default_timer()

    elevation_arr = np.zeros([ST.MAP_HEIGHT, ST.MAP_WIDTH], np.float32)
    climate_arr = np.zeros([ST.MAP_HEIGHT, ST.MAP_WIDTH], np.float32)
    rainfall_arr = np.zeros([ST.MAP_HEIGHT, ST.MAP_WIDTH], np.float32)

    randomizer = random.uniform(0.0001, 0.9999)

    # assign noise map values
    for y in range(ST.MAP_HEIGHT):
        for x in range(ST.MAP_WIDTH):
            noise_x = x / ST.MAP_WIDTH - randomizer
            noise_y = y / ST.MAP_HEIGHT - randomizer

            elevation_arr[y][x] = __elevation_mapper(noise_x, noise_y)
            climate_arr[y][x] = __climate_mapper(y, noise_x, noise_y)
            rainfall_arr[y][x] = __rainfall_mapper(noise_x, noise_y)

    # normalize to range [0, 1] and assign to relevant ST attributes
    ST.ELEVATIONS = (elevation_arr - elevation_arr.min()) / \
                    (elevation_arr.max() - elevation_arr.min())

    ST.CLIMATES = (climate_arr - climate_arr.min()) / \
                  (climate_arr.max() - climate_arr.min())

    ST.RAINFALLS = (rainfall_arr - rainfall_arr.min()) / \
                   (rainfall_arr.max() - rainfall_arr.min())

    stop = timeit.default_timer()
    print("GENERATION TIME: " + str(stop - start))

Losing your Loops

Austin Hastings' comment gives you a good hint where to look at. The main takeaway for you should be:

(Most) loops are damn slow in Python. Especially multiple nested loops.

NumPy can help to vectorize your code, i.e. in this case that more of the looping is done in the C backend instead of in the Python interpreter. I  would highly recommend to have a listen to the talk Losing your Loops: Fast Numerical Computing with NumPy by Jake VanderPlas. Although primarily tailored towards data science, it gives a good overview on the topic.

I did some slight modifications to your original script to include some of the vectorization ideas while still using your chosen Perlin noise library. (Sidenote: I changed the __ prefix to a single _, because that is the convention most Python programmers use for internal functions. See PEP8 style guide.)

# -*- coding: utf-8 -*-
from __future__ import division, print_function
import numpy as np
import noise
import timeit

class ST(object):
    MAP_HEIGHT = 1800
    MAP_WIDTH = 6400

def _noise(noise_x, noise_y, octaves=1, persistence=0.5, lacunarity=2.0):
    """
    Generates and returns a noise value.

    :param noise_x: The noise value of x
    :param noise_y: The noise value of y
    :return: numpy.float32
    """
    if isinstance(noise_x, np.ndarray):
        #rand_seed = np.random.randint(1, 9999, noise_x.size)
        rand_seed = np.ones((noise_x.size, ))   # just for comparison
        value = np.array([noise.pnoise2(x, y, octaves, persistence, lacunarity, r)
                          for x, y, r in zip(noise_x.flat, noise_y.flat, rand_seed)])
        return value.reshape(noise_x.shape)
    else:
        value = noise.pnoise2(noise_x, noise_y,
                              octaves, persistence, lacunarity,
                              1.0)   # just for comparison
                              #np.random.randint(1, 9999))
        return np.float32(value)

def _elevation_mapper(noise_x, noise_y):
    """
    Finds and returns the elevation noise for the given noise_x and
    noise_y parameters.

    :param noise_x: noise_x = x / ST.MAP_WIDTH - randomizer
    :param noise_y: noise_y = y / ST.MAP_HEIGHT - randomizer
    :return: float
    """
    return _noise(noise_x, noise_y, 8, 0.9)

def _climate_mapper(y, noise_x, noise_y):
    """
    Finds and returns the climate noise for the given noise_x and
    noise_y parameters.

    :param noise_x: noise_x = x / ST.MAP_WIDTH - randomizer
    :param noise_y: noise_y = y / ST.MAP_HEIGHT - randomizer
    :return: float
    """
    # find distance from bottom of map and normalize to range [0, 1]
    distance = np.sqrt((y - (ST.MAP_HEIGHT >> 1))**2) / ST.MAP_HEIGHT

    value = _noise(noise_x, noise_y, 8, 0.7)

    return (1.0 + value - distance) / 2.0

def _rainfall_mapper(noise_x, noise_y):
    """
    Finds and returns the rainfall noise for the given noise_x and
    noise_y parameters.

    :param noise_x: noise_x = x / ST.MAP_WIDTH - randomizer
    :param noise_y: noise_y = y / ST.MAP_HEIGHT - randomizer
    :return: float
    """
    return _noise(noise_x, noise_y, 4, 0.65, 2.5)

def create_map_arr():
    """
    This function creates the elevation, climate, and rainfall noise maps,
    normalizes them to the range [0, 1], and then assigns them to their
    appropriate attributes in the singleton ST.
    """
    # assign noise map values
    randomizer = np.random.uniform(0.0001, 0.9999)

    start_arr = timeit.default_timer()

    X, Y = np.mgrid[0:ST.MAP_WIDTH, 0:ST.MAP_HEIGHT]
    noise_x = X / ST.MAP_WIDTH - randomizer
    noise_y = Y / ST.MAP_HEIGHT - randomizer
    elevation_arr_np = _elevation_mapper(noise_x, noise_y)
    climate_arr_np = _climate_mapper(Y, noise_x, noise_y)
    rainfall_arr_np = _rainfall_mapper(noise_x, noise_y)

    duration_arr = timeit.default_timer() - start_arr

    start_loop = timeit.default_timer()

    elevation_arr = np.zeros([ST.MAP_HEIGHT, ST.MAP_WIDTH], np.float32)
    climate_arr = np.zeros([ST.MAP_HEIGHT, ST.MAP_WIDTH], np.float32)
    rainfall_arr = np.zeros([ST.MAP_HEIGHT, ST.MAP_WIDTH], np.float32)
    for y in range(ST.MAP_HEIGHT):
        for x in range(ST.MAP_WIDTH):
            noise_x = x / ST.MAP_WIDTH - randomizer
            noise_y = y / ST.MAP_HEIGHT - randomizer

            elevation_arr[y, x] = _elevation_mapper(noise_x, noise_y)
            climate_arr[y, x] = _climate_mapper(y, noise_x, noise_y)
            rainfall_arr[y, x] = _rainfall_mapper(noise_x, noise_y)

    duration_loop = timeit.default_timer() - start_loop

    print(np.allclose(elevation_arr, elevation_arr_np.T))
    print(np.allclose(climate_arr, climate_arr_np.T))
    print(np.allclose(rainfall_arr, rainfall_arr_np.T))

    print("GENERATION TIME: loop: {:.6f}, array: {:.6f}".format(duration_loop, duration_arr))

if __name__ == "__main__":
    create_map_arr()

The bottleneck is still in

value = np.array([noise.pnoise2(x, y, octaves, persistence, lacunarity, r)
                  for x, y, r in zip(noise_x.flat, noise_y.flat, rand_seed)])

and it would be highly favorable to use an implementation which supports 2D input, preferably from NumPy, directly (see further reading below).

Nevertheless, the modifications bring the execution time down to a third of the original time on my machine (which is not that powerful):

True
True
True
GENERATION TIME: loop: 338.094228, array: 101.549388

Those three Trues are from a little test I added to check if the generated maps are the same within reasonable accuracy. For this purpose the additional random value in _noise was disabled.

Further reading

There have also already been similar questions on Code Review (see, e.g. here), where a reviewer created a Perlin noise implementation purely in Numpy. There also seems to be a GitHub project also doing Perlin noise with Numpy. So maybe have a look at them if your not forced to stick with noise.


Separate an email address into its username and domain name This is my first ever Python program and I wanted to get everyone's opinions on it. The goal of the program is to separate an email address into its username and domain name.
import sys
import getopt

argv = sys.argv[1:]

# -------Error Handling-Start----------
try:
    opts, args = getopt.getopt(argv, "l:s:h", ["list ", "single ", "help "])
except getopt.GetoptError as err:
    print(f"{err}\n""\n"
          "Usage: <Options> <Input> \n"
          "-l, --list        Specify a list of emails to be sliced \n"
          "-s, --single      Specify a single email to be sliced \n"
          "-h, --help        Show options")
    opts = []
# -------Error Handling-End------------

for opt, file_arg in opts:
    if opt in ['-h', '--help']:
        sys.exit("Usage: <Options> <Input> \n"
                 "-l, --list        Specify a list of emails to be sliced \n"
                 "-s, --single      Specify a single email to be sliced \n"
                 "-h, --help        Show options")
# If option -h is present, display MAN page

for opt, email_arg in opts:
    if opt in ['-s', '--single']:
        email = email_arg
        username = email[:email.index('@')]
        domain = email[email.index('@') + 1:]
        print(f"Your username: {username}")
        print(f"Your domain name: {domain}\n")
# If option -s is present, split input email into username and domain then print the output

for opt, file_arg in opts:
    if opt in ['-l', '--list']:
        file = file_arg

        email_file = open(file, "r")

        for string in email_file:
            username = string[:string.index('@')]
            domain = string[string.index('@') + 1:]
            print(f"Your username: {username}")
            print(f"Your domain name: {domain}")
# If option -l is present read file specified then loop through each line while splitting each into username and domain

if len(sys.argv) == 1:
    sys.exit("Usage: <Options> <Input> \n"
             "-l, --list        Specify a list of emails to be sliced \n"
             "-s, --single      Specify a single email to be sliced \n"
             "-h, --help        Show options")
# If only one argument is supplied, print MAN page.

The general style looks good. The biggest opportunity for improvement is your use of getopt. You should use the newer and easier argparse instead. Most of your code is used to parse and use the command line arguments. All of that can be done in three lines with argparse.
import argparse

arg_parser = argparse.ArgumentParser()
arg_parser.add_argument("-f", "--file", dest="file", default=False, action="store_true")
arg_parser.add_argument("input", type=str)
args = arg_parser.parse_args()

Instead of using --single or --list, I added a single flag --file (or -f), as the former choice is not only ambiguous (what happens if you specify neither or both?) as well as potentially misleading in their name.
A help option with "-h" is introduced automatically.
"-f", "--file" are the flags you can use to specify this argument. dest=file means that the created object args wil have an attribute args.file that stores the resulting value. action="store_true" means that the flag has to be specified without a value (i.e. -f instead of -f True) to set it.
After that, you can access your inputs as a strings (because type=str) by args.input (because the first argument to add_argument was "input").
open of a file should always be paired with a close when you are done using it, as is also true for other I/O objects such as network connections. Instead of manually calling close, one should prefer to use Python's context managers with the with keyword, which automatically perform any cleanup.
if args.file:
    with open(args.input, "r") as file:
        emails = file.readlines()
else:
    emails = [args.input]

Your code to split the email into username and domain can be done with str.split, assuming that there is exactly one "@". If there are not "@" or more than one, you should probably notify the user that there is an error anyway. If you want to have actual "serious" code that goes into production in a live system, see Toby Speight's comment on pointers how to properly do that.
    if email.count("@") != 1:
        ...  # Notify the user
    username, domain = email.split("@")
    print(f"Your username: {username}")
    print(f"Your domain name: {domain}")

Finally, your use of sys.exit is incorrect. You should only use sys.exit(n) with an integer n. sys.exit(0) means "successful", anything else means "failed". Unless you have specific needs, sys.exit(1) is the default choice for failure.
To give user friendly error messages, the easiest way is to use print. You can show the messages as an error by specifying file=sys.stderr (as opposed to the default file=sys.stdout).
        print(f"{email} is not a valid email.", file=sys.stderr)
        sys.exit(1)

In total, this is what I'd propose your program could look like:
import sys
import argparse

arg_parser = argparse.ArgumentParser()
arg_parser.add_argument("-f", "--file", dest="file", default=False, action="store_true")
arg_parser.add_argument("input", type=str)
args = arg_parser.parse_args()

if args.file:
    with open(args.input, "r") as file:
        emails = file.readlines()
else:
    emails = [args.input]

for email in emails:
    if email.count("@") != 1:
        print(f"{email} is not a valid email.", file=sys.stderr)
        sys.exit(1)
    username, domain = email.split("@")
    print(f"Your username: {username}")
    print(f"Your domain name: {domain}")


Generating a co-occurrence matrix I am generating co-occurrence matrix (2000X2000) in Python.

Vocab is a list of the 2000 top words in corpus. It does not contain all words in corpus.
Corpus is a Pandas dataframe of 30000 (rows) sentences.
An example of a single row in the corpus:

  the energy classroom high the room filled scholars eager learn they embarking learning journey unlike no these scholars asked learn time rapidly changing technology communication as teacher job give tools successful our school special always looking ways give students best education possible my 4th grade students counting provide tools necessary success i want keep love learning alive giving students access technology critical success 21st century learning environment our school 1 3 ratio ipads kids we use variety apps allow kids collaborate build critical thinking skills even though ipads keyboard ipad cumbersome difficult kids use therefore i asking 15 keyboards plug ipads having wired keyboard allows kids practice keyboarding skills standard collaborate others effectively utilize technology accesskeyboards enhance collaboration choice critical thinkin

My code is currently taking 120s per iteration and 364 hours to complete run.
I am running this code in Google Colab with 25GB RAM.

The code works fine with small data like 10 row corpus and 5 word vocab.
Is there a better way to complete this task or speed up this process?

tqdm is required because if the code runs for longer than 12 hours, which it is, then it gets disconnected from Google Colab.

coocur_matrix = np.zeros((len(voca), len(voca)),np.float64)

#python
from tqdm import tqdm

window_size=5
corpus=corpu1
vocab = voca

for word1 in tqdm(vocab):
  for word2 in vocab:
    for sent in corpus:
      doc_tokens = []
      doc_tokens=sent.split()

      p1= [i for i in range(len(doc_tokens)) if doc_tokens[i] == word1]
      p2= [i for i in range(len(doc_tokens)) if doc_tokens[i] == word2]
      for k in p1:
        print(9)
        for l in p2:
          if (abs(l-k)<=window_size):
            if(word1!=word2):
              coocur_matrix[vocab.index(word1),vocab.index(word2)] += 1

print(coocur_matrix)

Readability

Your code is visually unappealing. You have multiple PEP 8 violations, and your variable names really don't speak volumes. You have 2 spaces of indentation which is pretty much un-heard of in Python. If we move your code into a function and perform a little clean up we can get something like:

import numpy as np

def get_indexes(tokens, word):
    return [
        index
        for index, token in enumerate(tokens)
        if token == word
    ]

def co_occurrence_matrix(corpus, vocabulary, window_size=5):
    matrix = np.zeros((len(vocabulary), len(vocabulary)), np.float64)
    for word_1 in vocabulary:
        for word_2 in vocabulary:
            for sent in corpus:
                tokens = sent.split()
                tokens_1 = get_indexes(tokens, word_1)
                tokens_2 = get_indexes(tokens, word_2)
                for k in tokens_1:
                    for l in tokens_2:
                        if abs(l - k) > window_size:
                            continue
                        if word_1 == word_2:
                            continue
                        matrix[
                            vocabulary.index(word_1),
                            vocabulary.index(word_2),
                        ] += 1
    return matrix

print(co_occurrence_matrix(corpu1, voca))

Performance

Now It doesn't look too bad, but it doesn't really look nice. From here we can see clearly that word_1 and word_2 are defined in the first two loops, however the check word_1 == word_2 is on the inner-most level of the loops. This means you're needlessly looping over sent, tokens_1 and tokens_2. These loops, totalling around 12000000, are useless and a waste of time.

It can be hard to see, but vocabulary.index(word_1) has a for loop in it. How else is it getting the index? This means you're looping 2000 times, this is a waste as you can define a variable to store the index when looping through vocabulary.

You don't need to iterate over vocabulary, you can just iterate over corpus. This is because if word_1 or word_2 is not in tokens then the for loop will iterate over an empty list and so will do nothing. And so we can derive word_1 and word_2 from corpus.

Since vocabulary only contains the top 2000 words in the corpus. Then a naïve flip of the loops is still going to waste some cycles. And so we can just filter sent to ones in the vocabulary. To achieve this I opted to use a closure as I think it makes the code cleaner and easier to read. This is because I don't have the walrus on Python 3.7 and wanted to use dict.get rather than perform two dictionary lookups per filter of each token.

import numpy as np
import itertools

def by_indexes(iterable):
    output = {}
    for index, key in enumerate(iterable):
        output.setdefault(key, []).append(index)
    return output

def co_occurrence_matrix(corpus, vocabulary, window_size=5):
    def split_tokens(tokens):
        for token in tokens:
            indexs = vocabulary_indexes.get(token)
            if indexs is not None:
                yield token, indexs[0]

    matrix = np.zeros((len(vocabulary), len(vocabulary)), np.float64)
    vocabulary_indexes = by_indexes(vocabulary)

    for sent in corpus:
        tokens = by_indexes(split_tokens(sent.split())).items()
        for ((word_1, x), indexes_1), ((word_2, y), indexes_2) in itertools.permutations(tokens, 2):
            for k in indexes_1:
                for l in indexes_2:
                    if abs(l - k) <= window_size:
                        matrix[x, y] += 1
    return matrix

print(co_occurrence_matrix(corpu1, voca))

This currently isn't fully optimized, as you can use combinations rather than permutations but that would require mirroring the non-empty values of the matrix. That would likely half the time it takes to run, but would require additional code.

This is answer is entirely hypothetical. This means using dict[] twice rather than dict.get may be faster. It also means that many of the assumptions I made may not translate into performance benefits due to the way Python is.


Invert rows and columns in an Excel file Annother little excercise with openpyxl:

Say you have a Excel file like this:

The Goal is to make make it look like this:

My code to acomplish this:

 inversion.py

"""
Inverts the content of table row and columns
"""

from copy import copy
import openpyxl
from openpyxl.utils import get_column_letter

def save_workbook_excel_file(workbook, filename: str):
    """Tries to save created data to excel file"""
    try:
        workbook.save(filename)
    except PermissionError:
        print("Error: No permission to save file.")

def invert_row_column(filename: str):
    """
    Main loop to invert rows and column
    """
    workbook = openpyxl.load_workbook(filename)
    sheet_names = workbook.sheetnames
    sheet = workbook[sheet_names[0]]
    workbook.create_sheet(index=0, title='tmp_sheet')
    tmp_sheet = workbook['tmp_sheet']

    data = []
    for row in sheet:
        cells = []
        for cell in row:
            cells.append(cell)
        data.append(cells)

    for x in range(0, len(data)):
        for y in range(0, len(data[x])):
            column = get_column_letter(x + 1)
            row = str(y + 1)
            tmp_sheet[column + row] = copy(data[x][y].value)

    sheet_name = sheet.title
    del workbook[sheet_name]
    tmp_sheet.title = sheet_name
    save_workbook_excel_file(workbook, 'updated_' + filename)

invert_row_column("test.xlsx")

I wonder how this can be improved?
Can the naming be better?
Is there a better / or shorter solution?

Another way to solve this, without using openpyxl and thereby slightly defeating the purpose of learning more about that, would be to use pandas, where this is quite short:

import pandas as pd

# This would load the first sheet by default, but we need the name to save it again
# df = pd.read_excel(filename)

sheets = pd.read_excel(file_name, sheet_name=None)  # all sheets
sheet_name, df = next(iter(sheets.items()))         # first sheet
df = df.T                                           # transpose
df.to_excel(file_name, sheet_name)                  # write back

This uses pandas.read_excel to read the file and pandas.DataFrame.to_excel to write it back. You need to have the xlrd module installed for this to work.

Feel free to wrap it in functions again if needed.

This should be faster than the manual iteration in Python, since the transpose should happen at C speed.


Reverse a domain name string For an interview question, I was asked to do the following:

  Given a dot separated domain name, reverse the domain elements.

Here are a few input/output examples:

codereview.stackexchange.com -> com.stackexchange.codereview
foo.bar -> bar.foo
a.. -> ..a

This was my proposed solution. I really feel like there's probably a better way to do this (in place?).

#include <iostream>
#include <string>

std::string
reverse(const std::string &str)
{
    std::string rstr = str;
    std::reverse(rstr.begin(), rstr.end());
    int size = rstr.size();
    int start = 0;
    int end = 0;
    while (end != size + 1) {
        if (rstr[end] == '.' || end == size) {
            std::reverse(rstr.begin() + start, rstr.begin() + end);
            start = end + 1;
        }
        ++end;
    }
    return rstr;
}

int
main()
{
    std::string examples[] = {
        "hello.world",
        "com.domain.something",
        "..a"
    };

    int size = sizeof(examples) / sizeof(examples[0]);
    for (int i = 0; i < size; ++i) {
        std::cout << examples[i] << " -> " << reverse(examples[i]) << std::endl;
    }

    return 0;
}

This looks quite fine to me. Granted, you make several passes over the entire string, but that doesn't matter much, as the time complexity remains \$O(n)\$, space complexity constant, and this is easy to read and understand.

You could reduce the number of passes by using extra storage, in which you could prepend the reversed segments. But due to the extra storage, such algorithm would not be better, it would be different. Whether one solution is better than the other depends on external constraints not stated in the problem. Talking about this is important at an interview, more important than whatever solution you gave.

In terms of the implementation, the only thing I would change is convert the main while loop to a for loop. The reason is that the start and end variables are only used inside the loop body, and the for loop would help confining them into that scope, which is a good thing, reducing the live time of those variables, and preventing accidental uses outside the loop.


Determine dependency injection type at runtime from config I have a .NET application with a .config file that has content like this:
<appSettings>
    <add key="SmtpConfiguration" value="Db" />
</appSettings>

Inside the application's code I define an ISmtpConfiguration interface and several different implementations. DbSmtpConfiguration gets configuration data from a database, AppSettingsSmtpConfiguration reads from some other entries in the .config, and DefaultSmtpConfiguration has values written directly in the code.
In my service provider configuration, I have this:
switch (ConfigurationManager.AppSettings["SmtpConfiguration"])
{
    case "Db":
        services.AddSingleton<ISmtpConfiguration, DbSmtpConfiguration>();
        break;
    case "AppSettings":
        services.AddSingleton<ISmtpConfiguration, AppSettingsSmtpConfiguration>();
        break;
    default:
        services.AddSingleton<ISmtpConfiguration, DefaultSmtpConfiguration>();
}

I'm not thrilled with that, so I have created a factory:
public class SmtpConfigurationFactory
{
    public static ISmtpConfiguration GetSmtpConfiguration()
    {
        var type = Type.GetType(ConfigurationManager.AppSettings["SmtpConfiguration"] + "SmtpConfiguration");

        if (type == null)
        {
            type = typeof(DefaultSmtpConfiguration);
        }

        return (ISmtpConfiguration) Activator.CreateInstance(type);
    }
}

I then use the factory like this:
services.AddSingleton(SmtpConfigurationFactory.GetSmtpConfiguration());

I'm not terribly thrilled with that either. My final thought was to change consumers of ISmtpConfiguration to accept SmtpConfigurationFactory instead, make the method not static, and then add the factory to my services:
services.AddSingleton<SmtpConfigurationFactory>();

Then the constructor for the consumer looks like:
public SmtpEmailService(SmtpConfigurationFactory configurationFactory)
{
    Configuration = configurationFactory.GetSmtpConfiguration();
}

I like that the best, but is there a better solution? The factory introduces the issue that implementations of ISmtpConfiguration cannot require dependencies of their own. DbSmtpConfiguration for example couldn't receive an IDbConfiguration telling it which database to connect to.

Since the value that determines what type to use doesn't change I personally would prefer your first solution of a switch statement in the registration.
The current implementation of the factory pattern would lose any type safety.  If a parameter gets added to the constructor then then the Activator.CreateInstance would fail.  For example the database one might take in a connection string or sql object while the default one wouldn't.  Plus a naming convention would need to be understood and knowledge passed down in maintenance mode.  As the saying goes juice not worth the squeeze.
If the value to return could change based on a parameter being passed in then I would agree factory pattern could be used but as it stands I think the switch statement in the container registration is the simplest and meets the requirement.
There is still ways to create the factory pattern that would give type safety and DI but it ends up looking a lot like the switch statement.


Overwrite disk space for the purpose of deleting old data I have written a little program that overwrites memory. It creates useless files in a folder named "folder" (I am so creative). The purpose is to overwrite old data. It's not a very professional program because I am a beginner, but I think it should do for which it is destined: exhaust memory and thus maybe overwrite old data that was stored on it. If you have tips for me how to imrove this program or how to make it faster in it's execution I would be thankful!

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

int create_files(int number_of_files, int number_of_values);

int main()
{
    int number_of_files;
    int number_of_values;

    srand(time(NULL));

    printf("before using this program create a folder with the name \"folder\"\nin the same folder this program is located\n");
    printf("number of files: ");
    scanf("%d", &number_of_files);
    printf("number of values: ");
    scanf("%d", &number_of_values);

    create_files(number_of_files, number_of_values);

}

void create_files(int number_of_files, int number_of_values)
{
    for(int i = 0; i < number_of_files; i++)
    {
        FILE * file;
        char file_name[30] = "./folder/";
        char file_number[20];
        sprintf(file_number, "%d", i);
        strcat(file_name, file_number);
        file = fopen(file_name, "w+");

        for(int j = 0; j < number_of_values; j++)
        {
            int random_value = rand();
            fprintf(file, "%d", random_value);
        }
        fclose(file);
    }
}

Make a habit of initializing variables. Your scanfmight fail:

scanf("%d", &number_of_files);

What happens if I answer "many"? Well, first of all, scanf will return 0, and next number_of_files won't get changed. So its value is unknown. If we use it, we end up with undefined behaviour, which is a fancy term to say "we have no idea what will happen".

So in order to at least prevent something like number_of_files = 1000000000 and number_of_values = 10000000 just because the input failed, make sure to initialize your values and maybe check whether scanf succeeded:

int main()
{
    int number_of_files  = 0;
    int number_of_values = 0;

    srand(time(NULL));

    printf("before using this program create a folder with the name \"folder\"\nin the same folder this program is located\n");
    printf("number of files: ");

    if(scanf("%d", &number_of_files) != 1) {
       // handle the error?
    }
    printf("number of values: ");

    if(scanf("%d", &number_of_values) != 1) {
       // handle the error?
    }

    create_files(number_of_files, number_of_values);
}

Note that your usage is misleading: the folder has to be located in the folder the program is run, not located.

Let the compiler help you

Note that I got rid of the int i? You declared it, but you've never used it. You probably had all of your program in your main and decided to refactor (good!), but you forgot not only the i, but to return something in create_files. Both of these mishaps could have been found with compiler flags:

$ gcc -Wall -pedantic main.c

main.c: In function 'int main()':
main.c:10:9: warning: unused variable 'i' [-Wunused-variable]
     int i;
         ^
main.c: In function 'int create_files(int, int)':
main.c:44:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^

Initialize the seed once

I've moved the srand call into main. That way there will be only one call to srand, regardless of how many times you call create_files. That's handy since

we can now reconstruct the flow if we print the seed
we can reconstruct the flow even if we call create_files multiple times
we don't have to worry about calling create_files twice in the same second

Simplify and continue to check errors

You can use sprintf to print your file_name directly:

char file_name[50] = "";
sprintf(file_name, "./folder/trash-%015d");

This will format file_name as "./folder/trash-000000000000001". Since every file name has the same length, the sorting is somewhat better, and the trash- prefix will simplify the users life in case they accidentally had a folder called folder (but they should really rename that folder).

That being said, you should check whether you were able to open that file at all, and whether you could actually write to it. Remember, you're trying to fill a disk (not memory) to the brim. If it's full, you cannot continue to write, and either fopen or fprintf will fail:

if(!(file = fopen(file_name, "w+")))
{
    perror("Could not open file");
    return i;
}

for(int j = 0; j < number_of_values; j++)
{
    int random_value = rand();
    if(fprintf(file, "%d", random_value) < 0)
    {
        fclose(file);
        return i;
    }
}

Note that I return i because I think you've wanted to return the number of created files, but I'm not sure. If you didn't want to return anything at all, change create_files return type to void.


Program for calculating and ranking scores I've recently completed a vanilla JS challenge . It is a method for calculating and ranking scores.
If possible, I would love to make it better. Any suggestions are welcome.
Challenge Directions:
The input array contains a series of objects that hold user scores. Scores are ranked both individually (within the object) as well as against the other score objects to find overall ranking.
MY CODE:

//STEP 1 - CALCULATE TOTAL SCORE & SCORE RANKS FOR EACH OBJECT
function calculateResults(input) {
    return input.map((scoreSeries) => {
        let final_rank = 0;
        calculatedScoreSeries = {...scoreSeries}

        Object.keys(scoreSeries).forEach((key) => {
          if (key !== 'category') {
            final_rank = final_rank + scoreSeries[key]  //Adding up the score.
            calculatedScoreSeries[key] = calculateSourceRank(scoreSeries, key) //calculating the rank
          }})
        calculatedScoreSeries.final_rank = final_rank //adding total_score prop to the new object
        return calculatedScoreSeries
    })
}
//to calculate the ranking of each source
function calculateSourceRank(scoreSeries, key) {
    let rank = 1;
    for (let i = 1; i < Object.keys(scoreSeries).length; i++) {
      if (scoreSeries[Object.keys(scoreSeries)[i]] > scoreSeries[key]) {
        rank++
      }
    }
   return rank
}

//STEP 2 - COMPARE OBJECTS FOR FINAL RANK
function calculateFinalRank(calculatedResults) {
    return calculatedResults.map((calculatedScoreSeries) => {
        let rankedScoreSeries = {...calculatedScoreSeries}
        let rank = 1;
        for (let i = 0; i < calculatedResults.length; i++) {
            if (calculatedResults[i].final_rank > calculatedScoreSeries.final_rank) {
            rank++
            }
          rankedScoreSeries.final_rank = rank
        }
    return rankedScoreSeries
  })
}

let output = calculateFinalRank(calculateResults(input))
console.log(output)

TEST INPUTS W EXPECTED OUTPUTS:

//each object's score sources are ranked (if two are equal they share the higher ranking).
//then all objects are compared for the overall rnking (if two are equal they share the higher ranking)
input = [ // test input 1
    {
    "category":"test3",
    "source1":50,
    "source2":100,
    "source3":30,
    "source4":10,
    "source5":10,
    },

    {
    "category":"test4",
    "source1":100,
    "source2":30,
    "source3":10,
    "source4":10,
    "source5":50,
    },

    {
    "category":"test5",
    "source1":5,
    "source2":10,
    "source3":10,
    "source4":40,
    "source5":5,
    },
    ]

    output = [
    {
    "category":"test3",
    "source1":2
    "source2":1
    "source3":3
    "source4":4
    "source5":4
    "final_rank":1
    },

    {
    "category":"test4",
    "source1":1
    "source2":3
    "source3":4
    "source4":4
    "source5":2
    "final_rank":1
    },

    {
    "category":"test5",
    "source1":4,
    "source2":2,
    "source3":2,
    "source4":1,
    "source5":4,
    "final_rank":3
    },
    ]

    input = [ // test input 2
    {
    "category":"cat1",
    "src1":20,
    "src2":30,
    "src3":40,
    "src4":50,
    "src5":50
    },

    {
    "category":"cat1",
    "src1":10,
    "src2":0,
    "src3":20,
    "src4":20,
    "src5":100
    },
    ]

    output = [
    {
    "category":"cat1",
    "src1":5,
    "src2":4,
    "src3":3,
    "src4":1,
    "src5":1,
    "final_rank":1
    },

    {
    "category":"cat1",
    "src1":4,
    "src2":5,
    "src3":2,
    "src4":2,
    "src5":1,
    "final_rank":2
    },
  ]

BUSINESS LOGIC:
1 - Category will always have at least one 'category'. More than 1 catgeory will be saved as an array under the category key. Category will always be the first key.
2 - each object in the input array will have an equal number of sources (although this number can be any amount)
3 - if a total score is equal to anothers, then they will share the higher ranking (same as individual score ranking)

Good things

I like the functional approach taken with this code, and that some ecmascript-6 features like arrow functions are used.

Suggestions

const vs let

It would be wise to default to using const for any variable that doesn't need to be re-assigned. If you later determine a value should be re-assigned then switch it to using let. This helps prevent accidental re-assignment in the future.

Use consistent indentation

Some lines appear to be indented with two spaces, while others are indented with four. It is wise to use uniform indentation throughout the code.

Use consistent line terminators

Many lines are terminated with a semi-colon but some are not. Unless you completely understand rules of Automatic semicolon insertion or are using a compiler/module bundler it is best to include line terminators.

multiple calls to Object.keys() in loop

Let us take a look at the following block:

for (let i = 1; i < Object.keys(scoreSeries).length; i++) {
  if (scoreSeries[Object.keys(scoreSeries)[i]] > scoreSeries[key]) {
    rank++
  }
}

For each iteration of the loop, Object.keys() is called twice. That function could be called once if the result is stored in a variable.

const scoreKeys = Object.keys(scoreSeries)
for (let i = 1; i < scoreKeys.length; i++) {
  if (scoreSeries[scoreKeys[i]] > scoreSeries[key]) {
    rank++
  }
}

The syntax could be simplified using a for...of loop:

const scoreKeys = Object.keys(scoreSeries)
for (const scoreKey of scoreKeys) {
  if (scoreSeries[scoreKey] > scoreSeries[key]) {
    rank++
  }
}

The whole function could be simplified with a more function approach using .filter():

//to calculate the ranking of each source
function calculateSourceRank(scoreSeries, key) {
    const scoreKeys = Object.keys(scoreSeries)
    return 1 + scoreKeys.filter(scoreKey => scoreSeries[scoreKey] > scoreSeries[key]).length
}

A similar approach could be applied to calculateFinalRank()


Solution to Project Euler #1 - multiples of 3 and 5 I have been programming for about ~2 years, and mostly wrote OOP and structural code. Recently, I have decided to pick up a functional programming language, and Haskell being too alien for me, looked to Racket (since it is high time I learned a LISP anyways) and am loving it. Since this is a new area of programming for me, I would appreciate any feedback you could give me on this program. It is the solution to the first Project Euler program.

;If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.
;Find the sum of all the multiples of 3 or 5 below 1000.

#lang racket

(define max 1000)

(define (multiple_of base test)
    (equal? (remainder test base) 0))

(define (primes current total)
    (if (< current max)
        (primes
            (+ current 1)
            (+ total (if
                (or (multiple_of 5 current) (multiple_of 3 current)) current 0)))
        total))

(primes 1 0)

; Output: 233168
; Success

In Scheme, we use hyphens to separate words, not underscores. Also boolean-returning procedures should end in ?. So it should be multiple-of?
Instead of (equal? x 0), use (zero? x).
Instead of a recursive loop, you can use for comprehensions:

(for/sum ((i (in-range 1000))
          #:when (or (multiple-of? 5 i)
                     (multiple-of? 3 i)))
  i)

Surely, that's much more readable. In my humble opinion. :-)
Notwithstanding the last comment, your formatting for your primes procedure is not ideal. Here's a more proper formatting:

(define (primes current total)
  (if (< current max)
      (primes (add1 current)
              (+ total (if (or (multiple-of? 5 current)
                               (multiple-of? 3 current))
                           current
                           0)))
      total))


Unscramble words faster in Python So I'm currently using python to unscramble words that are retrieved from an OCR program, and checked in the 'dictionary'.

The current code I am using to unscramble words is this:

import numpy as nm
import pytesseract
import cv2
import ctypes
from PIL import ImageGrab

def imToString():

    # Path of tesseract executable
    pytesseract.pytesseract.tesseract_cmd =r'C:\Program Files (x86)\Tesseract-OCR\tesseract'
    while(True):

        # ImageGrab-To capture the screen image in a loop.
        # Bbox used to capture a specific area.
        cap = ImageGrab.grab(bbox =(687, 224, 1104, 240))

        # Converted the image to monochrome for it to be easily
        # read by the OCR and obtained the output String.
        tesstr = pytesseract.image_to_string(
                cv2.cvtColor(nm.array(cap), cv2.COLOR_BGR2GRAY),
                lang ='eng')
        checkWord(tesstr)

def checkWord(tesstr):

    dictionary =['orange', 'marshmellow']

    scrambled = tesstr
    for word in dictionary:
        if sorted(word) == sorted(scrambled):
            print(word)

imToString()

I want to know if there is anyway to reduce the time it takes to look through the 'dictionary', as there are many more words to go through.

Like it was mentioned in the comments, it's difficult for us to tell you how to get your code faster witbout more information about the runtime context of your code. But based on what you have shown in code, I would do these modifications:

import numpy as nm
import pytesseract
import cv2
import ctypes
from PIL import ImageGrab

def im_to_string():

    # Path of tesseract executable
    pytesseract.pytesseract.tesseract_cmd = r'C:\Program Files (x86)\Tesseract-OCR\tesseract'
    while(True):

        # ImageGrab-To capture the screen image in a loop.
        # Bbox used to capture a specific area.
        cap = ImageGrab.grab(bbox=(687, 224, 1104, 240))

        # Converted the image to monochrome for it to be easily
        # read by the OCR and obtained the output String.
        tes_str = pytesseract.image_to_string(
                cv2.cvtColor(nm.array(cap), cv2.COLOR_BGR2GRAY),
                lang ='eng')
        check_word(tes_str)

words_dictionary = ['orange', 'marshmellow']
scrambled_words_dictionary = set(sorted(current_word) for current_word in words_dictionary)

def check_word(tes_str):
    if sorted(tes_str) in scrambled_words_dictionary:
        print(tes_str)

im_to_string()

This is based ont the assumption that the word dictionary doesn't change between screen grabs.

Here's some of the modifications I made to your code:

I defined the word dictionary outside of check_word() so that it doesn't need to be recreated each time to invoke that function
I created a set with the words scrambled because a set is optimized to check for membership. You are also only cheking against the scrambled words
I also changed the name of the dictionary variable because a dictionary has a specific meaning in Python and that could be slightly confusing for someone reading your code

If it's not what you were looking for, give us more precisions.