Searching string for character not in array I have a string which I want to check for characters other than those in this list {"1", "2", "3", "4", "5", "6", "7", "8", "9", "0", ".", "i"} however this check gets run several thousand times a second.

I am therefore looking for the most efficient way of performing this check, what I have tried so far:

If Not System.Text.RegularExpressions.Regex.IsMatch(Input, "[^0-9\.i]") Then

This is equivalent to the code below which I have also tried

Imports System
Imports System.Linq

Public Module Module1

    Public Sub Main()
        Console.WriteLine(IsValidString())
    End Sub

    Private Function IsValidString() As Integer
        'Dim Input As String = "Hello World" 'This is Invalid
        'Dim Input As String = "13.4i+4" 'This is Invalid
        Dim Input As String = "13.4i" 'This is valid
        If Function()
            Dim IsValid As Boolean = True
            For Each Character In Input
                If Not {"1", "2", "3", "4", "5", "6", "7", "8", "9", "0", ".", "i"}.Contains(Character) Then
                    IsValid = False
                End If
            Next

            Return IsValid
        End Function() Then
            Return -1
        Else
            'Do some other stuff
            Return 1 '
        End If
    End Function
End Module

I am looking for optimal performance possibly at the cost of readability as I am willing to write pages on how the code works and what it does if only I can speed up the execution of my app!

First we'll clean this up a bit, then we'll look at performance.
There is absolutely zero reason to be using an anonymous function. This is useful functionality that you will eventually want to call from somewhere else. As such, it deserves to be extracted into a proper method of it's own.
Private Function IsValidString() As Integer
    'Dim Input As String = "Hello World" 'This is Invalid
    'Dim Input As String = "13.4i+4" 'This is Invalid
    Dim Input As String = "13.4i" 'This is valid
    If IsValidString(Input) Then
        Return -1
    Else
        'Do some other stuff
        Return 1 '
    End If
End Function

Function IsValidString(Input As String) As Boolean
    Dim IsValid As Boolean = True
    For Each Character In Input
        If Not {"1", "2", "3", "4", "5", "6", "7", "8", "9", "0", ".", "i"}.Contains(Character) Then
            IsValid = False
        End If
    Next

    Return IsValid
End Function

Next, let's forget entirely about the other IsValidString() method for a second and focus on the code we want to focus on. Watch your casing. It's terribly confusing to make local variables PascalCased just like your classes & methods. Local variables should be camelCased.
Public Sub Main()
    Console.WriteLine(IsValidString("Hello World"))
    Console.WriteLine(IsValidString("13.4i+4"))
    Console.WriteLine(IsValidString("13.4i"))
End Sub

Function IsValidString(input As String) As Boolean
    Dim isValid As Boolean = True
    For Each character In input
        If Not {"1", "2", "3", "4", "5", "6", "7", "8", "9", "0", ".", "i"}.Contains(character) Then
            isValid = False
        End If
    Next

    Return isValid
End Function

We could go one step farther and Linq-ifiy it a bit into some very readable code.
Function IsValidString(input As String) As Boolean

    Dim validChars = {"1", "2", "3", "4", "5", "6", "7", "8", "9", "0", ".", "i"}

    Return input.All(Function(c) validChars.Contains(c))

End Function

But does it perform? Well, actually, yes it does. I wrote some benchmarking code and it turns out that the Linq version out performs yours by quite a bit. I was kind of surprised by this, but here are the results.

Sample Size: 100
Original; Mostly Invalid: 10ms
Linq; MostlyInvalid: 0ms
Original; Mostly Valid: 0ms
Linq; Mostly Valid: 0ms

Sample Size: 1000
Original; Mostly Invalid: 2ms
Linq; MostlyInvalid: 1ms
Original; Mostly Valid: 2ms
Linq; Mostly Valid: 1ms

Sample Size: 10000
Original; Mostly Invalid: 25ms
Linq; MostlyInvalid: 4ms
Original; Mostly Valid: 25ms
Linq; Mostly Valid: 23ms

Sample Size: 100000
Original; Mostly Invalid: 235ms
Linq; MostlyInvalid: 41ms
Original; Mostly Valid: 231ms
Linq; Mostly Valid: 255ms

Sample Size: 1000000
Original; Mostly Invalid: 2408ms
Linq; MostlyInvalid: 587ms
Original; Mostly Valid: 2347ms
Linq; Mostly Valid: 2083ms

Press any key to continue . . .

As you can see, the Linq version nearly always outperforms your method on mostly invalid input. However, on mostly valid input, it performs roughly the same; it's sometimes better, sometimes worse. I suspect this is because the Linq version bails out of the check earlier on invalid input.
Here's the final version of the benchmarking code. It exceed's .NetFiddle's memory limits, so you'll need to paste it into a C# console program to see the results of the higher sample sizes.
If you're either uncomfortable with Linq, or really looking to just push the performance to the absolute best it can be, then we can just modify your original algorithm to return early when we know that the answer is false.
Function IsValidString(input As String) As Boolean
    For Each character In input
        If Not {"1", "2", "3", "4", "5", "6", "7", "8", "9", "0", ".", "i"}.Contains(character) Then
            Return False
        End If
    Next

    Return True
End Function

This outperforms the both version above in every case, mostly valid or mostly invalid and it does it consistently each time I run the updated benchmarks.

Sample Size: 1000
Original; Mostly Invalid: 2ms
Linq; MostlyInvalid: 0ms
Improved; Mostly Invalid: 0ms
Original; Mostly Valid: 2ms
Linq; Mostly Valid: 2ms
Improved; Mostly Valid: 1ms

Sample Size: 10000
Original; Mostly Invalid: 25ms
Linq; MostlyInvalid: 4ms
Improved; Mostly Invalid: 3ms
Original; Mostly Valid: 22ms
Linq; Mostly Valid: 20ms
Improved; Mostly Valid: 17ms

Sample Size: 100000
Original; Mostly Invalid: 242ms
Linq; MostlyInvalid: 43ms
Improved; Mostly Invalid: 33ms
Original; Mostly Valid: 235ms
Linq; Mostly Valid: 238ms
Improved; Mostly Valid: 186ms

Sample Size: 1000000
Original; Mostly Invalid: 2479ms
Linq; MostlyInvalid: 609ms
Improved; Mostly Invalid: 336ms
Original; Mostly Valid: 2358ms
Linq; Mostly Valid: 2059ms
Improved; Mostly Valid: 1704ms

BUT........
Correctness
Is 0323.233.12.i a valid string? Both your and my method says it is. Your regex also suffers from the same problem.


Monte carlo simulation in C This is my first C program. I want to optimize this simulation.
Algorithm
The simulation algorithm is:

The system can go from \$i\$ to \$i+1\$ with probability \$e^{-L\theta(\rho_i)}\$, where \$\rho_i=i/L\$, and \$\theta(\rho)=\rho(2b-\rho)\$.
When system reaches \$n=0\$, it is revieved (resurected) to position based on how much time it spent on \$n>0\$.
At the end we are intrested in knowing \$\langle\rho\rangle=\sum_{t}\rho_t\$.

Code
Following is the code. I believe this code can be compactified also. I think I do not understand the norms of the ANSI C standard. Feel free to correct me anywhere. I also do not understand if I am properly generating random numbers or not!.
#include<stdio.h>
#include<stdlib.h>
#include<math.h>
#include<time.h>

#define LATTICE 10
#define SWEEPS 100000000
#define BETA 0.5

float rho[LATTICE];

/*
Following function will make variable defined above:
    rho = 1/10, 2/10, 3/10, ... 1.
*/
void initialize()
{
    for(int i = 0; i < LATTICE; i++)
        rho[i] = (i+1.0)/LATTICE;
    return;
}

/*
These are the rates for going form n to n+1 for a given b.
*/
void rates(double *death_rate, float b)
{
    double theta;
    for(int i=0; i < LATTICE; i++)
    {
        theta = rho[i]*(2*b - rho[i]);
        *death_rate = exp(-LATTICE*theta);
        death_rate++;
    }
    return;
}

/*
Following function generates uniform random number
between 0 to 1.
*/
double uniform_rand()
{
    return (double)rand()/(double)RAND_MAX;
}

/*
The following function revive the system when n becomes -1
with the weights = distribution.
*/
int revive(unsigned long long *distribution, unsigned long long norm)
{
    int n = -1;
    double cumsum = 0.0, u_rand = uniform_rand();
    while(cumsum <= u_rand)
    {
        cumsum += (double) *distribution/(double)norm;
        distribution++;
        n++;
    }
    return n;
}

/*
Following function calculate the average density.
*/
double rho_avg(unsigned long long *distribution, unsigned long long norm)
{
    int i;
    double avg_density = 0.0;
    for (i=0; i<LATTICE; i++)
    {
        avg_density += (rho[i]*(*distribution))/norm;
        distribution++;
    }
    return avg_density;
}

double monte_carlo(float b, int n)
{
    unsigned long long i;
    unsigned long long distribution[LATTICE]={0};
    double death_rate[LATTICE];
    srand(time(0));
    rates(death_rate, b);
    for(i = 1; i <= SWEEPS; i++)
    {
        distribution[n] += 1;
        if (uniform_rand() < death_rate[n])
        {
            n--;
            if (n == -1)
                n = revive(distribution, i);
        }
    }
    return rho_avg(distribution, SWEEPS);
}

int main(void)
{
    int i;
    double avg_density;
    initialize();
    avg_density = monte_carlo(BETA, LATTICE-1);
    printf("\nAverage density is %lf\n\n", avg_density);
    return 0;
}

Naming things
Naming things is one of the hard things in computer science. You should make sure names of functions and variables are clear and concise. In general, use nouns for variable names and verbs for function names. Looking at your choices of names in more detail:

LATTICE is not a lattice, it is the size of the lattice. So call it LATTICE_SIZE.
SWEEPS is the number of sweeps to perform, so maybe it is better to call it N_SWEEPS (N is a commonly used prefix meaning "number of").
rates() is a function, so use a verb for the function name, for example calculate_rates().
rho_avg() is a function again, so use a verb for that as well, like calculate_rho_avg().

You should also be consistent in how you name things. Is it rho or density? Pick one and stick with it. I would also write beta instead of b, to match how you write down other greek letters like rho and theta.
Use array indexing notation where appropriate
In rates(), you are using pointer arithmetic when you could just have used standard array notation:
   for(int i = 0; i < LATTICE; i++)
   {
        theta = rho[i] * (2 * b - rho[i]);
        death_rate[i] = exp(-LATTICE * theta);
   }

Similarly, in revive(), write:
    for(n = 0; cumsum <= u_rand; n++)
    {
        cumsum += (double)distribution[n] / (double)norm;
    }

    return n - 1;

