My own twist on the classic "Guessing Game" script in Python This is a (letter, or whatever the user decides to change it to) guessing game I wrote which implements both a mechanic to randomly change the order of elements in a list, as well as limit the number of attempts you get to complete the game. There's not much else to say about it. I'm just looking for some suggestions on where I can condense things and better optimize my code.

import random

samples = random.sample(['x', 'y', 'z'], 3)
keys = []

for i in samples:
    str(keys.append(i))

attempts_left = 3

print("X, Y, and Z are arranged in random order.")
print("The object of the game is to guess them in order before you run out of tries.\n")

while len(keys) > 0:

    for i, j in list(enumerate(keys, start=1)):
        key = j

        while attempts_left > 0:

            print("Round {}".format(i))
            guess = input("> ").lower()

            if guess == key:
                i += 1
                print("Correct\n")
                keys.remove(key)
                break

            elif guess not in ['x', 'y', 'z']:
                print("That's not one of the options.")

            else:
                print("Wrong\n")
                attempts_left -= 1

    if attempts_left > 0:
        print("You win!")
        break

    else:
        print("You LOSE!")
        print("Good DAY, sir!")
        break

Using the right functions

What you want to do is present the string xyz in some order. For that, there's random.shuffle(). Doing sampling is for taking a small number of elements from a large set (think, drawing a poker hand from a deck), and is not well suited for this particular problem. So we can start with:

answer = 'xyz'
random.shuffle(answer)

Note that a string in python is also an iterable, you don't have to make it a list of strings. Also your str() in your loop does nothing - and the loop itself is just a very verbose copy. Just the shuffle suffices.

Your outer loop

Your logic looks like:

while cond:
    ...
    if cond2:
        break
    else:
        break

That is, the outer loop never matters. len(keys) > 0 for sure the first time through and then we never check it again, so we can simply drop it.

Iteration

There's no reason to write list(enumerate(iterable)) if you're just looping over it. That unnecessarily has to bring the entire enumeration into memory to construct the full list. You only need one element at a time - so let the iterable give you one element at a time.

We can also make this much more readable by naming things better:

for round, letter in enumerate(answer):
   ...

Why are we enumerating anyway?

Enumerating is for enumerating. But here, the round number doesn't actually correspond to the letter number. It should just be an external variable:

round = 0
for letter in answer:
    while attempts > 0:
        round += 1

        print 'Round {}'.format(round)
        guess = input('> ').lower()

        if guess == letter:
            print('Correct!')
            break
        elif len(guess) > 1 or guess not in answer:
            print('Invalid option')
        else:
            print('Wrong!')
            attempts -= 1


Scraping images from a webpage and saving it to a customized folder using vba I've written a script in VBA which is able to scrape images from a webpage and save it to a customized folder successfully. Firstly, it scrapes the image link then downloads the image and rename it according to it's identity. It takes 2/3 seconds to accomplish the task. I tried to do the whole thing specklessly. Here is the script I tried with:

Sub SavingImages()

Dim http As New XMLHTTP60, htmldoc As New HTMLDocument
Dim htmlas As Object, htmla As Object, html As Object
Dim stream As Object, tempArr As Variant
Dim fileSource As String

With http
    .Open "GET", "https://www.yify-torrent.org/search/1080p/", False
    .send
    htmldoc.body.innerHTML = .responseText
End With

Set htmlas = htmldoc.getElementsByClassName("movie")

