CodeWars - Assembly Language - Check List For Value I'm doing assembly language problems on CodeWars, a website with practice problems.
Problem
https://www.codewars.com/kata/545991b4cbae2a5fda000158/train/nasm

Create a method that accepts a list and an item, and returns true if the item belongs to the list, otherwise false.

Solution In C
To give you an idea what the assembly code will be doing.
#include <stdbool.h>
#include <stddef.h>

bool include(const int* arr, size_t size, int item)
{
    int i = 0;
    loop:
    if ( i < size ) {
        if ( arr[i] == item ) {
            return true;
        }
        i++;
        goto loop;
    }
    return false;
}

Solution In NASM Assembly (Linux x64)
CodeWars provided the 7 lines at the top.
SECTION .text
global include
include:

    ; bool include(const int* arr, size_t size, int item)
    ; sizeof(int) = 4 bytes (32bit)
    ; sizeof(size_t) = 8 bytes (64bit)

    ;rdi = &arr    pointer, 8 bytes
    ;   arr[i]          signed int, 4 bytes (dd)
    ;rsi = size    size_t, unsigned int, 8 bytes
    ;edx = item    signed int, 4 bytes

    ; Avoid using registers that we need to preserve (RBX, RBP, R12-R15). Else we'd have to push and pop them onto the stack.

    mov rcx, 0      ; unsigned int i = 0;