Terminology
Death rates? Revive? That sounds very morbid! Unless you are simulating some system that predicts cell death or pandemics, this is not terminology normally used in Monte Carlo simulations. Your code looks like it implements the Metropolis algorithm, where your death_rate would be equivalent to the transition probability, although I'm not sure what the equivalent of revive() would be. If it is the Metropolis algorithm you are implementing, then it doesn't look like you have detailed balance. What system are you simulating exactly? It might help to document that in the code as well.
Avoid global variables
It is good practice to avoid using global variables. That makes it easier if your program grows and becomes part of something larger. Or perhaps you want to run multiple simulations at the same time using threads. This should be easy; you already have the arrays distribution[] and death_rate[] inside monte_carlo(), just move rho[] there as well and pasas a pointer to rho to rates().
You might want to do this in a more organized way, and create a struct to hold all the information relevant to your simulation:
struct simulation_parameters {
    unsigned long long distribution[LATTICE];
    double death_rate[LATTICE];
    double rho[LATTICE];
};

...

double monte_carlo(float beta) {
    struct simulation_parameters params = {0}; // sets everything in params to zero
    calculate_rho(params.rho); // should do what initialize() did
    calculate_death_rates(params.death_rate, beta);

    for (unsinged long long i = 1; i <= SWEEPS; i++) {
        distribution[n]++;
        if (uniform_rand() < params.death_rate[n]) {
            n--;
            if (n == -1)
                n = revive(params.distribution, i);
        }
    }

    return calculate_rho_avg(distribution, SWEEPS);
}

int main(void) {
    srand(time(0)); // srand() should only be called once, so do it in main()
    printf("Average density is %lf\n", monte_carlo(BETA));
}


Query result as an HTML table I am not a expert on php html. I built a website 2 months ago, and realized I using lot of php query result on web. Is there any approach without while my query result on multiple times

<?php
$proc = "{call p_rpt_str310Web (?,?,?)}";
$params = array($PlanPkID,$bycatID,$ByDateVal);
$result = sqlsrv_query($conn, $proc, $params);
while ($row = sqlsrv_fetch_array($result))
{
?>

<?php echo $count; ?>
<?php echo $row['CusName']; ?>
<?php echo $row['CusCount']; ?>
<?php echo $row['WorkTime']; ?>

<?php } ?>

I have code like this in my html multiple times is there any way to do something like this

There are many ways to improve this code. but the two most basic ones would be

To create a simple helper function in order to avoid the repetitive code when running a query.
To separate the database interaction from the presentation logic, making the application more flexible, maintainable and clean-looking.

And one way of improvement I would advise against of: a function that would automate the HTML table creation. Although for the most primitive cases it would do, in reality there are no primitive cases, and you'll end up with a monster that takes more code to configure than to write just plain HTML, and it would completely unreadable compared with a straightforward HTML template.

A function.

function sqlsrv($conn, $sql, $params)
    $result = sqlsrv_query($conn, $sql, $params);
    $return = [];
    while ($row = sqlsrv_fetch_array($result)) {
        $return = $row;
    }
}

this function accepts a connection, an arbitrary query and an array of parameters, and returns a simple array with returned rows.

You can put this function's definition into the same file where your connection is made.

Code separation

This one is simple. Always split your scripts into two parts: data manipulation (AKA business logic) and a template (aka presentation logic). So you can refactor your code like this

$data = sqlsrv($conn, "{call p_rpt_str310Web (?,?,?)}",[$PlanPkID,$bycatID,$ByDateVal]);
?>

    <? php foreqach ($data as $row): ?>

      <?= $count ?>
      <?= $row['CusName'] ?>
      <?= $row['CusCount'] ?>
      <?= $row['WorkTime'] ?>

    <?php endforeach ?>


Building tree from edges puzzle with performance requirement I'm working on a puzzle and have solved it, but my code is not fast enough to get the maximum score (if the code is faster than a certain achievable threshold, one gets awarded the maximum score). I have tried multiple approaches but it seems I can't figure it out. My Java code is appended at the end.

The puzzle is as follows: Input is a number >= 1 of lines of two integers separated by a space which represent edges in a graph. This graph is a tree with 0 as root. We have to calculate the "value" of the graph defined as:
Each node initially has value 1. If a node has children, they each increase its value by their value +1. The final value of a node it its new value + values of all it's children. So:

node value =  1 + (# of children) + (sum of children's values).

The value of the graph is the sum of values of its nodes.
Then print the value modulo 1234567.

Examples:

0 1
2 1
2 3

has value 16.

1 0
2 0
1 3
1 4

has value 17.

Here's my code, if someone has any idea where I'm being inefficient, thanks for the help!

public static void main (String[] args) {

    Scanner stdin = new Scanner(System.in);
    Stack<Tuple> allNodes = new Stack<>();
    List<Integer> input = new ArrayList<>();

    //add all input to input list
    while (stdin.hasNext()) {
        input.add(stdin.nextInt());
    }

    allNodes.push(new Tuple(0, -1));
    int index, childIndex, child, parent, iterator = 0;

    //search input for known nodes from parents and add them to allNodes
    while (input.size() > 0) {
        parent = allNodes.get(iterator++).id;
        while ((index = input.indexOf(parent)) >= 0) {
            //check whether index is even or odd
            childIndex = (index%2 == 0) ? index+1 : index-1;
            child = input.get(childIndex);

            allNodes.push(new Tuple(child, parent));

            input.remove(index);
            if (index > childIndex) { input.remove(childIndex); }
            else { input.remove(index); }
        }
    }

    int[] values = new int[allNodes.size()];
    Tuple currentNode;

    //compute values
    while (allNodes.size() > 1) {
        currentNode = allNodes.pop();
        values[currentNode.parent] += ++values[currentNode.id]+1;
    }
    values[0]++;

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

    System.out.println(sum%1234567);
    stdin.close();
}

public static class Tuple {

    int id;
    int parent;

    public Tuple (int id, int parent) {
        this.id = id;
        this.parent = parent;
    }
}

Would rather make this a comment, but I need 50 reputation for that, so:
It became quite an answer apparently.

I find given code quite confusing. From what I am picking up, it does excessive searching/iteration where it is not necessary. I'd propose to make a refactoring, creating a class representing your tree. Then you can cleanely implement your requirements. E.g.:

public class Node {

    private final int id;
    private final List<Node> children;

    public Node(final int id) {
        this.id = id;
        this.children = new ArrayList<>();
    }

    public void addChild(final Node child) {
        children.add(child);
    }
}

Now you can add some methods directly expressing your requirements:

    public int graphValue() {
        return nodeValue() + children.stream().collect(summingInt(Node::graphValue));
    }

    public int nodeValue() {
        return 1 + children.size() + children.stream().collect(summingInt(Node::nodeValue));
    }

When creating your tree, you could use something like a HashMap so you can retrieve an already existing node by id in constant time. Perhaps you might need more optimizations.

Some other comments:

Try to avoid using the Stack class: see Stack JavaDoc
Close the scanner as soon as possible; it is also AutoCloseable, so you can use try-with-resources: see try-with-resources documentation
Prefer !list.isEmpty() over list.size() > 0, it is more idiomatic and potentially more efficient. Some implementations might traverse all elements to calculate the size, but as soon as there is an element it is of course not empty.
You created a Tuple class, which is a generic thing. But the implementation is not generic. I'd give it a more specific and telling name. (or possibly make it truly generic)
The amount of +'s in values[currentNode.parent] += ++values[currentNode.id] + 1 is too damn high :D


Section 2: Classes - Design a StackOverflow Post Design a class called Post. This class models a StackOverflow post. It should have properties
  for title, description and the date/time it was created. We should be able to up-vote or down-vote
  a post. We should also be able to see the current vote value. In the main method, create a post,
  up-vote and down-vote it a few times and then display the the current vote value.

  In this exercise, you will learn that a StackOverflow post should provide methods for up-voting
  and down-voting. You should not give the ability to set the Vote property from the outside,
  because otherwise, you may accidentally change the votes of a class to 0 or to a random
  number. And this is how we create bugs in our programs. The class should always protect its
  state and hide its implementation detail.

  Educational tip: The aim of this exercise is to help you understand that classes should
  encapsulate data AND behaviour around that data. Many developers (even those with years of
  experience) tend to create classes that are purely data containers, and other classes that are
  purely behaviour (methods) providers. This is not object-oriented programming. This is
  procedural programming. Such programs are very fragile. Making a change breaks many parts
  of the code.

That is the information given to me for this exercise in my C# tutorial.  The following is my working code.  Still trying to grasp Object Oriented Programming.  I believe I did set the properties so that you can get the information but not give the ability to set the Vote property from the outside by setting the property to Private.  I also tried to protect the state of voting so that you could not up-vote or down-vote consecutively, yet allow it to change your vote and show the vote count along with protecting the state of the vote count.

Anyhow if anyone can see how to improve on this, point out a better way, or teach me something new I would greatly appreciate it.

using System;

namespace ExerciseTwo
{
    class Post
    {
        public string Title { get; set; }
        public string Description { get; set; }
        public DateTime TimeDateCreated { get; private set; }
        public int VoteCount { get; private set; }
        private bool HasVotedUp;
        private bool HasVotedDown;

        public Post(string title, string description)
        {
            Title = title;
            Description = description;
            TimeDateCreated = DateTime.UtcNow;
            VoteCount = 0;
        }

        public void VoteUp()
        {
            if (HasVotedUp)
            {
                throw new Exception("You have already up-voted.");
            }
            else
            {
                VoteCount++;
                HasVotedUp = true;
                HasVotedDown = false;
            }
        }

        public void VoteDown()
        {
            if (HasVotedDown)
            {
                throw new Exception("You have already down-voted.");
            }
            else
            {
                VoteCount--;
                HasVotedDown = true;
                HasVotedUp = false;
            }
        }
    }
}

Script to demo how it could work.

namespace ExerciseTwo
{
    class Program
    {
        static void Main()
        {
            Post post = new Post("Does my post work?", "Test to see if my post works.");
            System.Console.WriteLine($"Title: {post.Title}");
            System.Console.WriteLine($"Description: {post.Description}");
            System.Console.WriteLine($"Date Created: {post.TimeDateCreated}");
            System.Console.WriteLine($"Post Count: {post.VoteCount}");
            post.VoteDown();
            System.Console.WriteLine($"Post Count: {post.VoteCount}");
            post.VoteUp();
            System.Console.WriteLine($"Post Count: {post.VoteCount}");
        }
    }
}

Thank you for your help!

I don't know which version of C# you use, but as of C# 6 it's possible to set initial values on property definition, which has 2 advantages:

Can eyeball the initial values quickly by looking at the property definitions.
Doesn't require you to copy the same initial value assignment code into additional constructors you may create.

Example initial value definition:

public int VoteCount { get; private set; } = 0;

Also in the case of int with initial value 0 you don't have to explicitly set it because when an instance of the class is initialized all the ints are initialized with the default value 0 unless you specified otherwise. Just like the bools are false by default (you didn't set them in the constructor).

You could add another layer of protection to the creation date:

public DateTime TimeDateCreated { get; } = DateTime.UtcNow;