For Each htmla In htmlas
    Set html = htmla.getElementsByTagName("img")(0)

    fileSource = Replace(html.src, "about", "http")
    tempArr = Split(html.src, "/")
    tempArr = tempArr(UBound(tempArr))

    With http
        .Open "GET", fileSource, False
        .send
    End With

    Set stream = CreateObject("ADODB.Stream")

    stream.Open
    stream.Type = 1
    stream.write http.responseBody
    stream.SaveToFile ("D:\Test\Images\" & tempArr & ".jpg")
    stream.Close
Next htmla
End Sub

The code is quite clear to read and understand. There are only minor things I would improve:

better variable names - tempArr, for example, is a bad name - what about:

fileSource = Replace(html.src, "about", "http")
urlParts = Split(html.src, "/")
imageFileName = urlParts(UBound(urlParts))

and then use imageFileName instead of tempArr later on. Also think of better names for htmla and htmlas.
you can use with for the stream variable as well
avoid "hardcoding" - define "D:\Test\Images\" path as a constant, or as a parameter for your function
you can also iterate over the img elements directly if you do:

Set movieImages = htmldoc.querySelectorAll(".movie img")


Remove duplicates from csv based on conditions The task is basically this:

I am given the following csv file with lots of duplicate email addresses

Display Name,First Name,Last Name,Phone Number,Email Address,Login Date,Registration Date
John,John,Doe,99999999,johndoe@yahoo.com,4/20/2015 21:56,4/20/2015 21:56
John,John,DOE,99999999,johndoe@yahoo.com,3/31/2015 14:05,3/31/2015 14:05

I need to remove duplicates based on email address with the following conditions:

The row with the latest login date must be selected.
The oldest registration date among the rows must be used.

I used Python/pandas to do this.

How do I optimize the for loop in this pandas script using groupby? I tried hard but I'm still banging my head against it.

import pandas as pd

df = pd.read_csv('pra.csv')

# first sort the data by Login Date since we always need the latest Login date first
# we're making a copy so as to keep the original data intact, while still being able to sort by datetime
df['Login Date Copy'] = pd.to_datetime(df['Login Date'])
df['Registration Date Copy'] = pd.to_datetime(df['Registration Date'])

# this way latest login date appears first for each duplicate pair
df = df.sort_values(by='Login Date Copy', ascending=False)

output_df = pd.DataFrame()

# get rows for each email address and replace registration date with the oldest one
# this can probably be optimized using groupby
for email in df['Email Address'].unique():
    subdf = df.loc[df['Email Address'] == email]
    oldest_date = subdf['Registration Date Copy'].min()

    # get corresponding registration date for a particular registration date copy
    oldest_reg_date = df[df['Registration Date Copy'] == oldest_date]['Registration Date'].values[0]

    subdf['Registration Date'] = oldest_reg_date
    output_df = output_df.append(subdf)

# drop working columns
output_df.drop(['Login Date Copy', 'Registration Date Copy'], axis=1, inplace=True)

# finally, get only the first of the duplicates and output the result
output_df = output_df.drop_duplicates(subset='Email Address', keep='first')
output_df.to_csv('~/output.csv', index=False)

Using pandas.DataFrame.groupby is pretty much the same as what you are doing using the first line in your for loop. So you can change:

for email in df['Email Address'].unique():
    subdf = df.loc[df['Email Address'] == email]
    ...

by

for email, subdf in df.groupby('Email Address'):
    ...

Now, to improve this loop:

subdf['Registration Date Copy'].min() is not what you want since it gives you the value and you are interested by its index (as you are filtering and getting values[0] on the next line). Use argmin() instead to get the index so you can directly get the desired value with subdf['Registration Date'][<variable_name_you_stored_argmin_into>]
You can modify a specific cell of a DataFrame using df.set_value(index, column, value), no need to use an intermediate dataframe here. Since you will drop everything but the firsts elements of each group, you can change only the ones at subdf.index[0].

This yield:

df = pd.read_csv('pra.csv')

# Sort the data by Login Date since we always need the latest
# Login date first. We're making a copy so as to keep the
# original data intact, while still being able to sort by datetime
df['Login Date Copy'] = pd.to_datetime(df['Login Date'])
df['Registration Date Copy'] = pd.to_datetime(df['Registration Date'])
df.sort_values(by='Login Date Copy', ascending=False, inplace=True)

# Change the registration date of the first duplicate to the oldest one
for email, group in df.groupby('Email Address'):
    oldest_index = group['Registration Date Copy'].argmin()
    oldest_registration = group['Registration Date'][oldest_index]
    df.set_value(group.index[0], 'Registration Date', oldest_registration)

# Drop working columns
df.drop(['Login Date Copy', 'Registration Date Copy'], axis=1, inplace=True)

# Finally, get only the first of the duplicates and output the result
df.drop_duplicates(subset='Email Address', keep='first', inplace=True)
df.to_csv('~/output.csv', index=False)

I also changed a bit the formating of the comments and used inplace=True instead of df = df.whatever().

Finally, you may consider wrapping that in a function so you can easily change the values of the input/output parameters:

import pandas as pd

def main(input_csv, output_csv):
    df = pd.read_csv(input_csv)

    # Sort the data by Login Date since we always need the latest
    # Login date first. We're making a copy so as to keep the
    # original data intact, while still being able to sort by datetime
    df['Login Date Copy'] = pd.to_datetime(df['Login Date'])
    df['Registration Date Copy'] = pd.to_datetime(df['Registration Date'])
    df.sort_values(by='Login Date Copy', ascending=False, inplace=True)

    # Change the registration date of the first duplicate to the oldest one
    for email, group in df.groupby('Email Address'):
        oldest_index = group['Registration Date Copy'].argmin()
        oldest_registration = group['Registration Date'][oldest_index]
        df.set_value(group.index[0], 'Registration Date', oldest_registration)

    # Drop working columns
    df.drop(['Login Date Copy', 'Registration Date Copy'], axis=1, inplace=True)

    # Finally, get only the first of the duplicates and output the result
    df.drop_duplicates(subset='Email Address', keep='first', inplace=True)
    df.to_csv(output_csv, index=False)

if __name__ == '__main__':
    main('pra.csv', '~/output.csv')


GET and POST with Flask - optimum performance I have this code to POST and save a digest to redis, and retrieve it from redis on a GET. This uses Python and Flask, of course.

@app.route("/message/<shasum>", methods=['GET'])
def get_message(shasum):
    message = redis.get(shasum)
    if message:
        return jsonify(message=message)
    else:
        abort(404)

@app.route("/digest", methods=['POST'])
def get_digest():
    if request.headers['Content-Type'] == 'application/json':
        message = json.loads(request.data)
        sha = sha256(message['message'].encode('utf-8')).hexdigest()
        redis.set(sha, message['message'])
        return (jsonify(digest=sha), 201)
    else:
        return (jsonify(message="Bad Request"), 400 )

what else can I do to improve the performance of this little piece of code, specifically on the POST side and can it be improved on the GET side too?

You can avoid the Content-Type check by using request.get_json() method.

from werkzeug.wrappers import BaseRequest
...

try:
    data = request.get_json()
except BadRequest:
    return (jsonify(message="Bad Request"), 400)

message = data['message']
digest = sha256(message.encode('utf-8')).hexdigest()
redis.set(sha, message)
return (jsonify(digest=digest), 201)

Use better variable names, message can be replaced with data and message variable can be used to store the actual message from data. Similarly sha can be replaced with digest.
The else blocks in both functions can be removed as the corresponding if blocks are already returning. I personally find lesser indented code much easier to read and it also helps in keeping the Cyclomatic complexity low.
There are no check against the data being passed by the user, for example message can be passed as an empty string, None, integer etc. You probably want to use some sort of validation here.


Read file into vector I need to read a range of bytes from a file and return it as a std::vector<std::byte>.

My current code looks like this:

#include <cstdint>
#include <fstream>
#include <string>
#include <vector>

using bytes = std::vector<std::byte>;
using namespace std;

bytes read_block(uint32_t offset,
                 uint32_t length,
                 const string& filename) {
  ifstream is(filename, ios::binary);
  is.seekg(offset);
  byte data[length];
  is.read(reinterpret_cast<char*>(data), length);
  return bytes(data, data + length);
}

But as can be seen I read it into a byte array and then create the vector by copying the bytes in the constructor. I have some issues with this implementation:

Copying of bytes when creating output variable.
Use of C style array.
Use of casts.

I have left out error handling for now to focus on the specific issues above.

Nice:

using bytes = std::vector<std::byte>;

I would call it Bytes to make it clear it is a type rather than an object.

This is not a good idea:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

This is technically not valid C++.

  byte data[length];

Variable sized arrays (VSA) are an extension to the language supported by several compilers but not actually part of the C++ standard. Though it is part of the more recent versions C standard.

Also creating a local array means that the data is being created on the local stack frame (here I assuming a Von-Neumann like architecture). This means for the data to exist after the function completes means you will need to copy it out of the function. So an alternative data object is probably a good idea.

Also there is a limitation on the size of array allowed on the stack (there are hardware limitations on the size of stack frames on lots of architecture).

So a god idea would be to use the same data structure that you return.

  Bytes data(length);   // This is basically std::vector

Now in older versions of C++ there are some nice optimizations that can be applied to std::vector when it is returned from a function that allow the compiler to build the std::vector at the final destination so that it is not actually copied on return.

In modern versions of the compiler the language has the concept of move semantics built into the language. So returning a std::vector is not an expensive operation as the internal buffer is moved (just the pointer is copied) from the function to the destination result.

Also the compiler is now required to perform copy elision of objects when it can.

Error checking

This code does zero error checking. There are several things that can go wrong and you don't check for any of them. But you said you removed this (you should have left it in).

So a better version:

#include <vector>
#include <string>
#include <fstream>
#include <cstdint>

using Bytes = std::vector<std::byte>;

Bytes read_block(std::uint32_t      offset,
                 std::uint32_t      length,
                 std::string const& filename)
{
  // Not going to explicitly check these.
  // The use of gcount() below will compensate for a failure here.
  std::ifstream is(filename, std::ios::binary);
  is.seekg(offset);

  Bytes data(length);
  is.read(reinterpret_cast<char*>(data.data()), length);

  // We have to check that reading from the stream actually worked.
  // If any of the stream operation above failed then `gcount()`
  // will return zero indicating that zero data was read from the
  // stream.
  data.resize(is.gcount());

  // Simply return the vector to allow move semantics take over.
  return data;
}


Renaming video files based on episode title lookup I'm new to F#, so I've ported an old Python script I have to F#. It scans a directory for files named something like Game.Of.Thrones.S01E02.xvidblahblah.mp4 or The_Wire_s03e04.somethingelse.avi, and renames them to a nice form like TheWire_s03e04_Hamsterdam.avi (where Hamsterdam is the episode's title, found from a web api). I'd welcome any advice on how to make it more idiomatic or efficient.

open System.Text.RegularExpressions
open System
open System.IO
open FSharp.Data

let RegexPattern pattern input =
    let m = Regex.Match(input, pattern)
    if m.Success then Some(List.tail [for g in m.Groups -> g.Value])
    else None

let (|ShortShowCode|_|) input = RegexPattern @"(.*)[sS](\d{1,2})[^\d]?[eE](\d{1,2})" input
let (|LongShowCode|_|) input = RegexPattern @"(.*)Season ?(\d{1,2}) ?Episode ?(\d{1,2})" input

let uncamelcase phrase = Regex.Replace(phrase, @"(?=[A-Z0-9])", " ")
let compressSpaces phrase = Regex.Replace(phrase, @"[^a-zA-Z0-9]+", " ")
let trim (phrase:string) = phrase.Trim()

let prettify title = title |> uncamelcase |> compressSpaces |> trim

let compress title = Regex.Replace(title, @"\W", "")

let apiUrl showName = sprintf "http://api.tvmaze.com/singlesearch/shows?q=%s&embed=episodes" (Uri.EscapeUriString(showName))
type Shows = JsonProvider<"http://api.tvmaze.com/singlesearch/shows?q=The%20Sopranos&embed=episodes">

let extension filePath = Path.GetExtension(filePath).ToLower()
let fileNameOnly filePath = Path.GetFileName(filePath)

let rec walk dir =
    seq { yield! Directory.EnumerateFiles(dir, "*")
          for subDir in Directory.EnumerateDirectories(dir) do
              yield! walk subDir }

type VideoFileInfo (showName: string, season: int, number: int, fileName: string) =
    member this.ShowName = showName
    member this.Season = season
    member this.Number = number
    member this.FileName = fileName
    member this.Extension = extension fileName

let videoInfo vidFileName =
    match vidFileName with
    | ShortShowCode [showName; season; number]
    | LongShowCode [showName; season; number]
        -> Some <| new VideoFileInfo(prettify showName, season = Int32.Parse season, number = Int32.Parse number, fileName = vidFileName)
    | _ -> None

let episodeMatch (toMatch: VideoFileInfo) (episode: Shows.Episode) =
    (episode.Season = toMatch.Season && episode.Number = toMatch.Number)

let episodeInfo (epData: VideoFileInfo) =
    let show = epData.ShowName |> apiUrl |> Shows.Load
    show.Embedded.Episodes |> Seq.find (episodeMatch epData)

let titleImprovement vidInfo =
    let epInfo = episodeInfo vidInfo
    let newName = sprintf "%s_s%02ie%02i_%s%s" (compress vidInfo.ShowName) epInfo.Season epInfo.Number (compress epInfo.Name) vidInfo.Extension
    if vidInfo.FileName = newName then None
    else Some(vidInfo.FileName, newName)

let renameVideo (dir: string) (oldTitle: string) (newTitle: string) =
    File.Move(dir + oldTitle, dir + newTitle)
    // Prints the command to reverse this rename!
    sprintf "rename \"%s%s\" \"%s%s\"" dir newTitle dir oldTitle

let printeach list = list |> Seq.iter (printfn "%A")

let testfiledir = "C:\\Dummy\\vids\\"
let testfilenames = ["The.Sopranos.Season 5 Episode 6.mp4"; "I am not an episode.txt"; "GameOfThrones_s01e02_TheKingsroad.m4v"; "TheWire.S03E04.avi"]
let createTestFiles dir filenames = filenames |> Seq.iter (fun filename -> File.WriteAllText(dir + filename, ""))

[<EntryPoint>]
let main argv =
    createTestFiles testfiledir testfilenames
    walk testfiledir
    |> Seq.map fileNameOnly
    |> Seq.choose videoInfo
    |> Seq.choose titleImprovement
    |> Seq.map (fun (oldTitle, newTitle) -> renameVideo testfiledir oldTitle newTitle)
    |> printeach
    0

Looks good; there's nothing I'd really do differently. Good structure, no overly imperative code, appropriate use of pipes (including not piping for calling a single function with a single argument, which many people do, but I think reduces readability), pattern matching, even Active Patterns. Nice use of a type provider with a web data source too.

Two very minor things:

You don't need new when calling a constructor; the compiler only recommends it when creating an instance implementing IDisposable. (I'm very happy about not needing new, because I find it terribly annoying and redundant in C#.)
I've recently started either putting if-then-else on a single line if short enough, or giving the then part its own line as well because that better represents the structure of the construct.


Plain Page Flip Effect I've created a very plain version of page flip onscroll. I made it to look like turning the pages of a book.
It looks OK on browsers I have (Firefox 52.9, Chrome 49, IE8). Though the effect doesn't work on IE8 and similar old browsers, it still doesn't look messy. I want to use this presentation method on a website.

Are there any parts of the code that I need to create a fallback for?
Could this page be faster?
Are there any accessibility concerns?
Are there any 'Best Coding Practices' that I need to include here?
Any other improvements?

New Question:

Will this effect make viewing a page uncomfortable on touch devices?

CodePen: https://codepen.io/ZechariahRaman/pen/NOYbXL/

var h = 0,
  r = 1,
  y = 0,
  yh = 1,
  section = document.getElementsByTagName("section"),
  d = section[0];

function load() {
  h = window.innerHeight;
  r = 180 / h;
  for (var i = section.length - 1; i >= 0; i--) {
    section[i].style.position = "fixed";
  }
}

function turn() {
  y = window.pageYOffset;
  if (y == 0) {
    section[0].style.transform = "initial";
  }
  if (y >= 0 && y < h) {
    yh = y;
    d = section[0];
    section[1].style.transform = "initial";
  }
  if (y >= h && y < h * 2) {
    yh = y - h;
    d = section[1];
  }
  d.style.transform =
    "perspective(" + h * 5 + "px) rotate3d(1,0,0," + r * yh + "deg)";
}
* {
  padding: 0;
  margin: 0;
  box-sizing: border-box;
}

body {
  background-color: #274171;
  height: 300vh;
}

section {
  transform-origin: top;
  transition: all 0.3s ease;
}

#p1 {
  z-index: 3;
  background-color: #d9bd75;
}

#p2 {
  z-index: 2;
  background-color: #67a893;
}

#p3 {
  z-index: 1;
  background-color: #353cc9;
}

.page {
  margin: 1vh 9vw;
  height: 98vh;
  width: 80vw;
  display: flex;
}

p {
  margin: auto;
  font-size: 5em;
  padding: 0.5em;
  color: #fff;
  background-color: rgba(19, 41, 79, 0.7);
}

Hi

Short Story

The End

Declare variables in the smallest possible scope

It's recommended to declare variables in the smallest scope where they are needed.
That way, the variable will not be visible where it should not be,
and therefore it cannot be modified accidentally by mistake.
It also makes understanding a program easier when you don't need to keep more variables in your head than necessary.

Therefore, since most of the variables are not needed in the global scope,
move those declarations to the functions that need them.

Be careful with variable scope in JavaScript.
Looking at this code,
you may be mislead that the variable i is only visible within the loop:

  for (var i = section.length - 1; i >= 0; i--) {
    section[i].style.position = "fixed";
  }

But that's not the case. The variable is visible after the loop,
and even before the loop (with value undefined).

In this particular example you want to replace the var with the let keyword,
if your version of JavaScript supports it (like any non-ancient implementation).
Using let, the variable will only be visible within the loop.

Avoid redundant variable initializations

In the posted code all variables are initialized at the time of declaration.
But if the variables don't need an initial value, then this is just noise.