loop1:
    cmp rcx, rsi    ; if ( i < size ) {
    jae skip_loop

    mov r8d, [rdi + 4 * rcx]        ; make a temp variable so we can see this in step debugging
    cmp edx, r8d    ; if ( arr[i] == item ) {
    jne skip_if
    mov rax, 1      ; return true;
    ret

skip_if:
    inc rcx         ; i++;
    jmp loop1

skip_loop:
    mov rax, 0      ; return false;
    ret

Questions
I'm brand new to assembly. Any feedback on patterns and best practices would be appreciated. For example

Is there a standard pattern to use when writing loops?
Is there a standard pattern to use when writing if/elseif/else?
Are there better word choices and formatting for the labels?

First of all, props for the copious comments, particularly how you included a representation in C. The C representation itself has a signed vs unsigned comparison, which can cause weird bugs when and where you don't expect them, but I'm going to stick to the assembly code itself in this review. I would just recommend declaring the loop counter i as size_t, since that's what the type of the stop condition is.
I assembled your C function using gcc version 10.2.0 with -O3 -march=native, so I'll include the output here so I can walk through it step by step, comparing the two implementations. This is a really good idea, by the way, because working backwards through what the C compiler did helps you see real assembly language, not just practice examples you wrote. Compiler Explorer is a great tool for this.
Anyways, here is my input file.
#include <stdbool.h>
#include <stddef.h>

bool include(const int* arr, size_t size, int item) {
    for (size_t i = 0; i < size; ++i) {
        if (arr[i] == item) {
            return true;
        }
    }

    return false;
}

To assemble it, I use the following command. Note the -masm=intel argument; the default assembly syntax is AT&T for GNU tools.
gcc -S -O3 -march=native -masm=intel -o output.asm input.c

You can filter out auxiliary metadata and its containing labels using the following command.
cat output.asm | sed -E '/^\s+\./d;/^\.L[A-Z]/d'

And here is my output.
include:
        test    rsi, rsi
        je      .L4
        xor     eax, eax
        jmp     .L3
.L8:
        inc     rax
        cmp     rsi, rax
        je      .L4
.L3:
        cmp     DWORD PTR [rdi+rax*4], edx
        jne     .L8
        mov     eax, 1
        ret
.L4:
        xor     eax, eax
        ret

Notice that the first line is already different. In your version, you started by setting the rcx register to 0, using the mov instruction, whereas the compiler output test rsi, rsi. Why?
Well, as you noted, Intel x86-64 Linux assembly programming calling convention dictates that the rsi register contains the second argument to your function, in this case the size of the array.
From the Intel x86-64 documentation (pg. 1866), the test instruction performs a logical AND test on its arguments. If the result is zero, it sets the zero flag ZF equal to 1. The following instruction therefore makes sense, since the "jump near if equal" (je) instruction is carried out when the the zero flag is set (ZF=1).
In other words, the subroutine begins by checking whether or not the input array actually contains any items before actually doing anything with it. Note that you weren't checking for this edge case in your original code (nor did you verify the array pointer wasn't NULL), and it's a great example of compilers being awesome. Matt Godbolt (the guy who made Compiler Explorer) has an awesome talk about this kind of stuff that I highly recommend you check out if you like this sort of thing.
Anyways, if you look at the .L4 label, you'll notice it's semantically equivalent to your skip_loop. However, you literally set the rax register (i.e., the return value of the function) equal to zero by moving a 0 into it, whereas the compiler uses the exclusive-or xor instruction on eax with itself, which will obviously always be zero. You're not semantically wrong for doing it the way you did, but you can read this SO post that describes in significant detail why you should opt for the xor eax, eax method. The short version is that it's more efficient, and the longer version is that it's much more efficient, but there are other benefits, like power consumption. That post goes into a lot more detail, though, and it's a great read.
Your loop itself looks okay to me. The compiler used the rax register for the loop counter, which both you and the compiler then used to get the value of the array at the appropriate index. The only real difference between the two versions is that the compiler used an unconditional jump jmp instruction to skip the first part of its main loop, which contained the loop counter increment, whereas your code had that last.
I genuinely don't think this difference has any real impact, because both implementations contain two conditional jumps, which significantly impact performance because they trigger unconditional instruction fetches and involve more advanced processor features like branch prediction, which itself introduces problems via an optimization called speculative execution. (Long story short, optimization is complicated, you won't really know until you profile it, and you probably shouldn't even care about optimization until you have a working something to optimize, but you're "probably" fine.)
Something I found really interesting (although not particularly impactful or world-view shattering), was that believe it or not, creating that temporary variable and then comparing takes exactly as many bytes to encode as the direct comparison the compiler output in my version.
Here is a snippet from the objdump output for your version. (To generate this on your local machine, the command I used after assembling with nasm was objdump -Mx86-64,intel -D -S -s input.o.)
0000000000000005 <loop1>:
loop1:
    cmp rcx, rsi    ; if ( i < size ) {
   5:   48 39 f1                cmp    rcx,rsi
    jae skip_loop
   8:   73 14                   jae    1e <skip_loop>

    mov r8d, [rdi + 4 * rcx]        ; make a temp variable so we can see this in step debugging
   a:   44 8b 04 8f             mov    r8d,DWORD PTR [rdi+rcx*4]
    cmp edx, r8d    ; if ( arr[i] == item ) {
   e:   44 39 c2                cmp    edx,r8d
    jne skip_if
  11:   75 06                   jne    19 <skip_if>
    mov rax, 1      ; return true;
  13:   b8 01 00 00 00          mov    eax,0x1
    ret
  18:   c3                      ret

Now here's a snippet from the output for the compiler's version that contains the comparison operation.
0000000000000011 <include.L3>:
.L3:
    cmp     [dword rdi+rax*4], edx
  11:   39 94 87 00 00 00 00    cmp    DWORD PTR [rdi+rax*4+0x0],edx
    jne     .L8
  18:   75 ef                   jne    9 <include.L8>
    mov     eax, 1
  1a:   b8 01 00 00 00          mov    eax,0x1
    ret
  1f:   c3                      ret

Notice how in your version, the assignment to a temporary variable takes four bytes. You specified the r8d register as the destination register, so this isn't exactly groundbreaking stuff, but the following comparison instruction required only three bytes to encode:
44 8b 04 8f             mov    r8d,DWORD PTR [rdi+rcx*4]
44 39 c2                cmp    edx,r8d

The compiler's version skipped the intermediate variable assignment, but the resulting instruction required seven bytes to encode.
39 94 87 00 00 00 00    cmp    DWORD PTR [rdi+rax*4+0x0],edx

To explain why those extra zeroes at the end matter, I will borrow once again from this great post which you should definitely read.

Smaller machine-code size [...] is always an advantage: Higher code density leads to fewer instruction-cache misses, and better instruction fetch and potentially decode bandwidth.

To really drive this point home, let us read the conditional jump instruction documentation (pg. 1109 in the combined manual [vols 1-4]):

All conditional jumps are converted to code fetches of one or two cache lines, regardless of jump address or cache-ability.

I now leave this link to the latency numbers every programmer should know for your edification, although it should be noted this document is from 2012. Here's a cool updated version where you can look at the latency numbers by year (including 2020), but I actually just found this myself, so I admit I haven't vetted the source for accuracy. I am including it for completeness nonetheless.
As far as the labels themselves are concerned, since loop1, skip_if, and skip_loop are all logically related to the include subroutine, I would recommend using local labels to more intuitively organize your assembly code. Local labels are particularly useful because the subroutine name serves as a sort of namespace, allowing you to reuse the local label names defined therein. You can see the include version above assembled by gcc uses local labels.
The only recommendation I would make regarding loops is to be wary of using the right conditional jump for your situation. From the documentation:

The terms "less" and "greater" are used for comparisons of signed integers and the terms "above" and "below" are used for unsigned integers.

This isn't pedantry, either. Take for example, the  "jump if above or equal" jae instruction in your code. It follows a cmp instruction, which subtracts the second operand from the first and modifies the EFLAGS register accordingly. Specifically, the intermediate sub instruction performs both signed and unsigned subtraction, setting the overflow and carry flags, respectively. However, by using the jae instruction, you are implicitly only checking the carry flag, so hopefully your loop counter and stop conditions are of the same type.
The C standard defines how this should be done, which does help mitigate bugs by both converting as properly and safely as possible, and by providing helpful warnings and even error messages (depending on the compilation strictness settings). Of course, if you're going to be writing assembly language directly, this obviously does not help you.
For reference, the EFLAGS condition codes can be found in Volume 1 Appendix B of the Intel® 64 and IA-32 Architectures Software Developer’s Manuals, and the conditional jumps reference table starts on page 1106 of Volume 2.


Simple calculator in PHP utilizing MVC I have been struggling with understanding Model, View, Service, Controller architecture and wrote some code.

Is my Model class the Model part of MVC (did I define it properly)?
Is my Service class a Service Layer (did I define the layer properly)?
Did I connect my layers properly (is the 'wiring' set up correctly)?

 /*
 * Application Idea:  Assume UI has two input boxes and a button and an output box
 * box A is for a number
 * box B is for a number
 *
 * event of a button click sends responce of (A+B) into the output button
 * Implement Controller, Service, Model, View layers
 */
class Controller
{

    /**
     *
     * @var Service
     */
    private $service;

    /**
     *
     * @var Model
     */
    private $model;

    // add A B update C for all
    function addAB()
    {
        $a = $this->model->getA();
        $b = $this->model->getB();

        $c = $this->service->computeSum($a, $b);
        $this->model->setC($c);

        $view = new View();
        return $view->render($a, $b, $c);
    }

    public function getService()
    {
        return $this->service;
    }

    public function setService($service)
    {
        $this->service = $service;
    }

    public function getModel()
    {
        return $this->model;
    }

    public function setModel($model)
    {
        $this->model = $model;
    }
}

class ControllerFactory
{

    function getController(array $request)
    {
        $model = new Model();
        $model->setA($request['a']);
        $model->setB($request['b']);

        $service = new Service();

        $controller = new Controller();
        $controller->setService($service);
        $controller->setModel($model);

        return $controller;
    }
}

class Model
{

    private $a;

    private $b;

    private $c;

    public function getA()
    {
        return $this->a;
    }

    public function getB()
    {
        return $this->b;
    }

    public function getC()
    {
        return $this->c;
    }

    public function setA($a)
    {
        $this->a = $a;
    }

    public function setB($b)
    {
        $this->b = $b;
    }

    public function setC($c)
    {
        $this->c = $c;
    }
}

class Service
{

    function computeSum($a, $b)
    {
        return $a + $b;
    }
}

class View
{

    function render($a, $b, $c)
    {
        print "$a + $b = $c";
    }
}

class Router
{

    function routeRequest($request)
    {
        $controller = (new ControllerFactory())->getController($request);
        switch ($request['event'])
        {
            case 'add_event':
                $controller->addAB();
                break;
        }
    }
}

/*
 * request can come from Web Browser
 */
$request = array();
$request['event'] = "add_event";
$request['a'] = 2;
$request['b'] = 3;

$router = new Router();
$router->routeRequest($request);

Based on your example I would suggest you spend some more time on plain MVC before you try to work with a service layer. Something else to keep in mind is that the requirements of the application you're building determine whether you even need a service layer. In your example you don't. Actually since you're not really retrieving any data, your example doesn't need the model either.

The simplest way to describe the relationship between the controller and the view is that the controller does all the work and then sends data only to the view. The view then takes the data and displays it any which way it wants. This is valuable because it means you can make serious changes to one or the other or both without there being much risk of the changes causing undesired effect. It makes maintaining and expanding code much easier.

In your example you don't really make a clear separation between the view and the controller as far as the actual execution code goes. I would take a hint from MS.Net MVC and make better use of objects to separate the roles and interfaces of each layer properly.

Take this over-simplified example based on what you provided:

class View{}
class ViewData{}
class Controller{}
class ControllerData{}

$cData = new ControllerData();
$cData->event = "add_event";
$cData->a = 2;
$cData->b = 3;
$cObj = new Controller($cData);
$vData = $cObj->getData();   //returns a ViewData object
$vObj = new View($vData);
$vObj->renderView();


Permutations of a list of numbers I have written a program to find all the possible permutations of a given list of items. This precisely means that my program prints all possible P(n,r) values for r=0 to n.

package com.algorithm;

import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

public class Permutations<T> {
    public static void main(String args[]) {
        Permutations<Integer> obj = new Permutations<Integer>();
        Collection<Integer> input = new ArrayList<Integer>();
        input.add(1);
        input.add(2);
        input.add(3);

        Collection<List<Integer>> output = obj.permute(input);
        int k = 0;
        Set<List<Integer>> pnr = null;
        for (int i = 0; i <= input.size(); i++) {
            pnr = new HashSet<List<Integer>>();
            for(List<Integer> integers : output){
            pnr.add(integers.subList(i, integers.size()));
            }
            k = input.size()- i;
            System.out.println("P("+input.size()+","+k+") :"+
            "Count ("+pnr.size()+") :- "+pnr);
        }
    }
    public Collection<List<T>> permute(Collection<T> input) {
        Collection<List<T>> output = new ArrayList<List<T>>();
        if (input.isEmpty()) {
            output.add(new ArrayList<T>());
            return output;
        }
        List<T> list = new ArrayList<T>(input);
        T head = list.get(0);
        List<T> rest = list.subList(1, list.size());
        for (List<T> permutations : permute(rest)) {
            List<List<T>> subLists = new ArrayList<List<T>>();
            for (int i = 0; i <= permutations.size(); i++) {
                List<T> subList = new ArrayList<T>();
                subList.addAll(permutations);
                subList.add(i, head);
                subLists.add(subList);
            }
            output.addAll(subLists);
        }
        return output;
    }
}

output:
P(3,3) : Count (6) :- [[1, 2, 3], [2, 3, 1], [3, 2, 1], [3, 1, 2], [2, 1, 3], [1, 3,
2]]
P(3,2) : Count (6) :- [[3, 1], [2, 1], [3, 2], [1, 3], [2, 3], [1, 2]]
P(3,1) : Count (3) :- [[3], [1], [2]]
P(3,0) : Count (1) :- [[]]

My problem is increasing the numbers in the input list. Running time increases, and after 11 numbers in the input list, the program almost dies. Takes around 2 GB memory to run.

I am running this on a machine having 8GB RAM and i5 processor, so the speed and space is not a problem.

I would appreciate it if anyone can help me write more efficient code.

The number of permutations typically increases factorially. Since 3! = 6, 4! = 24, 5! = 120, 6! = 720, 7! = 5040, 8! = 40,320, 9! = 362,880, 10! = 3,628,800, 11! = 39,916,800, 12! = 479,001,600.

You can see that it get very large, very quickly. The output would be similarly huge.

The bottom line is, that past a certain point, there's just no way to keep the entire set in memory. A couple of numbers later, you wouldn't be able to afford a disk drive large enough to store it. A couple of number after that, there isn't enough paper on the planet to be able to print it. Somewhere in the 30's or 40's, there wouldn't be enough atoms in the entire universe to represent the results in an atomic scale computer.


Using Biopython, I would like to optimize this code I have the following code:

from Bio import AlignIO
import itertools

out=open("test.csv","a")
align = AlignIO.read("HPV16_CG.aln.fas", "fasta")
n=0
def SNP(line):
    result=[]
    result.append(str(n+1))
    result.append(line[0])
    result.append(align[y].id.rsplit("|")[3])
    result.append(x)
    return result

while n<len(align[0]):
  line = align[:,n]
  y=0
  for x in line:
    if line[0]!=x:
      print >> out, ','.join(map(str,SNP(line)))
      y=y+1
    else:
      y=y+1
  y=0
  n=n+1
out.close()

f=open("test.csv","rU")

out=open("test_2.csv","a")
lines=f.read().split()

for key, group in itertools.groupby(lines, lambda line: line.partition(',')[0]):
    print >>out, ','.join(group)

out.close()
f.close()

As you can see, I am currently writing two files. I really only need the second file.
Does anyone have any suggestions to combine both "subscripts" into one?

The input file "HPV16_CG.aln.fas" looks like this:

>gi|333031|lcl|HPV16REF.1| Alpha-9 - Human Papillomavirus 16, complete genome.
ACTACAATAATTCATGTATAAAACTAAGGGCGTAACCGAAATCGGTTGAACCGAAACCGG

>gi|333031|gb|K02718.1|PPH16 Human papillomavirus type 16 (HPV16), complete genome
ACTACAATAATTCATGTATAAAACTAAGGGCGTAACCGAAATCGGTTGAACCGAAACCGG

>gi|196170262|gb|FJ006723.1| Human papillomavirus type 16, complete genome
ACTACAATAATTCATGTATAAAACTAAGGGCGTAACCGAAATCGGTTGAACCGAAACCGG

I really appreciate all the help/suggestions to help me improve this!

First of all, read PEP8 (all of it!) and adhere to it. This is the de facto standard of Python code.

In the following, I’ll tacitly assume PEP8 as given.

Secondly, it would help if you had actually written what the expected output looks like. Given your input file, the script produces empty output.

I don’t understand what the while loop is supposed to do. align represents a single alignment record – you are reading a sequence file which contains multiple sequences. Furthermore, what is align[:, n]? Did you maybe intend to access the seq sub-object?

Likewise, the SNP method doesn’t at all make clear what its function is. And it implicitly uses global variables. This is error-prone and unreadable: use parameters instead. The function also accesses some object align[y] which won’t exist when y > 0, because AlignIO.read only reads a single alignment.

Furthermore, if you’re handling CSV files, I’d strongly suggest using the csv module. It’s not really less code in your particular case but it provides a more uniform handling and makes the code easily extensible.

Just to show what this could look like when done cleaner, here’s the production of the first file. I’d love to show the full code but I really don’t understand what the code is supposed to do since it doesn’t work for me, and I’m unfamiliar with the function of align[:,n] (and I think there’s an error in the access to align, anyway.

#!/usr/bin/env python

import csv
import itertools
import sys
from Bio import AlignIO

def snp(line, alignment, n, base, pos):
    """Documentation missing!"""
    id = alignment[pos].id.rsplit('|')[3] # why rsplit?
    return [ str(n + 1), line[0], id, base ]

def main():
    # TODO Use command line argumnents to provide file names

    alignment = Bio.AlignIO.read('HPV16_CG.aln.fas', 'fasta')

    with csv.writer('test.csv') as out:
        for n in range(len(alignment[0])):
            line = align[ : , n]
            for pos, base in line:
                if line[0] != x:
                    out.writerow(snp(line, alignment, n, base, pos))

if __name__ == '__main__':
    sys.exit(main())

In this code I tried to give the variables meaningful names but – again, since I don’t know what this is supposed to do – I had to guess quite a lot based on usage.

And by the way, range(len(x)) is actually kind of an anti-pattern but I can’t for the heck of it remember the “correct” usage (and even the documentation advises this).


Code for creating combinations taking a long time to finish I have the following code that I'm using to create combinations of elements in my dataset:

start_time = time.time()
question_pairs = []
import itertools as iter

for memberid in IncorrectQuestions.memberid.unique():
    combinations = IncorrectQuestions[IncorrectQuestions.memberid == memberid].groupby('memberid').questionid.apply(
        lambda x: list(iter.combinations(x, 2)))
    for elem in combinations:
        for el in elem:
            question_pairs.append(list(el))

print("--- %s seconds ---" % (time.time() - start_time))

the DataFrame IncorrectQuestions has about 40 Million records. Here's the sample dataset:

    memberid    created firstencodedid  questionid
0   9   2016-01-18 05:10:44 MAT.CAL.110 5696d0248e0e0869c96357d3
1   9   2016-01-18 05:10:44 MAT.CAL.110 5696cbc45aa413444ffd9973
2   9   2016-01-18 05:10:44 MAT.CAL.110 5696cf86da2cfe6f21d09879
3   34  2016-11-10 04:24:14 MAT.ARI.300 51d8cd415aa41337ec50425a
4   34  2016-11-10 04:24:14 MAT.ARI.300 559a84505aa4136cb37be676

The piece of code above takes way too long. It has been an hour and it is still running. Is there a way to optimize this piece of code so it takes less time?

The desired output would create all possible combinations of questionid for each memberid. For example, the combinations for memberid = 9 would be:

['5696d0248e0e0869c96357d3','5696cbc45aa413444ffd9973']

['5696d0248e0e0869c96357d3','5696cf86da2cfe6f21d09879']

['5696cbc45aa413444ffd9973','5696cf86da2cfe6f21d09879']

The combinations for memberid = 34 would be:

['51d8cd415aa41337ec50425a','559a84505aa4136cb37be676']

And finally, the combined output would be both the lists combined, i.e.:

['5696d0248e0e0869c96357d3','5696cbc45aa413444ffd9973']

['5696d0248e0e0869c96357d3','5696cf86da2cfe6f21d09879']

['5696cbc45aa413444ffd9973','5696cf86da2cfe6f21d09879']

['51d8cd415aa41337ec50425a','559a84505aa4136cb37be676']

I hope this makes things clearer.

Any pointers would be appreciated.

You take the combinations in a way too convoluted way. For starter, I would simplify the retrieval of "same memberid" questions:

for memberid in IncorrectQuestions.memberid.unique():
    questions = IncorrectQuestions[IncorrectQuestions.memberid == memberid].questionid

No need to groupby here since you’re already filtering by the one memberid you’re interested in. Thus producing combinations can be as simple as:

    for elem in itertools.combinations(questions, 2):
        question_pairs.append(list(elem))

Now, I don't see why you want to explicitly turn the tuples returned by itertools.combinations into lists as it adds up time and memory usage. So I suggest dropping it.

Now in the for loop, the use of unique() plus filtering on these unique values is the exact use-case for groupby. So you can rewrite it:

for member_id, questions in IncorrectQuestions.groupby('memberid'):
    for pair in itertools.combinations(questions.questionid, 2):
        question_pairs.append(pair)

I would then turn it into a function for better reusability/testing and use Python naming conventions:

def generate_combination_of_questions(dataframe):
    question_pairs = []
    for _, questions in dataframe.groupby('memberid'):
        for pair in itertools.combinations(questions.questionid, 2):
            question_pairs.append(pair)
    return question_pairs

But the combo var = [] + for loop + var.append should be turned into either a list-comprehension or a generator for better performances:

def generate_combination_of_questions(dataframe):
    return = [
        pair
        for _, questions in dataframe.groupby('memberid')
        for pair in itertools.combinations(questions.questionid, 2)
    ]

or

def generate_combination_of_questions(dataframe):
    for _, questions in dataframe.groupby('memberid'):
        yield from itertools.combinations(questions.questionid, 2)

The second version uses Python 3 syntax and is called as follow:

question_pairs = list(generate_combination_of_questions(IncorrectQuestions))

If you don't have Python 3 available, you can use the more verbose:

def generate_combination_of_questions(dataframe):
    for _, questions in dataframe.groupby('memberid'):
        for pair in itertools.combinations(questions.questionid, 2):
            yield pair

Your timing procedure can also be enhanced by the use of either time.perf_counter() or, better, the timeit module

Example code with timeit:

import itertools

def generate_combination_of_questions(dataframe):
    for _, questions in dataframe.groupby('memberid'):
        yield from itertools.combinations(questions.questionid, 2)

if __name__ == '__main__':
    import timeit
    setup = """\
import pandas as pd
from __main__ import generate_combination_of_questions
incorrect = pd.read_csv(<...>)"""

    print(timeit.timeit(
        'list(generate_combination_of_questions(incorrect))',
        setup=setup, number=1))

Reading at the various comments about the size of your dataset, you definitely want the generator version (the one with yield instead of return) as it will allow you to iterate over the results one by one and not store all of them in memory before. Otherwise, the list will fill up all your memory.

For instance, writing each pair into a file might look like:

import itertools
import csv
import pandas as pd

def generate_combination_of_questions(dataframe):
    for _, questions in dataframe.groupby('memberid'):
        yield from itertools.combinations(questions.questionid, 2)

def main(file_in, file_out):
    incorrect_questions = pd.read_csv(file_in)
    with open(file_out, 'w') as output:
        writer = csv.writer(output)
        for pair in generate_combination_of_questions(incorrect_questions):
            writer.writerow(pair)

if __name__ == '__main__':
    main(<path1>, <path2>)

But you're obviously free to do any form of processing while iterating over the generator; writing to a file is just the simplest example.


Search Range in Binary Search Tree I have implemented this algorithm, but I would like to get ride of the variable res, which is outside the Solution class, could anyone help me with this?

"""
Definition of TreeNode:
class TreeNode:
    def __init__(self, val):
        self.val = val
        self.left, self.right = None, None
"""
res = []
class Solution:
    """
    @param root: The root of the binary search tree.
    @param k1 and k2: range k1 to k2.
    @return: Return all keys that k1<=key<=k2 in ascending order.
    """
    def searchRange(self, root, k1, k2):
        # write your code here
        if root is None:
            return []
        if root.val > k1:
            self.searchRange(root.left, k1, k2)
        if root.val >= k1 and root.val <= k2:
            res.append(root.val)
        if root.val < k2:
            self.searchRange(root.right, k1, k2)
        return res

You should change searchRange to search_range as per PEP8, Python's style guide.
If this is Python2 then you should use Solution(object). (This is why we have version tags).
res should go in the function. As per RobertB's comment.
Just call searchRange twice and you will clearly see why.
root, k1 and k2 are all bad names.
Rename them to better ones.
E.G. node, minimum and maximum, respectively.
You should use k1 <= root.val <= k2 instead of root.val >= k1 and root.val <= k2.
There is currently no need to return []. return None or any return work as well.
If you wrap searchRange in another function, it can be much simpler.

If I were to do most of the above. (I didn't put it in a class.)

def search_range(root, minimum, maximum):
    nodes = []

    def recursive_search(node):
        if node is None:
            return None

        if node.val > minimum:
            recursive_search(node.left)
        if minimum <= node.val <= maximum:
            nodes.append(node.val)
        if node.val < maximum:
            recursive_search(node.right)

    recursive_search(root)
    return nodes


Simple object oriented design for an Allergy class Build a system for entering and displaying the allergies that patients may have.
  The allergy will have its own set of symptoms reactions. The allergy will also have a spectrum of severity which the clinician should be aware of, and allergies can be reported by the patient or by next of kin.

Please review my code and tell me how I can improve my object-oriented design.

class Allergy

    attr_accessor :name, :reporter, :time_reported
    attr_reader :symptoms

    def initialize(name, reporter, time_reported=Time.now)
        @name = name
        @reporter = reporter
        @symptoms = []
        @time_reported = time_reported
    end

    def add_symptoms(symptom, severity)
        symptom = Symptom.new(symptom, severity)
        @symptoms.push(symptom)
    end

end

class Symptom
    attr_accessor :symptom, :severity

    def initialize(symptom, severity)
        @symptom = symptom
        @severity = severity
    end

end

Two things to notice..

    def add_symptoms(symptom, severity)

Actually, you don't add many symptoms, but only one :)

Secondly, but this has nothing to do with object oriented. Why is the sympton name given to attr_accessor? Do you expect to change it at runtime (e.g: changing "Rash" to "Sneezes"?)
Same thing with time_reported, do you expect it to be updatable? :/

Secondly, but I think this is more a "taste" of matter.. I'd honestly prefer to pass something similar:

def add_symptoms(symptom)

with symptom already being the object instantiated. Why? Because if you are to then have multiple subclasses of Symptom (e.g:

class HeavySimptom
class RespiratorySimptom
class AmbiguousSymptom

) your code would be quite a pain to change


Splice from Array if index matches This code loops through an array called $rootScope.watchlist (uses AngularJS). It gets the current iteration of the loop to access that x iteration through $rootScope.watchlist and get the id index. If that matches the variable titleID, then it will splice (remove) that index only and then stop the foreach to stop unnecessarily continuing.

for (var i = $rootScope.watchlist.length - 1; i >= 0; i--) {
    if($rootScope.watchlist[i]['id'] == titleID) {
        $rootScope.watchlist.splice(i, 1);
        break;
    }
}

Is there a better way of doing such? Possibly using .some()?

A while back, I went through these exercises about functional programming in JS and since then I strive to use those techniques whenever possible. Bear in mind that functional techniques can lead to slower code, since a function call is made for each iteration instead of just running code within a loop block, but hopefully it is easier to work with each array element in that fashion - e.g. would you read/type if($rootScope.watchlist[i]['id'] == titleID) { or if(element['id'] == titleID) {?

After reading Igor's answer as well as this article about break with respect to functional programming, I believe the following should work:

$rootScope
.watchlist
.slice()         //  copy before reverse(), because reverse() is mutating
.reverse()
.some(function(element, i) {
    if(element['id'] == titleID) {
        $rootScope.watchlist.splice($rootScope.watchlist.length - 1 - i, 1);
        return true;
    }
});

Edit:

Igor suggests simplifying that callback passed to some(). You can use an ES-6 arrow function and a ternary operator to eliminate a few lines, like this:

.some((element, i) => (element['id'] == titleID)? $rootScope.watchlist.splice($rootScope.watchlist.length - 1 - i, 1) : false);

See a demonstration in this plunker. Click the button labeled remove watcher and see how the watcher is removed. This code presumes  $rootScope.watchlist is equivalent to $rootScope.$$watchers...


Speedup cython dictionary counter I wrote a simple cython script to optimize the collections.Counter of a dictionary counter and the python zip implementation (the main input is a list of tuples). Is there a way to speed it up?

%%cython --annotate
cimport cython
import numpy as np
cimport numpy as np
from collections import defaultdict

@cython.boundscheck(False)
@cython.wraparound(False)
def uniqueCounterListCython(list x not None):

    cdef:
        Py_ssize_t i,n

    n = len(x)
    dx = defaultdict(int)
    for i from 0 <= i < n:
        dx[x[i]] += 1
    return dx

@cython.boundscheck(False)
@cython.wraparound(False)
def zipCython(np.ndarray[long,ndim=1] x1 not None, np.ndarray[long,ndim=1] x2 not None):

    cdef:
        Py_ssize_t i,n

    n = x1.shape[0]
    l=[]
    for i from 0 <= i < n:
        l.append(((x1[i],x2[i])))
    return l

Sample input -

uniqueCounterListCython(zipCython(np.random.randint(0,3,200000),np.random.randint(0,3,200000)))

EDIT:
Found a kind of trivial way to speed things up - just merge the two functions:

@cython.boundscheck(False)
@cython.wraparound(False)
def uniqueCounterListCythonWithZip(np.ndarray[long,ndim=1] x1 not None, np.ndarray[long,ndim=1] x2 not None):

    cdef:
        Py_ssize_t i,n

    n = x1.shape[0]
    dx = defaultdict(int)
    for i from 0 <= i < n:
        dx[((x1[i],x2[i]))] += 1
    return dx

Any more suggestions?

You don't give us much context for this problem, so it's unclear to me exactly what you are trying to achieve. But in your example, you have a pair of NumPy arrays containing integers in the range 0–2, and you seem to want to count the number of occurrences of each pair of values.

So I suggest encoding pairs of integers in the range 0–2 into a single integer in the range 0–8, using numpy.bincount to do the counting, and then using numpy.reshape to decode the result, like this:

>>> import numpy as np
>>> x, y = np.random.randint(0,3,200000), np.random.randint(0,3,200000)
>>> counts = np.bincount(x * 3 + y).reshape((3, 3))
>>> counts
array([[22282, 22093, 22247],
       [22084, 22295, 22396],
       [22012, 22243, 22348]])

A quick check that I got the encoding/decoding right:

>>> counts[0,2] == np.count_nonzero((x == 0) & (y == 2))
True

This runs much faster than the code in your question (assuming I have interpreted your profile screenshots correctly):

>>> from timeit import timeit
>>> timeit(lambda:np.bincount(x * 3 + y).reshape((3, 3)), number=1000)
2.7519797360000666


Drawing a castle and towers with acm.graphics The following code is my solution of exercise 10 of chapter 5 of The Art and Science of Java. So here, using acm program and graphics, I need to draw a castle with two side towers and other three consecutive towers some distance away from the main building(castle and its two side towers). Is there any way to do this faster than that?

(The drawing is perfect as I spent plenty of time searching for the perfect location for every object on the canvas.)

import acm.program.*;
import acm.graphics.*;

public class Hw1 extends GraphicsProgram {

final double tower_width = 150;
final double tower_height = 600;
final double main_house_width = 400;
final double main_house_height = 500;
final double s_tower_width = 75;
final double s_tower_height = 300;
final double door_width = 50;
final double door_height = 100;
final double window_width = 100;
final double window_height = 100;

GRect create_tower(double x, double y) {
    GRect rect = new GRect(tower_width, tower_height);
    rect.move(x, y);
    return rect;
}

GRect create_main_home(double x, double y) {
    GRect rect = new GRect(main_house_width, main_house_height);
    rect.move(x, y);
    return rect;
}

GRect create_s_tower(double x, double y) {
    GRect rect = new GRect(s_tower_width, s_tower_height);
    rect.move(x, y);
    return rect;
}

GRect create_door(double x, double y) {
    GRect rect = new GRect(door_width, door_height);
    rect.move(x, y);
    return rect;
}

GOval create_window(double x, double y) {
    GOval circle = new GOval(window_width, window_height);
    circle.move(x, y);
    return circle;
}

GPolygon create_arch(double x, double y) {
    GPolygon arch = new GPolygon();
    arch.addVertex(x, y);
    arch.addVertex(x+150, y);
    arch.addVertex(x+75, y-150);
    return arch;
}

GPolygon create_s_arch(double x, double y) {
    GPolygon arch = new GPolygon();
    arch.addVertex(x, y);
    arch.addVertex(x+75, y);
    arch.addVertex(x+37.5, y-100);
    return arch;
}

GPolygon create_arch_door(double x, double y) {
    GPolygon arch = new GPolygon();
    arch.addVertex(x, y);
    arch.addVertex(x+50, y);
    arch.addVertex(x+25, y-50);
    return arch;
}

GPolygon create_house_arch(double x, double y) {
    GPolygon arch = new GPolygon();
    arch.addVertex(x, y);
    arch.addVertex(x+400, y);
    arch.addVertex(x+200, y-200);
    return arch;
}

public void run() {

    add(create_tower(700, 250));
    add(create_tower(150, 250));
    add(create_main_home(300, 350));
    add(create_s_tower(1000, 550));
    add(create_s_tower(1200, 550));
    add(create_s_tower(1400, 550));
    add(create_door(475, 750));
    add(create_window(350, 450));
    add(create_window(550, 450));
    add(create_arch(150, 250));
    add(create_arch(700, 250));
    add(create_s_arch(1000, 550));
    add(create_s_arch(1200, 550));
    add(create_s_arch(1400, 550));
    add(create_arch_door(475, 750));
    add(create_house_arch(300, 350));
}

}

here are my comments:

1) instead of having a create method for objects, why not have classes that you can instantiate? seems more clear to me

public class Tower extends GRect {
  final double width = 150;
  final double height = 600;
  public Tower() {
    super(width, height);
  }
}

2) instead of creating a GRect and then moving it, you can specify the location when you add it: add(new Tower(), 700, 250); this shows more clearly the intention of placing the tower at a certain location


Check if we can form a word across words in a sentence I've come across a simple challenge: given a phrase, check if we can form the given word vertically by stacking words in rows. For example, we can form the word "boom" from this phrase:

   every breath
            you take
every move
      you make

My Python code:

def can_spell(s, w):
    words = s.split()
    words_iter = iter(words)

    for char in w:
        try:
            next_word = next(words_iter)
            while char not in next_word:
                next_word = next(words_iter)
        except StopIteration:
            return False
    return True

print(can_spell('every breath you take every move you make', 'boom'))  # True

Is this the most efficient way to do it?

Staying close to your original appraoch, you could use another for loop with an else instead of try/except and while. The else is triggered when the inner loop has ended normally, without the break, i.e. when there are no more parts left in the sentence.

def can_spell(sentence, word):
    words_iter = iter(sentence.split())
    for char in word:
        for part in words_iter:
            if char in part:
                break
        else:
            return False
    return True

Or, you could use a nested generator expression checking that all characters in the word are in any of the remaining parts of the sentence-iterator. This works since any will only consume as many items from the iterator as necessary until it finds a match.

def can_spell(sentence, word):
    words_iter = iter(sentence.split())
    return all(any(char in part for part in words_iter) for char in word)

Following an entirely different route, you could use a regular expression, joining the characters of the word such that between each character there has to be at least one space (and, optionally, any other characters), e.g. b.* .*o.* .*o.* .*m for boom, and check if that regular expression can be found in the sentence:

import re
def can_spell(sentence, word):
    return re.search('.* .*'.join(word), sentence) is not None


Sorting three Decimal values I have been given three values: @lLength, @lWidth, @lHeight. I need to take these DECIMAL(4,2) values and set the values of @lMax, @lMid, @lMin with the values I've been given in order of value. So whichever value of Length, Width or Height is the greatest will be stored in Max, and the second created in Mid, and the lowest value in Min.

Note: I am limited to Microsoft SQL Server 2008.

-- Declarations are for testing purposes only
DECLARE @lLength DECIMAL(4,2) = CAST('1.0' AS DECIMAL(4,2))
DECLARE @lWidth  DECIMAL(4,2) = CAST('2.0' AS DECIMAL(4,2))
DECLARE @lHeight DECIMAL(4,2) = CAST('3.0' AS DECIMAL(4,2))
DECLARE @lMax    DECIMAL(4,2)
DECLARE @lMid    DECIMAL(4,2)
DECLARE @lMin    DECIMAL(4,2)

--Determine which is higher and store it.
IF @lLength > @lWidth BEGIN
    SET @lMax = @lLength
    SET @lMid = @lWidth
END ELSE BEGIN
    SET @lMax = @lWidth
    SET @lMid = @lLength
END

--Now determine where the 3rd value needs to be inserted
IF @lHeight < @lMid BEGIN
    SET @lMin = @lHeight
END ELSE IF @lHeight < @lMax BEGIN
    SET @lMin = @lMid
    SET @lMid = @lHeight
END ELSE BEGIN
    SET @lMin = @lMid
    SET @lMid = @lMax
    SET @lMax = @lHeight
END

--This select is for testing purposes only
SELECT @lMax as MAX, @lMid AS MID, @lMin AS MIN

The code above works, and I added declarations and a select to make that readily apparent. I am looking for a general review of this operation, perhaps a better way to do this. You can criticize the naming convention, but that is something I have no control over. Also, I do not have to worry about scalability of this operation unless our world gets a 4th physical dimension in the near future.

I don't know how you are using this information, but I think that you could create a very small table variable or temp table with like one column and then sort and select the information in order.

DECLARE @Table1 TABLE
(
     Dimension VARCHAR(10),
     Size DECIMAL(4,2)
)

INSERT INTO @Table1
(
     Dimension
     , Size
)
VALUES
('Length', @lLength),('Width', @lWidth),('Height',@lHeight)

SELECT * FROM @Table1 ORDER BY Size DESC

This eliminates 3 variables and all if statements (which are not very efficient in RDBMS's)
Setting variables over and over again is not very efficient in a RDBMS.

you can select these into the variables like this

SET @lMax = SELECT TOP (1) FROM @Table1 ORDER BY Size DESC
WITH orderedTable AS
(
    SELECT Size, ROW_NUMBER() OVER (ORDER BY Size) as `RowNumber`
    FROM @Table1
)
SET @lMid = SELECT Size FROM orderedTable WHERE RowNumber = 2;

WITH orderedTable AS
(
    SELECT Size, ROW_NUMBER() OVER (ORDER BY Size) as `RowNumber`
    FROM @Table1
)
SET @lMin = SELECT Size FROM orderedTable WHERE RowNumber = 3;

I am sure that Phrancis could come up with a better query than this, especially because I call the same CTE twice and I know that it could be more efficient perhaps with a temp table or something.

after seeing what @Phrancis is going to post I came up with something to get rid of the second CTE

because there are only 3 numbers you can subtract the middle number and the largest number to retrieve the minimum number using mathematics, like this

SET @lMax = SELECT TOP (1) FROM @Table1 ORDER BY Size DESC
WITH orderedTable AS
(
    SELECT Size, ROW_NUMBER() OVER (ORDER BY Size) as `RowNumber`
    FROM @Table1
)
SET @lMid = SELECT Size FROM orderedTable WHERE RowNumber = 2;
SET @lMin = SELECT (SUM(Size) - @lMid - @lMax) FROM @Table1 --Thanks @Phrancis

So the Whole thing looks like this

DECLARE @Table1 TABLE
(
     Dimension VARCHAR(10),
     Size DECIMAL(4,2)
)

INSERT INTO @Table1
(
     Dimension
     , Size
)
VALUES
('Length', @lLength),('Width', @lWidth),('Height',@lHeight)

SET @lMax = SELECT TOP (1) FROM @Table1 ORDER BY Size DESC
WITH orderedTable AS
(
    SELECT Size, ROW_NUMBER() OVER (ORDER BY Size) as `RowNumber`
    FROM @Table1
)
SET @lMid = SELECT Size FROM orderedTable WHERE RowNumber = 2;
SET @lMin = SELECT (SUM(Size) - @lMid - @lMax) FROM @Table1 --Thanks @Phrancis

I removed SELECT * FROM @Table1 ORDER BY Size DESC because we are filling in the variables in a different way.

If you were using this information in a program or something then I would tell you to just do it there, but it sounds like you are stuck in SQL Server


Calculate and displays the sum of all numbers from 1 to 20 This is a beginner's class. I just need help verifying if this code is correct, or edit the parts that need fixing.
n = int(input("Enter a number"))
sum = 0
for i in range(1,n+1):
    sum = sum + i
    print("The sum is:", sum)

Bug?
Do you want to display a cumulative sum every time the loop executes? That's what you do now. If not - if you want to only display the final sum - you need to de-indent your print.
Sum
You can do the whole sum using the actual sum function, like this:
total = sum(range(1, n + 1))

Note that you should not call a variable sum, because that shadows the built-in sum function.
Calculating the sum
This doesn't actually need a loop or a call to sum at all. Math will tell you that the entire sum will evaluate to \$n(n + 1)/2\$; an example of equivalence:
$$
\begin{align}
1 + 2 &= 3\\
\frac{2 * 3}{2} &= 3
\end{align}
$$


Word = smallWord1 + smallWord2 Look for all six letter words which are composed of two concatenated smaller words.

Please give me feedback regarding good coding standards.

import java.io.File
class KataExercise(val wordLength: Int) {
    fun concatenatedWords(): List<String> {
        val searchWords = dictionary
                .filter { it.length == wordLength }
        val result = mutableListOf<String>()
        for (subWord in smallDictionary) {
            val subWordLength = subWord.length
            (smallDictionary)
                    .filter { it.length + subWordLength == wordLength }
                    .map { it + subWord }
                    .filter { it in searchWords }
                    .forEach { result.add(it) }
        }
        return result.toList()
    }
    private val dictionary = File("dict.text").readLines()
            .filter { "'" !in it }
    private val smallDictionary: List<String> = dictionary
            .filter { it.length < wordLength}
}

Reduced mutation

val result = mutableListOf<String>()
for (subWord in smallDictionary) {
    val subWordLength = subWord.length
    (smallDictionary)
        .filter { it.length + subWordLength == wordLength }
        .map { it + subWord }
        .filter { it in searchWords }
        .forEach { result.add(it) }
}
return result.toList()

could be replaced with:

return smallDictionary
    .flatMap { first -> smallDictionary.map { first to it } }
    .filter { it.first.length + it.second.length == wordLength }
    .map { it.first + it.second }
    .filter { it in searchWords }

Optimisation

You could filter the 2 lists so the starting list is only words that start with the same letter as the target word, and the ending list is only words that end with the last letter of the target word, for a large wordlist this in theory could reduce the size of these lists by a large amount.


Simon Says in Scheme Ok, it's not completely like Simon Says, since it's text-based. There is nothing about memorizing based on visual cues and/or sound here, just memorizing long sequences of numbers. I wanted to do it in the language I write the least code in, Scheme (w/ Chicken), and see what happens.

(use posix)
(use (srfi 1))

(define button-count 4)

(define (clear-screen)
  (system "cls"))

(define (make-delay)
  (sleep 2))

(define (show-sequence sequence)
  (if (not (null? sequence))
    (begin
      (clear-screen)
      (make-delay)
      (write (car sequence))
      (make-delay)
      (show-sequence (cdr sequence)))
    (begin
      (clear-screen)
      (write-line "GO!"))))

(define (show-score score)
  (format #t "Your score: ~A~%" score))

(define (get-player-input)
  (read-line))

(define (make-sequence length)
  (map random (make-list length button-count)))

(define (player-sequence-matches? target-sequence)
  (cond
    [(null? target-sequence) #t]
    [(string=? (get-player-input) (number->string (car target-sequence)))
     (player-sequence-matches? (cdr target-sequence))]
    [else #f]))

(define (simon-says difficulty)
  (let [(current-sequence (make-sequence difficulty))]
    (show-sequence current-sequence)
    (if (player-sequence-matches? current-sequence)
      (+ 1 (simon-says (+ difficulty 1)))
      0)))

(define (play-simon-says)
  (show-score (simon-says 1)))

If you want to run it on something Unix, you'll need to change (system "cls") to (system "clear").

Ideally I'd like to use termcap or terminfo or the like to clear the terminal without shelling out. Alas, it seems Chicken doesn't have an egg for either termcap or terminfo. (It does have an egg for curses, but I consider that to be too heavyweight for just clearing a terminal.)
I'd probably use for-each for iterating through the items in show-sequence, rather than the recursive approach you have:

(define (show-sequence seq)
  (for-each (lambda (x)
              (clear-screen)
              (make-delay)
              (display x)    ;; not write
              (make-delay))
            seq)
  (clear-screen)
  (display "GO!\n"))

Not a huge fan of making a get-player-input function that's exactly the same as read-line. I'd just call read-line directly.
Your definition of make-sequence is quite clever, but it's not very idiomatic. Since you have SRFI 1 loaded, I'd just use list-tabulate:

(define (make-sequence len)  ;; not length
  (list-tabulate len (lambda (_) (random button-count))))

You can simplify player-sequence-matches? by using SRFI 1's every:

(define (player-sequence-matches? seq)
  (every (lambda (x)
           (= (string->number (read-line)) x))
         seq))

And since Chicken has cut builtin, you can simplify even further:

(define (player-sequence-matches? seq)
  (every (cut = (string->number (read-line)) <>) seq))

For calculating the total score, I'd rather accumulate the score, rather than use recursion to hold the score. Like so:

(define (simon-says difficulty)
  (let loop ((difficulty difficulty)
             (score 0))
    (define seq (make-sequence difficulty))
    (show-sequence seq)
    (if (player-sequence-matches? seq)
        (loop (add1 difficulty) (add1 score))
        score)))

This way, the function is totally tail-recursive.


Simple encoder by a beginner I decided to make a quick little encoder in Python as a little challenge for myself. I decided to submit my program as I am very interested in improving my programming skills and learning better ways to code. My program simply asks for a string and for a key. The key could be +3, +4, etc. The key will add the number inputted to the number corresponding to each letter, then return the new letters.

dic = {"A": 0, "B": 1, "C": 2, "D": 3, "E": 4, "F": 5, "G": 6, "H": 7, "I": 8, "J": 9, "K": 10, "L": 11, "M": 12, "N": 13, "O": 14, "P": 15, "Q": 16, "R": 17, "S": 18, "T": 19, "U": 20, "V": 21, "W": 22, "X": 23, "Y": 24, "Z": 25, " ": 26}
dic2 = {}
for x,v in dic.items():
    dic2[v] = x

program = True

while program:
    list = []
    encodeNums = []
    encodeStr = []
    task = input("Would you like to encode or decode?: ").lower()
    if task == "encode":
        string = input("What would you like to encode?: ").upper()
        key = input("What key would you like to use?: ")
        for x in string:
            list.append(dic[x])
        for x in list:
            encodeNums.append(eval("{}{}".format(x,key)) % 27)
        for x in encodeNums:
            encodeStr.append(dic2[x])
        print("".join(encodeStr))

I'd highly suggest learning comprehensions. The simplest is a list comprehension. This allows you to change say:

list = []
for x in string:
    list.append(dic[x])

To:

list = [dic[x] for x in string]

However, you can get better memory usage by using a generator comprehension, this works in roughly the same way as a list, however you can't index it. Which for this code is fine. To do this replace the square brackets with curved ones:

gen = (dic[x] for x in string)

You can also use a dictionary comprehension to simplify your creation of dic2:

dic2 = {v: k for k, v in dic.items()}

Purge all memory of eval, it leads to poor code and has severe security vulnerabilities. Instead cast the users input to an int. This can simplify decoding too. However you have to make sure the user does actually enter a number.

I don't like list = [] being outside the if statement. There's not much of a need for this. However it's fixed when using comprehensions.

I also think you should use while True rather than while program:. As you never change program.

Merging this all together gets you:

dic = {"A": 0, "B": 1, "C": 2, "D": 3, "E": 4, "F": 5, "G": 6, "H": 7, "I": 8, "J": 9, "K": 10, "L": 11, "M": 12, "N": 13, "O": 14, "P": 15, "Q": 16, "R": 17, "S": 18, "T": 19, "U": 20, "V": 21, "W": 22, "X": 23, "Y": 24, "Z": 25, " ": 26}
dic2 = {v: k for k, v in dic.items()}

while True:
    task = input("Would you like to encode or decode?: ").lower()
    if task == "encode":
        string = input("What would you like to encode?: ").upper()
        while True:
            try:
                key = int(input("What key would you like to use?: "))
                break
            except ValueError:
                pass
        list = (dic[x] for x in string)
        encodeNums = ((x + key) % 27 for x in list)
        encodeStr = (dic2[x] for x in encodeNums)
        print("".join(encodeStr))

Going forward, I'd change your code to encode and decode with the same code. This is as the Caesar cipher is a symmetric cypher, so simply using key = -key allows you to decode the encoding.

Allowing:

dic = {"A": 0, "B": 1, "C": 2, "D": 3, "E": 4, "F": 5, "G": 6, "H": 7, "I": 8, "J": 9, "K": 10, "L": 11, "M": 12, "N": 13, "O": 14, "P": 15, "Q": 16, "R": 17, "S": 18, "T": 19, "U": 20, "V": 21, "W": 22, "X": 23, "Y": 24, "Z": 25, " ": 26}
dic2 = {v: k for k, v in dic.items()}

while True:
    string = input("What would you like cypher?: ").upper()
    task = input("Would you like to encode or decode?: ").lower()
    while True:
        try:
            key = int(input("What key would you like to use?: "))
            break
        except ValueError:
            pass
    if task == "decode":
        key = -key

    list = (dic[x] for x in string)
    encodeNums = ((x + key) % 27 for x in list)
    encodeStr = (dic2[x] for x in encodeNums)
    print("".join(encodeStr))


Update bitmap in picturebox with data from a socket I'm working on a screen sharing project, and I receive a small block of images from a Socket constantly. I need to update them on a certain initial desktop bitmap I have.

After receiving every block, it copies its pixels according to a specific position (X and Y parameters), which is how the initial image gets updated. But then comes the part where I need to display it on a Picturebox. I handle the Paint event and redrawing it all again. The entire initial image is pretty big (1920X1080 in my case).

private void MainScreenThread()
{
    ReadData();//reading data from socket.
    initial = bufferToJpeg();//first intial full screen image.
    pictureBox1.Paint += pictureBox1_Paint;//activating the paint event.
    while (true)
    {
        int pos = ReadData();
        x = BlockX();//where to draw :X
        y = BlockY();//where to draw :Y
        Bitmap block = bufferToJpeg();//constantly reciving blocks.
        Draw(block, new Point(x, y));//applying the changes-drawing the block on the big initial image.

        this.Invoke(new Action(() =>
        {
            pictureBox1.Refresh();//updaing the picturebox for seeing results.
            // this.Text = ((pos / 1000).ToString() + "KB");
        }));
    }
}

private void pictureBox1_Paint(object sender, PaintEventArgs e)
{
    lock (initial)
    {
        e.Graphics.DrawImage(initial, pictureBox1.ClientRectangle); //draws at picturebox's bounds
    }
}

Because I'm aiming at high speed performance (it's kind of a real-time project), I would like to know if there's any method to draw the block itself on the picturebox instead of drawing the whole initial bitmap again. Plus, scaling it to the Picuturebox bounds  seems too inefficient to me.

The picturebox itself is not resizeable - it has a fixed width and height. But I need to scale the initial Bitmap every time (after processing the blocks) so it will fit the picturbox bounds to allow the client to see the full image. Despite their screen resolution, it's lower than the shared screen resolution (the server screen which is getting captured and shared).

This is my drawing method (works extremely fast, copying block with memcpy):

private unsafe void Draw(Bitmap bmp2, Point point)
{
    lock (initial)
    {
        BitmapData bmData = initial.LockBits(new Rectangle(0, 0, initial.Width, initial.Height), System.Drawing.Imaging.ImageLockMode.WriteOnly, initial.PixelFormat);
        BitmapData bmData2 = bmp2.LockBits(new Rectangle(0, 0, bmp2.Width, bmp2.Height), System.Drawing.Imaging.ImageLockMode.ReadOnly, bmp2.PixelFormat);
        IntPtr scan0 = bmData.Scan0;
        IntPtr scan02 = bmData2.Scan0;
        int stride = bmData.Stride;
        int stride2 = bmData2.Stride;
        int Width = bmp2.Width;
        int Height = bmp2.Height;
        int X = point.X;
        int Y = point.Y;

        scan0 = IntPtr.Add(scan0, stride * Y + X * 3);//setting the pointer to the requested line
        for (int y = 0; y < Height; y++)
        {
            memcpy(scan0, scan02 ,(UIntPtr)(Width * 3));//copy one line

            scan02 = IntPtr.Add(scan02, stride2);//advance pointers
            scan0 = IntPtr.Add(scan0, stride);//advance pointers//
        }

        initial.UnlockBits(bmData);
        bmp2.UnlockBits(bmData2);
    }

If your blocks don't change size, why not create adjacent picture boxes so that you update each one individually?

If that isn't providing enough performance, remove the picture box altogether and create a custom user control where you override the OnPaint method and do your drawing inside of it.

This way, the you will pass your user control an array of bitmaps to draw and where to draw them and, when OnPaint gets called, you simply use DrawImage() to place each one at the appropriate location.

No need for you to join the bitmaps manually.

Should a new a image arrive and you want to force redrawing, simply call Invalidate() on the user control which will force OnPaint to be called.
This can be optimized even further as Invalidate can be called only on a specific region of your control, allowing you to only redraw the parts that are necessary.


JavaScript text editing webapp Please review this JavaScript text editing webapp. Review the code quality. Give me suggestions to simplify the code.

var textarea = document.getElementById("textarea"),
  statusBar = document.getElementById("status-bar"),
  inputFile = document.getElementById("input-file"),
  appname = "Notepad",
  isModified = false,
  statusBarOn,
  filename;

function changeDocTitle(newFilename) {
  filename = newFilename;
  document.title = filename + " - " + appname;
}

function dontSave() {
  if (!textarea.value || confirm("You have unsaved changes that will be lost.")) {
    isModified = false;
    return true;
  }
}

function newDoc(text, newFilename) {
  if (!isModified || dontSave()) {
    textarea.value = text || "";
    changeDocTitle(newFilename || "untitled.txt");
  }
  if (statusBarOn) updateStatusBar();
  textarea.focus();
}

function openDoc() {
  if (!isModified || dontSave()) inputFile.click();
  textarea.focus();
}

function renameDoc() {
  var newFilename = prompt("Name this document:", filename);
  if (newFilename !== null) {
    if (newFilename === "")
      changeDocTitle("untitled.txt");
    else
      changeDocTitle(newFilename.lastIndexOf(".txt") == -1 ? newFilename + ".txt" : newFilename);
    return true;
  }
}

function saveDoc() {
  if (renameDoc()) {
    var blob = new Blob([textarea.value.replace(/\n/g, "\r\n")], {
      type: "text/plain;charset=utf-8"
    });
    saveAs(blob, filename);
    isModified = false;
  }
  textarea.focus();
}

function updateStatusBar() {
  var text = textarea.value,
    chars = text.length,
    words = text.split(/\b\S+\b/g).length - 1,
    lines = text.split("\n").length;
  statusBar.value = lines + " lines, " + words + " words, " + chars + " characters";
}

function showHideStatusBar(toState) {
  statusBarOn = toState;
  if (statusBarOn) {
    textarea.style.height = "calc(100% - 21px)";
    statusBar.removeAttribute("hidden");
    updateStatusBar();
  } else {
    textarea.style.height = "";
    statusBar.setAttribute("hidden", "");
  }
}

textarea.addEventListener("input", function() {
  isModified = true;
  if (statusBarOn) updateStatusBar();
});

inputFile.addEventListener("change", function() { // load file
  var file = inputFile.files[0],
    reader = new FileReader();
  reader.onload = function() {
    newDoc(reader.result, file.name);
  };
  reader.readAsText(file);
});

document.addEventListener("keydown", function(e) { // shortcuts
  var key = {
    66: function() { // B
      showHideStatusBar(statusBarOn ? false : true); // toggle
    },
    79: openDoc, // O
    82: newDoc, // R
    83: saveDoc, // S
    191: function() { // /
      alert("Welcome to " + appname + "!");
    },
    "noctrl9": function() { // tab
      var sStart = textarea.selectionStart,
        text = textarea.value;
      textarea.value = text.substring(0, sStart) + "\t" + text.substring(textarea.selectionEnd);
      textarea.selectionEnd = sStart + 1;
    }
  }, fn = e.ctrlKey ? key[e.keyCode] : key["noctrl" + e.keyCode];
  if (fn) {
    e.preventDefault();
    fn();
  }
});

onunload = function() { // store localStorage
  if (isModified) {
    localStorage.setItem("text", textarea.value);
    localStorage.setItem("filename", filename);
  } else
    localStorage.removeItem("text");
  localStorage.setItem("statusBarOn", statusBarOn);
};

onload = function() { // load localStorage
  if (localStorage.getItem("text")) {
    newDoc(localStorage.getItem("text"), localStorage.getItem("filename"));
    isModified = true;
  } else
    newDoc();
  if (localStorage.getItem("statusBarOn"))
    showHideStatusBar(JSON.parse(localStorage.getItem("statusBarOn")) ? true : false);
  else
    showHideStatusBar(true); // default
};

First of all, you should declare all of these in it's own scope, not in the global scope. That way, other code (or your own code) won't collide with naming. A simple IIFE would do:

(function(){
  //All code in here
}());

Your code assumes that it will run on modern browsers. But what if your user doesn't use one or can't use one (cough, enterprise, cough)? You code should check if these new APIs like FileReader and Blob exists. Modernizr is a handy library for this.
Likewise, you should also prepare for inconsistent APIs on browsers. addEventListener doesn't exist in older IE browsers. I suggest you use a library like jQuery for this as well as for other operations to normalize the results. It's easier, and you won't be repeating code.
Use libraries to simplify the job. True, libraries can add a bit of baggage to your code, but it shouldn't be an excuse not to use them. jQuery is just 40KB. In contrast, images and backgrounds in websites reach 1MB. How's that for size?
Scale. This code assumes you will only have one editor. What if you wanted more than just one in the near future? What if the app increases to 100k lines of code? Would you dig deep to change element id's and variable names for a second editor?

Or would prefer calling a function, and a set of configs in a few lines of code and create a second one? Something like:

$('#some-element,#another-element').myVeryCoolTextEditor({
  emoji : true,
  someCoolConfig : true,
});

I suggest you take a look at jQuery Boilerplate to easily structure plugins and plugin states.
Reusablility. A jQuery plugin is not the answer to everything, but I'm just suggesting it for a clean and structured code (Check the comments in the source). You can even go without it, as long as you keep your code reusable and "plugin-like". Think "Drop this script and a one-liner and it just works!"


Minimalist accordion implementation using vanilla JavaScript The following example is an accordion script in JavaScript that does not use jQuery. When I started looking for a simple solution to this requirement I did not accept the idea of having to use a powerful and robust library for a simple implementation. I didn't like the idea of use a machine gun when what I really need is a slingshot. So as a jQuery detox process I decide create my own solution. So far the code is simple enough and I’m planning using it in production as a custom widget in my blog running Wordpress. I’m very enthusiast of minimalist development and I have some concerns with this code.

Any suggestions about how I can achieve a simpler and elegant solution? I was thinking in implementing the element.classList properties.

Here is a live demo of the accordion script.

HTML

Archivos

2015
      <a href="#"

febrero 2015

enero 2015

2014
      <a href="#"

diciembre 2014

noviembre 2014

marzo 2014

febrero 2014

enero 2014

2013
      <a href="#"

diciembre 2013

septiembre 2013

CSS

.widget
{
  width:60%;
  margin:50px auto 0 auto;
  border:4px solid rgba(10,10,10,0.05);
  padding:10px;
  border-radius:10px;
  -webkit-box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
  -moz-box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
  box-shadow: 10px 10px 45px -20px rgba(0,0,0,0.75);
}

.widget-title
{
  font-weight:bold;
  text-transform:uppercase;
  color:#636363;
  padding-bottom:10px
}

.widget a
{
  color:#9b9b9b;
  text-decoration:none;
  padding:10px 0 10px 10px;
  display:inline-block;
}

.widget a + a
{
  padding:10px 16px;
  float:right;
  line-height:1.5;
}

.widget a + a:hover
{
  background-color:#ddd;
  color:#fff;
}

.widget a:hover
{
  color:#444
}

.widget ul
{
  list-style-type:none;
  margin:0;
  padding:0;
  background-color:#eee;
}

.widget ul
{
  line-height:1.5;
}

.widget li
{
  border-top:1px solid #ccc;
}

.widget li li
{
  line-height:1.5;
  border-top:1px solid #eaeaea;
  background-color:#fff;
}

.widget > ul li:first-child
li a:first-child {
  display:inline-block;
}

.hide {
  display:none;
}

JS

"use strict";

var elementum = document.querySelector(".widget_archive");
var gestus = elementum.getElementsByClassName("fa");

for (var i = 0; i < gestus.length; i++) {
  gestus[i].addEventListener("click", toogle);
}

function toogle()
{
  if (this.nextElementSibling.className === "")
  {
    this.nextElementSibling.className = "hide";
  }
  else
  {
    this.nextElementSibling.className = ""
  }
}

Yes, you should use classList. Right now you overwrite the entire className attribute of the elements. It works, but it also means that you can never use any other class name on the elements. So it's just not a very precise or robust way of doing things; it only works as long as you don't add other classes. And if you do need to add another class to the elements at some point, your script will mess up because the comparison with "" will always fail, and then any and all class names will get summarily removed by your script.

Of course, classList isn't supported everywhere, so you may have to use a shim/polyfill. The nextElementSibling property isn't supported everywhere either, by the way (which, incidentally is why jQuery exists; because browser compatibility is not simple).

I also wonder why you use querySelector in one line, but elementsByClassName in the next line. In both cases you want to find element(s) with a given class name. Of course, in the first case, you want exactly 1 element, and in the other you want a collection. However, if you want 1 particular element, you should probably add an id attribute to that element - not a class - and then use the good old document.getElementById. In other words: Using a class selector and expecting it to only exist on one element is backwards. Classes are meant to be used on an arbitrary number of elements. If you want unique identification, use an id instead.

Also, please don't mix languages in your code. I know it's only two variable names, but especially because it's only two variable names there's no reason to switch language. The DOM API is in English, and even all your CSS classes are in English, so it's simpler and cleaner to just stick to that (by the way, I'm not a native English speaker myself, I just prefer code to be consistent.)

Lastly, I always recommend that you use brace-on-same-line style in JavaScript. There are some assumptions about style in JS interpreters, and brace-on-same-line is one.


Congratulations. This was pretty readable code, by brainfuck standards. I was able to follow it.

By initializing your max cell to 256, you've made your code portable to work on non-wrapping brainfuck interpreters as well.  If you just wanted it to work on a wrapping brainfuck interpreter (i.e., one that works modulo 256), you could have left the cell set to 0.  I considered changing the program to just loop 256 times instead of comparing the character against the maximum value, but it turns out that 255 times is much easier than looping 256 times when the machine operates modulo 256.  You already have a comparison routine, so it's easy to just use it again.  (Though preferably not by copying and pasting the code — more on that below.)

While there may be simpler ways to print a number as a decimal string, you asked to optimize for readability, so I have opted to stay close to your original strategy.

The key to readability, I think, is to add strategic comments.  In brainfuck, any characters other than the eight commands (><+-.,[]) are ignored.  You can also add comments of the form [-][Any comment text goes here], with two caveats:

The pointer must be pointing to a cell containing 0 or a cell that you want to be zeroed.
Any character can appear inside, as long as square brackets occur in pairs.

The main goals of your comments should be to clarify:

What the intention of each chunk of code is.

It is not so important, in my opinion, to be able to follow every single micro-operation as long as you can understand the big picture.
What the memory layout is, and why it is laid out that way.

I've given "variable names" to all important cells.  There is also a long explanatory comment in the code below about what I call "frames", or groups of consecutive cells.
Which cell is the current one at any point in the program.

It's a relatively minor bug if a cell contains the wrong value.  However, if the pointer is not where it is supposed to be, the program blows up in a spectacular way.  Therefore, I have developed the notation to mark the current pointer location in parentheses at strategic points throughout the code.

You raise a valid concern about repeating the comparison routine three times in your code.  The way to handle that in brainfuck is to let the routine crawl along the cells, frame by frame, like a construction crew working on a section of highway.  Just put the appropriate signposts to let it know where to stop crawling.

[-][
    # See MEMORY_LAYOUT below for explanation of SETUP.
]
SETUP
    +++++ +++ [ -
        > +++++ ++      (Colon)
        > ++++          (Space)
        > +             (Newline)

        >>>>>>
        ++++ [ -
            > +++++ +++ (Loop_limit)
            <
        ]

        >>>>> >
        > +++++ ++      (C_limit)
        > +++++ +       (c_ascii)
        >>>>
        > +++++ ++      (X_limit)
        > +++++ +       (x_ascii)
        >>>>
        > +++++ ++      (I_limit)
        > +++++ +       (i_ascii)

        <<<<< <<<<< <<<<< <<<<< <<<<< <<<<
    ]
    > ++                (Colon)
    >> ++ >             (all_done)
    >> + >>>>           (Loop_limit)
    >> + >>>> ++        (C_limit)
    >> + >>>> ++        (X_limit)
    >> + >>>> ++        (I_limit)

    <<<<< <<<<< <<<<< <<<<< <<<<< <<<
END_SETUP

MEMORY_LAYOUT
[-][
    # Using Roman abbreviations i, x, and c for units, tens, and hundreds,
    # respectively.  Braces { } indicate a frame: a repeated consecutive group
    # of cells.  Cells named with Uppercase are constants.  At all times,
    # parentheses indicate the current cell; neighboring cells may be noted
    # as well in the code.

    # The main loop starts working from the last frame.  It compares the last
    # two cells in the frame, and performs a carry if applicable.  It then
    # proceeds to the preceding frame and repeats.  The first "frame" is
    # special, as noted by the first cell being 0.  The comparison routine
    # will not be performed on it.  However, when ascii reaches Loop_limit,
    # all_done will be incremented to 1, since it is located at the offset
    # within the frame where carrying would increment a value.

  { (0)  Colon=":"=58  Space=" "=32  Newline="\n"=10  not_all_done=0 all_done=0 }
  { Compare_frame_0=1  0 0 0  Loop_limit=256       ascii="\0"=0   }
  { Compare_frame_1=1  0 0 0  C_limit=":"=58       c_ascii="0"=48 }
  { Compare_frame_2=1  0 0 0  X_limit=":"          x_ascii="0"    }
  { Compare_frame_3=1  0 0 0  I_limit=":"          i_ascii="0"    }
]
END_MEMORY_LAYOUT

>>>>
+
WHILE (not_all_done) [ -
    # Print one line of output
    >>>>> >>>>> >>> .                   (c_ascii)
    >>>>> > .                           (x_ascii)
    >>>>> > .                           (i_ascii)
    <<<<< <<<<< <<<<< <<<<< <<<<< <<< . (Colon)
    > .                                 (Space)
    >>>>> >>>> .                        (ascii)
    INCR (ascii)
    +
    <<<<< <<< .                         (Newline)

    >>>>> >>>>> >>>>> >>>>> >>>>> >
    INCR (i_ascii)
    +

    <<<<<
    REPEAT (Compare_frame_?) [
        >>>>>
        IFEQ eq_flag=0 ascii_save=0 ?_limit (?_ascii)
            [ -
                < - < + >>
            ] ascii_save=ascii; diff=limit=limit MINUS ascii; ascii=0

            <<< + (eq_flag=1)
            >> [
                << - >> [ - > + < ]
            ]    ifneq { eq_flag=0 ; ascii=diff }; (diff=0)
            # Restore diff
            > [ - < + > ]
            # Restore limit and ascii
            << (ascii_save) [ - > + > + << ]
            < (eq_flag) [
            THEN
                [-][
                    # Perform carrying.  If ascii == Loop_limit, then
                    # "carrying" will set the all_done flag.
                ]
                <<< +
                >>>>> > ----- -----
                <<<
            ]
        END_IFEQ (eq_flag=0) ascii_save=0 ?_limit ?_ascii

        <<<<< <<<
        (Compare_frame_next)
    ]

    >>>>>
    BOOL_NEGATE_AND_CLEAR not_all_done=0 (all_done)
        < +
        > [ - < - > ]
        < (not_all_done)
    END_BOOL_NEGATE_AND_CLEAR
]

It is possible to reduce the frame size by one.  However, you would probably have to reuse the same cell for not_all_done and all_done, which would hurt readability.


Calculator for more than two numbers I am having this problem with my code.  I have variables firstnumber and secondnumber, as well as operand and result. Then I am using the string.Split to know whether it's an operator or a number.

After that I have used a while loop if a.Length is over 2 and if it is, I declared an double numbers which is a[i]. i is 3 and gets + 2 every time the while loop executes.

Then I declared a variable called operands which is a[b].  b = 4 and gets + 2 every time the loop executes. Then I want to compare the string operands and the string operator, to see if it is +, -, * or / in the end I have put an if statement, so that if a[i] == null, then the while loop breaks and the result is printed.

I know my code is messy, and I have probably done many things wrong here, so I would really appreciate every word of advice I can get.

 using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;

namespace ConsoleApplication1
{
 class Calculator
 {
     static void Main(string[] args)
     {
        Console.WriteLine("This is a calculator. enter the numbers this way: Example(4 + 3)");
        Console.WriteLine("!Use space between numbers!");
        start : string s = Console.ReadLine();
        overagain:  string [] avgränsare = { " " };
        string[] a = s.Split(avgränsare, StringSplitOptions.RemoveEmptyEntries);

        while (a.Length > 2)
        {
            int i = 3;
            int b = 4;
            double numbers = double.Parse(a[i]);
            i = i + 2;
            b = b + 2;

            double firstnumber = double.Parse(a[0]);
            string operand = a[1];
            double secondnumber = double.Parse(a[2]);
            double result = 0;
            string operands = a[b];
            goto overagain;

                if (a[i] == null)
                {
                  Console.WriteLine(result);
                  break;
                }
            }

        Console.WriteLine("Do you want to continue?");
        s = Console.ReadLine().ToLower();
        if (s == "y")
        {
            goto start;
        }
        else
        {
        }

        }
    }
 }

Using goto is not a good practice!!
Instead of using goto, use a do-while or a while loop. You want to do something as long as the answer to "Do you want to continue" is "Yes". This is perfect for a do-while loop.
You should also avoid using goto overagain and instead use a for-loop to iterate through the array. It could also be solved with recursion. Use anything you want except the goto statement!

Make your code self-documenting. Your "explanation" of your code in the beginning of your question confuses me more than helps me.
Self-document your code by using proper variable names. s can be called input instead, and i and b can be called something that explains what they are doing. Right now, I can't exactly understand what they are supposed to do.

Your last block of Console.WriteLine("Do you want to continue?");, including the if-else that comes afterwards should be indented one step further.

If you don't have anything to do in else, just skip that part. if ... { ... } is enough, you don't need else. You'd better remove this, or else...

Use English names for variables, especially when characters outside the range of A-Z, a-z, 0-9, _ are included (such as "ä"). The Swedish word "avgränsare" translates to "delimiter" in English.


Multiple games with same rpg-classes base Please, if you find a better title, tell me because I really didn't know what to put in here!

I like to guess how I could implement a game, and to be sure that my perception is working, I plan to develop small games. For instance, I plan to build a FFTA-like game, a FTL-like game, a tower-defense game, and so on. As a base, I created a bunch of rpg-classes (Warrior, Wizard, ...), and I want to use all of them in each of my futur game.

The only sure thing I know about what should be in common for every game is the stat system:

  Strength
  Constitution
  Agility
  Accuracy
  Intelligence
  Charisma
  Luck

I will continue with what I plan to do, but I am not really sure about my choices:

namespace General
{
    public interface IStatsOwner
    {
        int Strength { get; }
        int Constitution { get; }
        int Agility { get; }
        int Accuracy { get; }
        int Intelligence { get; }
        int Charisma { get; }
        int Luck { get; }
    }

    public abstract class WarriorBase : IStatsOwner
    {
        public int Strength { get { return 5; } }
        public int Constitution { get { return 4; } }
        public int Agility { get { return 3; } }
        public int Accuracy { get { return 3; } }
        public int Intelligence { get { return 2; } }
        public int Charisma { get { return 3; } }
        public int Luck { get { return 1; } }
    }

    public abstract class WizardBase : IStatsOwner
    {
        public int Strength { get { return 2; } }
        public int Constitution { get { return 3; } }
        public int Agility { get { return 3; } }
        public int Accuracy { get { return 3; } }
        public int Intelligence { get { return 5; } }
        public int Charisma { get { return 4; } }
        public int Luck { get { return 1; } }
    }

    public interface IGame
    {
        Type WarriorType { get; }
        Type WizardType { get; }
    }
}

namespace FFTALike
{
    public interface IDamageable
    {
        void TakeDamages();
    }

    public class Warrior : General.WarriorBase, IDamageable
    {
        public void TakeDamages() { Console.WriteLine($"Taking damages with CON={Constitution}"); }
    }

    public class Wizard : General.WizardBase, IDamageable
    {
        public void TakeDamages() { Console.WriteLine($"Taking damages with CON={Constitution}"); }
    }

    public Game : IGame
    {
        public Type WarriorType { get { return typeof(Warrior); } }
        public Type WizardType { get { return typeof(Wizard); } }
    }
}

namespace TowerDefense
{
    public interface ILocatable
    {
        Location Location { get; set; } // Location(int X, int Y)
    }

    public interface IShooter
    {
        void Shoot();
    }

    public class Warrior : WarriorBase, ILocatable, IShooter
    {
        public Location Location { get; set; }
        public void Shoot() { Console.WriteLine($"Shooting with STR={Strength}"); }
    }

    public class Wizard : WizardBase, ILocatable, IShooter
    {
        public Location Location { get; set; }
        public void Shoot() { Console.WriteLine($"Shooting with STR={Strength}"); }
    }

    public Game : IGame
    {
        public Type WarriorType { get { return typeof(Warrior); } }
        public Type WizardType { get { return typeof(Wizard); } }
    }
}

What I feel uncomfortable with is:

  I don't enforce the fact that IGame.WarriorType inherits WarriorBase
  I duplicate TakeDamages(), Location { get; set; } and Shoot() (I actually have 20 rpg-classes, so I isocuplate these implementations)

Please note that I want the final implementation to fit with SOLID principles, because I want these projects to be training projects.

I'm having trouble visualizing what your intention here is.

Let's talk about your code itself, then I'll move on to a slight redesign:

C# has a nice-new language feature called "Expression-Bodied members", basically, this:

public int Strength => 5;

That's equivalent, but much shorter than your version:

public int Strength { get { return 5; } }

So, I would recommend moving to something like that, if you want to keep this design.

But, I would redesign this:

Everything has the same stats, right? They all need the stats, they just might have different values?

public class Stats
{
    public int Strength { get; }
    public int Constitution { get; }
    public int Agility { get; }
    public int Accuracy { get; }
    public int Intelligence { get; }
    public int Charisma { get; }
    public int Luck { get; }
}

So we're using another new language feature here: auto-implemented readonly properties. Basically: don't put code in the get, and these can only be set by the constructor. Of course, we need a constructor:

public Stats(int strength, int constitution, int agility, int accuracy, int intelligence, int charisma, int luck)
{
    Strength = strength;
    Constitution = constitution;
    Agility = agility;
    Accuracy = accuracy;
    Intelligence = intelligence;
    Charisma = charisma;
    Luck = luck;
}

Now, because everyone has their own specific stats, but they all follow the same structure, we use this class in the object:

public abstract class Entity
{
    public Stats Stats { get; }
    public Entity(Stats stats)
    {
        Stats = stats;
    }
}

So, because everybody is the same type of "Entity" with different attributes, we then move on to the Warrior / Wizard / etc.:

public abstract class Warrior : Entity
{
    public Warrior()
        : base(new Stats(5, 4, 3, 3, 2, 3, 1))
    { }
}

public abstract class Wizard : Entity
{
    public Wizard()
        : base(new Stats(2, 3, 3, 3, 5, 4, 1))
    { }
}

Now you can use the ILocatable etc. in the specific games, but the basics are all the same across-the-board. We build a more extracted model: the things that have to be there are, and are enforced.

With this part:

public interface IGame
{
    Type WarriorType { get; }
    Type WizardType { get; }
}

We can do some trickery:

public abstract class Game<TWarrior, TWizard>
    where TWarrior : Warrior
    where TWizard : Wizard
{
    public Type WarriorType => typeof(TWarrior);
    public Type WizardType => typeof(TWizard);
}

Now, I don't know why you need the WarriorType or WizardType, you probably don't need them in this version, because we put those types on the Game class.

We obviously haven't handled the TakeDamages or Shoot, right? We need to find a way to deal with that on a game-by-game basis: enter ECS (entity-component-system).

We built the entities, they have stats (components), now we need "systems" to handle the components.

public class ActualWizard : Wizard { }
public class ActualWarrior : Warrior { }

public class ActualGame : Game<ActualWizard, ActualWarrior>
{
    public class ShootSystem
    {
        public static void Shoot(Entity source, Entity target)
        {
            Console.WriteLine($"Shooting with STR={source.Stats.Strength}");
        }
    }
}

So, now you would call ShootSystem.Shoot(myWarrior, myWizard);, which would be where you handle all of the logic to actually shoot the wizard with the warrior. This also means that all entities can use the same function, whatever they may be. (Paladin, etc.)

var myGame = new ActualGame();
var myWarrior = new ActualWarrior();
var myWizard = new ActualWizard();
myGame.ShootSystem.Shoot(myWarrior, myWizard);

You would add any stats specific to warriors to ActualWarrior, same with ActualWizard, but you don't put the logic there, you build a System for it.


Search string in file, replace it, and write back to file I have this simple function that just reads a file, replaces all occurances of a string, and write it back to the file:

def fileSearchReplace(targetFile, search, replace):
    with open(targetFile, 'r') as openFile:
        filedata = openFile.read()

    newData = filedata.replace(search, replace)

    with open(targetFile, 'w') as openFile:
        openFile.write(newData)

I was wondering if there are any better implementation?

Personally, I would do this kind of thing line by line. I don't like the idea of putting my entire file into memory, then overwriting it. What if something goes wrong in between? Basically I don't trust myself to write good code!

So, looping through line by line, I would write to a new file with any changes. In Python, you can open two files at once, which is perfect for reading from one and writing to another.

Applying PEP8 for naming, you'd end up with something like this:

def file_search_replace(open_file, search, replacement):

    with open(open_file, 'r') as read_file, open('new_file.txt', 'w+') as write_file:

        for line in read_file:
            write_file.write(line.replace(search, replacement))

I've changed a few of the names, notably replace to replacement so it's clear it's not the same as replace().

Finally, if you're happy with the result, you can easily rename the file to something else. Personally I'd keep the old one just in case.


Simplification of Python code (HTML extraction) I'd like to extract HTML content of websites and save it into a file. To achieve this, I have the following (working) code:

output = codecs.open("test.html", "a", "utf-8")

def first():
    for i in range(1, 10):

        root = lxml.html.parse('http://test'+str(i)+'.xyz'+'?action=source').getroot()

        for empty in root.xpath('//*[self::b or self::i][not(node())]'):
            empty.getparent().remove(empty)

        tables = root.cssselect('table.main')
        tables = root.xpath('//table[@class="main" and not(ancestor::table[@class="main"])]')

        txt = []

        txt += ([lxml.html.tostring(t, method="html", encoding="utf-8") for t in tables])

        text = "\n".join(re.sub(r'\[:[\/]?T.*?:\]', '', el) for el in txt)

        output.write(text.decode("utf-8"))
        output.write("\n\n")

Is this "nice" code? I ask because I'm not sure if it's a good idea to use strings, and because of the fact that other HTML texts I've seen use one tag-ending (for example
) per line. My code produces partly more tag-endings per line.

Is it possible not to use strings or/and to achieve that we receive not things like

	 but things like:

? Thanks for any proposition :)

Try something like this, you can definitely clean up the two list comprehensions you have, but for now this should suffice.

def first():
    with codecs.open("test.html", "a", "utf-8") as output:
        for i in range(1, 10):
            txt = []
            root = lxml.html.parse('http://test'+str(i)+'.xyz'+'?action=source').getroot()
            for empty in root.xpath('//*[self::b or self::i][not(node())]'):
                empty.getparent().remove(empty)

            #tables = root.cssselect('table.main')  <--Dont need this, its is being overwritten
            tables = root.xpath('//table[@class="main" and not(ancestor::table[@class="main"])]')

            txt += ([lxml.html.tostring(t, method="html", encoding="utf-8") for t in tables])
            text = "\n".join(re.sub(r'\[:[\/]?T.*?:\]', '', el) for el in txt)

            output.write(text.decode("utf-8") + "\n\n")


Pearson Correlation in Python I have written the following code and I am new to Python.
Please let me know if I can make improvements to the following program:

import math

X = [12,11,13,13, 9,10,10,13, 5,10,10,13,10,10, 5, 8, 9, 8, 8, 9, 9,10,11, 5,12]
Y = [11,10,10,10, 9,13,10,11, 6, 7,13,14,14,11,11,10,10, 7, 8,12,11,11, 8, 7,13]

Xmean = sum(X)/len(X)
Ymean = sum(Y)/len(Y)

x = [var-Xmean for var in X]
y = [var-Ymean for var in Y]

xy =[a*b for a,b in list(zip(x,y))]
sum_xy = sum(xy)

x_square = [a*a for a in x]
y_square = [b*b for b in y]

sum_x_square = sum(x_square)
sum_y_square = sum(y_square)

sum_x_square_sum_y_square = sum_x_square*sum_y_square
sqrt_sum_x_square_sum_y_square = math.sqrt(sum_x_square_sum_y_square)

r = sum_xy/sqrt_sum_x_square_sum_y_square

print(r)

The above code is written for this formula :

Note: I did not check whether your code matches the formula correctly and I am not an expert of the mathematical background. I only refer to your code.

Style

[12,11,13,13, 9,10,10,13, 5,10,10,13,10,10, 5, 8, 9, 8, 8, 9, 9,10,11, 5,12]

This looks very inconsistent. You should add a space after each comma.

Naming

Xmean and X are unusual names for variables. x_square is the style you should use for Python, which is lower case with underscores, also known as "snake case".

Your names however make no sense at all semantically. You can name local variables x and y if they are e. g. coordinates, or if you use them for iterating. If they represent lists of numbers, why not just call them numbers1 and numbers2? (This might still not be perfect, but it shows what the variables mean. Edit: Note that this mainly concerns the problem of having two variables that only differ in being lower/upper case, and not nowing what x is somewhere inside your code, whereas numbers tells you that it is a collection or sequence of numbers. See below.) Note that I used a lower case x to refer to your X. Having another x later is the next problem. How do you know the difference without reading your whole code again?

x = [var-Xmean for var in X]

This took me some time to understand. You can improve it by using a common iterator variable like i and adding spaces around operators. With these improvements, and also better variable names, this could be for example:

subtracted_average = [i - mean_x for i in numbers1]

sum_xy = sum(xy)

Saving intermediate results of calculations like this is good, when you need them more than once. But here you don't. So sum(xy) is more readible if it is inside of another formula than sum_xy.

sum_x_square_sum_y_square

If you read this, do you know what it means without reading the code next to it? Is it a sum of sums? A product of sums? Or a completely different thing altogether? When trying to find a good variable names, think about "what is this"? The answer might be e. g. "the product of sums of squared numbers". product_of_sums_of_squared_numbers is long, but it tells you what it is.

Implementing Formulas

Judge for yourself. What is more readible?

1.

sum_x_square = sum(x_square)
sum_y_square = sum(y_square)

sum_x_square_sum_y_square = sum_x_square*sum_y_square
sqrt_sum_x_square_sum_y_square = math.sqrt(sum_x_square_sum_y_square)

or 2.

result = math.sqrt(sum(x_square) * sum(y_square))

The second one is actually how you write formulas in maths. So when implementing a function to represent that formula, go the same way. (Hint: Maybe this code should be a function.)

Using Functions

def pearson_correlation(numbers_x, numbers_y):
    mean_x = sum(numbers_x)/len(numbers_x)
    mean_y = sum(numbers_y)/len(numbers_y)

    subtracted_mean_x = [i - mean_x for i in numbers_x]
    subtracted_mean_y = [i - mean_y for i in numbers_y]

    x_times_y = [a * b for a, b in list(zip(subtracted_mean_x, subtracted_mean_y))]

    x_squared = [i * i for i in numbers_x]
    y_squared = [i * i for i in numbers_y]

    return sum(x_times_y) / math.sqrt(sum(x_squared) * sum(y_squared))

Call it like this:

# Contrary to what I said above about variables in general,
# the names X and Y are ok here because they match the names in the formula.
X = [12, 11, 13, 13, 9, 10, 10, 13, 5, 10, 10, 13, 10, 10, 5, 8, 9, 8, 8, 9, 9, 10, 11, 5, 12]
Y = [11, 10, 10, 10, 9, 13, 10, 11, 6, 7, 13, 14, 14, 11, 11, 10, 10, 7, 8, 12, 11, 11, 8, 7, 13]

print(pearson_correlation(X, Y))


Script to download sequentially named files, rename them, and delete smaller files I've written a little script to download sequentially named files, rename them, and delete files smaller than an certain number of kilobytes. I came up with this but I'm not too happy.  Any advice for me?

#!/bin/sh
echo
echo -n "Start file #:"
read start
echo
echo -n "End file #:"
read end
echo

wget --progress=bar:force http://www.mybox.com/pictures/show_pic.asp?src_id={$start..$end} 2>&1 | progressfilt

sleep 1

for file in *.asp
do
    mv "$file" "${file%.asp}.jpg"
done

find * -maxdepth 1 -name "*.jpg" -size -300k -delete

echo "tasks completed...exiting"
exit

sh is not bash. This shouldn't be a problem for your script though, because I don't think there are any bashisms in there.
Use More Quotes™. If there had been another parameter in the URL you would have needed an ampersand (&) in there, and that's the special character to run a command in the background. That's one of many ways to trip up if you don't comment basically any non-trivial string.
You don't need to sleep, because the wget commands are synchronous.
That said, you might get a result much faster if you run them in parallel, by doing several wget […] & commands followed by a wait.
You don't need to exit at the end of a shell script. That's only useful if you want to exit prematurely or with a different exit code from the previous command.
If you use Bash you can use read -p "Start file #: " start etc. to avoid most of the echoing.
However, it is much more common to make shell scripts take parameters instead of being interactive, making the script even simpler:

start="$1"
end="$2"

Use set -o errexit -o nounset (and -o pipefail if you're really in Bash) to make sure you catch any common bugs early.
I'm not sure what progressfilt does, but you can suppress the wget progress bar by using --quiet.
To be done slightly faster you should swap around the deleting and the renaming.
Rather than find * you should use find . to avoid exceeding the maximum argument length. This is a good practice in general, but will only really break with lots of files.
One very common feature of *nix scripts is that they are silent unless there's something important to report. Therefore, I'd remove the last echo.

Your script should now be 9 SLOC, and clean as a whistle!