Without defining a setter, TimeDateCreated is set when you create an instance of Post and can never be changed again for that instance. It makes sense here because the only date you'd ever want to change on a post is the date it was edited.

Not a big deal for now, but it's better to develop early the habit of giving good variable names, this means being descriptive, concise and consistent with the naming of things around the code base. Most of your names are good, but TimeDateCreated is a bit counter-intuitive because once you'll get used to the name DateTime you'll expect this property to be named DateTimeCreated.

It's good that you protected VoteCount, but your vote function does something unwanted: if you first downvote and then upvote, you will be back to 0 votes but without the ability to upvote. I suggest you rethink this part of the code.


Using Python to model a single elimination tournament I'm learning python and I want to model a single elimination tournament like they use in athletic events like tennis, basketball, etc.  The idea would be to insert some meaningful metrics to determine the winner.  I've already got a database that I plan to connect to to get the metrics, but I am hoping to find a better way to "process" games in the tournament to move the winner to the next round, and eventually find the tournament winner.

So far this is the best I have come up with, but it doesn't scale (at all easily) to 32, 64, 128 entries & so-on.  The "primary key" that I need to retain is the "seed id" (1-16, in this case), which matches the unique identifier in the database, so that I can pull the correct metrics for deciding which entry will win a matchup.  Suggestions?

## this list represents the round 1 seeding order on the tournament sheet
## this doesnt easily scale to 32, 64, 128 entries
teamlist = [1,16,8,9,5,12,4,13,6,11,3,14,7,10,2,15]

#In a single elim tournament with a full field, # of games is # of teams-1
totalgames = len(teamlist) - 1

#Set defaults
gameid = 0
roundid = 0
nextround = []

#simulate all of the games
while gameid < totalgames:
    if gameid in [8,12,14]:  ##this is a manual decision tree, doesn't scale at all
        #if a new round begins, reset the list of the next round
        print "--- starting a new round of games ---"
        teamlist = nextround
        nextround = []
        roundid = 0

    #compare the 1st entry in the list to the 2nd entry in the list
    homeid = teamlist[roundid]
    awayid = teamlist[roundid + 1]

    #the winner of the match become the next entry in the nextround list
    #more realistic metrics could be substituted here, but ID can be used for this example
    if homeid < awayid:
        nextround.append(homeid)
        print str(homeid) + " vs " + str(awayid) + ":  The winner is " + str(homeid)
    else:
        nextround.append(awayid)
        print str(homeid) + " vs " + str(awayid) + ":  The winner is " + str(awayid)

    #increase the gameid and roundid
    gameid += 1
    roundid += 2
    print "next round matchup list: " + str(nextround)

print nextround

You have two lines marked as "doesn't scale".

The initial team list can be obtained from your database table of available teams (select of team details).

The other "problem line", is

 if gameid in [8,12,14]:  ##this is a manual decision tree, doesn't scale at all

But that's easily avoided by noticing that the games in a round are always half the previous round, and the initial round is half the number of teams!

In other words, you can do something like (if you include the initial round):

def NewRoundIds( teams_in_tournament ):
    round_ids = []
    game_id = 0
    games_in_next_round = len(teams_in_tournament)/2 #need to count the number of teams_in_tournament list
    while games_in_next_round > 0:
        round_ids += [game_id]
        game_id += games_in_next_round
        games_in_next_round /= 2
    return round_ids

new_round_game_ids = NewRoundIds( teamlist )
...
if gameid in new_round_game_ids:
    # etc

== edit ==

This puts it well outside the brief of the site, but it was interesting. The following I think does what you want, generate_tournament(16). It could do with a bit of tidying up, and it's the sort of thing that will certainly benefit from docstrings and doctests, which I shall leave as a exercise.

import math

def tournament_round( no_of_teams , matchlist ):
    new_matches = []
    for team_or_match in matchlist:
        if type(team_or_match) == type([]):
            new_matches += [ tournament_round( no_of_teams, team_or_match ) ]
        else:
            new_matches += [ [ team_or_match, no_of_teams + 1 - team_or_match ] ]
    return new_matches

def flatten_list( matches ):
    teamlist = []
    for team_or_match in matches:
        if type(team_or_match) == type([]):
            teamlist += flatten_list( team_or_match )
        else:
            teamlist += [team_or_match]
    return teamlist

def generate_tournament( num ):
    num_rounds = math.log( num, 2 )
    if num_rounds != math.trunc( num_rounds ):
        raise ValueError( "Number of teams must be a power of 2" )
    teams = 1
    result = [1]
    while teams != num:
        teams *= 2
        result = tournament_round( teams, result )
    return flatten_list( result )


Project Euler #34 Digit factorials I'm having trouble improving the performance of the solution to the Project Euler problem #34

145 is a curious number, as 1! + 4! + 5! = 1 + 24 + 120 = 145.
Find the sum of all numbers which are equal to the sum of the factorial of their digits.
Note: as 1! = 1 and 2! = 2 are not sums they are not included.

I've done the following :
    private static readonly int max = (int)Factorial(9)*7;
    private static void Main()
    {
        long sum = 0;
        Stopwatch sw = new Stopwatch();
        sw.Start();
        for (int i = 3; i <= max; i++)
        {
            if (IsCuriousNumber(i))
            {
                sum += i;
            }
        }
        sw.Stop();
        Console.WriteLine(sum);
        Console.WriteLine("Time to calculate in milliseconds : {0}", sw.ElapsedMilliseconds);
        Console.ReadKey();
    }

    private static long Factorial(int input)
    {
        int sum = 1;
        for (int i = 1; i <= input; i++)
        {
            sum *= i;
        }
        return sum;
    }

    private static bool IsCuriousNumber(long input)
    {
        char[] digits = input.ToString().ToCharArray();
        long sum = digits.Sum(t => Factorial((int) char.GetNumericValue(t)));
        return sum == input;
    }

It's pretty much self-explanatory .. We have a method to find the factorial of specific number and one more method that checks if a number is "curious" as described in the problem above. We do this by simply converting the input to a char array than we have a variable that just sum's up all the factorials of the digits in the number.

I ran your code and it ran in 1397 milliseconds, so that's not something to worry about. However, there is something very inefficient about your code: you calculate Factorial for every digit you encounter.
Calculate them in advance.