Order conditional terms by their values

It can improve readability when the terms in conditionals are in a consistent order. Take for example this piece:

  if (y >= 0 && y < h) {
    // ...
  }
  if (y >= h && y < h * 2) {
    // ...
  }

The first condition means y is between 0 and h,
and the second means y is between h and h * 2.
Although this is easy enough to see,
the current writing style doesn't really help to see it.

Consider this alternative:

  if (0 <= y && y < h) {
    // ...
  }
  if (h <= y && y < h * 2) {
    // ...
  }

Now, the meaning hasn't changed, but it's much easier to see.
And with this writing style,
it's also easier to see that the two conditions will not be true at the same time,
so the second if should in fact be an else if.


correct coding structure in React.js? I'm new to react and I'm still using the life cycle methods, so my question is since componentDidMount() acts like a constructor I'm planning to initialize everything there:

render() {
        return (

                Done
                {
                    this.props.done.map((d) => (

                    <div className={`done ${this.state.isActive ? 'active' : ''}`} id={this.props.done.id}
                    onClick={(e) => {
                        this.props.setTaskID(d.id);
                        this.setToActive(e); //3.Call <-----
                        this.props.arrowStyleToDone();
                    }}>

                    ))
                }

        )
    }

    setToActive  //1.Declare <----
    componentDidMount() {
        //2.Initialize  <-----
        this.setToActive = (e) => {
            if(!e.target.classList.contains('active')) {
                e.currentTarget.classList.add('active');
                e.currentTarget.classList.add('border-done')
                this.props.closeAllTasks();
                this.props.closeAllDoing();
            } else {
                e.currentTarget.classList.remove('active');
                e.currentTarget.classList.remove('border-done')
                this.props.disableArrowButton();
            }
        }
    }
}

My idea is that if I declare everything inside the class which looks very ugly in my opinion, but then initializing everything on componentDidMount() my web app will be faster because it won't need to declare and initialize everything every render.

Is this correct? and should I put the declaring and initialization on top of render()? but the componenDidMount() is called after the initial render.

Lifecycle hooks are functions that are invoked at different stages of a component. Here constructor() and componentDidMount() works different. From my point of view, state variables are initialized inside constructor() and api calls are done in componentDidMount() hook. we are not supposed to define function definitions inside lifecycle hooks. Your above component can be converted to

class Demo extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      isActive: false
    };
  }

  setToActive = (e, id) => {
    const {
      setTaskID,
      arrowStyleToDone,
      closeAllTasks,
      closeAllDoing,
      disableArrowButton
    } = this.props;
    setTaskID(id);
    arrowStyleToDone();
    if (!e.target.classList.contains("active")) {
      e.currentTarget.classList.add("active");
      e.currentTarget.classList.add("border-done");
      closeAllTasks();
      closeAllDoing();
    } else {
      e.currentTarget.classList.remove("active");
      e.currentTarget.classList.remove("border-done");
      disableArrowButton();
    }
  };

  componentDidMount() {} // no need for this lifecycle since we are not making any initial function call, like fetch from api etc.

  render() {
    const { done } = this.props;
    const { isActive } = this.state;
    return (

        Done
        {done &&
          done.map(item => (
            <div
              className={`done ${isActive ? "active" : ""}`}
              key={item.id}
              onClick={e => this.setToActive(e, item.id)}
            >

          ))}

    );
  }
}

Basic structure of React component is

Class component

class Class_Name extends Component{
    constructor(){
      this.state = {} // state variable
      this.toggleTab = this.toggleTab.bind(this);
    }

   // other functions
   toggleTab = ()=>{} // these are example functions.

   // life cycle hooks if you are using any

   render(){
    return (

    )

   }

}


"Choose a number between 0-9" game This is a simple "Choose a number between 0-9" Game. It works perfectly but I'd like to know if there's a way to make my code smaller with the exact same functionality.

package firstGame;

    import java.util.Scanner;

    public class Player {

        static Scanner p2Name = new Scanner(System.in);
        static Scanner p1Name = new Scanner(System.in);
        static Scanner p3Name = new Scanner(System.in);

        static Scanner p1Num = new Scanner(System.in);
        static Scanner p2Num = new Scanner(System.in);
        static Scanner p3Num = new Scanner(System.in);

        String name;
        int age;
        int size;

        public static void main(String[] args) {

            // Create a random number generator (0-9)
            int number = (int) (Math.random() * 10);

            // Get Player1's name
            System.out.print("Player 1, please enter your name : ");
            String p1 = p1Name.nextLine();
            System.out.println("Welcome " + p1 + "!");

            // Get Player1's chosen number
            System.out.println("Please enter a number between 0 and 9");
            int p1Choice = p1Num.nextInt();
            System.out.println("");

            // Get Player2's name
            System.out.print("Player 2, please enter your name : ");
            String p2 = p2Name.nextLine();
            System.out.println("Welcome " + p2 + "!");

            // Get Player2's chosen number
            System.out.println("Please enter a number between 0 and 9");
            int p2Choice = p2Num.nextInt();
            System.out.println("");

            // Get Player3's name
            System.out.print("Player 3, please enter your name : ");
            String p3 = p3Name.nextLine();
            System.out.println("Welcome " + p3 + "!");

            // Get Player3's chosen number
            System.out.println("Please enter a number between 0 and 9");
            int p3Choice = p3Num.nextInt();
            System.out.println("");

            //Output generated number
            System.out.println("The number I was thinking of was : " + number);

            //if 1 player wins, output this
            if (number == p1Choice || number == p2Choice || number == p3Choice) {
                System.out.println("We have a winner!");
            }

            //Player1 won or lost?
            if (number == p1Choice) {
                System.out.println(p1 + " won!");
            } else {
                System.out.println(p1 + ", you lose!");
            }

            //Player2 won or lost?
            if (number == p2Choice) {
                System.out.println(p2 + " won!");
            } else {
                System.out.println(p2 + ", you lose!");
            }

            //Player3 won or lost?
            if (number == p3Choice) {
                System.out.println(p3 + " won!");
            } else {
                System.out.println(p3 + ", you lose!");
            }

        }

    }

static Scanner p2Name = new Scanner(System.in);
        static Scanner p1Name = new Scanner(System.in);
        static Scanner p3Name = new Scanner(System.in);

        static Scanner p1Num = new Scanner(System.in);
        static Scanner p2Num = new Scanner(System.in);
        static Scanner p3Num = new Scanner(System.in);

You don't need all those scanners.  One is plenty.

        static Scanner scanner = new Scanner(System.in);

Then replace each call using one of the other variables with scanner.

It's possible that you may have to call scanner.nextLine() after calling scanner.nextInt().  I haven't tried it.

@BKSpurgeon in Java

Here's the @BKSpurgeon solution converted to Java:

Program.java

class Program {

    public static void main(String[] args) {
        GameMaster g = new GameMaster(3);
        g.printResults();
    }

}

Player.java

class Player {

    static Scanner scanner = new Scanner(System.in);

    private String name;
    private int number;

    public Player(int playerPosition) {
        name = inputName(playerPosition);
        printName();
        number = inputNumber();
        //scanner.nextLine(); // uncomment if needed
    }

    private void printName() {
        System.out.println("Welcome "+ name + "!");
    }

    private int inputNumber() {
        System.out.println("Please enter a number between 0 and 9");

        return scanner.nextInt();
    }

    private String inputName(int playerPosition) {
        System.out.println("Player " + playerPosition + ", please enter your name");

        return scanner.nextLine();
    }

    public String calculateResult(int winningNumber) {
        return name + ((winningNumber == number) ? " won!" : ", you lose!");
    }

}

GameMaster.java

public class GameMaster {

    private final int winningNumber;
    private final List<Player> players = new ArrayList<>();

    public GameMaster(int playerCount) {
        winningNumber = chooseWinningNumber();

        for (int i = 1; i <= playerCount; i++) {
            players.add(new Player(i));
        }
    }

    public void printResults() {
        for (Player p : players) {
            System.out.println(p.calculateResult(winningNumber));
        }
    }

    private static int chooseWinningNumber() {
        return (int) (Math.random() * 10);
    }

}

I made a few tweaks to comply with Java coding conventions, but the structure is mostly the same.

I haven't tried to run it, so beware of typos, etc.


Working with sockets Here is my code;

import socket
def read_line(sock):
    "read a line from a socket"
    chars = []
    while True:
        a = sock.recv(1)
        chars.append(a)
        if a == "\n" or a == "":
            return "".join(chars)

def read_headers(sock):
    "read HTTP headers from an internet socket"
    lines = []
    while True:
        line = read_line(sock)
        if line == "\n" or line == "\r\n" or line == "":
            return "".join(lines)
        lines.append(line)

def parse_headers(headerstr):
    "Return a dict, corresponding each HTTP header"
    headers = {}
    for line in headerstr.splitlines():
        k, _, v = line.partition(":")
        headers[k] = v.strip()
    return headers

http_request = """GET / HTTP/1.1
Host: {}

"""

site = "www.google.com.tr"
conn = socket.create_connection((site,80))
conn.sendall(http_request.format(site))

headersstr = read_headers(conn)
headersdict = parse_headers(headersstr)

if "Transfer-Encoding" in headersdict and headersdict["Transfer-Encoding"] == "chunked":
    content = ""
else:
    content = conn.recv(headersdict["Content-Length"])

print content
conn.close()

Can you please comment on how I am using sockets. Is it correct practive to read characters one by one from socket as I did in read_line function? Are there any other mistakes I am doing?

Reading one byte at a time is bad idea, because it won't buffer the input. Instead, you'll probably get a system call for each byte which will be really inefficient.

Easiest solution is to use makefile

myfile = sock.makefile('r')

Then you can treat myfile like a file, including using methods like readline. That'll be more efficient then your version.

It seems a bit of waste to have readheaders() convert all the headers into one long string, only to have parseheaders() split it apart again.

if "Transfer-Encoding" in headersdict and headersdict["Transfer-Encoding"] == "chunked":

I'd use if headersdict.get('Transfer-Encoding') == 'chunked':, if the key is not present, get will return None, and you'll have the same effect as before.

Of course, for serious stuff, you'd want to use the builtin python libraries for this sort of thing. But as long as you are doing it to learn, all well and good.


Next, greater permutation of digits of a number Was looking at this question (which I initially misunderstood completely), to which Peter Taylor posted a good answer outlining a much better algorithm.

For kicks, I implemented it in Ruby, but I feel like it can be done more cleanly:

def next_greater_permutation(integer)
  digits = integer.to_s.chars.map(&:to_i)               # get digits
  k = digits.each_cons(2).to_a.rindex { |a, b| a < b }  # find k
  return integer if k.nil?                              # no k found, return the input unaltered
  head, tail = digits[0..k], digits[k+1..-1]            # split digits into two arrays
  l = tail.rindex { |n| n > head.last }                 # find l (local to the tail)
  head[k], tail[l] = tail[l], head[k]                   # swap the two values
  (head + tail.reverse).join.to_i                       # glue back together, return
