ComplexProperties with Reflections I have used reflection for divide the complex property. For example, if I bind A.B[0], CustomerName means CustomerName is returned. Can anyone help to reduce the code?

var itemproperties = view.GetItemProperties();

if (itemproperties != null)
{
    if (propName.Contains("."))
    {
        string actualproperty = propName;
        string[] propertyNameList = propName.Split('.');
        int complexPropertyCount = propertyNameList.Count(), index = 0;
        bool isArrayproperty = actualproperty.IndexOf('[') != -1;

        if (isArrayproperty)
        {
            string strindex = actualproperty.Substring(actualproperty.IndexOf('[') + 1, actualproperty.IndexOf(']') - actualproperty.IndexOf('[') - 1);
            index = Convert.ToInt32(strindex);
            string property = actualproperty.Substring(0, actualproperty.IndexOf('['));
            for (int i = 0; i < propertyNameList.Length; i++)
            {
                if (propertyNameList[i].Contains('['))
                {
                    if (property.Contains("."))
                        property = propertyNameList[i].Substring(0, propertyNameList[i].IndexOf('['));
                    propertyNameList[i] = property;
                    break;
                }
            }
        }

        for (int iterator = 0; iterator < complexPropertyCount - 1; iterator++)
        {
            var tempProperyDescriptor = itemproperties.Find(propertyNameList[iterator], true);
            if (tempProperyDescriptor != null)       // Fix for SD 8318
            {
                record = tempProperyDescriptor.GetValue(record);
                itemproperties = TypeDescriptor.GetProperties(record);
            }
        }

        actualproperty = propertyNameList[complexPropertyCount - 1];

        if (isArrayproperty)
        {
            IList arrayList = ((IEnumerable)record) as IList;

            foreach (var item in arrayList)
            {
                var propertyInfo = item.GetType().GetProperty(actualproperty);

                if (propertyInfo == null)
                    return null;
                if (arrayList.IndexOf(item) == index)
                {
                    return propertyInfo.GetValue(item, null);
                }
            }
            return null;
        }

        var propertyinfo = record.GetType().GetProperty(actualproperty);

        if (propertyinfo == null && view is CollectionViewAdv)
        {
            var sourceType = (view as CollectionViewAdv).SourceType;
            if (sourceType != null && typeof(ICustomTypeDescriptor).IsAssignableFrom(sourceType))
                return itemproperties.Find(actualproperty, true).GetValue(record);
            return null;
        }

        if (!itemaccessor.ContainsKey(propName))
        {
            PropertyAccessor accessor = new PropertyAccessor(propertyinfo);
            itemaccessor.Add(propName, accessor);
            var val = accessor.GetValue(record);
            return val;
        }
    }

    if (record == null)
        return null;
    return itemaccessor[propName].GetValue(record);
}

return null;

Considering that you simply return null if the itemproperties are null, do that immediately:

if (itemproperties == null)
{
    return null;
}

if (propName.Contains("."))
{
}

That way you don't need to have 80+ lines inside an if.

The same is probably true for your next check -- if (propName.Contains(".")) -- but since you don't provide us with enough information I cannot help you there: where does propName come from? Where does record come from?

I see duplicate code:

actualproperty.Substring(0, actualproperty.IndexOf('['));
propertyNameList[i].Substring(0, propertyNameList[i].IndexOf('['));

Shouldn't that be converted to a method call?


Creating multiple files from a list This piece of code is to get random samples from two String lists, pos and neg. Based on a specified sampleSize, it writes an equal number of strings into 4 files. This codes seems slow, perhaps due to the file creating operation, since each file is created for a single string. Is there a better way to do this?

// pos and neg are ArrayList<String>
        Collections.shuffle(pos);
        Collections.shuffle(neg);

        int i = 0;
        int j =0;
        while(i < sampleSize ){
            if(i<sampleSize/2) {
                BufferedWriter bw1 = new BufferedWriter(new FileWriter(("test/neg/" + i + ".txt")));
                bw1.write(neg.get(i));
                bw1.close();

                BufferedWriter bw2 = new BufferedWriter(new FileWriter(("test/pos/" + i + ".txt")));
                bw2.write(pos.get(i));
                bw2.close();

            }else{
                BufferedWriter bw1 = new BufferedWriter(new FileWriter(("train/neg/" + j + ".txt")));
                bw1.write(neg.get(j));
                bw1.close();

                BufferedWriter bw2 = new BufferedWriter(new FileWriter(("train/pos/" + j + ".txt")));
                bw2.write(pos.get(j++));
                bw2.close();
            }
            i++;
            System.out.println(i);
        }

The execution time will be dominated by I/O, since you are writing so many files.  The only suggestion I have that might change the I/O performance is to try the NIO API, namely Files.write().

There is another possible inefficiency if sampleSize is much smaller than the size of the collections.  In that case, shuffling the entire list would be overkill, and you might be better off writing your own function to perform sampling without replacement.  (That would basically be a Fisher-Yates Shuffle that terminates early.)

There are four similar copies of the code, which you might want to consolidate, especially if it is not important to preserve the behaviour of your System.out.println(i) progress indicator.

My first observation is that the pos and neg lists can be treated independently.  My second observation is that you could simplify the loop if you interleave the output of the test and train cases.

I think that this code would make it more obvious that the number of files created is sampleSize, for each of the lists neg and pos.

private static void writeSampleFiles(List<String> list, String name, int sampleSize) throws IOException {
    Collections.shuffle(list);
    for (int i = 0; i < sampleSize; i++) {
        Path path = Paths.get(i & 1 == 0 ? "train" : "test",
                              name,
                              (i >> 1) + ".txt");
        Files.write(path, list.get(i).getBytes());
    }
}

// elsewhere...
writeSampleFiles(neg, "neg", sampleSize);
writeSampleFiles(pos, "pos", sampleSize);

Note that this isn't exactly identical to your original code.  The shuffled entries are extracted in a different order, which could be an issue if you are concerned about "dealing from the top of the deck".  If the execution aborts prematurely due to an IOException, then the difference in the processing order would be visible to the user.


Yet another lightweight Enum for Python 2.7 I'm trying to develop a lightweight enum for Python 2.7.  This question is downstream of the SO question here; for context, a streamlined version of the bulleted addendum to that question is replicated below:

  My desired feature set is:

  Only having to type the enum name (or value) once in the source definition.
  Valid enum values exposed for IDE autocomplete.
  Enum values stored internally as comprehensible strings.
  Straightforward membership testing using native Python in.
  Ability to import individual enum subclasses via, say, from .enums import myEnum, to avoid cluttering the local namespace with other enums I don't care about.
  I would rather avoid introducing an additional package dependency by using enum34, as I don't (think I) need the feature set.

After reviewing the SO post here on recommendations for non-enum34 implementations of enums in Python 2.7, I also would add:

  Clean, readable definition of enum values.

To note, every form of dynamic namespace construction I've tried, in efforts to accomplish #1, has broken #2.  I'm pretty sure at this point they're mutually exclusive.

In any event, my current solution goes as follows, defining an enum superclass with a metaclassed type with an iterator that returns all members with string content identical to the respective member names:

class SuperEnum(object):
    class __metaclass__(type):
        def __iter__(self):
            for item in self.__dict__:
                if item == self.__dict__[item]:
                    yield item

Each enum is then subclassed from this SuperEnum, e.g.:

class myEnum(SuperEnum):
    this = 'this'
    that = 'that'
    other = 'other'

The usage is working just as I want (except of course for #1), including for the IDE autocomplete of #2:

>>> myEnum.other
'other'
>>> 'this' in myEnum
True
>>> 'thing' in myEnum
False
>>> e = myEnum.other
>>> e in myEnum
True

This construction is far more concise than anything I'd set up around the time I posted the above-linked SO question, so the need to type the value name twice in the enum class definition isn't terribly bothersome.  I'm still interested to know if there's any way to make #1 and #2 play nice together, though.

More generally: what might the weaknesses of this construction be? For my application, performance is of minor concern. There's always the possibility of value collisions, as described here, but using strings as the values dramatically reduces the likelihood.  Security issues? Compatibility problems of some kind?

An interesting bit of code.  :)

The downsides would be the things missing from enum341:

printing a variable gives no indication that it's an enum and not a string
enums that have the same value will compare equal
depending on the value of the member, is won't always work
etc.

Plus the surprise other developers will have when your enum doesn't work like the now-standard Python Enum2.

On the bright side, you shouldn't have any problems with pickling.

1 Disclosure:  I am the author of the Python stdlib Enum, the enum34 backport, and the Advanced Enumeration (aenum)  library.

2 See this answer for the standard Enum usage.


Simple rainfall animation in Pygame I made the following code. It's a simple animation of 'raindrops' falling and splattering once they reach the ground. The images I use can be found here: https://imgur.com/a/VYV07ef

import pygame, sys
from pygame import locals
import random
import itertools

def listenToQuit():
    for event in pygame.event.get():
        if event.type == locals.QUIT:
            pygame.quit()
            sys.exit()

def makeNewDrop(display, drops):
    # there isa 15% chance we'll make a new drop, each frame
    x = display.get_width() * random.random() # a random xpostion
    return RainDrop(x, display, drops)

class RainDrop(pygame.sprite.Sprite):
    img = pygame.image.load('assests/rain_drop.png')

    def __init__(self, x, display, group):
        # call the super constructor, and pass in the
        # group you've created and it is automatically added to the group every
        # time you create an instance of this class
        pygame.sprite.Sprite.__init__(self, group)
        self.y = 0
        self.x = x
        self.fall_step = 5
        self.display = display
        self.die = False
        self. animation = iter([
            pygame.image.load('assests/splatter_1.png'),
            pygame.image.load('assests/splatter_2.png'),
            pygame.image.load('assests/splatter_3.png'),
            pygame.image.load('assests/splatter_4.png')
        ])

    def update(self):
        self.checkIfShouldDie()
        self.fall()
        if self.die:
            try:
                self.img = next(self.animation)
            except StopIteration:
                self.kill()

        self.display.blit(self.img, (self.x,self.y))

    def fall(self):
        self.y = self.y + self.fall_step

    def checkIfShouldDie(self):
        if self.img.get_rect().bottom + self.y >= self.display.get_height():
            self.die = True
            self.y = self.display.get_height() - self.img.get_rect().bottom - 5

if __name__ == '__main__':
    pygame.init()
    drops = pygame.sprite.Group()
    cooldown = 0

    fps = 30
    clock = pygame.time.Clock()

    main_display = pygame.display.set_mode((400,400),0,32)
    pygame.display.set_caption("rain!")

    while True:
        main_display.fill((255,255,255))

        if random.random() >= 0.85 and cooldown == 0:
            # a 15% change we'll make a new drop, each frame
            # assuming we didn't make a drop in the last few frames
            drop = makeNewDrop(main_display, drops) # automatically added to the drops group
            cooldown = cooldown + 5

        drops.update()

        # reduce cooldown
        cooldown = 0 if cooldown <= 0 else cooldown - 1
        listenToQuit()
        pygame.display.update()
        clock.tick(30)