You have declared your sum variable inside the Factorial(i) method as an int, while the return type is long. However, you are never dealing with a long value, so you can just change the return type to int.
(You're only calculating up to 9!, which fits well within the range for int)

Also, converting each number to a string and back again is kinda slow too.

class Euler034
{
    //Calculate the factorials in advance and just look them up.
    private static readonly int[] fact = Enumerable.Range(0, 10).Select(Factorial).ToArray();
    private static readonly int max = fact[9] * 7;

    public static void Main()
    {
        //Convenient method for creating and starting a Stopwatch
        Stopwatch watch = Stopwatch.StartNew();
        int sum = 0;
        for (int i = 4; i <= max; i++) {
            if (IsCuriousNumber(i)) sum += i;
        }
        watch.Stop();
        Console.WriteLine(sum);
        Console.WriteLine("Time to calculate in milliseconds : {0}", sw.ElapsedMilliseconds);
        Console.ReadKey();
    }

    private bool IsCuriousNumber(int number) {
        return number == Digits(number).Select(i => fact[i]).Sum();
    }

    private static int Factorial(int number)
    {
        int sum = 1;
        for (int i = 1; i <= number; i++) {
            sum *= i;
        }
        return sum;
    }

    private static IEnumerable<int> Digits(int number)
    {
        while (number > 0) {
            yield return number % 10;
            number /= 10;
        }
    }
}


Knight's Travails in ruby The task was to build a knight_moves method which when called would display the simplest path from one given square to the other given square.

My Solution:

class Square
  attr_accessor :x, :y, :children, :parent
  def initialize(x,y, parent=nil)
    @x = x
    @y = y
    @children = []
    @parent = parent
  end
end

def create_children(board)
  potentials = []
  potentials.push(
    [board.x + 2, board.y + 1],
    [board.x + 2, board.y - 1],
    [board.x + 1, board.y + 2],
    [board.x + 1, board.y - 2],
    [board.x - 2, board.y + 1],
    [board.x - 2, board.y - 1],
    [board.x - 1, board.y + 2],
    [board.x - 1, board.y - 2]
    )

  valid_children = potentials.select do |space|
    space[0].between?(0,8) &&
    space[1].between?(0,8)
  end

  valid_children = valid_children.map do |space|
    Square.new(space[0], space[1], board)
  end
  @children = valid_children
end

def knight_moves(first_square,final_square)
  first_children = Square.new(first_square[0],first_square[1])
  create_children(first_children)
  bfs(final_square, @children)
end

def bfs(search_value,children)
  queue = children

  loop do
    current = queue.shift
    if [current.x,current.y] == search_value
      display_path(current)
      break
    else
      create_children(current).each {|child| queue << child}
    end
  end
end

def display_path(current)
  parent = current.parent
  array = []
  while !parent.nil?
    array << [parent.x,parent.y]
    parent = parent.parent
  end
  array.reverse!
  array << [current.x,current.y]
  puts "Your path is:"
  array.each {|i| p i}
end

Method Call

knight_moves([2,5],[4,7])

Bug

This:

knight_moves([2,5],[2,5])

prints:

Your path is:
[2, 5]
[4, 6]
[2, 5]

Not the shortest path. In knight_moves, simply call bfs with [first_child] as the children argument. There's no need to call create_children in knight_moves; bfs can compute the first level of the search tree the same way it computes all other levels.

Global variable

In create_children, you're setting @children which acts like a global variable as this method isn't inside a class. Simply return valid_children without setting any variables.

(You're not setting Square#children because the method isn't inside Square. You actually never use Square#children. Remove this field from Square).

Naming

Rename:

Square -> Node (The class represents a node in the search tree. A true Square class would only have x and y fields).
create_children -> child_nodes
first_children -> root
board argument -> node (or parent)
display_path -> print_path

Missing Square class

The code sometimes uses an [x, y] array for squares, and sometimes the x and y fields of Node. This makes the code full of wrapping of unwrapping:

# lines that unwrap an [x, y] array:
Node.new(space[0], space[1], board)
first_child = Node.new(first_square[0],first_square[1])

# lines that create an [x, y] array:
if [current.x,current.y] == search_value
array << [current.x,current.y]

You can eliminate this by creating a Square class and use it for representing squares everywhere in the code (including in Node):

class Square
  attr_reader :x, :y

  def initialize(x, y)
    @x = x
    @y = y
  end

  def ==(other)
    [self.x, self. y] == [other.x, other.y]
  end

  def to_s
    [self.x, self.y].to_s
  end
end

I already implemented equality (for the comparison in bfs) and to_s (for printing in print_path). Then change Node to use a Square: (also, use attr_reader)

class Node
  attr_reader :square, :parent
  def initialize(square, parent=nil)
    @square = square
    @parent = parent
  end
end

Now replace any [x, y] array in the code with a Square:

# in child_nodes:
square = node.square
...
Square.new(square.x + 2, square.y + 1),
...
space.x.between(0, 8) &&
...
Node.new(space, node)

# in knight_moves:
Node.new(first_square)

# in bfs:
if current.square == search_value

# in print_path:
array << parent.square
array << current.square
puts i # instead of "p i", so to_s is used

# calling knight_moves:
knight_moves(Square.new(2, 5), Square.new(4, 7))

Missing Square methods

Most of child_nodes deals with squares, so most of its functionality should be inside Square methods. The only thing it does that doesn't involve only squares is creating nodes, so it should do just that:

def child_nodes(node)
  node.square.knight_moves.map do |square|
    Node.new(square, node)
  end
end

Square#knight_moves is a new method that returns an array of the squares that can be reached from the current one. It should look something like this:

def knight_moves
  [
    Square.new(self.x + 2, self.y + 1),
    ...
  ].select { |square| square.valid? }
end

This requires adding a Square#valid? to check if a square's coordinates are legal.

Revisiting knight_moves

knights_moves does too little now, it only creates a root node and put it in a one-element array. I would remove it and move its functionality to bfs (Make bfs accept first_square and final_square as arguments, and call it directly).

Seperating logic and printing

Don't print the path in bfs, just return it (the array of squares) and let the main code print it. You can simplify the code for getting the path:

def path_from_root(current)
  array = []
  loop do
    array << current.square
    current = current.parent
    break if current.nil?
  end
  array.reverse
end


Searching JSON recursively for an item with a "selected" attribute I have the following function, which I believe could be improved with functional-programming.

        function findSelectedInList(list){
        for(var i=0;i<=list.length;i++){
            var currentItem = list[i];
            if(currentItem.selected === "selected"){
                return currentItem;
            }
            var selectedNode = findSelectedInList(currentItem.children)
            if(selectedNode){
                return selectedNode;
            }
        }
        return void 0;
    }

Here are some sample inputs:

    var treeList = [ { "roleName": "User",
                   "roleId": "role1",
                   "children": [ { "roleName": "subUser1", "roleId": "role11", "children": [], "selected": "selected" }]
                 }];

    var treeList2 = [ { "roleName": "User", "roleId": "role1", "children": [],"selected":"selected"}];

Initially, I considered suggesting using Array.filter() but then that wouldn't allow breaking out of the loop once a selected item was found.

One approach is to use Array.some() or Array.find() (and disregard the return value) with a ternary operator. A variable can be initialized as the first line, then the .some() callback returns the result of setting that variable to either the current item if it is selected, otherwise the return value of the recursive call.

function findSelectedInList(list) {
  var selected;
  list.some(function(currentItem) {
    return selected = currentItem.selected === "selected" ? currentItem : findSelectedInList(currentItem.children);
  });
  return selected;
}

And one could use ecmascript-6 features like arrow functions and the let keyword), to condense it slightly:

function findSelectedInList(list){
  let selected;
  list.some((currentItem) => selected = currentItem.selected === "selected" ? currentItem : findSelectedInList(currentItem.children));
  return selected;
}

To see this demonstrated, expand the snippet below.

var treeList = [{
  "roleName": "User",
  "roleId": "role1",
  "children": [{
    "roleName": "subUser1",
    "roleId": "role11",
    "children": [],
    "selected": "selected"
  }]
}];

var treeList2 = [{
  "roleName": "User",
  "roleId": "role1",
  "children": [],
  "selected": "selected"
}];
var treeList3 = [{
  "roleName": "User",
  "roleId": "role1",
  "children": []
}];

function findSelectedInList(list) {
  var selected;
  list.some(function(currentItem) {
    return selected = currentItem.selected === "selected" ? currentItem : findSelectedInList(currentItem.children);
  });
  return selected;
}
console.log('treeList: ', findSelectedInList(treeList));
console.log('treeList2: ', findSelectedInList(treeList2));
console.log('treeList3: ', findSelectedInList(treeList3));

I know it doesn't really cover the find or some methods, but these functional JS exercises are a good thing to go through.


Searching for a value from one CSV file in another CSV file I am writing a script that takes one CSV file searches for a value in another CSV file then writes an output depending on the result it finds.

I have been using Python's CSV Distreader and writer.  I have it working, but it is very inefficient because it is looping through the 2 sets of data until it finds a result.

There are a few bits in the code which are specific to my setup (file locations etc), but I'm sure people can see around this.

# Set all csv attributes

cache = {}
in_file = open(sript_path + '/cp_updates/' + update_file, 'r')
reader = csv.DictReader(in_file, delimiter= ',')
out_file = open(sript_path + '/cp_updates/' + update_file + '.new', 'w')
out_file.write("StockNumber,SKU,ChannelProfileID\n")
writer = csv.DictWriter(out_file, fieldnames=('StockNumber', 'SKU', 'ChannelProfileID'), delimiter=',')
check_file = open(sript_path + '/feeds/' + feed_file, 'r')
ch_file_reader = csv.DictReader(check_file, delimiter=',')

#loop through the csv's, find stock levels and update file

for row in reader:
    #print row
    check_file.seek(0)
    found = False
    for ch_row in ch_file_reader:
        #if row['SKU'] not in cache:
        if ch_row['ProductCode'] == row[' Stock']:
            Stk_Lvl = int(ch_row[stk_lvl_header])
            if Stk_Lvl > 0:
                res = 3746
            elif Stk_Lvl == 0:
                res = 3745
            else:
                res = " "
            found = True
            print ch_row
            print res

            cache[row['SKU']] = res
    if not found:
        res = " "
        #print ch_row
        #print res
        cache[row['SKU']] = res
    row['ChannelProfileID'] = cache[row['SKU']]
    writer.writerow(row)

This is a few lines from my in_file and also the outfile is the same structure.  It just updates the ChannelProfileID depending on the results found.

"StockNumber","SKU","ChannelProfileID"
"10m_s-vid#APTIIAMZ","2VV-10",3746
"10m_s-vid#CSE","2VV-10",3746
"1RR-01#CSE","1RR-01",3746
"1RR-01#PCAWS","1RR-01",3746
"1m_s-vid_ext#APTIIAMZ","2VV-101",3746

This is a few line from the check_file:

ProductCode, Description, Supplier, CostPrice, RRPPrice, Stock, Manufacturer, SupplierProductCode, ManuCode, LeadTime
2VV-03,3MTR BLACK SVHS M - M GOLD CABLE - B/Q 100,Cables Direct Ltd,0.43,,930,CDL,2VV-03,2VV-03,1
2VV-05,5MTR BLACK SVHS M - M GOLD CABLE - B/Q 100,Cables Direct Ltd,0.54,,1935,CDL,2VV-05,2VV-05,1
2VV-10,10MTR BLACK SVHS M - M GOLD CABLE - B/Q 50,Cables Direct Ltd,0.86,,1991,CDL,2VV-10,2VV-10,1

You can see it selects the first line from the in_file, looks up the SKU in the check_file then writes the out_file in the same format as the in_file changing the ChannelProfileID depending what it finds in the Stock field of the check_file, it then goes back to the first line in the check_file and performs the same on the next line on the in_file.

As I say this script is working and outputs exactly what I want, but I believe it is slow and inefficient due to having to keep loop through the check_file until it finds a result.

What I'm after is suggestions on how to improve the efficiency.  I'm guessing there's a better way to find the data than to keep looping through the check_file.

What you want is a mapping from the product code (the key) to a stock level/result code (the value). In Python this is known as a dictionary. The way to do it is to go through your check file at the start, and use the information in it to create a dictionary containing all the stock level details. You then go through your input file, read in the product code, and retrieve the stock code from the dictionary you created earlier.

I've rewritten your code to do this, and it works for the example files you gave. I have commented it fairly thoroughly, but if there is anything unclear in it just post a comment and I'll try to clarify.

import csv

# Open the check file in a context manager. This ensures the file will be closed
# correctly if an error occurs.
with open('checkfile.csv', 'rb') as checkfile:
    checkreader = csv.DictReader(checkfile)

    # Create a function which maps the stock level to the result code.
    def result_code(stock_level):
        if stock_level > 0:
            return 3746
        if stock_level == 0:
            return 3745
        return " "

    # This does the real work. The middle line is a generator expression which
    # iterates over each line in the check file. The product code and stock
    # level are extracted from each line, the stock level converted into the
    # result, and the two values put together in a tuple. This is then converted
    # into a dictionary. This dictionary has the product codes as its keys and
    # their result code as its values.
    product_result = dict(
        (v['ProductCode'], result_code(int(v[' Stock']))) for v in checkreader
    )

# Open the input and output files.
with open('infile.csv', 'rb') as infile:
    with open('outfile.csv', 'wb') as outfile:
        reader = csv.DictReader(infile)

        # Use the same field names for the output file.
        writer = csv.DictWriter(outfile, reader.fieldnames)
        writer.writeheader()

        # Iterate over the products in the input.
        for product in reader:
            # Find the stock level from the dictionary we created earlier. Using
            # the get() method allows us to specify a default value if the SKU
            # does not exist in the dictionary.
            result = product_result.get(product['SKU'], " ")

            # Update the product info.
            product['ChannelProfileID'] = result

            # Write it to the output file.
            writer.writerow(product)


Auto backup Chrome bookmarks This script, once added to the Windows Scheduler, automatically creates a backup of your Google Chrome bookmarks.

import shutil
import os
import datetime

def setup():
    print ('Enter your Chrome bookmark directory.')
    print ('Standard one is: C:\\Users\\<USER>\\AppData\\Local\\Google\\Chrome\\User Data\\Default')
    bookmarkfolder = input('> ')

    print ('Enter the folder you want to save the backup files in.')
    print ('For example: C:\\Users\\<USER>\\Desktop\\Chrome Bookmarks')
    backupfolder = input('> ')

    with open('user.txt', 'w') as txt:
        txt.write(bookmarkfolder + '\n' + backupfolder)

    copy()

def copy():
    userList = []
    date = datetime.date.today()

    with open('user.txt', 'r') as txt:
        for line in txt:
            userList.append(line.strip())

    shutil.copy(userList[0] + '\\Bookmarks', userList[1])
    try:
        os.rename('Bookmarks', 'Bookmarks' + str(date).replace('-',''))
    except WindowsError:
        os.remove('Bookmarks' + str(date).replace('-',''))
        os.rename('Bookmarks', 'Bookmarks' + str(date).replace('-',''))

def checksetup():
    with open('user.txt', 'r') as setupfile:
        setupfile.seek(0)
        for line in setupfile:
            if '\\' in line:
                copy()
            else:
                setup()

checksetup()

The logic isn't so easy to follow, especially since the loops in copy
and checksetup imply that there are more lines, but really there will
be only two at most, the bookmark and backup folder.  I'd suggest making
that more obvious and avoiding for line in file: in case you only ever
want to have a fixed number of lines in that file.

The code also doesn't work with no "user.txt" file present, so some
more checks and error handling are necessary to handle that case as
well.

Some of the paths and other constructs are duplicated, I'd always try to
minimise the amount of duplication, so with that in mind, consider more
constants and variables to extract common code.

The .seek(0) in checksetup isn't necessary with mode 'r'; if you
open a file (for reading) you'll already be at the start; however, the
mode should probably be 'a+' anyway to create the file if it doesn't
exist.
If you don't like the escapes all the time, consider using the r""
syntax instead.
Use the if __name__ == "__main__": ... construct as is customary,
that way this file could be imported and used without running
checksetup as is it would currently.
Python typically uses lower case names with underscores between words,
c.f. PEP8.
For the date formatting use the strftime method instead to get the
desired output directly.
The setup method could use some more error checks, e.g. whether the
folders exist.
The "user.txt" file is opened three times in the worst case; it
would also be better to pass the two values necessary for the script
in the script as function arguments instead of parsing the file so
often; I'll leave that be for the moment though.  In general I'd say
that copy should have the signature
copy(bookmark_directory, backup_directory) and should be called with
the values from the file instead of parsing it.

After all that you can also look into command line arguments using
e.g. argparse for
parsing options, that way users won't have to deal with pesky manual
input instead of reusing the shell completion (for specifying the
folders).

So, it should look more like this perhaps:

import shutil
import os
import datetime

SETUP_FILE = 'user.txt'

def setup():
    print ('Enter your Chrome bookmark directory.')
    print (r'Standard one is: C:\Users\<USER>\AppData\Local\Google\Chrome\User Data\Default')
    bookmarkfolder = input('> ')

    print ('Enter the folder you want to save the backup files in.')
    print (r'For example: C:\Users\<USER>\Desktop\Chrome Bookmarks')
    backupfolder = input('> ')

    with open(SETUP_FILE, 'w') as txt:
        txt.write(bookmarkfolder + '\n' + backupfolder + '\n')

    copy()

def copy():
    with open(SETUP_FILE, 'r') as txt:
        user_list = [line.strip() for line in txt]

    shutil.copy(user_list[0] + r'\Bookmarks', user_list[1])

    new_name = datetime.date.today().strftime('Bookmarks%Y%m%d')
    try:
        os.rename('Bookmarks', new_name)
    except WindowsError:
        os.remove(new_name)
        os.rename('Bookmarks', new_name)

def checksetup():
    try:
        with open(SETUP_FILE, 'r') as txt:
            txt.seek(0)
            line = txt.readline()

        if '\\' in line:
            copy()
        else:
            setup()
    except FileNotFoundError:
        setup()

if __name__ == "__main__":
    checksetup()


Determine fragment identifier from HTML page in Swift For an iOS app which helps me rolling back vandalism on Stack Exchange, I have a piece of Swift code which downloads a revision page (example) and tries to find the 'spacer' fragment just above a certain revision. It might not be clear what I'm talking about, so here's a picture from Firefox + developer tools:

I have the HTML content of this page, and the GUID of the revision (8f9ab85f-1401-41e9-8f75-8a07b10bad32) from the Stack Exchange API. I'm looking for that element just above the revision header, since those are the only HTML elements with IDs on the page. I need that spacer-9617187a-fe48-4212-9a1a-f3a366e62736 so I can link directly to https://codereview.stackexchange.com/posts/189958/revisions#spacer-9617187a-fe48-4212-9a1a-f3a366e62736

For that, I've written a few lines of Swift code. The problem is that string handling in Swift confuses the **** out of me. Most of the language feels rather good, but I'd rather do string manipulation in SQL than in Swift...

Here is what I have so far. It works, but I was wondering if it could break in cases I haven't foreseen, or if it can be made more understandable/manageable by a future me. You see, even Stack Exchange's syntax highlighter has problems understanding it...

The input parameters for this piece of code are html (a String containing the content of the revisions page, e.g. https://codereview.stackexchange.com/posts/189958/revisions) and revisionGUID (8F9AB85F-1401-41E9-8F75-8A07B10BAD32 in the example above - the API returns them in upper case). fragment is eventually used as output parameter. The 43 is the length of spacer- plus a GUID.

// Find fragment just above selected revision
let range = html.range(of: #"onclick="StackExchange.revisions.toggle('"# + revisionGUID.lowercased() + #"')""#)!
let index = html.range(of: #"<tr id=""#, options: .backwards, range: html.startIndex..<range.lowerBound)!.upperBound
let fragment = String(html[index..<html.index(index, offsetBy: 43)])

I don't know how stable the precise HTML structure of those pages is, could that change in the future? Using a HTML parsing library might be a more robust approach.

Some remarks concerning the Swift implementation:

Don't force-unwrap optionals. If one of the searched strings is not found, your program will terminate with a runtime error. Use optional binding with if let or guard let instead, and handle the failure case properly.
Instead of converting revisionGUID to lowercase you can do a case-insensitive search.
The first search string can be created with string interpolation instead of concatenation, that makes the expression slightly shorter:

#"onclick="StackExchange.revisions.toggle('\#(revisionGUID)')""#

Use a regular expression with positive look-ahead and look-behind for the second search. That allows to find the precise range of the spacer, without relying on a particular length.
Put the code in a function, and add documentation.

Putting it together, the function could look like this:

/// Find spacer fragment for GUID on revisions page
/// - Parameter html: HTML of a revisions page
/// - Parameter revisionGUID: A revision GUID from the StackExchange API
/// - Returns: The spacer fragment, or `nil` if not found

func findFragment(html: String, revisionGUID: String) -> String? {
    let pattern1 = #"onclick="StackExchange.revisions.toggle('\#(revisionGUID)')""#
    guard let range1 = html.range(of: pattern1, options: .caseInsensitive) else {
            return nil
    }
    let pattern2 = #"(?<=<tr id=")[^"]+(?=")"#
    guard let range2 = html.range(of: pattern2,
                                  options: [.backwards, .regularExpression],
                                  range: html.startIndex..<range1.lowerBound) else {
            return nil
    }

    return(String(html[range2]))
}


Sorting structures lexicographically I am swapping structure lexicographically by name.
Is there any way to improve the code when the structure entries are heavy.

Here is the code

#include<stdio.h>
#include<string.h>
struct College
{
    char name[100];
    char city[100];
    int yearOfEstablishment;
    float passPercentage;
};
void displayCollegeDetails(struct College c)
{
    printf("Name : %s\n",c.name);
    printf("City: %s\n",c.city);
    printf("Year of establishment : %d\n",c.yearOfEstablishment);
    printf("Pass percentage: %.2f\n",c.passPercentage);
}
void swap(struct College *a,struct College *b)
{
    struct College temp;
    temp = *a;
    *a = *b;
    *b = temp;
}
int main()
{

    int n,i,j;
    //char name[100],city[100];
    //float paasPercentage;
    printf("Enter the number of colleges\n");
    scanf("%d",&n);
    struct College c[n];
    if(n<=0)
    {
        printf("Invalid Input\n");
        return 0;
    }

    for(i=0; i<n; i++)
    {
        fflush(stdin);
        printf("Enter the details of college %d\n",i+1);
        printf("Enter name\n");
        scanf("%s",c[i].name);
        printf("Enter city\n");
        scanf("%s",c[i].city);
        printf("Enter year of establishment\n");
        scanf("%d",&c[i].yearOfEstablishment);
        printf("Enter pass percentage\n");
        scanf("%f",&c[i].passPercentage);
    }
    for(i=0; i<n-1; i++)
    {
        for(j=i+1; j<n; j++)
        {
            if(strcmp(c[i].name,c[j].name)>0)
            {
                swap(&c[i],&c[j]);
            }
        }
    }
    printf("Details of colleges\n");
    for(i=0; i<n; i++)
    {
        fflush(stdin);
        printf("College %d\n",i+1);
        displayCollegeDetails(c[i]);
    }
    return 0;
    }

Is there any way to improve the code when the structure entries are heavy.

1st idea is to use a different sort method.  This code runs in O(n*n) time with the for(i=0; i<n-1; i++) for(j=i+1; j<n; j++) loops.

Various sorts operate in O(n*lo2(n)) time and are not too difficult to code.

2nd idea: Make an array of pointers to the list and sort that.  Then only pointers need to be moved during the heavy part of the algorithm.  When done, the final list can then be arranged per the pointer list.

3rd.  Use a hash table for even faster sorting.  Of course, this is more code and complexity.

General coding observations

fflush(stdin); is non-portable.  Code should be able to be written without it.  Better to use fgets() and then scan the string.
printf("Enter city\n"); scanf("%s",c[i].city); is problematic for 1) it does not limit input to 99 characters as needed per the buffer size.  Many cities have a space in their name "New York" and "%s" does not save spaces.  Use fgets() and lop off the tailing '\n' as needed.  name[] too.
Best to validate input:  check return value from *scanf() functions.
The check it too late. Do it before declaring College[]

// struct College c[n];
if(n<=0)
  ...
struct College c[n];

A point on improved sorting methods.

By sorting on name[] only, code could have multiple entries with the same name like "John Adams".  With OP's slow O(n*n) method, a benefit to it is that it is a stable sort.  OP's sorting would not change the sequence concerning ties.

Now consider the standard function qsort()  with a simple compare function that only reference name[], then the order of entries on ties is not guaranteed.

  "If two elements compare as equal, their order in the resulting sorted array is unspecified."  C11 §7.22.5.2

Many fast methods do not guarantee stability.

A classic solution is to include a unique index number with each College[].  Then when there is a tie on the name[], the index could be the tie breaker, ensuring a stable sort.  With the reference table idea far above, possible the "index" could be the College[] address (a pointer) in a clever usage, thus incurring no additional memory cost.


Golden Section Search in Lisp I implemented the golden section search algorithm recursively in Lisp. My code is:

(defconstant phi 0.618033988749895D0)

(defun linemin (f x1 x2 &key (xi1 (- x2 (* (- x2 x1) phi)))
                             (xi2 (+ x1 (* (- x2 x1) phi)))
                             (tol 0.001))
   (if (> tol (- xi2 xi1))
      (/ (+ xi1 xi2) 2)
      (if (> (funcall f x1) (funcall f x2))
         (linemin f xi1 x2 :tol tol :xi1 xi2)
         (linemin f x1 xi2 :tol tol :xi2 xi1))))

I sought to make this tail-recursive. Does Lisp provide a way of ascertaining whether the function was optimized and turned into a loop at compile time? Btw I'm using Lispworks.

Any other input is appreciated, of course.

The Common Lisp standard does not require implementations to eliminate tail calls, but this is an optimization supported by many implementations. It is the case for Lispworks. You can write your code so that it works portably with multiple implementations, provided you wrap your declarations as required for each one of them (e.g. #+sbcl). Otherwise,  you have to use an iterative approach, which is not hard to implement in your case.

Remarks

The code looks well-formatted, but consider using meaningful names:

Write tolerance instead of tol
Write line-min (?) or golden-section-search instead of linemin
x1, x2, xi1 and xi2 are a little hard to read. Granted, what you write is close to mathematical notations where variable names are short and written with indices, but maybe there are more interesting name to give (low, high?), especially when those names are used as keyword arguments.


Balanced expression checker This program detects whether an expression is balanced or not. I would like to know how I can make this more efficient.

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

struct Stack{
char data;
struct Stack *next;
};

void push(struct Stack **top,char data)
{
        struct Stack *new_node;
        new_node=malloc(sizeof(struct Stack));
        new_node->data=data;
        new_node->next=*top;
        *top=new_node;
}
int compare(char str,char op)
{
        if( (op == '(' && str == ')') || (op == '{' && str == '}') || (op == '[' && str == ']') || (op == '<' && str == '>') )
                return 1;
        else
                return 0;
}
int pop(struct Stack **top,char str)
{
        char op,ret;
        struct Stack *temp=*top;
        if(*top==NULL)
                return 0;

        else if(*top!=NULL)
        {
                // Pop the element and then call comapre function
                op=(*top)->data;
                free(temp);
                *top=(*top)->next;
                return(ret=compare(str,op));
        }
return 1;
}
void display(struct Stack *top)
{
        struct Stack *temp=top;
        while(temp!=NULL)
        {
                printf("%c ",temp->data);
                temp=temp->next;
        }
printf("\n");
}
int isNotEmpty(struct Stack *top)
{

        if(top!=NULL)
                return 1;
        else
                return 0;
}
int main(void)
{
        struct Stack *top=NULL;
        int i=0,y,flag=0;
        char str[]="((A+B)+(C+D)>";
        printf("\n Running the programe to check if the string is balanced or not ");
        for(i=0;str[i]!='\0';++i)
        {
                if( str[i] == '(' || str[i] == '[' || str[i] == '{' || str[i] == '<' )
                        push(&top,str[i]);
                else if( str[i] == ')' || str[i] == ']' || str[i] == '}' || str[i] == '>')
                {
                        y=pop(&top,str[i]);
                        if(!y)
                        {
                                printf("\n Unbalanced Expression ");
                                flag=1;
                        }
                }
        }
        if(!flag)
        {
                if(isNotEmpty(top))
                        printf(" \nUnbalanced Expression ");
                else
                        printf(" \n Balanced Expression ");
        }
        printf("\n");
        return 0;
}

pop() function
For this review, I'll only look at the pop() function, as it is representative of the whole program:

int pop(struct Stack **top,char str)
{
        char op,ret;
        struct Stack *temp=*top;
        if(*top==NULL)
                return 0;

        else if(*top!=NULL)
        {
                // Pop the element and then call comapre function
                op=(*top)->data;
                free(temp);
                *top=(*top)->next;
                return(ret=compare(str,op));
        }
return 1;
}

Purpose of the function
The first thing is that the function doing more than what its name says.  Not only is it popping an element off the stack, it is also trying to determine the correctness of the close brace with the open brace.  Ideally, your pop function should just pop an element off the stack and let the caller do the rest.  It's not a good idea to combine multiple unrelated tasks into single functions.
Naming
I don't particularly like top as the name of your stack but I can live with it.  But str is a terrible name for a character.  Anyone who reads your code will think that str is a string.  Especially because in main you have a string named str.  I don't really like op either.  What does it mean?
Useless variable
The variable ret is useless and can be removed.
Useless statements
The else condition is always true and can be removed.  The return 1 at the end can never be reached and can be removed.
Use of freed variable
You call free() before you set the stack to the next element, which is a mistake.  You are actually reading the next pointer from memory that is already freed.  You should reverse the order of those two statements.
Indentation
Your indentation is a bit strange with the return 1 unindented.
Rewrite
If you want to keep it in one function:
int popAndCompare(struct Stack **pStack, char closeBrace)
{
    struct Stack *top = *pStack;
    char openBrace;

    if (top == NULL)
        return 0;

    // Pop the element and then call compare function
    *pStack   = top->next;
    openBrace = top->data;
    free(top);
    return compare(closeBrace, openBrace);
}

Otherwise, separate the tasks:
char pop(struct Stack **pStack)
{
    struct Stack *top = *pStack;
    char ret;

    if (top == NULL)
        return '\0';

    *pStack = top->next;
    ret     = top->data;
    free(top);
    return ret;
}

int main(void)
{
    // ...
            char openBrace = pop(&stack);
            if (!compare(str[i], openBrace)
            {
                printf("\n Unbalanced Expression ");
                flag=1;
            }
    // ...
}


Print element in a linked list in reverse I solved this problem in C++ and was looking for feedback on my code. I'm not sure if storing the data in an array and looping through it in reverse and printing was the right approach since this challenge was to use linked lists, but I didn't know how else to do it:

  You are given the pointer to the head node of a linked list and you
  need to print all its elements in reverse order from tail to head, one
  element per line. The head pointer may be null meaning that the list
  is empty - in that case, do not print anything!

/*
 Print elements of a linked list in reverse order as standard output
 head pointer could be NULL as well for empty list
 Node is defined as
 struct Node
 {
 int data;
 struct Node *next;
 }
 */
void ReversePrint(Node *head)
{
    // creates nodes named current, prev, next
    struct Node *current, *prev, *next;
    // sets current and previous to their respective values. current is head because it is starting position while previous is where we want the head to be pointing, which is null, since we are reverse a linked list
    current = head;
    prev = NULL;
    // allocate memory to an array and initialize i=0
    int array[100], i=0;
    while (current != NULL) {
        // store data into an array to print out later
        array[i] = current->data;
        i+=1;
        // updates next [current, link] -> [int, link] = next
        next = current->next;
        // updates link of current.next to value of previous
        current->next = prev;
        // updates previous value
        prev = current;
        // updates current value
        current = next;
    }
    // at the end of list, sets head to the last value in linked list which is previous
    head = prev;
    // goes through array in reverse order and prints
    for(int j=i-1; j>=0; j--) {
        cout<<array[j]<<endl;
    }
}

This approach is fine if you know the linked list will always be less than 100 elements. To fix this issue while still using the same method, you could iterate through the linked list first to count the number of elements needed in your array.

If I were doing this problem, I'd look at using a stack... just push all the elements onto the stack, then pop them all off again to print them. This way we avoid iterating through the linked list twice (once to count, once to store in an array), while still solving to problem.


Aggregating values of a path-dependent type I have a type representing an immutable order which contains immutable order lines that belong to it. I want to use path-dependent types for this. Obviously you cannot instantiate a path-dependent type without having the value it depends on, so I decided to take a function from the constructor. Since (order: Order) => Seq[order.OrderLine] is impossible, I use a simple structural type.

final class Order private() {
  case class OrderLine(x: Int, y: Double) { }

  def this(makeOrderLines: { def apply(order: Order): Seq[order.OrderLine] }) = {
    this()
    _orderLines = makeOrderLines(this)
  }

  private var _orderLines: Seq[OrderLine] = null
  def orderLines = _orderLines
}

object Main extends App {
  val order = new Order(new {
    def apply(order: Order) =
      Seq(order.OrderLine(1, 1.0), order.OrderLine(2, 2.0))
  })
  println(order.orderLines)
}

I find this approach very ugly. Especially not being able to use a lambda expression irks me (the lambda expression would return Seq[Order#OrderLine] instead of Seq[order.OrderLine] since it has be of type Function1[_, _] where path-dependent types are not possible).

Is there a better way of doing this?

Taking a shower can do wonders. :)

You can take tuples instead and convert those to this.OrderLines in the constructor.

final class Order private() {
  case class OrderLine(x: Int, y: Double) { }

  def this(rawOrderLines: Seq[(Int, Double)]) = {
    this()
    _orderLines = rawOrderLines.map { case (x, y) => OrderLine(x, y) }
  }

  private var _orderLines: Seq[OrderLine] = null
  def orderLines = _orderLines
}

object Main extends App {
  val order = new Order(Seq((1, 1.0), (2, 2.0)))
  println(order.orderLines)
}

It’s simpler and more concise, although the call-site may still be rather confusing (not that it wasn’t already).


Modify or not modify your objects when creating computing functions? See the examples below:

function map_cloning(obj,fn){
    result = {};
    for (var key in obj)
        result[key] = fn(obj[key]);
    return result;
};
function map_modifying(obj,fn){
    for (var key in obj)
        obj[key] = fn(obj[key]);
    return obj;
};

function sum_cloning(vector1,vector2){
    return [vector1[0] + vector1[1], vector2[0] + vector2[1]];
};
function sum_modifying(vector1,vector2){
    vector1.x += vector2.x;
    vector1.y += vector2.x;
    return vector1;
};

function tail_cloning(obj){
    return obj.slice(1);
};
function tail_modifying(obj){
    obj.splice(0,1);
    return obj;
};

Often you have to chose between cloning or not an object before making a computation. Which is best?

I suggest cloning. If it was possible (like in Java), I'd even suggest making your objects immutable...

Why do I suggest cloning? Consider being after a busy Friday, with business users ranting all day on some totally unimportant feature, and you see this code fragment all of a sudden, having a, b and c variables:

b=sum_modifying(a,b);
 //some other code here
a=sum_modifying(c,a);
 //some other code here
b=sum_modifying(c,a);

Will you be able to track back what's going on? I woudn't, for sure...

Even this is a tough situation, but at least, the value of the variables change only with the assignment.

b=sum_cloning(a,b);
 //some other code here
a=sum_cloning(c,a);
 //some other code here
b=sum_cloning(c,a);

However, I'd suggest a third alternative: add functions to the objects themselves, to enable schemes like this:

result = a.add(b);

I'd find this the most readable of these.
(I stole the idea mainly from Java's BigDecimal.)


Upgrade script for MediaWiki website with one external addon (wave 1) I have a MediaWiki 1.33.0 website with only one external addon installed (external extension), which is ContactPage.
I have created the following script with that logic:

  if there is an upgrade to either MediaWiki core OR ContactPage these will be upgraded respectively;
  else, the script keeps the original state (that is, the state before the upgrade attempt). Successful upgrading might
  require further manual changes although I didn't have a case of
  needing to make manual changes.

I tested the script and it seems to work as expected; if no new version was released, the current ones are kept.

The script:

# Declare MediaWiki download variables (ensure latest versions are downloaded because as of 08/10/19 there aren't version-agnostic download links):

latest_mediawiki_core="https://releases.wikimedia.org/mediawiki/1.33/mediawiki-1.33.0.tar.gz"
latest_contactpage_extension="https://extdist.wmflabs.org/dist/extensions/ContactPage-REL1_33-abdcab9.tar.gz"

# Declare other important variables:

current_date="$(date +%Y-%m-%d-%H-%M-%S)"
war="$HOME/public_html" # Web Application Root # Change to your Web Application Root if needed
domain="example.com" # Change to relevant domain
db-username_and_db-name="DB_CREDENTIALS" # Change to relevant DB credentials

# Create backup directories:

mkdir -p "${war}/mediawiki_general_backups"
mkdir -p "${war}/mediawiki_specific_backups"

# General backups:

zip -r "${war}/mediawiki_general_backups/${domain}-directory-backup-${current_date}.zip" "${war}/${domain}"
mysqldump \
-u "${db-username_and_db-name}" \
-p "${db-username_and_db-name}" \
> "${war}/mediawiki_general_backups/${db-username_and_db-name}-${current_date}.sql"

# Specific backups:

rm "${war}"/mediawiki_specific_backups/*
rm "${war}"/mediawiki_specific_backups/.* # If I won't run this, a specific backup of .htaccess, in that directory, won't get deleted;

cp "${war}/${domain}"/.htaccess*                  "${war}/mediawiki_specific_backups"/.htaccess*
cp "${war}/${domain}"/LocalSettings.php           "${war}/mediawiki_specific_backups"/LocalSettings.php
cp "${war}/${domain}"/robots.txt                  "${war}/mediawiki_specific_backups"/robots.txt
cp "${war}/${domain}"/${domain}.png               "${war}/mediawiki_specific_backups"/${domain}.png
cp "${war}/${domain}"/googlec69e044fede13fdc.html "${war}/mediawiki_specific_backups"/googlec69e044fede13fdc.html

# Downloads and configurations:

rm -rf "${war}/${domain}"
mkdir "${war}/${domain}"
wget "${latest_mediawiki_core}" -O - | tar -xzv --strip-components 1 -C "${war}/${domain}"
wget "${latest_contactpage_extension}" -O - | tar -xzv -C "${war}/${domain}"/extensions/
cp -a "${war}/mediawiki_specific_backups"/* "${war}/${domain}"

# Create a new sitemap:

mkdir -p "${war}/${domain}/sitemap"
php "${war}/${domain}"/maintenance/generateSitemap.php \
--memory-limit=50M \
--fspath=/"${war}/${domain}/sitemap" \
--identifier="${domain}" \
--urlpath=/sitemap/  \
--server=https://"${domain}" \
--compress=yes

# Update DB (One might need to change LocalSettings.php before doing so):
php "${war}/${domain}"/maintenance/update.php

Make the process approach atomic

The script first wipes out ${war}/${domain},
then populates it with content from wget commands,
and restores content from backup.

The wget commands are a serious risk here.
If any of the source sites is down or experiencing instability at the time the script runs,
your site can end up in an inconsistent state.

At the minimum, it would be better to fetch all external resources before you start rewriting your content.

An even better way would be to try to make the process as atomic as possible:
build up the new content in a dedicated directory isolated from production,
but on the same partition,
and when all is ready, make the switch in fast directory rename operations.

Separate your backups from your production environment

The $war directory looks clearly in the heart of the production environment.
In that directory, you create backup directories side by side with the $domain directory.
I suggest to create the backups elsewhere.
Ideally on a different filesystem partition.

Closely related to this, the backup directories are somewhere in $war/...,
and the original content is also somewhere in $war/....
I think this invites mistakes.
If you had distinct variable name prefixes such as $backup_... and $content,
that would reduce the risk of mistakes.

Syntax error

I don't understand what this is doing in a Bash script:

db-username_and_db-name="DB_CREDENTIALS"

This is an invalid statement in all Bash versions that I know.

Bash scripts should have a shebang

The posted code has no shebang.
Since the question is tagged bash,
maybe it's implied it's really there in your version,
but just for the record, every Bash script should start with a shebang.

Watch out for things that may go wrong

Toby already mentioned this in his review,
but I think it deserves to be stressed.
Many things can go wrong in this script:

If mysqldump fails for some reason, the script will keep going without a backup created. That can lead to data loss, and it's to be avoided.
If any of the wget commands fail for some reason, the script will keep going and update the sitemap and the DB. That can lead to inconsistent state of your site that's difficult to debug.

My suggestion is similar to Toby's:
put set -euo pipefail near the top of the script,
to make it stop executing when something goes wrong unexpectedly.

Double-quote variables used in command arguments

You did that correctly for the most part,
you missed just one here:

cp "${war}/${domain}"/${domain}.png               "${war}/mediawiki_specific_backups"/${domain}.png

Use more variables

The term ${war}/mediawiki_general_backups comes up repeatedly.
It would be better if it was in a variable.
Editors with Bash support will make it possible to rename, see, find easily all occurrences.

Make long pipelines easier to read and understand

Take for example this:

wget "${latest_mediawiki_core}" -O - | tar -xzv --strip-components 1 -C "${war}/${domain}"

I think it's better to write flags before arguments.
The text closer to the left is naturally easier to read.
And the flags pack a lot of important details in little space,
and the URL argument is long and not very interesting to read.

Secondly, it's easier to understand code when there is a single statement per line.
A good way to achieve that is by splitting the line at each command in the pipeline.

That is, I suggest writing like this:

wget -O- "${latest_mediawiki_core}" \
| tar -xzv --strip-components 1 -C "${war}/${domain}"

Simplify

Instead of this:

rm "${war}"/mediawiki_specific_backups/*
rm "${war}"/mediawiki_specific_backups/.* # If I won't run this, a specific backup of .htaccess, in that directory, won't get deleted;

Why not simply:

rm -r "${war}"/mediawiki_specific_backups


Making sequence unique with removal records I'm working on a tool to detect all exact tandem repeats in a DNA/RNA sequence. Part of the algorithm requires finding Longest Common Extension (LCE) values, which although can be computed in constant time, in practice it is (usually) better to use a naive \$O(n^2)\$ solution. The problem is that DNA sequences can contain long contiguous sequences of N's (unknown base). This can completely destroy the performance of the algorithm as it doesn't play nice with the \$O(n^2)\$ LCE algorithm. Solution: collapse all stretches of N's into a single N, no other repeats are missed (other than N's).

The problem is that then the repeats reported by the algorithm are in respect to the modified sequence, so I need to record how many N's have been removed before that position in the original sequence to get the correct position.

More generally: The algorithm should take as input a sequence sequence and a character c, collapse all contiguous sub-sequences of c into a single c (in-place), and record how many cs have been removed up until the next non-c character past the last removed c.

Here is my current attempt at this algorithm:

#include <map>
#include <cstddef>   // std::size_t
#include <algorithm> // std::adjacent_find, std::find_if_not, std::unique
#include <iterator>  // std::begin, std::end, std::distance, std::next

template <typename SequenceType>
std::map<std::size_t, std::size_t> collapse(SequenceType& sequence, char c)
{
    std::map<std::size_t, std::size_t> result {};

    auto first = std::begin(sequence);
    auto last  = std::end(sequence);

    std::size_t position {}, num_removed {};

    while (first != last) {
        auto it1 = std::adjacent_find(first, last, [c] (char lhs, char rhs) { return lhs == c && lhs == rhs; });

        if (it1 == last) break;

        auto it2 = std::find_if_not(it1, last, [c] (char b) { return b == c; });

        position    += std::distance(first, it1);
        num_removed += std::distance(it1, it2) - 1;

        result.emplace(position, num_removed);

        first = it2;
    }

    if (!result.empty()) {
        sequence.erase(std::unique(std::next(std::begin(sequence), result.cbegin()->first), last,
                                   [c] (char lhs, char rhs) { return lhs == c && lhs == rhs; }), last);
    }

    return result;
}

I'm wondering if there's a better way that would avoid having to potentially scan the entire sequence twice (note the last call to std::unique)?

Generally, this seems to be readable code.  I would just note a few possible improvements:

Use a std::vector instead of a std::map

The std::map may be appropriate if you plan on doing frequent lookups of the key values, but a std::vector might be a less expensive storage mechanism than std::map otherwise.

Prefer for to while loop

The while loop may be more intuitively written as a for loop, as shown in the code below the next suggestion.

Consolidate loops

As you've asked in your question, yes, the separate loop can be eliminated.  Once the pair of iterators (it1 and it2) are found, simply use erase with those as parameters:

template <typename SequenceType>
std::map<std::size_t, std::size_t> collapse(SequenceType& sequence, char c)
{
    std::map<std::size_t, std::size_t> result {};

    std::size_t position {}, num_removed {};
    auto last = std::end(sequence);

    for(auto first = std::begin(sequence); first != last; last = std::end(sequence) ) {
        auto it1 = std::adjacent_find(first, last, [c] (char lhs, char rhs) { return lhs == c && lhs == rhs; });

        if (it1 == last) break;

        auto it2 = std::find_if_not(it1, last, [c] (char b) { return b == c; });

        position    += std::distance(first, it1);
        num_removed += std::distance(++it1, it2);

        result.emplace(position, num_removed);
        first = sequence.erase(it1, it2);
    }

    return result;
}

Because std::erase returns an iterator following the last erased value, this also provides a handy way to update first.  Also, by incrementing it1 within the num_removed update, we eliminate the need to subtract one and automatically correct the first iterator so that we eliminate only duplicate Ns and not all of them.

Consider the use of std::regex

It occurs to me that rather than use compress at all, perhaps std::regex might be better suited to solving this particular problem by eliminating the need to do preprocessing at all.  That is, with the proper regex, the notion of "one or more N's" is very quickly and easily represented.  Naturally, this assumes a more restrictive possibility range than a generic sequence object, but if your sequences are already std::string types, this may work very well.


Getting a remote file to local storage I've prepared a function using cURL to get a remote file to the local storage:

/**
 * cURL - Get Remove File
 *
 * Function using CURL the get a remote file to the local storage.
 *
 * @param str $url              Full remote URL
 * @param str $xml_path         Internal path to the storage the file
 * @param str $xml_file         Filename of the xml file to save
 * @param arr $access           Access credentials (usr;pwd)
 *
 * @return bollean
 */
function get_remote_file($url, $xml_path, $xml_file, $access = null) {

  $curl_handle = curl_init();
  curl_setopt($curl_handle, CURLOPT_HEADER, 0);
  curl_setopt($curl_handle, CURLOPT_URL, $url);
  curl_setopt($curl_handle, CURLOPT_CONNECTTIMEOUT,2);
  curl_setopt($curl_handle, CURLOPT_RETURNTRANSFER,true);
  if ($access) {
    curl_setopt($curl_handle, CURLOPT_USERPWD, $access["usr"].":".$access["pwd"]);
  }
  curl_setopt($curl_handle, CURLOPT_SSL_VERIFYPEER, false);
  $buffer = curl_exec($curl_handle);
  curl_close($curl_handle);

  if ($buffer != false) {
    file_put_contents($xml_path.$xml_file, $buffer);
    return true;
  }

    return false;

}

While is working fine, even with password protected files, I'm not too comfortable with CURL, so I wonder if this can be optimized or improved.

I'm not very familiar with cURL either, but looking at the documentation I noticed a curl_setopt_array() function that may be worth looking into.

$options = array(
    CURLOPT_HEADER => FALSE,
    CURLOPT_URL =>  $url,
    //etc...
);

curl_setopt_array( $curl_handle, $options );

Looking at the documentation for curl_exec() shows that the only time its output will be FALSE is on failure. The only time it is really necessary to compare a value explicitly with a boolean is if that value will at some point be a boolean and at all other times won't, but may be mistaken for it (0, '', 'false', etc...). However, at this point you would then use absolute equality comparison instead of a loose one. For instance, the below example will output "asdf", "false", '', and 0 from the first if statement, but the second and third if statements will only ever output "asdf". This is because the first if statement uses an absolute, or strict, comparison to the boolean. The third if statement shows a cleaner way of doing the second. Any variable you can loosely compare with a boolean, can loosely be used as a boolean, so there's no need to type out the actual comparison. I would use that last if statement because its not likely you want to create an empty file, and unless you have a file with only a "false" value in it, then you are not likely to notice the difference.

$buffer = array( FALSE, 'asdf', 'false', '', 0 );

foreach( $buffer AS $test ) {
    if( $test !== FALSE ) {//absolute
        var_dump( $test );
    }
    if( $test != FALSE ) {//loose
        var_dump( $test );
    }
    if( $test ) {//another way of writing loose
        var_dump( $test )
    }
}

The only other thing I would suggest here is that you verify that your $xml_file exists before trying to write to it.

if( $buffer && is_file( $xml_path . $xml_file ) ) {


Combining two Arrays of hashes based on a hash key I have two arrays of variable lengths: people1 and people2. I want to create an array of arrays where each inner array is a pair of elements from people1 and people2 The pairing should be based on a matching key. I'm using Ruby 2.2.0.

For example, given this input:

people1 = [{ name: :jon, id: 1 }, { name: :jay, id: 3 }, { name: :ray, id: 5 }]
people2 = [{ name: :jon, id: 2 }, { name: :ray, id: 7 }]

I would like this result:

pairs = [
  [{ name: :jon, id: 1 }, { name: :jon, id: 2 }],
  [{ name: :ray, id: 5 }, { name: :ray, id: 7 }]
]

Here's what I came up with:

people1.collect do |p1|
  if p2 = people2.find { |p2| p1[:name] == p2[:name] }
    [p1, p2]
  else
    nil
  end
end.compact

I looked at Array#zip, but it doesn't seem to take a condition. Is there a more idiomatic way to do this?

First create a hash that maps names to the hashes for one list:

people2_by_name = Hash[people2.map { |h| [h[:name], h] }]

Filter the other list

people1.select { |h| people2_by_name.has_key?(h[:name]) }
       .map { |h| [h, people2_by_name[h[:name]]] }

This will only work properly if there are no duplicate names in each, but it is \$\mathcal{O}(n)\$ in the number of hashes.


Small text-based RPG This is my first text-based game I made. It's very small because I just learned Python and wanted to start small and then code bigger programs. Please comment on this.

import random
import time
import sys

def fight_enemy(enemy_name, min_enemy_damage, max_enemy_damage, min_player_damage, max_player_damage):
    enemy_damage_dealt = random.randint(min_enemy_damage, max_enemy_damage)
    player_damage_dealt = random.randint(min_player_damage, max_player_damage)

    if enemy_damage_dealt > player_damage_dealt:
        print("Uh-oh! You died!")
        game_over()
    elif enemy_damage_dealt < player_damage_dealt:
        print("You killed the {enemy_name}".format(enemy_name=enemy_name))
        its_getting_late()
    else:
        print("You walk away unscathed, but the {enemy_name} still lives.".format(enemy_name=enemy_name))
        its_getting_late()

def fight_enemy_2(enemy_name, min_enemy_damage, max_enemy_damage, min_player_damage, max_player_damage):
    enemy_damage_dealt = random.randint(min_enemy_damage, max_enemy_damage)
    player_damage_dealt = random.randint(min_player_damage, max_player_damage)

    if enemy_damage_dealt > player_damage_dealt:
        print("Uh-oh! You died!")
        game_over()
    elif enemy_damage_dealt < player_damage_dealt:
        print("You killed the {enemy_name}".format(enemy_name=enemy_name))
        game_end()
    else:
        print("You walk away unscathed, but the {enemy_name} still lives.".format(enemy_name=enemy_name))
        game_end()

def intro():
    name = input("Enter you name: ")
    print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
    print("Welcome to the Wildlife %s" %(name))
    print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
    time.sleep(1)
    desert()

def desert():
    print("You are in the middle of a Desert in Africa")
    time.sleep(2)
    print("All you see is Sand. EVERYWHERE!!!")
    time.sleep(2)
    print("You start walking")
    time.sleep(2)
    print("You see a Snake")
    ch1 = str(input("Do you wanna attck the Snake? [y/n]: "))

    if ch1 in ['y', 'Y', 'YES', 'Yes', 'yes']:
        time.sleep(2)
        fight_enemy('Snake', 1, 7, 1, 7)
    elif ch1 in ['n', 'N', 'No', 'NO', 'no']:
        print("You slowly back away")
        time.sleep(1)
        print("You managed to get away from the Snake")
        print("But you fell in a sinkhole and died")
        game_over()

def game_end():
    print("Congratulations you won!!!")
    ch4 = str(input("Do you wanna play again? [y/n] "))

    if ch4 in ['y', 'Y', 'YES', 'Yes', 'yes']:
        intro()
    elif ch4 in ['n', 'N', 'No', 'NO', 'no']:
        print("Thank you for playing")

def its_getting_late():
    time.sleep(1)
    print("Its starting to get late")
    ch2 = str(input("Do you wanna go to sleep? [y/n]: "))

    if ch2 in ['y', 'Y', 'YES', 'Yes', 'yes']:
        print("You were shot dead last Night")
    elif ch2 in ['n', 'N', 'No', 'NO', 'no']:
        print("You stay up")
        print("You see a person")
        ch3 = str(input("Do you wanna attack the person? [y/n]: "))

        if ch3 in ['y', 'Y', 'YES', 'Yes', 'yes']:
            time.sleep(1)
            fight_enemy_2('Enemy', 4, 9, 1, 7)
        elif ch3 in ['n', 'N', 'No', 'NO', 'no']:
            print("You try to run away")
            print("But the Enemy is faster than you and stabs you")
            game_over()

def game_over():
    print("You lost")
    ch5 = str(input("Do you wanna play again? [y/n] "))

    if ch5 in ['y', 'Y', 'YES', 'Yes', 'yes']:
        intro()
    elif ch5 in ['n', 'N', 'No', 'NO', 'no']:
        print("Thank you for playing")

intro()

You could simplify the yes/no answer checks:

if answer.lower() in ('y', 'yes'):
    # ...

And instead of writing these repeatedly, you could create a helper function to encapsulate this logic:

def is_answer_yes(answer):
    return answer.lower() in ('y', 'yes')


Scoped time sink implementation I have written a little class that is intended to be used to spend unused time in a loop body, sleeping.

That is, I have a loop. I want every iteration to take at least X seconds. If the operations actually complete quicker, the excess time shall be spent sleeping.

My implementation is based on a class that is created in a scope, and whose destructor sleeps for the required amount of time when the object's lifetime ends.

Is this a sane approach to the problem? Are there any flaws in the implementation? Is there potential for improvements regarding coding style?

#include <chrono>
#include <thread>

class TimeSink {
public:
    typedef std::chrono::milliseconds duration_ms;

    TimeSink(duration_ms const & min_duration)
    : min_duration(min_duration), start(std::chrono::steady_clock::now()) {}

    ~TimeSink() {
        time_point end = std::chrono::steady_clock::now();
        auto elapsed = end-start;
        auto remaining_time_to_sleep = min_duration - elapsed;
        if (remaining_time_to_sleep > epsilon) {
            std::this_thread::sleep_for(remaining_time_to_sleep);
        }
    }

private:
    typedef std::chrono::time_point<std::chrono::steady_clock> time_point;
    const duration_ms epsilon = duration_ms(0);
    duration_ms min_duration;
    time_point start;
};

int main() {
    TimeSink t(TimeSink::duration_ms(1000));
}

Allow users to customize your TimeSink class

We achieve this by making it a template class:

template <typename TimeUnit, typename Clock>
class time_sink;

This will allow your class to be used in different scenarios; it removes restrictions.

Since the TimeUnit template parameter is the one most likely to be changed, we can provide a default argument for Clock:

template <typename TimeUnit, typename Clock = std::chrono::steady_clock>
class time_sink;

Your destructor can be simplified

Instead of doing...

time_point end = std::chrono::steady_clock::now();
auto elapsed = end-start;
auto remaining_time_to_sleep = min_duration - elapsed;
if (remaining_time_to_sleep > epsilon)
{
    std::this_thread::sleep_for(remaining_time_to_sleep);
}

...we can do:

auto time_spent = Clock::now() - begin;
if ( time_spent < min_duration )
{
    std::this_thread::sleep_for( min_duration - time_spent );
}

I find this clearer and easier to understand.

Consider using std::chrono::high_resolution_clock as your default clock type

This stack overflow question should tell you why that's the preferred clock type for timing function execution.

template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
class time_sink;

Concise usage with the std::chrono_literals namespace and a template function

This would simply make the code shorter. Note that there is a repetition, as we must define time_sink's time unit type.

using namespace std::chrono_literals;
using time_sink = time_sink<std::chrono::milliseconds>;
time_sink{ 500ms };

In order to make usage fully concise (avoid repetition), we can use a template function and implement the idea proposed by 5gon12eder, in this comment which simply creates the time_sink object and uses template function type deduction to fill in the details.

template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
time_sink<TimeUnit, Clock> make_time_sink( TimeUnit const minimum_duration )
{
    return time_sink<TimeUnit, Clock>{ minimum_duration };
}

This allows us to create a time_sink in a short and clear way:

using namespace std::chrono_literals;
make_time_sink( 500ms );
// rest of function code, etc.

The improved code

Here's what the final code could look like:

#include <chrono>
#include <thread>

template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
class time_sink
{
public:
    time_sink( TimeUnit const minimum_duration ) :
        min_duration{ minimum_duration },
        begin{ Clock::now() }
    {}

    ~time_sink()
    {
        auto time_spent = Clock::now() - begin;
        if ( time_spent < min_duration )
        {
            std::this_thread::sleep_for( min_duration - time_spent );
        }
    }

private:
    TimeUnit min_duration;
    std::chrono::time_point<Clock> begin;
};

template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
time_sink<TimeUnit, Clock> make_time_sink( TimeUnit const minimum_duration )
{
    return time_sink<TimeUnit, Clock>{ minimum_duration };
}

Sample usage

As 5gon12eder points out, you must also ensure that you keep the return of make_time_sink in order to prevent time_sink's destructor from running early, since destructors are called at the "end of the full expression, for nameless temporaries" (source).

void f()
{
    using namespace std::chrono_literals;

    // we keep the return of make_time_sink in order to prevent
    // time_sink's destructor from executing before the end of the function
    auto ts = make_time_sink( 500ms );
    // ... do work ...
}

int main()
{
    f();
}