end

(The k and l names are in keeping with the names used in the algorithm; usually I'd use more verbose names.)

It's not super complex or anything, but I just feel like there's a bit too much going on. I'm used to array-manipulation in Ruby being a matter of finding the right methods and chaining them, rather than fiddling with indices. Just feels like I'm missing some obvious shortcut.

To wit, the algorithm (quoted from Wikipedia) is:

  Find the largest index k such that a[k] < a[k + 1]. If no such index exists, the permutation is the last permutation.
  Find the largest index l greater than k such that a[k] < a[l].
  Swap the value of a[k] with that of a[l].
  Reverse the sequence from a[k + 1] up to and including the final element a[n].

This is a great question with deep implications imo.  And I think the correct answer is that there is no good answer.
I managed some minor cleanup which is almost not worth posting:
def next_greater_permutation(integer)
  digits = integer.to_s.chars
  k = digits.each_cons(2).to_a.rindex { |a, b| a < b }
  return integer if k.nil?

  l = digits.rindex { |n| digits[k] < n }
  digits[k], digits[l] = digits[l], digits[k]
  [digits[0..k], digits[k+1..-1].reverse].join.to_i
end

But when you say "I'm used to array-manipulation in Ruby being a matter of finding the right methods and chaining them," what you are saying is that ruby code -- and this applies to functional code in general -- looks pleasing and natural when the problem you're solving can be modeled as a series of successive data transformations.
In the case of an array, that means a series of transformations of the array which require no additional temporary variables.  When you do need temporary variables, the solution starts to get uglier, even if you try to paper over the fact by hiding your variables in the accumulator of the faux-functional method reduce (or something similar).
From this perspective, I think it becomes clear that the algorithm you're implementing is by nature procedural, and there is no way around those k and l temporary variables (certainly not k).  In fact, I suspect (but don't know) that there might even be a way to make this intuition rigorous.
Of course, you could imagine a standard library (or a monkey patched one) which provided additional array methods like swap(i,j) and reverse_after(i) that would make this solution prettier.  But again, you'd still be stuck with what is truly making you uneasy imo -- the need of k and l.
So, my non-answer to your question is that, at bottom, this question is unanswerable -- that is, minor improvements may be possible, but not a fundamental one.
UPDATE
Thinking about this more, I just wanted to elaborate on what I see as the larger problem posed by this question -- that is, what categories of problem cannot be solved cleanly with a functional pipeline.
I think it's worth pointing out that this problem can be solved with a pipeline, although probably not with the built-in ruby functional constructs.  Essentially, you need to transform the data, and then pass both the result of that transformation and the original data down the pipe, to functions of more than one argument.  In the J language, forks and hooks provide this mechanism in an elegant way.  But even then, you'd still be passing your k value down multiple steps of the pipe, and at that point you've simply implemented a local temporary variable through a pipe.  And I don't think you've gained anything in readability -- in fact, you've probably lost something.  Indeed, you can view the lines of a  procedural program as steps in a pipe each of which have access to the results of every calculation above them.
So I think the general lesson here may be something like: If you need to use the result of a calculation in multiple places in a sequence of functions, then a procedural solution is probably going to be less verbose and more readable than a purely functional one, unless you're using a language like J with special constructs for this -- and even then it might be a draw.
The only question that remains would be: Is there some clever equivalent version of this algorithm which wouldn't require the use of temporary variables or their equivalent?  I'd still guess the answer is no.


Get host ip address behind load balancer or without lb Description

Aplication type: WEB API

.NET Framework: 4.5

MVC: 5

I need caller host ip in my custom action filter and then in action. On some enviroments we have configured load balancer which stores caller ip in header "HTTP_X_FORWARDED_FOR". I nedd my code works with LB or without.

CODE

Filter

using System.Collections.Generic;
using System.Web;
using System.Web.Http.Controllers;
using System.Web.Http.Filters;

namespace FAPTest.TestApp.ActionFilters
{
    public class AccessControlFilter : ActionFilterAttribute
    {
        public override void OnActionExecuting(HttpActionContext actionContext)
        {
            string ip = HttpContext.Current.Request.UserHostAddress;

            string sourceIp = HttpContext.Current.Request.ServerVariables.Get("HTTP_X_FORWARDED_FOR");

            string ipAddress = ip;

            if (!string.IsNullOrEmpty(sourceIp))
            {
                ipAddress = sourceIp;
            }

            actionContext.Request.Properties.Add(new KeyValuePair<string, object>("ipAddress", ipAddress));
        }
    }
}

Controller

using System.Web.Http;

namespace FAPTest.TestApp.Controllers
{
    public class TestController : ApiController
    {
        [HttpGet]
        public string Test()
        {
            object o = null;
            string ipAddress = null;

            if (Request.Properties.TryGetValue("ipAddress", out o))
            {
                ipAddress = (string)o;
            }

            return ipAddress;
        }
    }
}

Filter registration:

using System.Web.Http;
using System.Web.Mvc;
using System.Web.Optimization;
using System.Web.Routing;
using FAPTest.TestApp.ActionFilters;

namespace FAPTest.TestApp
{
    public class WebApiApplication : System.Web.HttpApplication
    {
        protected void Application_Start()
        {
            AreaRegistration.RegisterAllAreas();
            GlobalConfiguration.Configure(WebApiConfig.Register);
            FilterConfig.RegisterGlobalFilters(GlobalFilters.Filters);
            RouteConfig.RegisterRoutes(RouteTable.Routes);
            BundleConfig.RegisterBundles(BundleTable.Bundles);

            GlobalConfiguration.Configuration.Filters.Add(new AccessControlFilter());
        }
    }
}

Questions

Is this code safe?

What can I improve?

Are there some pitfals?

Here is a pattern I've used successfully.  However, you need to be aware that the X-FORWARDED-FOR header may contain a comma separated list of ip addresses.  This solution also helps with protocol sniffing, because most secure sites will need to force the web browser to SSL, and if you are using SSL OFFLOADING, you'll need to use the x-forwarded-proto header to determine if a redirect is required.

        string ipAddress = string.Empty;
        string protocol = string.Empty;
        string port = string.Empty;

        if (Request.RequestContext.HttpContext.Request.Headers.GetValues("X-Forwarded-For") != null)
        {
            protocol = (Request.RequestContext.HttpContext.Request.Headers.GetValues("X-Forwarded-Proto")).FirstOrDefault();
            ipAddress = (Request.RequestContext.HttpContext.Request.Headers.GetValues("X-Forwarded-For")).FirstOrDefault();
            port = Request.RequestContext.HttpContext.Request.Headers.GetValues("X-Forwarded-Port").FirstOrDefault();
        }

        if (string.IsNullOrWhiteSpace(protocol))
        {
            protocol = (this.Request.IsSecureConnection ? "HTTPS" : "HTTP");
            ipAddress = this.Request.UserHostAddress;
        }

Example format of header value:

X-Forwarded-For: client, proxy1, proxy2

See https://en.wikipedia.org/wiki/X-Forwarded-For


Validating input values in C# I began studying C# 5.0 a few days ago and am trying to avoid duplicated code for validation of input values.

class transcript
{
  //Use lamda expression and Func for validation logic
  Func<byte, byte> validate = (grade) =>
  {
    if (grade > 100)
      throw new ArgumentOutOfRangeException("grade can`t be more than 100");
    else
      return grade;
  };

  public string name { get; set; }

  public byte kor
  {
    get { return kor; }
    set { kor = validate(value); }
  }

  public byte eng
  {
    get { return this.eng; }
    set { eng = validate(value); }
  }
}

Per the Capitalization Conventions on MSDN, types, methods, and properties should be PascalCasemeaning they start with an uppercase letter.

  Func<byte, byte> validate = (grade) =>
  {
    if (grade > 100)
      throw new ArgumentOutOfRangeException("grade can`t be more than 100");
    else
      return grade;
  };

There is no reason to use a Func here. Just use a normal method, like this

private byte Validate(byte grade)
{
    // the method body can go here
}

  public byte kor
  {
    get { return kor; }
    set { kor = validate(value); }
  }

Have you tested this code? The getter and setter both look like a StackOverflowException (via infinite recursion) to me. You need a backing field, like this

private byte kor;
public byte Kor
{
    get { return kor; }
    set { kor = Validate(value); }
}

It's uncommon to use byte instead of int, even if you know your data will fit in a byte. Only optimize for size if you know that you need to do so.


MPMC lock-free queue I'm pretty new to the world of lock-free programming and would love some feedback on this code. I have been testing the queue with multiple producers and consumers, and with no unexpected output (yet). Is there anything I could have missed or should improve?

template <typename Type, std::size_t Capacity = 100>
struct concurrent_queue
{
    static_assert(Capacity > 2, "not enough room in queue!");

    bool try_push(Type const & object) const
    {
        auto past_writer = writer.load();
        auto next_writer = increment(past_writer);

        if (next_writer != reader.load())
        {
            if (writer.compare_exchange_weak(past_writer, next_writer))
            {
                array[past_writer] = object;

                return true;
            }
        }

        return false;
    }

    bool try_push(Type & object) const
    {
        auto past_writer = writer.load();
        auto next_writer = increment(past_writer);

        if (next_writer != reader.load())
        {
            if (writer.compare_exchange_weak(past_writer, next_writer))
            {
                array[past_writer] = std::move_if_noexcept(object);

                return true;
            }
        }

        return false;
    }

    bool try_pop(Type & object) const
    {
        auto past_reader = reader.load();
        auto next_reader = increment(past_reader);

        if (past_reader != writer.load())
        {
            if (reader.compare_exchange_weak(past_reader, next_reader))
            {
                object = std::move_if_noexcept(array[past_reader]);

                return true;
            }
        }

        return false;
    }

    bool empty() const
    {
        return reader.load() == writer.load();
    }

private:

    std::size_t increment(std::size_t index) const
    {
        return (index + 1) % Capacity;
    }

    mutable std::atomic_size_t reader { 0 };
    mutable std::atomic_size_t writer { 0 };

    mutable std::array<Type, Capacity> array;
};

Welcome to Code Review. This queue implementation is not truly concurrent-ready.

Arbitrary initial capacity

template <typename Type, std::size_t Capacity = 100>

Why have you decided that the default size should be 100? Different people have different needs; having default values like this isn't a good idea because there is no true advantage to having a default size of 100.

While this is subjective, I suggest you remove that default size and just have users be required to specify the size that they want.

Compare/exchange confusion

There seems to be a misunderstanding in regards to the use of compare/exchange. The compare/exchange operations work as follows:

Let X be the atomic variable
Let E be the expected value of X
Let N be the new value of X

The operations has the following signature:

X.compare_exchange(E, N);

This roughly translates (but is not an implementation of the compare/exchange operation) to:

bool compare_exchange(E, N)
{
    if( X == E )
    {
        X = N;
        return true;
    }
    else
    {
        E = X;
        return false;
    }
}

What this means is that when the comparison fails, E is updated to the current value of X. If the comparison succeeds, X is updated with the value of N. Therefore, you don't need to update X again! That is the whole point of a compare/exchange operation; you're basically saying: "I was the last to modify X."

Issue #1: Unsynchronized access to writer

In your code, you perform a store to the atomic variable with a copy of a value you last obtained from the update performed in the compare/exchange operation, but this is not atomic.

if (next_writer != reader.load())
{
    if (writer.compare_exchange_weak(past_writer, next_writer))
    {
        // thread can be stalled here and try_pop() will access garbage memory
        // since at this point writer != reader
        array[past_writer] = object;
        return true;
    }
}

Imagine a first call to try_push() in thread A makes it inside the if (next_writer != reader.load()) statement and gets stalled just before executing array[past_writer] = object;. Now another thread B calls try_pop() and successfully gets past if (past_reader != writer.load()); the writer now has a value different than reader because of the compare/exchange operation.

However, the store of an element has not happened yet since thread A is still stalled. What is thread B going to pop? It's going to access whatever garbage memory is at that location.

Issue #2: Malfunctioning empty function

The load from reader and the load from writer followed by a comparison is not thread safe.

bool empty() const
{
    return reader.load() == writer.load();
}

Consider what happens if writer.load() returns value A, but before the expression is evaluated, writer is changed to some other value B. This could cause empty() to say that the queue is empty when it isn't or that it isn't when it actually is.

Conclusion

These are two of many issues that are currently in your code. I've decided to stop here since that's a pretty bad problem. This current implementation if not safe at all. I recommend you do some more reading and learning about thread interaction. Good luck.


List class/controller to prevent memory exhaustion In my Outlook spam add-on when I receive a new email I create a new task to process the email.

That way I don't block Outlook itself.

To prevent the List collection from growing exponentially, (traditionally Outlook is not closed for a long, long time), I have created a simple TasksController class that will help me keeping the list to a manageable size.

Then I can simply call:

TasksController.Add( ProcessNewEmail() )
...
private async Task<bool> ProcessNewEmail()
{
   /// ... do stuff
}

This is the class itself

internal class TasksController : IDisposable
{
  private static TasksController _controller;

  private static TasksController Controller => _controller ?? (_controller = new TasksController());

  private readonly object _lock = new object();
  private readonly List<Task> _tasks;

  private TasksController()
  {
    _tasks = new List<Task>();
  }

  public void Dispose()
  {
    WaitAll();
  }

  private void _Add( Task run )
  {
    //  remove all the completed tasks
    RemoveAllCompleted();

    // and add the new task
    lock (_lock)
    {
      _tasks.Add(run);
    }
  }

  private void _WaitAll()
  {
    lock (_lock)
    {
      Task.WaitAll(_tasks.ToArray());
    }
    RemoveAllCompleted();
  }

  private void RemoveAllCompleted()
  {
    lock(_lock)
    {
      _tasks.RemoveAll(t => t.IsCompleted);
    }
  }

  public static void WaitAll()
  {
    Controller._WaitAll();
  }

  public static void Add(Task run)
  {
    Controller._Add( run );
  }
}

Is deriving from IDisposable necessary?
Is the class itself thread safe?

Is the class itself thread safe

Looks fairly safe except TasksController Controller part.

Firstly, I would suggest to use .NET classes for storing concurent data instead of List<T> depending on your needs you can use ConcurrentBag<T>, ConcurrentQueue<T>.
Secondly, create lock for TasksController Controller and use it inside getter. Something like this:

private static TasksController Controller {
    get{
        lock(_controllerLock){
           if(_controller == null){ (_controller = new TasksController();}
           return _controller
        }
    }}

Is deriving from IDisposable necessary

I would say: no. All tasks should be disposed automatically when they finish.

However, there is only one but, you might to cancell your tasks when object is being disposed. That means if they do some heavy calculations which you do not need anymore after main object disposes. See this answer for information how to do that.

For more information about disposing tasks see MSDN blog post


#define or NSString * const - Benefits and drawbacks of string literal constant syntax? I'm interested in discussing string literals in Objective-C and would like to know 1. if there's a preferred paradigm / style and 2. what (if any) the performance implications might be.

There are parts of the Cocoa / CocoaTouch frameworks that use strings as identifiers. Some examples in Cocoa / CocoaTouch...

-[NSNotificationCenter addObserver:selector:name:object:]
-[UITableView dequeueReusableCellWithIdentifier:]
-[UIViewController performSegueWithIdentifier:sender:]

I find myself most often declaring a global variable within the class like so...

NSString * const kMySegueIdentifier = @"Awesome Segueueueueue";

For segue identifiers, I will often times expose the variable in the header file extern NSString * const kMySegueIdentifier; so that other modules can reuse it.

The same behaviors can be accomplished with preprocessor macros: #define kMySegueIdentifier @"Awesome Segueueueueue". I believe this would also prevent the app from consuming memory to hold these globals. I cringe a little at this syntax however because it exposes the "implementation details" of my string literal constants.

Both of these lines accomplish an end goal of abstracting the string into being easy to remember, type correctly, and will generate compile warnings / errors, is one actually better then the other? What are the situations that would arise where one would be preferred over the other?

Actually, they are completely equal (obviously sans the extern keyword on the constant define).  When literal strings are declared @"", the compiler expands them out to a compile-time constant expression, which looks familiar to us all: (static) NSString *const; -albeit with a lot more compiler magic thrown in.

Nearly the same process occurs with macros, but with one extra step: replacement.  Macros are placeholders, which the compiler replaces with the value you #define at compile-time, which is why CLANG can show you errors and warnings.

Where the difference lies is how much work the compiler has to do to replace your abstractions, not in the "memory overhead" they will incur (which means there is absolutely no speed or performance to squeeze out).  Besides, NSString* is a brilliant class cluster that's been optimized over the years, especially with literal constants, where a sort of caching occurs in the binary itself.  That way, literals used over and over again don't get reallocated over and over again.  Though, to make one thing perfectly clear: #define'd literals do NOT reduce memory overhead!


Multiple .on() events My app has a feature that when you click the new data button, HTML is loaded to the page via ajax.  Because I am using AJAX, all events I want to add has to be bound using the on() method.

There are 4 on('click') functions that are bound to 4 different HTML elements:

// save data
$('#container').on( {
    click: function() {
       // save code here
    }
}, "a.save_data" );

// cancel data
$('#container').on( {
    click: function() {
       // cancel code here
    }
}, "a.cancel_data" );

// edit data
$('#container').on( {
    click: function() {
        // edit code here
    }
}, "a.edit_data" );

// delete data
$('#container').on( {
    click: function() {
        // delete code here
    }
}, "a.delete_data" );

Each of these functions share multiple jQuery wrappers to select certain elements within the HTML:

$(this).closest('div.content_wrap');
$(this).closest('div.Content');

Please note: The reason I am using selectors such as $(this).closest() is because the user has the ability to add the same HTML multiple times on to the page.

First, is there a better way to organize all of the on() functions?  Maybe combine them into one object OR create a function?

Second, because all 4 functions use the same jQuery wrappers, is there any way to declare them in a variable globally somewhere, so i don't keep retyping them.  I wasn't able to figure out how declare variables with $(this) and apply it to more than one function.

To me, how I am doing it doesn't seem very DRY and I was wondering if someone can help me better organize this.

Here's how I'd probably do it (basically, it's the same as in the answer I linked to in the comments, I'm just adding it here for clarity)

var handlers = {
  save_data:   function (event, content, wrap) {
    // save code
  },

  cancel_data: function (event, content, wrap) {
    // cancel code
  },

  edit_data:   function (event, content, wrap) {
    // edit code
  },

  delete_date: function (event, content, wrap) {
    // delete code
  }
};

$('#container a').on('click', function (event) {
  var klass = this.className,
      content,
      wrap;

  if(typeof handlers[klass] === "function") {
    content = $(this).closest("div.Content"),
    wrap    = $(this).closest("div.content_wrap");
    handlers[klass].call(this, event, content, wrap);
  }

  // you want to always "absorb" the click event, you can
  // do so here or in the if-block above
  //   event.preventDefault();
});

You can write the handler functions just like you'd write them in your orginal code (i.e. this will refer to the right element, etc.). However, the click event handler will pass along the the content and wrap arguments, so you don't have to have code to select those elements in every handler function.

Of course, if your links have multiple classes, the className trick won't work. That's why I'd advocate (as I do in the answer I linked to) using a data-* attribute if possible.


Apply Coupon Validation in Order I have created a mvc application for booking space. In this for booking user can applied coupon for booking.

I have implemented one coupon service and validate coupon.

public class Coupon
{
    public int ID { get; set; }
    public string Name { get; set; }
    public string Description { get; set; }
    public Enum.Enum_DiscountType DiscountType { get; set; }
    public Enum_DiscountTypeAppliedOn DiscountTypeAppliedOn { get; set; }
    public decimal DiscountValue { get; set; }
    public decimal? MaximumDiscountAmount { get; set; }
    public string CouponCode { get; set; }
    public DateTimeOffset StartDate { get; set; }
    public DateTimeOffset EndDate { get; set; }
    public Enum_CouponLimitationType CouponLimitationType { get; set; }
    public int? LimitationTimes { get; set; }
    public bool Status { get; set; }
    public Enum_CouponStatus CouponStatus
    {
        get
        {
            return Status ? Enum_CouponStatus.Active: Enum_CouponStatus.InActive;
        }
    }
    public Enum_CouponAssignedTo CouponAssignedTo { get; set; }
    public string Postcodes { get; set; }
    public bool UseMaximumDiscountAmount { get; set; }
    public virtual ICollection<CouponUsageHistory> CouponUsageHistory { get; set; }
    public virtual ICollection<Order> Orders { get; set; }
}

My coupon service contains below class for validation

 public async Task<bool> IsAppliedCouponValid(Coupon coupon, string userID, Order order, int? categoryID)
    {
        if (coupon == null)
            return false;

        if (coupon.Status == false)
            return false;

        //check date range
        var now = DateTime.UtcNow;
        var startDate = coupon.StartDate;
        if (startDate.CompareTo(now) > 0)
            return false;

        var endDate = coupon.EndDate;
        if (endDate.CompareTo(now) < 0)
            return false;

        //check coupon applied limitations
        var isValid = await CheckCouponLimitations(coupon, userID);
        if (!isValid)
            return false;

        return true;
    }

CheckCouponLimitations method contains

 private async Task<bool> CheckCouponLimitations(Coupon coupon, string userID)
    {
        if (coupon == null)
            throw new ArgumentNullException("coupon");

        switch (coupon.CouponLimitationType)
        {
            case Model.Enum.Enum_CouponLimitationType.Unlimited:
                {
                    return true;
                }
            case Model.Enum.Enum_CouponLimitationType.NTimesOnly:
                {
                    var totalUsage = await GetAllCouponUsageHistory(coupon.ID);
                    return totalUsage.Count < coupon.LimitationTimes.Value;
                }
            case Model.Enum.Enum_CouponLimitationType.NTimesPerCustomer:
                {
                    //registered customer
                    var totalUsage = await GetAllCouponUsageHistory(coupon.ID, userID);
                    return totalUsage.Count < coupon.LimitationTimes.Value;
                }
            default:
                break;
        }
        return false;

    }

I have implemented like above and also calculate order after applied coupon also generate new coupon dynamically.

Please suggest me to best way to design this code and give some feedback for review code.

IsAppliedCouponValid()

Don't omit braces although they might be optional. Omitting braces can lead to hidden and therfor hard to find bugs.
The intermediate variables startDate and endDate won't buy you much. Instead I would just use the coupon.StartDate and coupon.EndDate. Don't get me wrong, using intermediate variables can be a good thing but not for such simple stuff.
A construct like

if (!bool)
{
    return false;
}
return true;

can be rewritten as return bool;
You "validate"  the passed Coupon coupon but you don't validate string userID. The method parameters Order order, int? categoryID aren't used and should be removed.
Sometimes you use if (bool == false) and sometimes you use if (!bool) where I prefer the first because it is easier to see that we check if the value is false. Whatever style you choose, you should stick to it.
The check for the coupon regarding null and coupon.Status could be added in a condition using ||.
Comments like //check date range or //check coupon applied limitations are superflous and unneeded comments because they tell the reader of the code what is done by the code. Telling what the code is doing should be done by the code itself by having meaningful names. The why something is done in the way it is done is where we need and want comments.

Applying these points will lead to

public async Task<bool> IsAppliedCouponValid(Coupon coupon, string userID)
{
    if (userID==null) { throw new ArgumentNullException(nameof(userID)); }

    if (coupon == null || coupon.Status == false)
    {
        return false;
    }

    var now = DateTime.UtcNow;

    if (coupon.StartDate.CompareTo(now) > 0)
    {
        return false;
    }

    if (coupon.EndDate.CompareTo(now) < 0)
    {
        return false;
    }

    return await CheckCouponLimitations(coupon, userID);
}


Extension method to enumerate a hierarchical object I have a domain object that I have no control over which itself contains a collection of this same type. This is a classical hierarichal representation like in a tree node hierarchy. I would like to be able to enumerate the whole hierarchy in a single call so I came up with this extension method:

public static class MyObjectExtensions
{
    public static IEnumerable<MyObject> FindAllObjectsRecursive(this MyObject myObject)
    {
        var objects = new List<MyObject>();

        objects.Add(myObject);

        foreach (MyObject innerObject in myObject.Objects)
        {
            objects.AddRange(innerObject.FindAllObjectsRecursive());
        }

        return objects;
    }
}

Unfortunately I am not really satisfied by this method because it would definitely cause issues in a large tree span scenario. So I was wondering if anyone would have a solution for that. I did think of using a parameter for the list but I was thinking there must be a better solution.

Here is how I intend to use this method:

var myObject = new MyObject();
var allObjects = myObject.FindAllObjectsRecursive();

And here is the domain object:

public class MyObject
{
    public IEnumerable<MyObject> Objects { get; set; }
}

There is a way, and it is beautiful. Instead of using recursion, you use iteration by placing the top level object on a Stack (or Queue depending on whether you want a depth-first or a breadth-first search). Then you go into a loop where you pop off the first item, check to see if it has descendants, and add them to the stack. By the time the stack/queue is empty you'll have traversed the entire tree.

You were halfway there with your decision to manage a collection that represents the entire set of objects, but the nature of the Stack/Queue allows you to access deeper and deeper properties without having to go deeper and deeper in the call stack.

Example from MSDN on how to traverse a directory tree:
http://msdn.microsoft.com/en-us/library/bb513869.aspx

Whether you choose to implement this as an extension method or not is up to you. I also strongly advise asking yourself whether you really need a collection of all descendants, or if you simply want to iterate through all descendants of a certain object? The answer to that question will determine whether the traversal belongs in what would be acting on the top-level object or whether the traversal should be abstracted away.


HackerRank ACM ICPC Team You are given a list of \$N\$ people who are attending ACM-ICPC World Finals. Each of them are either well versed in a topic or they are not. Find out the maximum number of topics a 2-person team can know. And also find out how many teams can know that maximum number of topics.

Note Suppose \$a\$, \$b\$, and \$c\$ are three different people, then \$(a,b)\$ and \$(b,c)\$ are counted as two different teams.

Input Format

The first line contains two integers, \$N\$ and \$M\$, separated by a single space, where \$N\$ represents the number of people, and \$M\$ represents the number of topics. \$N\$ lines follow.
Each line contains a binary string of length \$M\$ If the \$i\$'th line's j'th character is 1, then the i'th person knows the \$j\$'th topic; otherwise, he doesn't know the topic.

Constraints

\$2 ≤ N ≤ 500\$
\$1 ≤ M ≤ 500\$

Output Format

On the first line, print the maximum number of topics a 2-person team can know.
On the second line, print the number of 2-person teams that can know the maximum number of topics.

Imports System

Module Solution
    Public Sub Main()
        Dim input As String() = Console.Readline().Split(" ")
        Dim people As Integer = CInt(input(0))
        Dim topics As Integer = CInt(input(1))

        Dim peopleKnowledge(people-1)() As Char
        Dim maxScore As Integer = 0
        Dim teamsWithMaxScore As Integer = 0
        Dim score As Integer = 0

        For i As Integer = 0 To people-1
            peopleKnowledge(i) = Console.ReadLine().ToCharArray()
        Next i

        For i As Integer = 0 to people-2
            For j As Integer = i+1 To people-1
                score = GetTeamScore(peopleKnowledge(i),peopleKnowledge(j),topics)

                If score > maxScore Then
                    maxScore = score
                    teamsWithMaxScore = 1
                ElseIf score = maxScore Then
                    teamsWithMaxScore += 1
                End If
            Next j
        Next i

        Console.WriteLine(maxScore)
        Console.WriteLine(teamsWithMaxScore)
    End Sub

    Private Function GetTeamScore(m1 As Char(),m2 As Char(), topics As Integer) As Integer
        Dim score As Integer = 0

        For i as Integer = 0 to topics-1
            If m1(i).Equals("1"c) Or m2(i).Equals("1"c) Then
                score += 1
            End If
        Next i

        Return score
    End Function
End Module

Trying to become acquainted with VB.NET so I welcome any and all suggestions. This solution executes in 0.61s with \$N = M = 500\$; would love to know about any performance gains.

Public Sub Main()
    Dim input = Console.ReadLine().Split(" ")
    'The variable name should describe what it is more specifically, "people" could mean
    '   a List(Of Person), or be a Boolean For If there are people, etc.
    'You don't need to explicitly state the data type.  I don't, it's a matter of
    '   personal (or company) style, i.e. no consensus either way.
    'Take note that you are setting the largest possible number of people or topics here
    '   to be <= Int32.MaxValue (2,147,483,647) by casting use CInt().  This is fine
    '   because your spec says it will be <= 500.  You could also use Int16
    '   (max of 32,767).  My point is that you should be somewhat aware of the sizes of
    '   data types you are using.
    Dim peopleCount = CInt(input(0))
    Dim topicsCount = CInt(input(1))

    'Arrays are great for low level or highly structured data, but this is neither.
    '   Lists provide many benefits over an array which we'll get to in a second.
    'The data is being input to you in a text representation of 0's and 1's, but the
    '   actual concept they represent is are they nowledgeable or not which is a Boolean.
    Dim peopleKnowledge As New List(Of List(Of Boolean))
    Dim maxScore As Integer = 0
    Dim teamsWithMaxScoreCount As Integer = 0

    For i = 0 To peopleCount - 1
        peopleKnowledge.Add(New List(Of Boolean))

        Dim CharArray = Console.ReadLine().ToCharArray()
        For Each ch In CharArray
            'Nowhere in your code was any validation that the only text input on the lines
            '   was 0, 1, Or CrLf (return) which is fine if you are the only person who
            '   will ever use it, but other users won't always remember rules like that.
            Dim IsKnowledgeable = If(ch = "1", True, False)
            peopleKnowledge(i).Add(IsKnowledgeable)
        Next

        'No need for the "i" in "Next i"
    Next

    'Lists allow us to use For Each which helps the developer not have to worry about
    '   getting the numbers in the loops right.
    'By starting i at 0 and j at 2, I assume you were trying (correctly) to prevent getting
    '   team scores for a team of the same person twice.  The way you had it set up though
    '   only prevented the very first person being added to itself.  Every other person did
    '   have a team score calculated for double their score.
    For Each PersonA In peopleKnowledge
        For Each PersonB In peopleKnowledge
            'This line prevents the getting a team score for a team of the same person twice.
            'PersonA and PersonB are both *references* to an object in peopleKnowledge.
            '   This comparison compares if they are pointing at the same object, not if
            '   the contents of their lists are the same.
            If PersonA Is PersonB Then Continue For

            Dim score = GetTeamScore(PersonA, PersonB)

            If score > maxScore Then
                maxScore = score
                teamsWithMaxScoreCount = 1
            ElseIf score = maxScore Then
                teamsWithMaxScoreCount += 1
            End If
        Next
    Next

    Console.WriteLine(maxScore)
    Console.WriteLine(teamsWithMaxScoreCount)
End Sub

Private Function GetTeamScore(PersonA As List(Of Boolean), PersonB As List(Of Boolean)) As Integer
    Dim score As Integer = 0

    'Nowhere in your code was any validation that every person's topic list of 1's and 0's
    '   were equal in length which is fine if you are the only person who will ever use it,
    '   but other users won't always remember rules like that.
    For i = 0 To PersonA.Count
        If PersonA(i) Or PersonB(i) Then
            score += 1
        End If
    Next

    Return score
End Function


Getting the previous sibling of a node using Roslyn I am currently creating an analyzer using the roslyn sdk. I ran into a case where it was simpler to check child tokens for Wait()/.Result.

The code i am sharing works perfectly fine - However i was unable to find a better way on google nor through browsing the docs.

This way of getting the previous sibling of a node feels very weird - Is there a better way?

        if (context.Node is IdentifierNameSyntax identifierName)
        {
            if (identifierName.Identifier.Text == TaskIdentifierWait || identifierName.Identifier.Text == TaskIdentifierResult)
            {
                var siblingsAndSelf = identifierName.Parent.ChildNodes().ToImmutableArray();
                var selfIndex = siblingsAndSelf.LastIndexOf(identifierName);
                if (selfIndex <= 0)
                    return;

                var previousSibling = siblingsAndSelf[selfIndex-1];

You could iterate over the ChildNodes only once with the Zip extension where the first collection starts at the first element and the other one at the second element with Skip(1). This will produce tuples with pairs such as Previous & Current. Now you can filter it and pick at the same time the Last element that satisfies the condition:

var childNodes = new[] { "a", "a", "b", "c", "a" };

var previousSibling =
    childNodes
        .Zip(childNodes.Skip(1), (p, c) => (Previous: p, Current: c))
        .Last(t => t.Current == "c").Previous; // b


Using jQuery string replacements to make labels bold Instead of having multiple statements like the following, Is it possible to club them into one liner in jQuery?

       var replaced = $("#myContents").html().replace(/Employee:/g,'Employee:');
       $("#myContents").html(replaced);

       var replaced = $("#myContents").html().replace(/SSN:/g,'SSN:');
       $("#myContents").html(replaced);

       var replaced = $("#myContents").html().replace(/DOB:/g,'DOB:');
       $("#myContents").html(replaced);

       var replaced = $("#myContents").html().replace(/Start Date\/Time:/g,'Start Date\/Time:');
       $("#myContents").html(replaced);

       var replaced = $("#myContents").html().replace(/End Date\/Time:/g,'End Date\/Time:');
       $("#myContents").html(replaced);

I think yes, the ugliest I think is the follow:

// Define one regexp with all the piece you need and save
// the match with ()
var re = /(Employee|SSN|DOB|Start Date\/Time|End Date\/Time):/g

// Then change the replace as follow
var replaced = $("#myContents").html().replace(re, '<b>$1:</b>');

$("#myContents").html(replaced);

Of course you can compose your regexp as chunks as follow:

var terms = [];

...
terms.push("Employee");
...
terms.push("End Date\/Time");

...

var re = new RegExp("("+terms.join("|")+"):", 'g');

...


Structured instructions for simple extraction script I have a rather simple task in trying to extract keywords from input.dat which looks like:

func1
{
        yes         true;
        keyword123  (1.1 0 -0.3);
        gamma       (0 1 0);
        dir         (1 0 0);
func2
{
        yes         false;
        keyword123  (1.1 0 -0.3);
        gamma       (0 0 1);
        dir         (1 0 0);

In this case I want to get the first entry in the vector of keyword123. If the lines containing the keyword aren't identically, the script should stop with an error. I feel that this should be a one- or twoliner in PERL using RegEx but it took me some time to get the following script:

#!/usr/bin/perl -w

use strict;

my $path = "./input.dat";

# array @grepped of lines containing keyword
my @grepped=();
open(FILE,"$path") or die "error on opening $path: $!\n";
while (my $line = <FILE>) {
        if ($line=~ /keyword123/i){push(@grepped,"$line")};
}
close(FILE);

# get array @unique of unique lines
my %seen;
my @unique = grep {!$seen{$_}++} @grepped;

# continue with $unique if keyword-line is unique
my $unique;
if (scalar @unique eq 1) {
        $unique=$unique[0];
} else {
        die "entry not unique, exiting: $!\n";
}

# get string between brackets
$unique =~ /\((.+?)\)/;
my $final = $1;

# cut after first space
$final =~ s/\s.*//s;
print "keyword123: $final\n";

The script is giving the correct output but this can't be the best work, so any help/hint would be highly appreciated.

This is how I would implement it:

#!/usr/bin/perl
use warnings;
use strict;

my $file = 'input.dat';
open my $IN, '<', $file or die $!;

my $uniq;
while (my $line = <$IN>) {
    next unless $line =~ /^\s*keyword123/;
    if (defined $uniq and $line ne $uniq) {
        die "Entry not unique. Exiting.\n";
    }
    $uniq = $line;
}
$uniq =~ s/.*\((.*)\).*/$1/;
$uniq =~ s/\s.*//;
print "keyword123: $uniq";

You can check the uniqueness each time you read the keyword line in. Once you find a non-unique line, you can stop right at the moment, no need to search the rest of the file.


Most performant way to check if elements are overlapping I know you can calculate if an element is overlapping another by comparing the top|bottom|right|left properties within the getBoundingClientRect method. However, you need to loop through the elements to do so. I'm curious to know what is the performant way of checking to see if an element is overlapping any other element. For example, checking to see if label one is overlaying any other element. Then checking to see if label two is overlapping any other element including label one without using a loop each time, or is this the only way? Currently I'm just checking one element against the previous.

// dismiss -- just creating less CSS
const randomColor = "#" + ((1 << 24) * Math.random() | 0).toString(16);
document.querySelectorAll('.label').forEach((x, i) => {
  document.querySelectorAll('.label')[i].style.background = "#" + ((1 << 24) * Math.random() | 0).toString(16);
});

//function to check for overlapping
function overlayCheck() {
  let points = document.querySelectorAll('.label');
  let rightPos = (elem) => elem.getBoundingClientRect().right;
  let leftPos = (elem) => elem.getBoundingClientRect().left;
  let topPos = (elem) => elem.getBoundingClientRect().top;
  let btmPos = (elem) => elem.getBoundingClientRect().bottom;

  for (let i = 1; i < points.length; i++) {
    if (!(
        rightPos(points[i]) < leftPos(points[i - 1]) ||
        leftPos(points[i]) > rightPos(points[i - 1]) ||
        btmPos(points[i]) < topPos(points[i - 1]) ||
        topPos(points[i]) > btmPos(points[i - 1])
      )) {
      points[i].innerHTML = `${points[i].innerHTML} C`;
    }
    if (i === points.length - 1) {
      return
    }
  }
}

overlayCheck();
.parent {
  position: relative;
  display: flex;
}

.label {
  height: 75px;
  left: 0;
  width: 100px;
  margin-left: 10px;
  background-color: orange
}

.cover {
  width: 220px;
  position: absolute;
  left: 0;
  height: 50px;
  bottom: 0;
}

Label 1

Label 2

Label 3

Label 4

Label 5

Label 6

I figured out a way thru trial and error. I'll post this here for anyone else who could use it.

// dismiss -- just creating less CSS
const randomColor = "#" + ((1 << 24) * Math.random() | 0).toString(16);
document.querySelectorAll('.label').forEach((x, i) => {
  document.querySelectorAll('.label')[i].style.background = "#" + ((1 << 24) * Math.random() | 0).toString(16);
});

//function to check for overlapping
function overlayCheck() {
  let points = document.querySelectorAll('.label');
  let rightPos = (elem) => elem.getBoundingClientRect().right;
  let leftPos = (elem) => elem.getBoundingClientRect().left;
  let topPos = (elem) => elem.getBoundingClientRect().top;
  let btmPos = (elem) => elem.getBoundingClientRect().bottom;

  for (let i = 0; i < points.length; i++) {
    for (let j = 0; j < points.length; j++) {
      let isOverlapping = !(
        rightPos(points[i]) < leftPos(points[j]) ||
        leftPos(points[i]) > rightPos(points[j]) ||
        btmPos(points[i]) < topPos(points[j]) ||
        topPos(points[i]) > btmPos(points[j])
      );

      if (isOverlapping && j !== i) {
        points[i].innerHTML = `${points[i].innerHTML} C`;
      }
    }
  }
}
overlayCheck();
.parent {
  position: relative;
  display: flex;
}

.label {
  height: 75px;
  left: 0;
  width: 100px;
  margin-left: 10px;
  background-color: orange
}

.cover {
  width: 220px;
  position: absolute;
  left: 0;
  height: 50px;
  bottom: 0;
}

Label 1

Label 2

Label 3

Label 4

Label 5

Label 6


Demographics information for EDI report I am going through some of my companies EDI reports to see if I can make them faster.  The query below runs in less than one second in our AWS RDS environment, but it takes ~55 seconds on a blade in the office, so there is room for improvement.  I do not see any common performance culprits based upon Google searches, but I am not an expert.

When I say, "EDI", I am expressing that the purpose of this query is to transfer data from one computer system to another. For instance, that is why I am converting all of the dates to strings, because that is what the target system wants (in "mm/dd/yyyy" format).

Some background info:

There are about 11,000 rows in each of the two tables
Two of tables are de-normalized "Snapshot" tables
The query returns 3500 records
The two Snapshot tables have non-unique, non-clustered indexes
The third table (CaseOpenEnrollmentPeriod) has a unique clustered index, although it is only four records long.

DECLARE @MostRecentSnapshotTime datetime
SET @MostRecentSnapshotTime = (SELECT MAX(SnapshotLoadStartTime) AS Expr1
                       FROM   Snapshot.SnapshotLog)

SELECT
   SPE.SSN
  ,SPE.SSN
  ,CONVERT(NVARCHAR(10), SPE.BirthDate, 101)
  ,SPE.LastName
  ,SPE.FirstName
  ,LEFT(SPE.MiddleName, 1)
  ,SPE.Address1
  ,SPE.Address2
  ,SPE.City
  ,SPE.StateCode
  ,LEFT(SPE.ZipCode, 5)
  ,'SampleText'
  ,'SampleText'
  ,CASE WHEN SPE.MaritalStatusCode = 'SampleText' THEN 'SampleText' END
  ,SPE.Gender
  ,SPE.[Status]
  ,CASE
    WHEN SEE.PlanCode = 'MED' THEN 'MED 16'
    WHEN SEE.PlanCode = 'MEDP' THEN 'MEDPlus 16'
    WHEN SEE.PlanCode = 'MEDH' THEN 'MEDHeavy 16'
    WHEN SEE.PlanCode = 'MEDHP' THEN 'MEDHeavyPlus 16'
    WHEN SEE.PlanCode = 'MVP' THEN 'MVP 16'
    ELSE NULL
   END
  ,'4'
  ,CASE
    WHEN SEE.TierCode =  'EO' THEN 'E'
    WHEN SEE.TierCode =  'ESP' THEN 'ES'
    WHEN SEE.TierCode =  'ECH' THEN 'EC'
    WHEN SEE.TierCode =  'EFam' THEN 'F'
    ELSE NULL
   END
  ,'4'
  ,SPE.DepartmentCode
  ,CONVERT(NVARCHAR(10), SPE.HireDate,                  101)
  ,CONVERT(NVARCHAR(10), COEP.BenefitsEffectiveDate,    101)
  ,CONVERT(NVARCHAR(10), SEE.EffectiveDate,             101)
  ,CONVERT(NVARCHAR(10), SEE.StopDate,                  101)
  ,CASE WHEN NULLIF(LTRIM(SPE.TerminationDate), '') IS NOT NULL AND SEE.LifeEventActionID IS NOT NULL THEN CONVERT(VARCHAR(4), SEE.LifeEventActionID)
        WHEN NULLIF(LTRIM(SPE.TerminationDate), '') IS NOT NULL AND SEE.LifeEventActionID IS NULL THEN 'AI'
        ELSE ''
   END

  ,SEE.EnrollerID
  ,'1'
  ,(SEE.IssCost * 12 / SPE.PayCycle)
  ,CASE WHEN SPE.PayCycle = 52 THEN 'Weekly'
        WHEN SPE.PayCycle = 12 THEN 'Monthly'
        WHEN SPE.PayCycle = 24 THEN 'Semi-Monthly'
        ELSE 'Other'
   END
  ,SPE.WorkPhone
  ,SPE.HomePhone
  ,SPE.Email
  ,ISNULL(SPE.HeightInInches, '')
  ,ISNULL(SPE.WeightInPounds, '')

FROM Snapshot.EmployeeElection SEE
  JOIN Snapshot.PersonEmployee SPE
    ON  SEE.EmployeeID = SPE.AssignedID AND
        SEE.ConfirmationID = SPE.ConfirmationID AND
        SEE.CaseOpenEnrollmentPeriodID = SPE.CaseOpenEnrollmentPeriodID AND
        SEE.LoadDateTime = SPE.LoadDateTime
  JOIN CaseOpenEnrollmentPeriod COEP
    ON COEP.CaseOpenEnrollmentPeriod_ID = SEE.CaseOpenEnrollmentPeriodID
WHERE SEE.LoadDateTime = @MostRecentSnapshotTime
  AND PlanID <> 8
  AND SPE.ConfirmationID > 0;

Here is the XML execution plan. I am new to execution plans, so I am not sure if this is the info you need or not.

Aliases
The alias names SEE, SPE and COEP are not descriptive. It's often tempting to make acronyms for tables and use those as aliases. It's all well and good for people who are already familiar with the structure of the database, but when you show this code to an "outsider" those aliases become a bit of an obstacle in understanding the code.
I think something like this would read a lot better:
FROM Snapshot.EmployeeElection AS EmpElecs --was SEE
  JOIN Snapshot.PersonEmployee AS Emps --was SPE
    ...
  JOIN CaseOpenEnrollmentPeriod AS Periods --was COEP

Execution plan
An update after you posted the execution plan.
See that RID Lookup (Heap) operation on SnapshotPersonEMployee? That to me indicates that the SQK engine could not use the index for that query, see Identifying Key and RID Lookup Issues and How to Resolve by Aaron Bertrand on the DBA.SE site. Quoting him:

These lookups occur when an index does not satisfy the query (non-covered query) and therefore additional data needs to be retrieved from the clustered index or the heap. Non-covered queries can be a problem because, for every row in the index, the additional column(s) must then be fetched; this can have a significant impact on large data sets and impact overall performance.

If hard to pinpoint the exact problem, I would suggest starting by looking at the columns in this join and see if they are indexed, if not that might be your problem right there. If not all those columns are indexed then it likely has to go lookup in the table itself as hinted by your execution plan.
  JOIN Snapshot.PersonEmployee SPE
    ON  SEE.EmployeeID = SPE.AssignedID AND
        SEE.ConfirmationID = SPE.ConfirmationID AND
        SEE.CaseOpenEnrollmentPeriodID = SPE.CaseOpenEnrollmentPeriodID AND
        SEE.LoadDateTime = SPE.LoadDateTime

You could either look into adding indexes, which will speed up performance for all queries that use them, use perhaps move the non-indexed columns to an HAVING clause at the end, which might move these matches to the result set instead of the initial lookup. (maybe).

Date conversions
You've mentioned that the code is meant for an EDI which accepts dates as string (in mm/dd/yyyy format), I have to deal with things like that pretty frequently, and chances are you probably cannot change the EDI itself (or not without significant effort/expense, at least) so that's understandable.
I was going to suggest casting the dates instead of converting, but then I realized that casting does not allow to pick the output format, it would come out as yyyy-mm-dd which is default SQL Server/T-SQL date format. Unfortunately, that's probably one of the most expensive set of operations in your query. There may not actually be a way to improve these, given the limitations.
Date-Time handling can be annoying, and more so when you have to convert between actual date-time values and string representations of them.


Calculating the percentage on Python 2.7 An attack with magnitude M will be successful with a chance of (100-M)%. That is, higher magnitude means higher risk of missing it. For instance, if M is 30, the chance of succeeding would be 70%, whereas, if M is 10, the chance of succeeding would be 90%. If an attack with magnitude M is successful, the attacked hero's health points will decrease by M. If the user writes more than
50 or less than 1, the game should warn the player, and re-ask for another attack magnitude as below.

import random

# ---

char1 = raw_input("Player One: ")

while char1 == "":
    char1 = raw_input("Please enter your name: ")

char2 = raw_input("Player two: ")
while char2 == char1:
    char2 = raw_input(char1 + " name is taken, please choose another name: ")
while char2 == "":
    char2 = raw_input("Please enter your name: ")
print char1, "and", char2, "welcome to the game."

# ---

health1 = 50
health2 = 50

print char1, '|' * health1
print char2, '|' * health2

toss = random.randint(0, 1)

if toss == 0:
    print char1, "will start the game"
else:
    print char2, "will start the game"

# ---

while health1 > 0 and health2 > 0:
    if toss == 0:
        n = input(char1 + " select n force: ")
        health2 -= n
        print char2, '|' * health2 + char1,'|' * health1
        toss = 1 # change player
    else:
        n = input(char2 + " select n force: ")
        health1 -= n
        print char1, '|' * health1 + char2,'|' * health2
        toss = 0 # change player

# ---

if health1 > 0:
    print char2, 'wins'
else:
    print char1, 'wins'

I take it from your code the game works like this:

Each player enters their names. The names can't be blank nor the same with each other.
A coin is tossed to determine the turn order.
The turn player decides the amount of damage (M) he wants to deal against the other player. The chance of dealing damage is calculated by (100 - M).

With that, I have this code:

player1, player2 = input_player_names()
print("Welcome to the game {} and {}".format(player1, player2))
print("Tossing coin... Heads for {}; tails for {}".format(player1, player2))
turn_order = determine_turn_order([player1, player2])
print(turn_order[0], "plays first")
play(turn_order)

And the respective implementations:

def input_player_names():
    player1 = ""
    player2 = ""
    while player1 == "":
        player1 = input("Player 1, please enter your name.")
    while player2 == "" or player2 == player1:
        player2 = input("Player 2, please enter your name.")
    return player1, player2

def determine_turn_order(player_list):
    turn_order = []
    toss = random.randint(0, 1)
    turn_order.append(player_list[toss])
    turn_order.append(player_list[toss + 1 % 2])
    return turn_order

def play(turn_order):
    player_health_points_list = [50, 50]
    turn_number = 0
    while there_are_no_winners_yet(player_health_points_list):
        turn_player = turn_order[turn_number % 2]
        player_is_sure = False
        chance = 100
        damage_to_deal = 0
        while not player_is_sure:
            damage_to_deal = input(turn_player + " enter damage you want to deal: ")
            damage_to_deal = int(damage_to_deal)
            chance = chance - damage_to_deal
            if chance < 50:
                player_is_sure = bool(input("You have only a {}% chance of dealing damage. Are you sure you want to push through? Input 1 for yes; 0  for no.".format(chance)))
            else:
                player_is_sure = True

        # Here's how I'd calculate determine whether the attack hits or not
        if chance > random.randint(1, 100):
            player_health_points_list[(turn_number + 1) % 2] -= damage_to_deal
        turn_number = turn_number + 1
        print(turn_order[0], '|' * player_health_points_list[0])
        print(turn_order[1], '|' * player_health_points_list[1])
    declare_winner(turn_order, player_health_points_list)

def there_are_no_winners_yet(player_health_points_list):
    return player_health_points_list[0] > 0 and player_health_points_list[1] > 0

def declare_winner(turn_order, player_health_points_list):
    print("The winner is {}".format(turn_order[0] if player_health_points_list[0] > player_health_points_list[1] else turn_order[1]))


TCPListener server to discover clients on a network I am currently writing a program to sync files (music, photos, etc) from my PC to an Android device. In order to do this, I have 2 application: one that is running on my PC, and one that shall be running on the Android device. When the Android app will open, it will try to connect to a specified IP:port on the PC application, which has a server just listening for requests in order to show the clients in a WPF list style view. This server is written in F#.

Here is the server implementation I have come up with:

type TCPListenerServer(discoveryPort:int) =
    let server = new TcpListener (IPAddress.Loopback, discoveryPort)

    let activeConnections = new List<TcpClient>()

    let serverLoop () =
        let rec loop (pendingConnection:Task<TcpClient>) = async {
            let newPendingConnection, client =
                match pendingConnection.Status with
                | TaskStatus.Created | TaskStatus.WaitingForActivation | TaskStatus.WaitingToRun | TaskStatus.Running  ->
                    (None, None)
                | TaskStatus.Faulted | TaskStatus.Canceled ->
                    raise (new System.NotImplementedException())
                | TaskStatus.RanToCompletion ->
                    let connectionTask = server.AcceptTcpClientAsync ()
                    (Some connectionTask, Some pendingConnection.Result)
                | _ ->
                    raise (new System.NotImplementedException())

            // Add the new client to the list
            Option.iter (fun c -> activeConnections.Add c) client

            // Switch the new pending connection if there is one
            let connectionAttempt = defaultArg newPendingConnection pendingConnection

            Async.Sleep 1000 |> Async.RunSynchronously
            return! loop connectionAttempt
        }

        try
            server.Start ()

            let connectionTask = server.AcceptTcpClientAsync ()

            loop connectionTask
        finally
            server.Stop ()

    member x.Start () =
        serverLoop () |> Async.Start

    member x.Stop () =
        server.Stop ()

    member x.ActiveConnections =
        activeConnections

I wanted to know if my implementation was acceptable. I am unsure what is best: using a .NET List, or using a mutable variable that holds an F# list for activeConnections.

Also, usually I would keep the state of my object encapsulated in the serverLoop and just passing the updated list to each iteration, but doing this I cannot expose it so that I may have access to it from the outside (to bind on it in my WPF for example). Is there a better way to do this?

I would consider altering member x.ActiveConnections for the reasons I mention on the answer here: Create database connection and run the insert, delete, update queries class

Basically, consumers of your class can actually modify the ActiveConnections list (call .Remove, etc.), so you should find a way to eliminate that.

In F#, I would probably do the following:

member x.ActiveConnections = activeConnections.ToArray()

Or as Dan Lyons stated in comments:

member x.ActiveConnections = activeConnections.AsReadOnly()

This guarantees that you are exposing a non-modifiable copy of the list to the consumer of your class. You could optionally open System.Linq and use:

member x.ActiveConnections = activeConnections.ToArray().AsEnumerable()

Both of these have the same effect: the internal List<T> isn't able to be modified.

I also noticed you use x. as the prefix for all your member declarations, I'm not sure what the standard is (quick google pulled nothing) but I always try to use member this.Name for public-facing members, to remind myself that they're part of the same object. (F# doesn't seem to care much, but it still feels right.)