How can this code be improved? Specifically the part where I iterator over animations and catch the StopIteration error seems very hacky to me.

PEP8

The official Python style guide will suggest that:

there be two newlines between functions at the global scope, and only one newline between functions in the class scope (i.e. check_if_should_die)
functions be named in lower_snake_case, i.e. listen_to_quit

Hard exit

Currently, you have a forever-loop that only exits on sys.exit. Instead, simply return a boolean from listenToQuit (which can be called should_quit), and if the return value is true, break out of the loop.

Abbreviated imports

pygame.sprite.Sprite can just be Sprite if you from pygame.sprite import Sprite.

Typo

assests -> assets

Generator

    self. animation = iter([
        pygame.image.load('assests/splatter_1.png'),
        pygame.image.load('assests/splatter_2.png'),
        pygame.image.load('assests/splatter_3.png'),
        pygame.image.load('assests/splatter_4.png')
    ])

can be

self.animation = iter(
   pygame.image.load(f'assets/splatter_{i}.png')
   for i in range(1, 5)
)

Generally I don't think your iter/StopIteration approach is that bad. That said, you can rework it by changing your outer while True into a for drop in drops, where drops is an iterable through four instances of a drop object tied to a specific asset.


Simulating data generating process correctly My aim is to simulate the following model by means of a Monte Carlo simulation. I wonder if my R code is correct for generating the data.

Could somebody check?

The model:

$$Y = \sum_{j=1}^{100} (1+(-1)^{j}A_j X_j + B_j \sin(6X_j)) \sum_{j=1}^{50} (1+X_j/50) + \epsilon$$

where

\$A_1, \dots, A_{100}\$ are i.i.d. \$∼ \text{Unif}([0.6,1])\$
\$B_1, \dots, B_{100}\$ are i.i.d. \$∼ \text{Unif}([0.8,1.2])\$ and independent of \$A_j\$
\$X \sim \text{Unif}([0,1])\$ where all components are i.i.d. \$∼ \text{Unif}([0, 1])\$
\$\epsilon \sim N(0,2)\$ and \$X_j\$ represents the \$j\$th column of the design matrix

You can find the model here, p. 14

This is my code attempt

n_sim <- 10
n_sample <- 200
n_reg <- 100
sd_eps <- sqrt(2)

X <- replicate(n_reg, runif(n_sample, 0,1))
A <- replicate(n_reg, runif(1, 0.6,1))
B <- replicate(n_reg, runif(1, 0.8,1.2))

f_1 <- vector(mode = 'integer', length = n_sample)
f_2 <- vector(mode = 'integer', length = n_sample)

for (d in seq(100)){
  part1 <- 1 + (-1)^d*A[d]*X[,d]+B[d]*sin(6*X[,d])
  f_1 <- f_1 + part1
}

for (d in seq(50)){
  part2 <- 1 + X[,d]/50
  f_2 <- f_2 + part2
}

# True DGP Train ----
f_true <- f_1*f_2
y <- replicate(n_sim, f_true) + replicate(n_sim, rnorm(n_sample, 0,sd_eps))

The first thing that jumps out from the definition is that, if you have X, A, B and epsilon, you can compute y deterministically. This means you can readily test your implementation. You should always strive to find ways to define pure functions in your R code, and try to use vectorisation instead of for loops.

Based on your existing code, I'll assume for a given model that X is a matrix (n_sample, 100), A and B are vectors of length 100 and epsilon is a vector of length n_sample.

Based on your implementation, the function would look something like

compute_y <- function(X, A, B, epsilon) {
  n_sample <- nrow(X)
  # note that your f_[1|2] stored `double`s not `integers`
  f_1 <- numeric(n_sample)
  f_2 <- numeric(n_sample)

  for (d in seq(100)){
    part1 <- 1 + (-1)^d*A[d]*X[,d] + B[d]*sin(6*X[,d])
    f_1 <- f_1 + part1
  }
  for (d in seq(50)){
    part2 <- 1 + X[,d]/50
    f_2 <- f_2 + part2
  }

  f_1 * f_2 + epsilon
}

But that's a bit scruffy.

The easiest bit to clean up is the bit that defines f_2:

f_2 <- numeric(n_sample)
for (d in seq(50)) {
    part2 <- 1 + X[,d]/50
    f_2 <- f_2 + part2
}

Here you're only using the first 50 columns of X. You could rewrite it as:

f_2 <- numeric(n_sample)
W <- 1 + X[, 1:50]/50
for (d in seq(50)) {
    f_2 <- f_2 + W[,d]
}

But in the latter, you're summing along the rows of W. So you could ditch the for loop altogether:

W <- 1 + X[, 1:50] / 50
f_2 <- rowSums(W)

This gives us:

compute_y <- function(X, A, B, epsilon) {
  n_sample <- nrow(X)

  f_1 <- numeric(n_sample)

  for (d in seq(100)){
    part1 <- 1 + (-1)^d*A[d]*X[,d] + B[d]*sin(6*X[,d])
    f_1 <- f_1 + part1
  }

  f_2 <- rowSums(1 + X[, 1:50] / 50)

  f_1 * f_2 + epsilon
}

There is a way to replace the for-loop that computes f_1.

First note you're adding 1 to f_1 one hundred times, so you might as well start with f_1 storing the value 100

f_1 <- rep(100, n_sample)

for (d in seq(100)){
  part1 <- (-1)^d*A[d]*X[,d] + B[d]*sin(6*X[,d])
  f_1 <- f_1 + part1
}

For speed, I'll just show you how to do it:

tX <- t(X)
a <- colSums(c(-1, 1) * A * tX)
b <- colSums(B * sin(6 * tX))
f_1 <- 100 + a + b

That code would be a bit faster, but I don't think it looks as clean as your definition of f_1.

If you want you can move the code that defines X, A, B, and epsilon into a model-definition function.


Return error messages for sending emails How can I optimize this piece of code and avoid the multiple if/else call for returning error message ?

    if ($cardStatus == true) {

        $cardEmailContent = $this->setCardEmailContent();
        $sendUserEmail    = $this->sendCardToUser($cardEmailContent);

        if ($sendUserEmail) {
            $this->sendAdminEmail();
            $this->updateCardDetails();
        } else {
            return $sendUserEmail;
        }

    } else {
        return $this->cardApiError;
    }

By switching the if condition to check if $cardStatus is false and the same for $sendUserEmail you will reduce some of the nesting.

if (!$cardStatus) {
    return $this->cardApiError;
}

$cardEmailContent = $this->setCardEmailContent();
$sendUserEmail    = $this->sendCardToUser($cardEmailContent);

if (!$sendUserEmail) {
    return $sendUserEmail;
}

$this->sendAdminEmail();
$this->updateCardDetails();

by using these guard clauses it avoids the upcoming arrow antipattern.


Take a Ten Minute Walk: A City Tour Solution I recently came across the following problem on Codewars:

  Take a Ten Minute Walk

  You live in the city of Cartesia where all roads are laid out in a perfect grid.
  You arrived ten minutes too early to an appointment, so you decided to take
  the opportunity to go for a short walk. The city provides its citizens with a
  Walk Generating App on their phones -- every time you press the button it sends you an array of one-letter strings representing directions to walk
  (eg. ['n', 's', 'w', 'e']).
  You know it takes you one minute to traverse one city block, so create a function
  that will return true if the walk the app gives you will take you exactly ten
   minutes (you don't want to be early or late!) and will, of course, return you to
  your starting point. Return false otherwise.

I solved the problem with the following function:

function isValidWalk(walk) {
  var directions = {
    'n': 0,
    's': 0,
    'e': 0,
    'w': 0
  }
  walk.forEach( function(direction) {
    directions[direction]++;
  }
  var displacement = {
    x: directions['n'] - directions['s'],
    y: directions['e'] - directions['w']
  }
  /*it take 1 min to traverse 1 block & I only have 10 minutes,
  & I also want to attend appointment, so i want to come from where i start.*/
  return walk.length === 10 && displacement.x === 0 && displacement.y === 0;
}

var directions = {
  'n': 0,
  's': 0,
  'e': 0,
  'w': 0
}

Quotes are optional on all valid property names.

walk.forEach( function(direction) {
  directions[direction]++;
}

Syntax error here. Plus, making whitespaces consistent would make this code less of an eye sore. Consider using an IDE that will yell at you for inconsistent formatting.

var displacement = {
  x: directions['n'] - directions['s'],
  y: directions['e'] - directions['w']
}

Storing in an object is unnecessary. x and y can just be variables here.

An alternate approach is to represent the directions into 2 columns, vertical and horizontal. Then the directions on the same axis are represented 1 and -1. This way, knowing if you're back is a matter of checking if both axes sum up to zero. No subtraction required.

   V  H
N  1  0
S -1  0
E  0  1
E  0  1
W  0 -1
Result: [0, 1]
// Off by 1 to the east. Also, length is less than 10.

This is also a good functional programming exercise on how to think of functions as data transformations instead of machine commands. In the following code, each array method transforms the data from directions to a boolean in three discrete steps.

const directionVector = {
  N: [1, 0],
  S: [-1, 0],
  E: [0, 1],
  W: [0, -1]
}

const isValidWalk = walk => walk.length === 10 && walk
  .map(direction => directionVector[direction])
  .reduce((prev, next) => [prev[0] + next[0], prev[1] + next[1]], [0, 0])
  .every(v => v === 0)

// length check goes first so code doesn't even try if it's not 10
// map converts direction into vectors: N -> [V, H]
// reduce sums all vectors: [V1, H1] + [V2, H2] + ... = [Vs, Hs]
// every checks if sum values are all zero: Vs === 0, Hs === 0

// Too short
console.log(isValidWalk(['N', 'S', 'E', 'W', 'W', 'E']))

// Too long
console.log(isValidWalk(['N', 'S', 'E', 'W', 'N', 'S', 'E', 'W', 'N', 'S', 'E']))

// We're back!
console.log(isValidWalk(['N', 'S', 'E', 'W', 'N', 'S', 'E', 'W', 'N', 'S']))

// We're lost
console.log(isValidWalk(['N', 'S', 'E', 'W', 'N', 'S', 'E', 'W', 'N', 'E']))


Creating nodes for linked list in a while loop in C I am trying to create nodes for a linked list in a while loop. The code works, but it is not very pretty. How would I get the same results the proper way?

root = malloc(sizeof(struct node)); // Allocates memory for first node
pointer = root; // Set pointer to first node
char buf[MAXBUF]; // Buffer for each line

int size = 1; // Counts nodes.

/* Write to list */
while(fgets(buf, MAXBUF, f) != NULL) // Puts each line in a node through buffer
{
    strncpy(pointer->data, buf, MAXBUF); // Puts line in character array in node
    pointer->next = malloc(sizeof(struct node)); // Allocates memory for new node
    size++; // Increments for each node. Root is 1, so first round in loop is 2
    pointer = pointer->next; // Sets pointer to next node
}
size--; // Removes 1 count for last node
free(pointer); // Frees last allocated node
pointer = NULL;

The technique you're trying to accomplish in building your ordered linked list is called forward-chaining. A pointer-to-pointer makes the task trivial for building a simple linked list while retaining its input order. Error checking memory allocations not-withstanding, it is done something like this:

struct node *root = NULL, **pp = &root;
char buf[MAXBUF] = {0};
size_t size = 0;

while (fgets(buf, MAXBUF, f) != NULL)
{
    *pp = calloc(1, sizeof(**pp));
    strncpy((*pp)->data, buf, MAXBUF-1);
    pp = &(*pp)->next;
    ++size;
}
*pp = NULL; // terminate the last node

How It Works

The parts of this that mandate further elaboration follow this general algorithm:

Utilize a pointer-to-pointer to always hold the address of the pointer that will be populated with the address of the next node to add. The initial value of this pointer-to-pointer is the address of the root pointer: &root.
Acquire a new node, storing its address in *pp, where pp is the pointer-to-pointer from (1)
Once the node is finished being configured, change the address stored in pp to be the address of the next member pointer held within the node just added. This step sets up the target location where the next node will be added if needed.
When finished processing input, the last-node-added's next member pointer has its address currently held in pp. If no nodes were added to the list, then pp still holds the address of root. To terminate the list, this pointer must be set to NULL.

So on to the code:

struct node *root = NULL, **pp = &root

The above line declares the root pointer, initialized to NULL (optional, but I hate indeterminate pointers anywhere in-code), and a pointer-to-pointer that holds the address of root. From there...

*pp = malloc(sizeof(**pp));

This allocates a new node, storing its address at whatever pointer is currently being addressed by the pointer-to-pointer pp. On the initial pass that pointer is the root pointer. On subsequent passes it will always hold the address of the `next member within the last-node-added on a prior iteration.  But it always holds the address of pointer that is to receive the next new node.

After the current node is finished being configured copy, then this is done:

pp = &(*pp)->next;

This stores the address of the just-added-node's next member in the pointer-to-pointer. When we loop around for the next iteration, this is where we will hang the next new node. Finally, after the loop finishes, this is done:

*pp = NULL;

This sets the pointer pointed-to by pp to NULL. Now think about what pointer that is:

If no items were populated in the list, then pp still holds the address of root and this will reestablish root as NULL; exactly what you want if the list is empty.
If any nodes were read, then pp will hold the address of the last added-node's next pointer, which should set to NULL to terminate the linked list. Again, exactly what this code does.

Addendum: Printing the list

Per request, printing the list after this is simply:

struct node *ptr = root;
while (ptr)
{
    printf("data: %s\n", ptr->data);
    ptr = ptr->next;
}

For C99 and later,

for (struct node *ptr = root; ptr; ptr = ptr->next)
    printf("data: %s\n", ptr->data);


Sum even fibonacci numbers up to a limit After I have tackled the first problem I come to Project Euler Problem 2 which states

Each new term in the Fibonacci sequence is generated by adding the previous two terms. By starting with 1 and 2, the first 10 terms will be:
1, 2, 3, 5, 8, 13, 21, 34, 55, 89, ...
By considering the terms in the Fibonacci sequence whose values do not exceed four million, find the sum of the even-valued terms.

If we take into account that

adding 2 odd numbers will result in an even number
adding an odd to an even number will result in an odd number

we only need to take every third term.
But before we can get there we need to get all the fibonacci numbers like so
private static IEnumerable<int> GetFibonacciNumbers()
{
    var first = 0;
    var second = 1;
    while (true)
    {
        var nextFibonacciNumber = first + second;
        yield return nextFibonacciNumber;

        first = second;
        second = nextFibonacciNumber;
    }
}

which will generate infinite terms of the fibonacci numbers. We will then restrict this by using the TakeWhile() extension method.
Getting only the even terms up to a specific limit can be achieved by
private static IEnumerable<int> GetEvenFibonacciNumbersTill(int upperLimit)
{
    var counter = 0;
    foreach (var currentTerm in GetFibonacciNumbers().TakeWhile(i => i < upperLimit))
    {
        counter++;
        if (counter == 3) // every third term is even
        {
            yield return currentTerm;
            counter = 0;
        }
    }
}

which is called by the int Solve() method like so
private static readonly int fourMillions = 4000000;
public static int Solve()
{
    return GetEvenFibonacciNumbersTill(fourMillions).Sum();
}

As usual any improvements in style, performance, naming or memory usage are welcome.

If you dig a little deeper in the math, it is very straightforward to realize that every third Fibonacci number can be computed with the formula:

\$F_{n+6} = 4 F_{n+3} + F_{n}\$

You can reuse your function if you work the recursion backwards a couple of steps, and do something like:

private static IEnumerable<int> GetEvenFibonacciNumbers()
{
    var first = 2;
    var second = 0;
    while (true)
    {
        var nextFibonacciNumber = first + 4*second;
        yield return nextFibonacciNumber;

        first = second;
        second = nextFibonacciNumber;
    }
}

This will not only make your code ~3x faster, but will render your other functions trivial to write.


A clock implemented using Moment.js This is a POC for a clock using moment.js, eventually I plan to recreate this in Raect.js. Any opimisations or a better way to do it?

<!DOCTYPE html>

    <title>Html clock</title>

    <link rel="stylesheet" type="text/css" href="stylesheet.css">

<select id='timezone_list'></select>

I have created a function to prevent the one second delay.
Added moment().tz(timezone) as variable
Added all elements as variable
Added the date and time directly to the elements
Added a shorter way to add options

var list = moment.tz.names();
var timezone;

// elements
var elTimezoneList = document.getElementById('timezone_list');
var elTime = document.getElementById('show_time');
var elDate = document.getElementById('show_date');

// loop over timezones and set options
for(var i = 0; i < list.length; i++) {
   elTimezoneList.appendChild(new Option(list[i], list[i]));
}

// time function to prevent the 1s delay
var setTime = function() {
  var time = moment().tz(timezone);

  // set time in html
  elTime.innerHTML= time.format('h:mm:ss a');

  // set date in html
  elDate.innerHTML = time.format('MMMM Do YYYY');
}

// on change select
elTimezoneList.onchange = function(){
  // set selected timezone
  timezone = elTimezoneList.options[elTimezoneList.selectedIndex].text;

  setTime();
  setInterval(setTime, 1000);
}
<!DOCTYPE html>

    <title>Html clock</title>

    <link rel="stylesheet" type="text/css" href="stylesheet.css">

    <select id="timezone_list"></select>


Learn Python the Hard Way #48 — lexicon exercise I'm trying to get some feedback on my code for the lexicon exercise (#48) in Learn Python the Hard Way. I have only recently started learning Python and want to make sure I have my fundamentals down before progressing. I have some experience coding (only with kdb+/q in my work environment) but none in Python, so all advice is welcomed.

verb=['go','kill','eat']
direction=['north','south','east','west']
noun=['bear','princess']
stop=['the','in','of']
vocab={'verb':verb,'direction':direction,'noun':noun,'stop':stop}

def scan(sentence):
    wordlist=sentence.split()
    result=[] #initialize an empty list
    for word in wordlist:
        found=False
        for key,value in vocab.items():
            if word.lower() in value:  #convert to lower case so that we can handle inputs with both cases
                result.append((key,word))
                found=True
                break
        if not found:
            try:
                word=int(word)
                result.append(('number',word))
            except ValueError:
                result.append(('error',word))
    return result

Python version 2.6.

verb=['go','kill','eat']
direction=['north','south','east','west']
noun=['bear','princess']
stop=['the','in','of']

There is not a lot of point in storing these in separate variables only to stick them inside vocab just put the list literals inside vocab

vocab={'verb':verb,'direction':direction,'noun':noun,'stop':stop}

Python convention say to make constant ALL_CAPS. I'd also not abbreviate the names. Code is read more then written, so make it easy to read not to write.

def scan(sentence):
    wordlist=sentence.split()
    result=[] #initialize an empty list

Pointless comment. Assume you reader understand the language. You don't need to explain what [] means.

    for word in wordlist:

I'd use for word in sentence.split():

        found=False

Boolean logic flags are delayed gotos. Avoid them when you can

        for key,value in vocab.items():
            if word.lower() in value:  #convert to lower case so that we can handle inputs with both cases

Your dictionary is backwards. It maps from word types to lists of words. It'd make more sense to have a dictionary mapping from word to the word type.

                result.append((key,word))
                found=True
                break
        if not found:

Rather then this, use an else block on the for loop. It'll be execute if and only if no break is executed.

            try:
                word=int(word)
                result.append(('number',word))

Put this in the else block for the exception. Generally, try to have as little code in try blocks as possible. Also I wouldn't store the result in word again, I'd put in a new local.

            except ValueError:
                result.append(('error',word))

    return result

My approach:

WORD_TYPES = {
   'verb' : ['go', 'kill', 'eat'],
   'direction' : ['north', 'south', 'east', 'west'],
   'noun' : ['bear', 'princess'],
   'stop' : ['the','in','of']
}
# invert the dictionary
VOCABULARY = {word: word_type for word_type, words in WORD_TYPES.items() for word in words}

def scan(sentence):
    tokens = []
    for word in sentence.split():
        try:
            word_type = VOCABULAR[word]
        except KeyError:
            try:
                 value = int(word)
            except ValueError:
                 tokens.append( ('error',word) )
            else:
                 tokens.append( ('int', value) )
        else:
            tokens.append( (word_type, word) )
     return tokens

Alternately, using some regular expressions:

classifications = []
for word_type, words in WORD_TYPES.items():
    word_expression = '|'.join("(?:%s)" % re.escape(word) for word in words)
    expression = r"\b(?P<%s>%s)\b" % (word_type, word_expression)
    classifications.append(expression)
classifications.append(r"\b(?P<int>\d+)\b")
classifications.append(r"\b(?P<error>\w+)\b")
parser = re.compile('|'.join(classifications))

def scan(sentence):
    return [(match.lastgroup, match.group(0)) for match in parser.finditer(sentence)]

EDIT

If your version of python is to old to use the dict comphrensions you can use:

dict((word, word_type) for word_type, words in WORD_TYPES.items() for word in words)


Bare bones battleship game This is basically the beginnings of a battleship game. There is only 1 ship which takes up 3 of six locations on a 1 x 6 grid. The locations are created by finding a random location on the grid and then taking the next two points as the other 2 locations.

I basically just make sure the the guess is between 1 and 6, make sure the number has not been guessed before by comparing it to guessarr (an array that contains all the valid guesses), then compare the guess to the random locations to determine if it's a hit or miss. After guessing all 3 random locations, the user wins.

How would I go about making this code object-oriented?

var guess;
var guesses = 0;
var hits = 0;
var isSunk = false;
var  guessCheck=-1;
var guessarr = [];
var valid = true;
var randomeloc = Math.floor(Math.random () * 5);
var location1 = randomeloc
var location2 = randomeloc + 1;
var location3 = randomeloc +2;

while (isSunk == false) {
    guess = prompt("Enter a guess between 1 and 6");
    if (guess < 0 || guess > 6) {
        alert("Enter a valid number between 0 and 6");
    }else if(guess > 0 || guess < 6) {
        guessCheck= guessarr.indexOf(guess);
        if (guessCheck >= 0) {
        valid = false;
        alert("You've already entered this number. Try again");
        } else if (valid = true){
            guesses = guesses +1;
            if (guess == location1 ||
                guess == location2 ||
                guess == location3) {
                hits = hits + 1;
                alert ("You got a hit!");
                guessarr.push(guess);
            }
            else {alert("miss");
            }
            if (hits == 3) {
            isSunk= true;
            alert("you won!");
            }
        }
    }
}

Be careful that the random can be between 0 and 4 when it should be between 1 and 4. If you want to make it more object oriented, I would separate the interface (alert and prompt) from the game logic. As this is just a game with 1 ship, I would only do a Game and Ship objects to represent it, but for more complex versions other objects may apply. Here is how I would implement it having custom columns:

function Game(columns){
  this.guesses;
  this.guessarr;
  this.columns = columns;
  this.ship;
  this.ended;

  return this;
}

Game.prototype.getRandomNumber = function(min,max){ // interval [)
  return Math.random() * (max - min) + min;
}

Game.prototype.getRandomShipPosition = function(shipSize){
  return this.getRandomNumber(1,this.columns - shipSize + 2);
}

Game.prototype.init = function(){
  this.guesses = 0;
  this.guessarr = [];
  this.ship = new Ship(this.getRandomShipPosition(3), 3);
  this.ended = false;
}

Game.prototype.makeGuess = function(guess){
  if(isNaN(Number(guess))){
      return "Enter a valid number";
  }
  if(guess <= 0 || guess > this.columns){
      return "Guess has to be between 1 and " + this.columns;
  }
  if(this.guessarr.indexOf(guess) != -1){
      return "You've already entered this number. Try again";
  }
  this.guessarr.push(guess);
  if(this.ship.makeGuess(guess)){
      if(this.ship.isSunk){
          this.ended = true;
      }
      return "You got a hit!";
  }else{
      return "Miss";
  }
}

function Ship(start,size){
  this.start = start;
  this.hits = 0;
  this.size = size;
  this.isSunk = false;

  return this;
}

Ship.prototype.makeGuess = function(guess){
    if(guess >= this.start && guess <= this.start + this.size){
        this.hits+=1;
        if(this.hits == this.size){
            this.isSunk = true;
        }
        return true;
    }
    return false;
}

var game = new Game(6);
game.init();
while(game.ended == false){
    guess = Number(prompt("Enter a guess between 1 and 6"));
    alert(game.makeGuess(guess))
}
alert("you won!")


Chaining Internal Class Methods in Python I'm learning object-oriented programming in Python. I've created a class RepositoryMaker that contains all methods required to create a Git repository. The idea is to be able to create a single instance of this class with a constructor that only requires pertinent info to create a new repo.

My code looks like this:

class RepositoryMaker:
    """Creates new Azure DevOps repositories"""
    def __init__(self, project_name, repo_name):
        self.project_name = project_name
        self.repo_name = repo_name
        with open("./repo_request_body.json", "r") as file:
            self.data = json.load(file)

    def get_project_id(self):
        """Returns the Azure DevOps project ID for a project name"""
        _request_object = make_request(
            "_apis/projects/",
            self.project_name,
            "?api-version=5.1-preview.1",
            request_method = "get"
            )
        return _request_object.get("id")

    def update_project_id(self):
        """Updates JSON dictionary with new ID"""
        new_id = self.get_project_id()
        self.data["project"]["id"] = new_id

    def update_repo_name(self):
        """Updates JSON dictionary with new repo name"""
        self.data["name"] = self.repo_name
        return self.data

    json_content_type = {
        'Content-Type': 'application/json',
        'Authorization': 'Basic XXXXXXXXXXXXXXXX'
        }

    def create_repository(self):
        """Creates a new repo under the specified project"""
        self.update_project_id()
        repo_request_body = self.update_repo_name()
        make_request(
            self.project_name,
            "/_apis/git/repositories",
            "?api-version=5.1-preview.1",
            request_method = "post",
            data = json.dumps(repo_request_body),
            headers = self.json_content_type
        )

x = RepositoryMaker(project_name, repo_name)
x.create_repository()

My concern is chaining together class methods internally within the class. For example, create_repository() is calling update_project_id() which is calling get_project_ID().

Is this convention acceptable within Python? Is there a better way to group together a set of related functions than using a class?

Class methods calling each other is perfectly acceptable in general, but this should be done toward the end of making the code easier to follow by encapsulating complexity.  Two things in your example potentially work against that goal:

All of the methods and attributes are public.  (In Python you denote a "private" member by starting its name with _.)
Data is passed between the methods via instance attributes.

This means that someone reviewing this code for possible bugs needs to assume that a caller might call any of these methods while the object is in any arbitrary state -- that's a lot of combinations to consider!

If the only intended usage of this class is the one you've given (create a single instance, call create_repository exactly once), then there isn't much benefit to making it a class with state (since that state is never meant to be reused beyond that one function call).  The other typical way of grouping related functions is to simply define them locally within a larger function:

def create_repository(project_name: str, repo_name: str) -> None:
    """Creates a new repo under the specified project"""

    with open("./repo_request_body.json", "r") as file:
        data = json.load(file)

    def get_project_id() -> str:
        """Returns the Azure DevOps project ID for a project name"""
        _request_object = make_request(
            "_apis/projects/",
            project_name,
            "?api-version=5.1-preview.1",
            request_method = "get"
        )
        return _request_object.get("id")

    def update_project_id() -> None:
        """Updates JSON dictionary with new ID"""
        new_id = get_project_id()
        data["project"]["id"] = new_id

    def update_repo_name() -> None:
        """Updates JSON dictionary with new repo name"""
        data["name"] = repo_name

    json_content_type = {
        'Content-Type': 'application/json',
        'Authorization': 'Basic XXXXXXXXXXXXXXXX'
        }

    update_project_id()
    update_repo_name()
    make_request(
        project_name,
        "/_apis/git/repositories",
        "?api-version=5.1-preview.1",
        request_method = "post",
        data = json.dumps(data),
        headers = json_content_type
    )

create_repository("project_name", "repo_name")

But given that each of these functions is only called once, is there any value to actually giving them individual names and making the reader jump around between them?  I'd probably just write this as one function where everything is written in exactly the order it happens in:

def create_repository(project_name: str, repo_name: str) -> None:
    """Creates a new repo under the specified project"""
    # Load the repo request data from disk.
    with open("./repo_request_body.json", "r") as file:
        data = json.load(file)

    # Get the Azure DevOps project ID for a project name
    _request_object = make_request(
        "_apis/projects/",
        project_name,
        "?api-version=5.1-preview.1",
        request_method="get"
    )
    new_id = _request_object.get("id")

    # Update JSON dictionary with new ID
    data["project"]["id"] = new_id

    # Update JSON dictionary with new repo name
    data["name"] = repo_name

    # Make the server request.
    json_content_type = {
        'Content-Type': 'application/json',
        'Authorization': 'Basic XXXXXXXXXXXXXXXX'
    }
    make_request(
        project_name,
        "/_apis/git/repositories",
        "?api-version=5.1-preview.1",
        request_method="post",
        data=json.dumps(data),
        headers=json_content_type
    )

create_repository("project_name", "repo_name")

IMO this is a lot easier to follow, in that dependencies between the different blocks of code are very obvious and I can read it from top to bottom and easily keep track of what's happening when.

Taking it one step further, I'd eliminate the named variables that only get used once, which shortens the new_id part of the code (again, less for the reader to keep track of if they can see at a glance that function calls chain together to produce a single result, rather than having to try to figure out if a given value is going to be used again later):

def create_repository(project_name: str, repo_name: str) -> None:
    """Creates a new repo under the specified project"""
    # Load the repo request data from disk.
    with open("./repo_request_body.json", "r") as file:
        data = json.load(file)

    # Update JSON dictionary with new ID from server API
    data["project"]["id"] = make_request(
        "_apis/projects/",
        project_name,
        "?api-version=5.1-preview.1",
        request_method="get"
    ).get("id")

    # Update JSON dictionary with new repo name
    data["name"] = repo_name

    # Make the server request to create the repo.
    make_request(
        project_name,
        "/_apis/git/repositories",
        "?api-version=5.1-preview.1",
        request_method="post",
        data=json.dumps(data),
        headers={
            'Content-Type': 'application/json',
            'Authorization': 'Basic XXXXXXXXXXXXXXXX',
        }
    )


Compute and print the sum and mean of name and value pairs I'm just looking for more feedback on style/approach regarding this small problem. Also, if you have tips to make it more readable or C++-like, please let me know.

Also, why is the problem asking for the sum and mean of each name and all names, while the sum of a single element is itself? Am I missing anything in terms of answering the question?

#include <stdio.h>
#include <stdlib.h>
#include <map>
#include <string>
#include <iostream>
#include <sstream>
#include <algorithm>
#include <iterator>

/********************************************************************
 * Read a sequence of possibly whitespace-separated (name,value)
 * pairs, where the name is a single whitespace-separated word and
 * the value is an integer or floating-point value. Compute and print
 * the sum and mean for each name and the sum and mean for all names
 * ******************************************************************/

using namespace std;

std::multimap<string,float> _map;

istream& operator>>(istream& stream, pair<string,float> &in ) {
    return stream >> in.first >> in.second;
}

ostream& operator<<(ostream& stream, pair<string,float> &out ) {
    return stream << "(" << out.first
                  << ", " << out.second << ")" << endl;
}

int main( int argc, char *argv[ ] ) {
    istream *is = &cin;
    do {
        pair<string,float> input;
        (*is) >> input;
        _map.insert(input);
    } while( is->peek( ) != EOF );

    ostream *os = &cout;
    multimap<string,float>::iterator mit = _map.begin( );
    float sum = 0.0;
    while( mit != _map.end( ) ) {
        pair<string,float> p_pair = (*mit);
        (*os) << p_pair;
        sum+=p_pair.second;
        mit++;
    }
    float mean = static_cast<float>( sum/_map.size( ) );
    (*os) << "Sum: " << sum << " Mean: " << mean << endl;
}

Although it might be a bit longer (I haven't bothered to count lines), I think I'd prefer something on this order:

#include <map>
#include <string>
#include <numeric>
#include <iostream>
#include <utility>
#include <iterator>
#include <algorithm>

typedef std::multimap<std::string, float> vtype;
typedef std::pair<std::string, float> ptype;

namespace std {
std::istream &operator>>(std::istream &is, ptype &v) {
    return is >> v.first >> v.second;
}
}

float add_second(float a, vtype::value_type b) {
    return a + b.second;
}

template <class FwdIt>
float average(FwdIt start, FwdIt end) {
    float total = std::accumulate(start, end, 0.0f, add_second);
    return total / std::distance(start, end);
}

int main() {
    vtype values((std::istream_iterator<ptype>(std::cin)),
                  std::istream_iterator<ptype>());

    vtype::iterator start, end;

    for (start = values.begin(); start != values.end(); start=end) {
        end = values.upper_bound(start->first);
        std::cout << start->first << ": " << average(start, end) << "\n";
    }
    std::cout << "Overall: " << average(values.begin(), values.end()) << "\n";
    return 0;
}

If you preferred, you could use an std::vector instead of an std::multimap. That would require some minor changes to the code (e.g., using std::upper_bound instead of std::multimap::upper_bound, and adding a call to std::sort after reading the data, but would otherwise be pretty similar. It would, however, probably be a bit more efficient (less CPU time and memory).

While I suppose others might disagree, I prefer this for the simple reason that I find it easier to understand. We start by reading in all the data. We then find each run of identical keys, and compute the average for that run and print it out. When we reach the end of the data, we do the same for the overall average.

In fairness, if you were working with huge amounts of data, this probably wouldn't be the best way do do things. It does spend more time walking through the same data than is strictly necessary. At the same time, unless you're going to rewrite the code to overlap reading the data with computing the result, it's unlikely to make much real difference -- almost regardless of how you do the computation, that I/O will almost certainly occupy the vast majority of the overall time.


YUYV OpenGL Fragment Shader I made a shader to display a YUYV surface (data arranged as: Y0 U0 Y1 V0). It's working, but I don't know anything about shaders, or openGL, or anything graphic related as a matter of fact. I feel more like a mad scientist patching random bits of code together than a programmer. Thus, I would appreciate if anyone could look it up and tell me what I could improve, performance-wise, logic-wise or syntax-wise. I also have some questions:

What desktop hardware will that work on? Is there any restriction, or will it work on any  10 years old desktop?
Would that work on an android tablet?
Will any interpolation of pixel be performed, if my rendering surface is the same size as the texture? Is it possible to have interpolation (with some flag, not by implementing it)?

I load my YUYV texture as RGBA texture of width/2 by height. The rest should be self-explanatory:

#version 330 core
uniform sampler2D tex;
uniform mat4 convMat; /* Color conversion matrix */
uniform vec2 dim; /* Width and height of image */
in vec2 UV;
out vec3 color;

void main(){
    vec4 yuv;
    ivec2 iUV = ivec2(UV*dim*vec2(0.5, 1)); /* Absolute coordinate of desired pixel in texture */

    yuv = texelFetch(texy, iUV, 0);
    if (int(UV.x*dim.x) % 2 == 1){
        /* There are 2 Y in a texture RGBA pixel, choose the right one based on absolute pixel coordinate */
        yuv.x = yuv.z;
    };
    yuv.z = yuv.w;
    yuv.w = 1;
    color = (convMat*yuv).rgb;
};

Okay, trying to answer a few of your questions to the best of my knowledge:

  What desktop hardware will that work on? Is there any restriction, or will it work on any 10 years old desktop?

Probably not in a 10yo PC. Your code is currently targeting GLSL version 3.3, which was released circa 2010. You can check a version history here. However, if you need to be backward compatible, there are changes you can make to this code to target older OpenGL versions. I'll mention those at the end.

  Would that work on an android tablet?

No. Android devices use OpenGL-ES, which is a variant of Desktop OpenGL version 2.0. You wouldn't have the texelFetch() function neither integer types in the current OpenGL-ES used by both Android and iOS devices.

  Will any interpolation of pixel be performed, if my rendering surface is the same size as the texture? Is it possible to have interpolation (with some flag, not by implementing it)?

What exactly do you mean by interpolation? By any chance are you talking about texture filtering? By using texelFetch(), you have opted-out of texture filtering. It is not clear to my why you did this however. If you were to switch your texture sampling to a function like texture() (or texture2D if targeting older versions) you would be able to have filtered lookup and also avoid the non-portable texelFetch(). texture()/texture2D() also take the UVs as normalized 0-1 range texture coordinates, so it would make your shader faster too by eliminating the integer conversions.

Backward compatibility: You can make this code backward compatible with older OpenGL version by replacing texelFetch() with texture()/texture2D(), as I've mentioned before. That would also be necessary for current OpenGL-ES compat. You would also have to remove any integer conversions. Older OpenGL and OpenGL-ES hardware can only operate on floating point types! (no int or ivec).

A comment on performance: Branching is still an expensive operation on a lot of graphics hardware out there. So the most obvious improvement here would be to eliminate the conditional if test. You might be able to pull it off using several clever tricks. Maybe the mod function might be of help.


Updating a jquery progressbar via button clicks I'm trying to get my progress bar animated. I'd like it to smoothly transition in between the starting and destination values, but only be initiated by clicking the continue button.

The code I currently have works great, but when clicking forward or backward, it brings you directly to the values:

//PROGRESS BAR
$("#progress").progressbar(
{
    value: 0
})
//UPDATING PROGRESS BAR WHEN CONTINUE BUTTON CLICKED
var currValue=0,toValue=0;
$("#cont").button().click(function()
{
    currValue = $("#progress").progressbar("value");
    if(currValue+25 <= 100)
    {
        toValue=currValue+25;
        animateProgress();
    }
});
//DECREASING PROGRESS BAR WHEN GO BACK BUTTON CLICKED
$("#back").button().click(function()
{
    currValue = $("#progress").progressbar("value");
    if(currValue-25 > 0)
    {
        toValue=currValue-25;
        animateProgress();
    }
});
});

function animateProgress() {
  if (currValue < toValue) {
    $("#progress").progressbar("value", currValue+1);
    currValue = $("#progress").progressbar("value");
    setTimeout(animateProgress, 4);
  }
  else if (currValue > toValue) {
    $("#progress").progressbar("value", currValue-1);
    currValue = $("#progress").progressbar("value");
    setTimeout(animateProgress, 4);
  }
}

I've seen a couple examples of animated progressbars, but none by clicking and stopping when a certain value was reached.

Think of it more like 4 individual "slides" and the progress bar showing you which slide you're on.

EDIT: here's the working fiddle...http://jsfiddle.net/ryanhagz/8u76B/20/

Anyone tried something similar?

Not sure if it is helpful, but since we are on codereview, allow me to give it a shot.

there is a progress element available in HTML5 that works very easily and is fairly well supported (http://caniuse.com/progressmeter), except for in IE off course. I would suggest using this in stead of the (heavy) jquery-ui you use now.
When writing js code, avoid global variables at all cost. They take up resources, and some day they will come and haunt you when you accidentally overwrite a variable that you forgot about. Keep your variables in function scope whenever possible.
when you see yourself writing the same code more then once, with just a minor difference in one of the values, try to think of a way to reuse your code. It will decrease the number of bytes, make your code better maintainable, and almost certainly improve the quality of your code.
Be careful with setTimeout. It can be a very useful and powerful tool, but it costs quite some resources. When working with jQuery, try to use their functions in stead. They have been used and tested millions of times, and you are a lot less likely to mess things up when you use them.

With those tips in mind, I tried to rewrite your code.The full version is in the updated fiddle, and the javascript is here:

// changing progressbar when button is clicked
$(".button").click(function () {
    animateProgress(parseInt($(this).data('diff')));
});

// animate progress by a step indicated by diff
function animateProgress(diff) {
    var currValue = $("#progress").val();
    var toValue = currValue + diff;

    toValue = toValue < 0 ? 0 : toValue;
    toValue = toValue > 100 ? 100 : toValue;

    $("#progress").animate({'value': toValue}, 500);
}


Longest word simple online challenge For lack of better things to do I went on coderbyte and did one of the simple challenges since I have not written code for a little while.

The problem was stated as following:

Have the function LongestWord(sen) take the sen parameter being passed and return the largest word in the string. If there are two or more words that are the same length, return the first word from the string with that length. Ignore punctuation and assume sen will not be empty.

Few notes to TL;DR the comments:

I would have used const std::string& but function definition was given by CoderByte
string[string.length()] is guaranteed to return '\0' since C++11.
My code does not look for any terminating zeroes.

I'm looking for more discussion regarding the structure, the approach to the problem and general look & feel.

This is my solution:

// The function definition was given by CoderByte, I would have used
// const std::string& if it was up to me
string LongestWord(string sen) {
  // code goes here
  int start = -1;
  int bstart = -1;
  int blen = 0;

  // lequal since std string is compatible with c strings, it will have \0 at
  // the end, saving a special case after the loop.
  for (int i = 0; i <= sen.length(); ++i)
  {
      if (start == -1 && isalpha(sen[i]))
      {
          start = i;
      }
      else if (!isalpha(sen[i]))
      {
          if (start >= 0 && i - start > blen)
          {
               blen = i - start;
               bstart = start;
               start = -1;
          }
      }
  }

  return sen.substr(bstart, blen);
}

I ran the test cases and all was well, so I figured I'd check the top solution on the website:

string LongestWord(string sen) {

  // code goes here
  string sen2 = "";
  for(int i=0;i<sen.length();i++)
  {
    if(isalpha(sen[i])||sen[i]==' '||'0'<=sen[i]&&sen[i]<='9')
    {
      sen2.append(sen.substr(i,1));
    }
  }
  char* sench = (char*)sen2.c_str();
  string longest = "";
  int longestLen = 0;
  for(const char* pch=strtok(sench," ");pch;pch=strtok(NULL," "))
  {
    if(strlen(pch)>longestLen)
    {
      longest = pch;
      longestLen=strlen(pch);
    }
  }
  return longest;
}

From my point of view my solution is less complicated, has fewer moving parts, is easier to digest and at first glance I expect it to perform better due to less looping(although I have not tested this).

I feel like I am missing something, this happens with some frequency where I notice people use more complex solutions and I wonder if I'm basically writing tutorial code or something. Is this just me doubting myself or have I missed something?

Your solution is certainly neater than the other one. Programming challenge websites often contain horrible code.

There's a few things to improve though:

Use the container index type for indexing into a container. This ensures the index values cover the necessary range.

In this case, we should use std::size_t instead of int for indexing the std::string (or std::string::size_type if we're being paranoid). This would mean rethinking the algorithm slightly (perhaps we need a std::optional<std::size_t>).

Note that for storing the difference between indices (i.e. word length) we should use std::ptrdiff_t (or std::string::difference_type).

In C++ it's more idiomatic to use iterators, instead of indices. This makes it easier to genericize functions (e.g. finding the longest run satisfying an arbitrary precondition on an arbitrary sequence).

I'd be inclined to implement this with an outer loop to iterate each word, and an inner loop to find the start and end points.

It's more verbose, but perhaps a bit easier to follow:

#include <algorithm>
#include <cctype>
#include <string>

std::string get_longest_word(std::string const& input)
{
    auto const is_in_word = [] (unsigned char c) { return std::isalpha(c); };

    auto max_word_length = std::string::difference_type{ 0 };
    auto max_word_start = input.end();
    auto max_word_end = input.end();

    for (auto i = input.begin(); i != input.end(); )
    {
        auto word_start = std::find_if(i, input.end(), is_in_word);

        if (word_start == input.end())
            break;

        auto word_end = std::find_if_not(word_start, input.end(), is_in_word);

        auto const word_length = std::distance(word_start, word_end);

        if (word_length > max_word_length)
        {
            max_word_length = word_length;
            max_word_start = word_start;
            max_word_end = word_end;
        }

        i = word_end;
    }

    return std::string(max_word_start, max_word_end);
}

Which is on the way to becoming:

template<InputItT, PredicateT>
std::pair<InputItT, InputItT> find_longest_run(InputItT begin, InputItT end, PredicateT predicate);


Moving the player across an ASCII art "world" Wow, this one is definitely going to need some improvement.

So, just for fun, I decided to make a program where the player moves across a 2-dimensional ASCII-art map. If the player types something such as down, the on screen representation moves down and "loads" a bit more of the map.

from console import clear

class Move(object):
    up_down = 1
    right_left = 1

class Actions(object):
    action = {
        "down": 1,
        "up": -1,
        "left": -1,
        "right": 1
    }

def set_move_values(action):
    if action == "down":
        Move.up_down += Actions.action["down"]
    elif action == "up":
        Move.up_down += Actions.action["up"]
    elif action == "left":
        Move.right_left += Actions.action["left"]
    elif action == "right":
        Move.right_left += Actions.action["right"]

def render_map(x, y):
    for _ in range(y):
        print(''.join(['.' for _ in range(x + 4)]))
    print(''.join(['.' for _ in range(x + 2)])), '@',
    print(''.join(['.' for _ in range(1)]))
    for _ in range(1):
        print(''.join(['.' for _ in range(x + 4)]))

def run_program():
    while True:
        render_map(Move.right_left, Move.up_down)
        set_move_values(raw_input('> '))
        clear()

if __name__ == "__main__":
    run_program()

All I'm really looking for is, what general stuff can I improve? Also, if possible, I'd like something like right 5 to work to. Note: This is Python 2.7, I just prefer Python3's print function better. Also, console is an ios module, as I wrote this script on ios.

A few comments:

You already did a good job with PEP8. The only error I get back is that it expects two blank lines between functions
Regarding PEP257, please add some docstrings and comments to make clear what's the code goal to the potential reader
If find the Move class confusing. The up_down and left_right class attributes seem coordinates. I'd prefer to have a class named, for example, Player with x and y instance attributes. Note the differenct between class and instance attributes if you want to have multiple players in the future
The translation between actions and movements isn't very effective because you need to hardcode all four cases and use up/down/left/right multiple times
If iteration is all what is needed, I recommend using xrange instead of range
If just one iteration is needed (range(1)), just drop the loop for better readability
Use the multiplication operator to get long strings of the same charater instead of loops ('.' * n)

The code I'd have written would be more or less as follows (more comments below, console library not used):

"""ASCII world game."""

class Player(object):

    """Player object with some actions (just move for now)."""

    def __init__(self):
        """Initialize coordinates."""
        self.x = 1
        self.y = 1

    def move(self, dx, dy):
        """Move player to some position.

        :param dx: How much to move in the x axis
        :type dx: int
        :param dy: Move much to move in the y axis
        :type dy: int

        """
        # Ignore invalid movements
        if (self.x + dx) < 1 or (self.y + dy) < 1:
            return

        self.x += dx
        self.y += dy

class GameMap(object):

    """GameMap that shows where the player is."""

    X_OFFSET = 4

    def render(self, player):
        """Render map and player position."""
        def render_empty_row():
            """Render row in which there is no player."""
            print('.' * (player.x + self.X_OFFSET))

        for _ in xrange(player.y):
            render_empty_row()

        print('{} @ {}'
              .format('.' * (player.x - 1),
                      '.' * (self.X_OFFSET / 2)))
        render_empty_row()

class Command(object):

    """Command reader from stdin."""

    # Map text command to player movement
    CMD_TO_MOVE = {
        'u': (0, -1),
        'up': (0, -1),
        'd': (0, 1),
        'down': (0, 1),
        'l': (-1, 0),
        'left': (-1, 0),
        'r': (1, 0),
        'right': (1, 0),
    }

    # Default value for unknown command
    NO_MOVE = (0, 0)

    def read(self):
        """Read command (up, down, left, right) from stdin.

        The command is converted to a movement for the player using the same
        coordinate system.

        :returns: Player movement
        :rtype: tuple(int, int)

        """
        command_str = raw_input('> ')
        return self.CMD_TO_MOVE.get(command_str, self.NO_MOVE)

class Game(object):

    """Game object to keep state and read command for next turn."""

    def __init__(self):
        """Get all the objects needed to run the game."""
        self.player = Player()
        self.game_map = GameMap()
        self.command = Command()

    def run(self):
        """Render map and read next command from stdin."""
        while True:
            self.game_map.render(self.player)
            self.player.move(*self.command.read())

if __name__ == "__main__":
    game = Game()
    game.run()

What I tried to show:

Object oriented design: use the language of the problem space to define a class for each type of object in the problem. In general, when you think about the problem, if it's a name it should be a class, if it's a verb it should be a method
Use docstrings for every class and method (use sphinx format for arguments and return values if possible)
Make a difference about what is command (word) and a movement (delta in the coordinate system)
Use constants instead of harcoded values (X_OFFSET)

Note that not everything should be a class. The command reader could be a function, but I preferred to use class in this case for consistency.

I hope this helps.


Handle data insertion after validation I have an application that has a form. Validation and data insertion works fine. However, it bulks out my controller. I've always been told skinny controllers.

For example:

class myController extends BaseController
{
    public function store(myRequest $request)
    {
        $company_id = Company::insertGetId([
            'name'      => $request->name,
            'contact'   => $request->contact,
            'email'     => $request->email,
            'telephone' => $request->telephone,
        ]);

       $this->handleQuestion($request->question_one, '1', $company_id->id);
       $this->handleQuestion($request->question_two, '2', $company_id->id);
       $this->handleQuestion($request->question_three, '3', $company_id->id);
       $this->handleQuestion($request->question_four, '4', $company_id->id);
       $this->handleQuestion($request->question_five, '5', $company_id->id);
       $this->handleQuestion($request->question_six, '6', $company_id->id);
       $this->handleQuestion($request->question_seven, '7', $company_id->id);

       Outstanding::create([
            'company_id' => $company_id->id,
       ]);
    }

    public function handleQuestion($answer, $q_id, $company_id)
    {
        if( $question ) {
            CompanyAnswers::create([
                'company_id'  => $company_id,
                'question_id' => $q_id,
                'answer'      => $answer,
            ]);
        }
    }
}

The database schema has been created with a 2NF relation in mind.

companies

id (PK)
name
contact
email
telephone

company_answers

id (PK)
company_id (FK)
question_id (FK)
answer

outstanding_companies

id (PK)
company_id (FK)
question_id (FK)
answer

How is the application intended to work?

When the form is filled out, we create the company.
We ask seven questions - questions are not mandatory. (This is why we have a company_answers table - to avoid redundant data.) The results of the questions are stored in company_answers.
We insert the application in outstanding_companies so we know that this application is outstanding and needs attention.

My Problem

Have I totally misunderstood database normalisation and hindered myself by adding more code to handle data insertion?
If I have understood database normalisation, what is a better approach for handling the data insertion?
Is skinny controllers true? If so, where do I put data insertion logic?

After a lot of digging, I have finally arrived at (what I think) a better solution.

Laravel had a feature in 5.0 called CommandBus with 5.1 they have changed the name & the class structure (slightly) to Jobs.

Jobs are Queuable or you can run them synchronously. The real benefit from using jobs are:

Loosely coupled to the controller
Easier testing
Re-usable

This was a really good feature for me & my project because an application can be submitted in multiple controllers.

After implementation my controller now looks like this:

public function postGuest(Requests\VoucherFormRequest $request)
{
    //return the newly created id
    $company_id     = $this->dispatchFrom('App\Jobs\CreateCompanyJob', $request);

    //set the application to outstanding
    $outstanding_id = $this->dispatch(new SetApplicationToOutstandingJob($company_id));

    //return the user_id this application needs mapping to.
    $userId         = $this->dispatch(new InsertApplicationAnswersJob($request->only('question'), $company_id));

    //finally map the application to the user chosen by our auto delegation.
    $this->dispatch(new MapOutstandingAppToUserJob($outstanding_id, $userId));

    return redirect(route('guest-published'));
}

This is the solution that I have found & seems to be a huge improvement on what I had. Most likely, there will be a better solution so feedbacks always welcome!

Handling Questions

Handling questions was done by putting them in a multidimensional array within my form.

{!! Form::text('question[6]', '', array('class' => 'app-create-input')) !!}

With this approach all I had to do was iterate through the array & handle each parameter within the loop.

foreach ($this->answers as $key => $value) {
    CompanyAnswers::create([
        'company_id'  => $this->company_id,
        'question_id' => $key,
        'answer'      => $value
    ]);
}

Normalisation

In regards to my normalisation, I cannot see any glaring issues. Even after further research. However I don't claim to be a DBA so I could very well be missing something.


Cyclomatic complexity script As part of my programming training, I was told to research metrics for programming. I went ahead and turned the metrics into code starting with the cyclomatic complexity by McCabe. It works for bash sourcecode which I was targeting in this example.

My approach relies heavily on the while read line to read the Code line by line.
I tried to use more Regex at first but I soon hit a wall and decided to do it this way. Im familiar with Regex but inexperienced in combining them with sed or awk. So I'd like to ditch the while read line if that is possible for this kind of problem, and general readability tips for bash scripting would also be nice (im using vim to write the code).

if [ "$1" == "" ]
then
    echo "Type in file name: "
    read filename
else
# $1 is the first input parameter
filename=$1
fi

counter=1
caseflag=0
casecounter=0

while read line
do

 if [ "$caseflag" == "1" ]
 then
    if [[ $line == *"#"* ]]
    then
        #takes care of the comments in the SC
        temp=${line%%#*}
        tempcounter=$(echo $temp|awk '/;;$/{counter++}END{print counter}')
        let counter=$counter+$tempcounter > /dev/null 2>&1
    else
        tempcounter=$(echo $line|awk '/;;$/{counter++}END{print counter}')
        let counter=$counter+$tempcounter > /dev/null 2>&1
    fi
 fi
#loop detection
 temp=`echo $line | cut -d ' ' -f1`
 if [ "$temp" == "until" ] || [ "$temp" == "if" ] || [ "$temp" == "else" ] || [ "$temp" == "elif" ] || [ "$temp" == "while" ] || [ "$temp" == "for" ]
 then
    let counter=$counter+1
 fi
#case detection
 if [ "$temp" == "case" ]
 then
    caseflag=1
    let casecounter=$casecounter+1

 elif [ "$temp" == "esac" ]
 then
    caseflag=0
 fi

done < $filename

let counter=$counter-$casecounter > /dev/null 2>&1
echo "Cyclomatic complexity score: $counter"

bash grammar is notoriously hard to parse. Your script is easily misled by many valid bash constructs.

For example, it unconditionally consider # as a comment, whereas it could be escaped, or a part of a special parameter $#, or a part of ${parameter#word} expansion.

It will consider the assignment (stupid but valid)

    x=\;;

as an end of case. It will miss a loop in

    cat file | while read line

It will count the contents of a

    <<EOF
    while
    for
    until
    EOF

as loops.

It incorrectly handles nested case statements.
awk is a powerful programming language. If you feel you need to invoke it, consider using it for an entire script. It is particularly inefficient to invoke awk for a single line of input.
bash is also a very powerful programming language. The constructs like

    temp=`echo $line | cut -d ' ' -f1`

can be more clearly (and more efficiently) expressed in bash, e.g.

    read temp rest <<< $line

Testing for the presence of # to remove the comment is redundant. The expansion works correctly even if it is not there (in which case the line remains unchanged).


parsing log file and get time difference between two timestamp actions in python I am quite a beginner in python, so probably the taks I would like to perform is relative simple. I have log files with first column timestamp and second column describes the action, and I need to get the time difference between two actions. The log file called log.txt looks like:

2017-05-11T12:22:12.760 End step: first action

2017-05-11T12:22:13.724 Start step: other action

2017-05-11T12:22:15.069 End step: other action

2017-05-11T12:22:15.933 Start step: first action

I wrote a basic script that parses trough the directories, searches for specific words and calculate the time difference. However, the code is really basic and would like to improve it a little bit, for example by defining a config function that arises error and keeps running when the key words are missing. Any suggestion would be really appreciate. the code I use looks like:

   from datetime import datetime
   from datetime import timedelta
   import re
   import os
   import numpy as np

   inputDir = os.path.dirname(os.path.realpath(__file__))

   for subdir, dirs, files in os.walk(inputDir):
     for file in files:

     filepath = subdir + os.sep + file

     if 'log.txt' in filepath:
     filename=open(os.path.join(filepath))

         with open("OutputTime.csv","a") as outfile:

           for line in filename:
           line = line.rstrip()

           if re.search('Start step: first action', line):
              start_first_step=line[:23]
              start_step=datetime.strptime(start_first_step, "%Y-%m-%dT%H:%M:%S.%f")

           if re.search('End step: first action', line):
              end_first_step=line[:23]
              end_step=datetime.strptime(end_first_step, "%Y-%m-%dT%H:%M:%S.%f")
              minutes=end_step - start_step
              minutes=minutes.total_seconds()/60
              print minutes

In general, the indentation should be 4 spaces for each indentation level in Python. See PEP-0008 (the Python style guide). Loops and if statements should count as a level. The whole script could be shifted to the left, as well. Some of the variable names are a bit confusing because 'files' and 'file' can become easy to mix up as loop variables. One other thing of note is the spacing between things. I'd consider making fewer new lines between the bits of code. Some of the indentations seem like they should throw exceptions, so if you see IndentationErrors, that's what it's chirping about, I'd look at indenting after the blocks (for loops, and such).


Correct implementation of the factory pattern in php All, during my vacation I am trying to learn myself the basic principles of OOP and the factory pattern in php. I read a lot of tutorials and websites (e.g. http://www.phptherightway.com) and began to write some classes.

Is the underlying implementation correct?

Additional information:
I have set up a singleton class for my MysqlConnection. Furthermore I created a MysqlQuery class to handle the queries to the database.

Thanks for the advice!

Factory class

This is the factory class with the different CRUD-operations and the queries for the database

class OrganisationFactory
{
    public function getOrganisations()
    {
        $r = [];
        $q = new MysqlQuery("SELECT ID, Name, Created FROM organisations;", [0], MysqlConnect::get('core'));
        $q->select('', []);
        foreach($q->getResult() as $r)
        {
            $r[] = new Organisation($q['ID'], $q['Name'], $q['Created']);
        }
        return $r;
    }

    public function getOrganisation($ID)
    {
        $r = 0;
        $q = new MysqlQuery("SELECT ID, Name, Created FROM organisations WHERE ID = ?;", [0], MysqlConnect::get('core'));
        $q->select('i', [$ID]);
        foreach($q->getResult() as $r)
        {
            $r = new Organisation($q['ID'], $q['Name'], $q['Created']);
        }
        return $r;
    }

    public function createOrganisation($obj)
    {
        /* @var $obj Organisation */
        $q = new mysqlQuery('INSERT INTO organisations (Name, Created) VALUES (?, ?);', [0], mysqlConnect::get('core'));
        $q->insert('ss', [$obj->getName(), $obj->getCreated()]);
        $obj->setID($q->getResult());
        return $obj;
    }

    public function updateOrganisation($obj)
    {
        /* @var $obj Organisation */
        $q = new mysqlQuery('UPDATE organisations SET Name = ?, Created = ? WHERE ID = ?;', [0], mysqlConnect::get('core'));
        $q->update('ssi', [$obj->getName(), $obj->getCreated(), $obj->getID()]);
        return $obj;
    }

    public function deleteOrganisation($obj)
    {
        /* @var $obj Organisation */
        $q = new mysqlQuery('DELETE FROM organisations WHERE ID = ?;', [0], mysqlConnect::get('core'));
        $q->delete('i', [$obj->getID()]);
        return $obj;
    }
}

Base Class

This is the base class of the object including the variables, constructor and the getters and setters

class Organisation
{
    //variablen
    private $_ID = 0;
    private $_Name = '';
    private $_Created = '';

    //constructor
    function __construct($ID, $Name, $Created) {
        $this->_ID = $ID;
        $this->_Name = $Name;
        $this->_Created = $Created;
    }

    //getter & setters
    public function getID() { return $this->_ID; }
    public function setID($ID) { $this->_ID = $ID; }

    public function getName() { return $this->_Name; }
    public function setName($Name) { $this->_Name = $Name; }

    public function getCreated() { return $this->_Created; }
    public function setCreated($Created) { $this->_Created = $Created; }
}

No, this is not 100% true to the Factory Pattern - almost, but not 100%.

From Wikipedia:

  In class-based programming, the factory method pattern is a creational pattern (1) that uses factory methods to deal with the problem of creating objects without having to specify the exact class (2) of the object that will be created. This is done by creating objects by calling a factory method (3)—either specified in an interface and implemented by child classes, or implemented in a base class and optionally overridden by derived classes—rather than by calling a constructor (4).

(emphasis, mine)

The key points here are:

This is a creational pattern (it creates objects) - Your code satisfies this
Objects get created without having to specify the exact class. The factory pattern deals with inheritance - You are specifying an exact class, and only have one class.
A method is used to create the object
Code using the "factory" doesn't call a constructor directly

Your code satisfies points 3 and 4, with one fatal problem: The createOrganization method also INSERTs a record into the database. If a factory method is written correctly, the only thing it does is create an object. A factory method or class should never do anything else.

The major reason why you haven't actually created a "factory" is because you do not utilize inheritance, where you create a concrete instance and return it as an abstract parent class or interface.

Instead, what you have created is a Data Access object.

Now, a data access object can utilize a factory. Let's pretend your Organisation class is abstract, and it has two concrete child classes:

Business
NonProfitOrganisation

Sample code:

class OrganisationDataAccess
{
    private function createOrganisation($data) : Organisation {
        if ($data['type'] == 'business') {
            return new Business($data['ID'], $data['Name'], $data['Created']);
        } else if ($data['type'] == 'non-profit') {
            return new NonProfitOrganisation($data['ID'], $data['Name'], $data['Created']);
        }

        throw new Exception('Invalid organisation type: ' + $data['type']);
    }

    public function getOrganisation($id) {
        $data = // get from database

        $org = $this->createOrganisation($data);

        // Populate $org with more data

        return $org;
    }
}

Now the createOrganisation class is a true factory method. It instantiates concrete child classes and returns them as the abstract parent.

Now, simply because you aren't 100% true to the definition doesn't mean your createOrganisation method isn't useful - it eliminates duplication of code when creating new objects. But I would suggest a name change for your class: OrganisationDataAccess or OrganisationRepository is a better fit.

If, in the future, you really do need sub classes you have the option of adding an OrganisationFactory class if need be.


Return the prime numbers that are dividers of a some numbers in a list What I need to do is to return the prime numbers that are dividers of a some numbers in a list, so if
I = [12, 15] # result = [[2, 12], [3, 27], [5, 15]]

and this is the code that I used for that:
def sum_for_list(lst):
    r = []
    for num in lst:
        for i in range(2, abs(num)+1):
            if num%i == 0 and i%2 != 0 and all(i%x != 0 for x in r) and i not in r:
                r.append(i)
            elif i == 2 and num%i == 0 and 2 not in r:
                r.append(i)

    newList = []
    for r in r:
        z = 0
        for y in lst:
            if y%r == 0:
                z += y
        newList.append([r,z])

    return sorted(newList)

BTW, if you know about good resources to study efficient algorithms, I would really appreciate that too :)

Here are some suggestions for improving readability of your code. The more readable / easily understandable your code is, the easier it is to properly review regarding performance.

Naming
This is probably the most important point regarding your code. More or less none of the variables have meaningful names. This makes it that much harder to understand your algorithm.
Single letter variable names (r, z, y) are only a good idea when following well-known conventions like iterating over an index (for i in range(...)). The same goes for lst and newList, these names say nothing about content and function of the variables. Without fully understanding your algorithm I would presume some names like divisor, numbers, candidate_primes, results, etc. might be useful.
Variables should be named in snake_case instead of camelCase. PEP 8
for r in r:: Using the same variable name for different variables (in this case: the single elements of an iterable and the iterable itself) is confusing to the reader and might lead to some unexpected behaviour in some cases.

Conditions
Line 5: After checking all(i%x != 0 for x in r), you don't need to additionally check if i not in r. Since i % i == 0, the first condition would already fail if i in r.

Comments & docstrings
You should include comments and docstrings in your code to explain at least the key functionality / algorithmic approach.

You can remove the entire elif case from this for loop:
for num in lst:
    for i in range(2, abs(num)+1):
        if num%i == 0 and i%2 != 0 and all(i%x != 0 for x in r) and i not in r:
            r.append(i)
        elif i == 2 and num%i == 0 and 2 not in r:
            r.append(i)

like this:
for num in lst:
    if num % 2 == 0 and 2 not in r:
        r.append(2)

    for i in range(3, abs(num) + 1):
        if num % i == 0 and i % 2 != 0 and all(i % x != 0 for x in r) and i not in r:
            r.append(i)

This will save you some unnecessary checks for each i in range(3, abs(num) + 1) for each num.


Python interactive console which automatically reload modules I just started to program Python. As my first application I implemented an interactive console which automatically python modules before a new code block is evaluated. Here is the code:

### module "module_reloading_console.py"
import sys
import code
import imp
import readline
import os

class ModuleReloadingConsole(code.InteractiveConsole):

    def __init__(self):
        super().__init__()
        readline.parse_and_bind("tab: complete")
        self.stored_modifier_times = {}
        self.check_modules_for_reload()

    def runcode(self, code):
        self.check_modules_for_reload()
        super().runcode(code)
        self.check_modules_for_reload() # maybe new modules are loaded

    def check_modules_for_reload(self):
        for module_name, module in sys.modules.items():
            if hasattr(module, '__file__'):
                module_modifier_time = os.path.getmtime(module.__file__)

                if module_name in self.stored_modifier_times:
                    if module_modifier_time > self.stored_modifier_times[module_name]:
                        imp.reload(module)
                    self.stored_modifier_times[module_name] = module_modifier_time
                else:
                    self.stored_modifier_times[module_name] = module_modifier_time

ModuleReloadingConsole().interact("Welcome to ModuleReloadingConsole")

Usage:

~: python3 module_reloading_console.py
Welcome to ModuleReloadingConsole
>>> import test
>>> test.foo
23
>>> test.foo # in the meanwhile change foo to '42' in test.py
42

Because I am really new to Python I want to now which improvements you have for the above code. Are there common Python conventions I have not considered?

marginal note: this code recipe (automatically upgrade class instances on reload()) would be a good improvement. Maybe also this article about dynamically reloading a python code might improve the code...

Automatic tools

PEP8: I'd say your code's OK style wise, but this is what the checker said:

1:1: E266 too many leading '#' for block comment
19:40: E261 at least two spaces before inline comment
27:80: E501 line too long (86 > 79 characters)
29:80: E501 line too long (82 > 79 characters)
31:80: E501 line too long (82 > 79 characters)

Pylint: Apart for missing doc strings, the only thing it finds is that in your runcode method, you shadow a module name, code. It may be better to call you argument code_to_run or inp so that code always refers to the module. But, as the method's short, its not a big deal.

Human tool

Your check_modules_for_reload method looks like this:

def check_modules_for_reload(self):
    for module_name, module in sys.modules.items():
        if hasattr(module, '__file__'):
            module_modifier_time = os.path.getmtime(module.__file__)

            if module_name in self.stored_modifier_times:
                if module_modifier_time > self.stored_modifier_times[module_name]:
                    imp.reload(module)
                self.stored_modifier_times[module_name] = module_modifier_time
            else:
                self.stored_modifier_times[module_name] = module_modifier_time

Things to note:

Your if statement checking if you have already seen the module contains the line:

self.stored_modifier_times[module_name] = module_modifier_time

As this occurs in both branches of the if statement, you can move it out, after the branching.
Currently your code involves 4 indentation levels. You can reduce this by inverting the check_modules_for_reload logic and using continue.

The code now looks like:

def check_modules_for_reload(self):
    for module_name, module in sys.modules.items():
        if not hasattr(module, '__file__'):
            continue
        module_modifier_time = os.path.getmtime(module.__file__)

        if module_name in self.stored_modifier_times:
            if module_modifier_time > self.stored_modifier_times[module_name]:
                imp.reload(module)
        self.stored_modifier_times[module_name] = module_modifier_time

We see that the nested if could be reduced to:

if module_name in self.stored_modifier_times and module_modifier_time > self.stored_modifier_times[module_name]

That's long and horrid. But we can use dict.get combined with the fact module_modifier_time > module_modifier_time is False to produce:

if self.stored_modifier_times.get(module_name, module_modifier_time) < module_modifier_time

Which is slightly shorter. The code is now:

def check_modules_for_reload(self):
    for module_name, module in sys.modules.items():
        if not hasattr(module, '__file__'):
            continue

        module_modifier_time = os.path.getmtime(module.__file__)
        if self.stored_modifier_times.get(module_name, module_modifier_time) < module_modifier_time:
            imp.reload(module)
        self.stored_modifier_times[module_name] = module_modifier_time

Two lines shorter, two indentation levels and a bit more DRY. It might not be much, but the code was good to start with. Other things:

os.getmtime may throw OSError, but assuming no ones deleting modules or doing weird things this should not matter.
imp.reload could throw a whole horde of exceptions, crashing the shell. E.g.:

>>> m.a
Traceback (most recent call last):
File "rl.py", line 32, in <module>
    ModuleReloadingConsole().interact("Welcome to ModuleReloadingConsole")
File "/usr/lib/python3.4/code.py", line 234, in interact
    more = self.push(line)
File "/usr/lib/python3.4/code.py", line 256, in push
    more = self.runsource(source, self.filename)
File "/usr/lib/python3.4/code.py", line 74, in runsource
    self.runcode(code)
File "rl.py", line 17, in runcode
    self.check_modules_for_reload()
File "rl.py", line 28, in check_modules_for_reload
    imp.reload(module)
File "/usr/lib/python3.4/imp.py", line 315, in reload
    return importlib.reload(module)
File "/usr/lib/python3.4/importlib/__init__.py", line 149, in reload
    methods.exec(module)
File "<frozen importlib._bootstrap>", line 1153, in exec
File "<frozen importlib._bootstrap>", line 1129, in _exec
File "<frozen importlib._bootstrap>", line 1467, in exec_module
File "<frozen importlib._bootstrap>", line 1572, in get_code
File "<frozen importlib._bootstrap>", line 1532, in source_to_code
File "<frozen importlib._bootstrap>", line 321, in _call_with_frames_removed
File "/home/matthew/m.py", line 1
    a=5svnls2
            ^
SyntaxError: invalid syntax

After which the shell exits.


Optimizing multiple if-else from static class with LINQ I have a static class:

public static class ConfigurationDetails
{
    public static string SharepointServer { get; set; }
    public static string Username { get; set; }
    public static string Password { get; set; }
    public static string SharepointDomain { get; set; }
    public static string ServiceLibrary { get; set; }
    public static string SqlServer { get; set; }
    public static string SQLUsername { get; set; }
    public static string SQLPassword { get; set; }
    public static string SQLDomain { get; set; }
}

which needs to be updated from the values of a dictionary.

Using if the code is below:

var configValues = ReadConfiguration();

if (configValues.ContainsKey("SharepointServer"))
{
    ConfigurationDetails.SharepointServer = configValues["SharepointServer"];

}
if (configValues.ContainsKey("ServiceLibrary"))
{
    ConfigurationDetails.ServiceLibrary = configValues["ServiceLibrary"];

}
if (configValues.ContainsKey("Username"))
{
    ConfigurationDetails.Username = configValues["Username"];

}
if (configValues.ContainsKey("Password"))
{
    ConfigurationDetails.Password= configValues["Password"];

}

Now this will do the job, but there would be a lot of ifs. I've tried with LINQ, but this is the closest I've gotten:

configValues.Where(kv => kv.Key == "SharepointServer").ToList().ForEach(kv => ConfigurationDetails.SharepointServer = kv.Value);

and it's not doing any good. Any other ideas, please?

One solution would be to encapsulate the logic into a method. A problem with that is that there is no simple way to pass a property to the method.

If the case where the key is not in the dictionary means that the value is null, then you could write an extension method for IDictionary that returns the value, if it's present, or null:

public static string GetValueOrDefault(this IDictionary<string, string> dict, string key)
{
    string result;
    dict.TryGetValue(key, out result);
    return result;
}

You would then use it like this:

ConfigurationDetails.SharepointServer = configValues.GetValueOrDefault("SharepointServer");

Another option (assuming the names of properties and keys always match) would be to use reflection to set all the properties at the same time. Something like:

var properties = typeof(ConfigurationDetails).Getproperties();
foreach (var property in properties)
{
    string value;
    if (configValues.TryGetValue(property.Name, out value))
    {
        property.SetValue(null, value);
    }
}


Javascript contact form validation code I'm a UI / UX Designer trying to sharpen my JS skills.  I wanted someone else to check this code snippet and maybe have a Pro tip to improve or shorten it.

Javascript:

var formContainer = $('.contact-form-container');
var formError = $('.form-error');
var tosLabel = $('.tos-label');
var tosCheckbox = $('.tos-checkbox');

function checkBoxValidation(tosLabel, tosCheckbox, formError) {
    if (!tosCheckbox.is(':checked')) {
        tosLabel.removeClass('tos-is-checked');
        formError.addClass('tos-error');
    } else if (tosCheckbox.is(':checked')) {
        tosLabel.addClass('tos-is-checked');
        formError.removeClass('tos-error');
    }
}

tosCheckbox.change(function() {
    checkBoxValidation(tosLabel, tosCheckbox, formError);
});

formContainer.submit(function () {
    checkBoxValidation(tosLabel, tosCheckbox, formError);
});

HTML:

    <form data-abide novalidate method="post" action="{{ path(route) }}" class="contact-form-container">
        {% if contact.error %}

{{ contact.error }}

        {% elseif contact.success %}

{{ contact.success }}

        {% endif %}

            Vor- und Nachname
                <input type="text" required pattern="[a-zA-Z]+" name="name" value="{{ contact.name }}">
                Vor- und Nachname angeben.

            Ihre E-Mail Adresse
                <input type="email" required name="email" value="{{ contact.email }}">
                Ihre E-Mail Adresse ist nicht korrekt.

            Ihre Nachricht
                <textarea placeholder="Schreiben Sie eine Nachricht" name="message" required>{{ contact.message }}</textarea>
                Ihre Nachricht angeben.

                <input id="checkbox1" class="tos-checkbox" type="checkbox" name="privacy" {{ contact.privacy?'checked' }} required>
                Ich akzeptiere die Bestimmungen für den Datenschutz.

                Akzeptieren Sie die Bestimmungen zum Datenschutz.

        <input type="submit" class="button" value="Senden">
    </form>

Here is a suggestion:

var tosCheckbox = $('.tos-checkbox');
var checkBoxValidation = (function(label, checkbox, errorDisplay) {
    var checked = checkbox.is(':checked');
    return function(e) {
        if (e.type == 'submit' && !checked) e.preventDefault();
        tosLabel.toggleClass('tos-is-checked', checked);
        formError.toggleClass('tos-error', !checked);
    }
})(tosLabel.closest('label'), tosCheckbox, formError.closest('label').find('.form-error'));

tosCheckbox.change(checkBoxValidation);
tosLabel.closest('form').submit(checkBoxValidation);

Baiscally I tried to make the code dependent/related to tosCheckbox, finding both label, error and form from that element. This avoids pointing to the wrong thing like '.form-error' which I see more than one in the page. This way only the one inside the label will receive the class.

DRYed also the event listener callbacks, the class add/remove and added a check to prevent the form submit event. Not sure you need this, but its a heads up in case you are not stoping the submit in case of error.

Hope it helps.