File existence promise I hate to have to write a custom exists promise every time I need to know if a file exists, so I came up with this. Thoughts?

var Promise = require("bluebird")
var fs = Promise.promisifyAll(require("fs"))

function fsExists(_path){
  return fs.lstatAsync(_path)
  .then(function(){ return true })
  .catch(function(){ return false })
}

module.exports = fsExists

I think the catch and then parts are unnecessary. A then accepts a resolve and reject handler, which you can treat a boolean.

var Promise = require("bluebird")
var fs = Promise.promisifyAll(require("fs"))

module.exports = fs.lstatAsync

Usage would simply be

var stat = require('yourFsStatModule');

stat('path/to/file').then(function(){
  // it exists
}, function(){
  // it doesn't exist
});


Is my error handling feasible when dealing items not found in an array? internal void ShiftSwapInstrument(SwapCurve shiftedCurve, string swapToShift)
 {
    if (SwapExists(shiftedCurve, swapToShift))
    {
       var rateToShift = shiftedCurve.Swaps.Single(r => r.Description() == swapToShift);
       rateToShift.Rate = rateToShift.Rate + 1 / 10000.0;
       return;
    }
    throw new PdhSwapRateNotFoundException(swapToShift);
 }

 private bool SwapExists(SwapCurve shiftedCurve, string swapToShift)
 {
    var swap = Array.Find(shiftedCurve.Swaps, r => r.Description() == swapToShift);
    return swap == null ? false : true;
 }

The above code is what I've written to handle processing when an item is not found in an array. The exception should 'never' be thrown - it's truly exceptional if it does happen - thus I can use Single without reservation within the if-block.

My question is whether my exception handling is correct/feasible when items are not found in my array and is it correct for me to return; when I do?

Secondary question

I noted the following about Array.Find on MSDN

  Return Value

  Type: T The first element that matches the conditions defined by the
  specified predicate, if found; otherwise, the default value for type
  T.

Am I correct in my understanding, that since SwapCurve & Swaps are reference types the default value will be null?

Your error handling looks correct to me as long as your application is catching PdhSwapRateNotFoundException at some point and logging appropriate warning/error to notify user about this.

A slight cosmetic modification to your code could be like:

{
    if (!SwapExists(shiftedCurve, swapToShift))
        throw new PdhSwapRateNotFoundException(swapToShift);

    var rateToShift = shiftedCurve.Swaps.Single(r => r.Description() == swapToShift);
    rateToShift.Rate = rateToShift.Rate + 1 / 10000.0;
    return; // you don't actually need this.
}

And you're correct default value of reference type is null.

Edit:
I assume shiftedCurve.Swaps will not be null, otherwise Find will throw an exception. And if you think it could be the case then you may want to update your code to check for shiftedCurve.Swaps == null and handle it appropriately.


Asynchronous wrapper for database connection I'm using a database that has a library for access to it. Connection is done synchronously, it's blocking and I do not have a way to abort it. To connect, I have to write:

var _loader = new DatabaseDriverLoader();
DatabaseDriver _driver = _loader.Connect(connectionString, login, password);

I wanted to improve it:

Be able to connect asynchronously (not waiting for actual connection)
Be able to cancel the connection if I need to

Here's a wrapper that I created:

public class DatabaseConnection : IDisposable
{
    private DatabaseDriver _driver;
    private DatabaseDriverLoader _loader

    public event EventHandler ConnectionFailed;
    public event EventHandler ConnectionEstablished;
    public event EventHandler ConnectionLost;

    public async Task ConnectAsync(string connectionString, string login, string password, CancellationToken cancellationToken)
    {
        var connectionTask = Task.Run(() =>
        {
            try
            {
                var _loader = new DatabaseDriverLoader();
                DatabaseDriver _driver = _loader.Connect(connectionString, login, password);
            }
            catch (Exception e) { }
        });

        await Task.Run(async () =>
        {
            while(!connectionTask.IsCompleted && !cancellationToken.IsCancellationRequested)
            {
                await Task.Delay(100);
            }
        });

        cancellationToken.ThrowIfCancellationRequested();

        if (_driver == null)
        {
            ConnectionFailed?.Invoke(this, EventArgs.Empty);
            return;
        }
        ConnectionEstablished?.Invoke(this, EventArgs.Empty);

        _driver.ConnectionToServerLost += OnConnectionLost;
    }

    public void Dispose()
    {
        if (_driver != null)
        {
            _driver.ConnectionToServerLost -= OnConnectionLost;
            _driver.Dispose();
        }
        _loader?.Dispose();
    }

    public DatabaseDriver GetDriver()
    {
        return _driver;
    }

    private void OnConnectionLost(object sender, EventArgs e) => ConnectionLost?.Invoke(sender, e);
}

Here's how I would use that class:

private async Task ConnectWithDatabase()
 {
  //Delete old connection instance
  _dbConnectionTokenSource?.Cancel();
  if (_dbConnection != null)
  {
      _dbConnection.ConnectionEstablished -= OnDbConnectionEstablished;
      _dbConnection.ConnectionLost -= OnDbConnectionLost;
      _dbConnection.ConnectionFailed -= OnDbConnectionFailed;

      _dbConnection.Dispose();
  }

//Create new connection instance
  _dbConnection = new DatabaseConnection();
  _dbConnection.ConnectionEstablished += OnDbConnectionEstablished;
  _dbConnection.ConnectionLost += OnDbConnectionLost;
  _dbConnection.ConnectionFailed += OnDbConnectionFailed;

  _dbConnectionTokenSource = new CancellationTokenSource();
  var token = _dbConnectionTokenSource.Token;

  try
  {
      await _dbConnection.ConnectAsync(
          ConnectionString,
          Login,
          Password,
          token)
      .ConfigureAwait(false);
  }
  catch (TaskCanceledException e)
  { }
 }

I wanted to ask you kindly for review of this code, because I'm not really sure if this is done correctly.
I don't know if it is a good idea to use an event for successful connection. Maybe Connect method should return successful Task when connection occurs?

I'm not sure if cancellation is done correctly. In my use case there might be situations where DatabaseConection is canceled and disposed before connection occurs successfully.

I am not sure if it's a good idea to create new threads in DatabaseConnection, because clients of this class might not want it? - at least I read somewhere that it's not a good design, but I'm not sure why.

I will be happy to hear any opinion, always good to learn something new.

I would first start by saying that such a class is generally regarded as a bad practice because it doesn't achieve what's supposed to do, or simply there are better ways of doing it.

But let's go to the actual code, in the ConnectAsync method.

var connectionTask = Task.Run(() =>
{
    try
    {
        var _loader = new DatabaseDriverLoader();
        DatabaseDriver _driver = _loader.Connect(connectionString, login, password);
    }
    catch (Exception e) { }
});

Here you delegate the connection to the main class and initialize the _driver field. The first problem is that, since it's wrapped in a Task, it uses a separate thread and that new thread is the blocked one instead of the calling one. In a desktop program, it can be  a good thing (since it keeps the UI thread free to handle the UI), but in a web site you free the original thread and stall another one, which is what you want to avoid with an async method.

Even worse, the empty catch block will swallow any exceptions, so you won't get any clue why a connection fails if that happens.

The next part also has its problems:

await Task.Run(async () =>
{
    while(!connectionTask.IsCompleted && !cancellationToken.IsCancellationRequested)
    {
        await Task.Delay(100);
    }
});

This employs a second thread and loops until the first one finishes or the cancellation is requested. While it sounds neat in principle, it actually ends up using double the resources it should for the ability to "cancel" the connection (more on that latter), which is far suboptimal. Callers expect NO threads consumed when you see an async DB connection, but instead this code stalls 2 of them.

What to do instead?

In short, leave the decision to the caller! This class introduced some side effects that callers normally don't expect. In web pages, it consumes far more resources than necesary, and in desktop programs it can be simply invoked via Task.Run method directly, instead of wrapping in a class, if keeping the UI responsive is desired. This removes the need for this class, using the provided driver directly.

About your other points

  Be able to connect asynchronously (not waiting for actual connection)

It clearly does that, but I would simply use the first chunk of code (awaiting for it) directly, and remove the need for this, because of unwanted side effects.

  Be able to cancel the connection if I need to

Here is the bigger catch. It does NOT cancel the connection at all!  When the cancellation token is triggered, the loop is interrupted, but the first thread is left running, and its result is ignored. This means that if the connection succeeds, you have a floating connection that won't be disposed of properly, and the involved resources will still be consumed. The only practical effect of cancellation is the early return.

  Maybe Connect method should return successful Task when connection occurs?

Yes, that's a good practice. I don't see a particular use for an event here (not that it's a bad thing to have one, but it serves little purpose). Most important, in its current form, the code does return a task and completes it on sucessful connection, and that's what you use in your "demo" code in fact. That's a good thing.


Validating that a Java program has two equal-length arguments I'm writing a small almost one method Java class for a job application and the advice given was to write it as though it was a piece of commercial software.

The processResult method needs two string arguments of equal length. I thought the best thing to do is check that the args has at least 2 items, then inside the processResult method check that the strings have the same length.

I am unsure about throwing a generic exception in my main method and then later using the try/catch statement. Is this a good practice to do? Would it be better practice to handle these as part of one try catch? Are custom Exceptions a good way to go for this type of problem?

Here is what I have at the moment.

public class Task {
    public static void main(String[] args) throws Exception {
        if (args.length < 2) {
            throw new Exception("Need 2 string arguments");
        }

        try {
            int result = processResult(args[0], args[1]);
            System.out.println(result);
        } catch (IllegalArgumentException e) {
            System.err.println(e.getMessage());
        }
    }

    private static boolean lengthsEqual(String a, String b) {
        return Integer.compare(a.length(), b.length()) == 0;
    }

    private static int processResult(String inputOne, String inputTwo) {
        if (!lengthsEqual(inputOne, inputTwo) {
            throw new IllegalArgumentException("Strings must be the same length");
        }

        // method logic

        return 0;
    }

}

I took your code and rewrote it a bit, so that it satisfies my personal requirements for production-ready code.

if (args.length < 2) {
    System.err.println("usage: Task <arg1> <arg2>");
    System.exit(1);
}

I removed the throw new Exception since passing the wrong number of arguments is not a programming error but a wrong invocation of the program. Only programming errors should print a stack trace. Instead of saying "Need 2 string arguments" I am following the "usage:" pattern that has been successfully established by Unix programs. Since you didn't provide any context in your question, the best variable names I could come up with are <arg1> <arg2>. If your task were to copy a file, the usage line should better be usage: Copy <source> <target>, of course.

} catch (IllegalArgumentException e) {
    System.err.println(e.getMessage());
    System.exit(1);
}

I added the System.exit since it was missing. If the program fails, it must report this via System.exit or by throwing an exception from the main function.

private static int processResult(String inputOne, String inputTwo) {
    if (inputOne.length() != inputTwo.length()) {
        throw new IllegalArgumentException("Strings must be the same length");
    }

There's no need to have a separate function for checking the lengths of the strings. Using IntelliJ it was quite simple to inline the method call to lengthsEqual (I just pressed Ctrl+Alt+N) and to remove the redundant call to Integer.compare (which was already marked in dark gray, so I just had to press Alt+Enter there).

It's unfortunate that the exception message says "String must be the same length". This message does not tell which strings are meant, and since that message is printed to System.err later, it should be worded with the same care as the usage message.

I disagree with bag's answer, especially the part that commercial code needs to be extensible. It doesn't. It needs to be easy to read, and it needs to clearly tell its intention. If the business requires that the two strings be equal (for whatever reason), the code should say exactly this, and the code should match the wording of the original requirement as closely as possible, so that later changes to the requirements can be programmed as easily as possible.


Repository pattern for books in EF6 I am working on a generic repository. And I wondered if anyone could give some feedback on it.

Interface for DbContext:

public interface IDbContext
{
    IDbSet<T> Set<T>() where T : class;
    DbEntityEntry Entry<T>(T entity) where T : class;
    void SaveChanges();
}

Context class

public class BookContext : DbContext, IDbContext
{
    public BookContext()
        : base("BookDB")
    { }

    public new IDbSet<T> Set<T>() where T : class
    {
        return base.Set<T>();
    }

    public new DbEntityEntry Entry<T>(T entity) where T : class
    {
        return base.Entry(entity);
    }

    public new void SaveChanges()
    {
        base.SaveChanges();
    }
}

Repository interface

public interface IRepository<T>
{
    IQueryable<T> FindAll();
    IQueryable<T> Find(Expression<Func<T, bool>> predicate);
    T FindById(int id);
    void AddOrUpdate(T entity);
    void Remove(T entity);
}

Repository implementation

public class Repository<T> : IRepository<T>
    where T : class, IEntity
{
    protected readonly IDbContext Context;

    public Repository(IDbContext context)
    {
        Context = context;
    }

    public IQueryable<T> FindAll()
    {
        return Context.Set<T>();
    }

    public IQueryable<T> Find(Expression<Func<T, bool>> predicate)
    {
        return Context.Set<T>().Where(predicate);
    }

    public T FindById(int id)
    {
        return Context.Set<T>().SingleOrDefault(o => o.Id == id);
    }

    public void AddOrUpdate(T entity)
    {
        if (entity == null)
            throw new ArgumentNullException("entity");

        if (entity.Id == 0)
        {
            Context.Set<T>().Add(entity);
        }
        else
        {
            Context.Entry(entity).State = EntityState.Modified;
        }

        Context.SaveChanges();
    }

    public void Remove(T entity)
    {
        if (entity == null)
            throw new ArgumentNullException("entity");

        Context.Set<T>().Remove(entity);
        Context.SaveChanges();
    }
}

Additional repository

public interface IBookRepository
{
    Book FindByIsbn(string isbn);
    IQueryable<Book> FindByAuthor(Author author);
}

Book repo implementaion

public class BookRepository : Repository<Book>, IBookRepository
{
    public BookRepository(IDbContext context) : base(context)
    {
    }

    public Book FindByIsbn(string isbn)
    {
        return Context.Set<Book>().SingleOrDefault(b => b.Isbn.Equals(isbn));
    }

    public IQueryable<Book> FindByAuthor(Author author)
    {
        return Context.Set<Book>().Where(b => b.Author.Id == author.Id);
    }
}

I am planning to use this repo with autofac IoC. I am not sure If I should use Unit of work pattern and should I make my repos disposable. Any feedback would be appreciated.

1) I would not expose IQueryable via IRepository. IQueryable is still a virtual set of objects that hasn't been retrieved from a database yet. If you have (for example) some problem with database performance, you would have to go through all your application to find out what queries you construct and send to DB.

Instead I would expose methods for specific purposes on specific repositories. These methods would return IList<T> (or ICollection<T>, depends on your preference). Then you have all the code for accessing a database on a single place. For example, your IBookRepository would look like:

public interface IBookRepository : IRepository<Book>
{
    Book FindByIsbn(string isbn);
    IList<Book> FindByAuthor(int authorId);
    IList<Book> FindByTitle(string title);
    ...
}

And your generic repository would be:

public interface IRepository<T>
{
    T FindById(int id);
    void AddOrUpdate(T entity);
    void Remove(T entity);
}

2) It is not necessary to make your repositories disposable. So far you have nothing to dispose there. Your repositories get IDbContext (which is IDisposable) from outside, thus it should not be disposed inside a repository. If your repositories get created via an IoC container, the container will also handle disposing of all dependencies.

3) UnitOfWork: You can implement UnitOfWork pattern if you want to (e.g.) wrap all your database work inside transactions. I would recommend such IUnitOfWork interface:

public interface IUnitOfWork
{
    void SaveChanges();
    void BeginTransaction();
    void CommitTransaction();
    void RollbackTransaction();
}

Implemetation of IUnitOfWork would have dependency on IDbContext. Then the only places where you work with a database (via IDbContext) are UnitOfWork and Repository classes. At places where you want to save your work to a DB, use IUnitOfWork, not IDbContext.


Collatz Conjecture (c++) I am new to C++ and this is my first program. The objective of this program is to fully complete the sequence called the Collatz conjecture.

  It concerns a sequence defined as follows: start with any positive integer n. Then each term is obtained from the previous term as follows: if the previous term is even, the next term is one half the previous term. Otherwise, the next term is 3 times the previous term plus 1. The conjecture is that no matter what value of n, the sequence will always reach 1.

I am not familiar with the language just yet and would appreciate some (constructive) feedback. The program works as intended, however, I have a feeling that some of my code is not as elegant as it could/should be.

#include <iostream>
#include <ctime>

using namespace std;

bool isValid(int num) {
  if(num <= 0) {
    return false;
  } else {
    return true;
  }
}

bool isEven(int num) {
  if (num % 2 == 0) {
    return true;
  } else {
    return false;
  }
}

void calculate(int num) {
  if (num == 1) {
    cout << endl << "Finished!" << endl;
    return;
  } else if(isEven(num)) {
    num = num / 2;
    cout << num << " ";
    calculate(num);
  } else {
    num = (num * 3) + 1;
    cout << num << " ";
    calculate(num);
  }
}

int main() {

  int input;
  cout << "Please enter an integer greater than zero." << endl;
  cin >> input;

  if(isValid(input)) {
    calculate(input);
  } else {
    cout << "Not a valid input." << endl;
    return 0;
  }

  return 0;
}

Good job checking the input for validity and breaking up the logic into simple functions! However, some improvements can be made.

Avoid using namespace std

This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.

Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.

Avoid std::endl in favor of \n

std::endl flushes the stream, which can cause a loss in performance. In some cases you can also reduce calls to operator<< (e.g. cout << endl << "Finished!" << endl; becomes cout << "\nFinished!\n";).

Simplify your boolean expressions

You can remove some of your explicit return true and return false statements by returning the expression in the if condition. For example, you can shorten isEven() like so:

bool isEven(int num) {
  return num % 2 == 0;
}

Similarly, you can shorten isValid().

Don't mix logic and writing to cout in calculate()

Currently calculate() is not very reusable since it writes to cout. What if you wanted to write to a different stream, or if you just wanted to calculate the next term in the sequence without writing to any stream? I suggest redefining calculate() to simply return the next term in the sequence:

int next(int prev) {
  if (isEven(prev))
    return prev / 2;
  else
    return prev * 3 + 1;
}

Then in main() you can output each number returned by next() to cout, as well as check for the condition in which the term equals 1. You would replace the calculate(input); statement in main() with something like this:

while (input != 1) {
  input = next(input);
  std::cout << input << " ";
}

std::cout << "\nFinished!\n"


Sending Ajax requests and updating DOM I wrote some ugly JavaScript that is responsible for sending Ajax requests and update the DOM. I'm using it to show user progress of background jobs.

this.AjaxPoller = {
  poll: function() {
    setTimeout(this.AjaxPoller.request, 5000);
  },

  request: function() {
    $(".ajax_poller[data-url]").each(function(i, elem) {
      url = $(elem).data("url");

      $.getJSON(url, function(data) {
        if (checkProgress(data)) {
          location.reload();
        };

        $.each(data, function( key, val ) {
          $(key+ ' .progress_info').html(val);
          $(key + ' .progress-bar').width(val);
        });
      });
    });
  }
};

var checkProgress = function(obj) {
  correct = true;

  for (key in obj) {
    if (obj[key] != '100%') correct = false;
  }
  return correct;
}

What can I refactor in this code to make it better?

I personally like to rely on named functions rather than inline anonymous functions.

this.AjaxPoller = {
  poll: poll,
  request: request
};

function request() {
  $(".ajax_poller[data-url]").each(fetchDataForElement);
}

function fetchDataForElement(i, elem) {
  var url = $(elem).data("url");
  $.getJSON(url, handleJson);
}

function handleJson(data) {
  if (checkProgress(data)) {
    location.reload();
  }
  $.each(data, function(key, val) {
    $(key+ ' .progress_info').html(val);
    $(key + ' .progress-bar').width(val);
  });
}

function poll() {
  setTimeout(this.request, 5000);
}

function checkProgress(obj) {
  var correct = true;
  for (key in obj) {
    if (obj[key] != '100%') correct = false;
  }
  return correct;
}


Heap's algorithm - integer permutation Integer permutation using Heap's algorithm:

import java.util.Arrays;

public class IntegerPermutation {

    private static void swap(Integer[] integerArray, int i, int j) {
        Integer temp = integerArray[i];
        integerArray[i] = integerArray[j];
        integerArray[j] = temp;
    }

    private static void permutationHelper(Integer[] integerArray,
            int currentPosition) {

        if (currentPosition == 1) {
            System.out.println(Arrays.toString(integerArray));
        } else {
            for (int i = 0; i < currentPosition; i++) {
                permutation(integerArray, currentPosition - 1);

                if (currentPosition % 2 == 0) {
                    swap(integerArray, i, currentPosition - 1);
                } else {
                    swap(integerArray, 0, currentPosition - 1);
                }
            }
        }
    }

    public static void permutation(Integer[] integerArray, int lengthOfTheArray) {
        permutationHelper(integerArray, lengthOfTheArray);
    }

    public static void main(String[] args) {

        Integer[] a = new Integer[] { 1, 2, 3 };
        IntegerPermutation.permutation(a, a.length);
    }
}

Can you please critique my code and provide your thoughts on where I should improve my code?

permutation -> permutationHelper -> permutation -> ...

As @MartinR pointed out,
the permutation and permutationHelper functions calling each other are confusing.
In fact, permutation is actually pointless,
as it does nothing but call permutationHelper.

To give permutation some purpose,
you could do this:

Make permutationHelper call itself instead of permutation
Drop the array length parameter from permutation: it can derive it from the array it receives, and pass that to permutationHelper

Why Integer[] instead of int[] ?

In the posted program, there's really no need for Integer objects.
You can work with int[], which is slightly lighter.

Code duplication

There are several duplicated elements in this code:

permutation(integerArray, currentPosition - 1);

if (currentPosition % 2 == 0) {
    swap(integerArray, i, currentPosition - 1);
} else {
    swap(integerArray, 0, currentPosition - 1);
}

First of all, the currentPosition - 1 is duplicated.
It would be good to store it in a local variable.

Secondly, swap is called with almost the same parameters, except one.
This could be expressed more compactly using a ternary operator.

int nextPosition = currentPosition - 1;
permutation(integerArray, nextPosition);
swap(integerArray, currentPosition % 2 == 0 ? i : 0, nextPosition);

Initializing arrays

A simpler way to initialize arrays is using { ... }, like this:

int[] a = { 1, 2, 3, 4 };


Create JSON file from WordPress ACF options I am using a WordPress multisite network and want to pull information from options pages in Advanced Custom Fields to create a json file. I currently have this code running whenever an options field is updated on a subsite. The json file lives on the main site.

The code works and runs successfully, however, it takes forever and often results in 502 errors due to running out of time. This is solved by refreshing the page and attempting to update the options value again.

How can I optimize my code and make it run faster or more efficiently?

functions.php

require_once( 'httpful.phar' );

function create_locations_json( $post_id ) {
    // check if acf field updated is on options page
    if ($post_id == 'options' || $post_id == 0) {
        $stageurl = array();
        $posts = array();
        $args = array(
            'public'     => true,
            'limit'      => 500
        );

        // get all public sites in multisite network
        $sites = wp_get_sites($args);
        foreach ($sites as $site) {
            // loop through sites and pull stage variable from options
            switch_to_blog($site['blog_id']);
            $stage = get_field('stage', 'option');
            if (isset($stage)) {
                $stageurl[] = $site['domain'];
            }
            restore_current_blog();
        }

        // loop through sites to generate json content
        foreach ($stageurl as $i => $stages) {
            $mainurl = parse_url(network_site_url());

            // check if http (local) or https (production)
            if($mainurl['scheme'] == 'https'){
                $url = "https://" . $stageurl[$i] . "/wp-json/acf/v2/options";
            } else {
                $url = $stageurl[$i] . "/wp-json/acf/v2/options";
            }

            // send api url to Httpful to get acf json
            $response = \Httpful\Request::get($url)->send();

            // get fields from acf json
            $name = "{$response->body->acf->small_location_name}";
            $sitestatus = "{$response->body->acf->stage}";
            $city = "{$response->body->acf->city}";
            $state = "{$response->body->acf->state}";
            $email = "{$response->body->acf->email}";

            // get lat and lng from google maps api
            $mapsurl = "https://maps.googleapis.com/maps/api/geocode/json?address=" . urlencode($city) . ",+" . $state . "&key=KEYHERE";
            $mapsresponse = \Httpful\Request::get($mapsurl)->send();
            $lat = "{$mapsresponse->body->results[0]->geometry->location->lat}";
            $lng = "{$mapsresponse->body->results[0]->geometry->location->lng}";

            // create different json responses based on stage
            if ($sitestatus > 1) {
                $address = "{$response->body->acf->address_1}";
                $address2 = "{$response->body->acf->address_2}";
                $postal = "{$response->body->acf->zip}";

                $posts[] = array('id' => $i, 'name' => $name, 'site_status' => $sitestatus, 'address' => $address, 'address_2' => $address2, 'city' => $city, 'state' => $state, 'postal' => $postal, 'lat' => $lat, 'lng' => $lng, 'email' => $email, 'web' => $stageurl[$i]);
            }
            else {
                $posts[] = array('id' => $i, 'name' => $name, 'site_status' => $sitestatus, 'city' => $city, 'state' => $state, 'lat' => $lat, 'lng' => $lng, 'email' => $email, 'web' => $stageurl[$i]);
            }
        }

        // overwrite whole json file with new array
        file_put_contents(plugin_dir_path( __FILE__ ) . '../library/json/locations.json', json_encode($posts));
    }
    else {
        return $post_id;
    }

    return $post_id;
}

// run when acf field is updated
add_action('acf/save_post', 'create_locations_json', 20);

Your problem is likely making those API requests in a loop.  You might consider using a REST library which can support multiple concurrent calls to your end services (perhaps something based on curl_multi_exec()).  This would allow you to collapse overall API request time down to the length of whichever request happens to take the longest.

I am not familiar with Httpful which it seems you are using, but in glancing at the information available for that library, I didn't see support for making concurrent requests.  This may mean you need to change libraries or build something yourself to do this.

I will link to an example class I had once written in PHP to give you an idea for how this may be accomplished.

https://github.com/mikecbrant/php-rest-client

You are free to use it if you find it useful.


Converting strings to PascalCase I have a method which I use on occasion to convert strings (like enum and property names) to PascalCase for display.

It's fairly simple, so there's probably not much to comment on, but as usual I appreciate any advice on improving it.

/// <summary>
/// Converts a string of dash-separated, or underscore-separated words to a PascalCase string.
/// </summary>
/// <param name="s">The string to convert.</param>
/// <returns>The resulting PascalCase string.</returns>
public static string ToPascalCase(this string s)
{
    string[] words = s.Split(new char[2] { '-', '_' }, StringSplitOptions.RemoveEmptyEntries);

    StringBuilder sb = new StringBuilder(words.Sum(x => x.Length));

    foreach (string word in words)
    {
        sb.Append(word[0].ToString().ToUpper() + word.Substring(1));
    }

    return sb.ToString();
}

Feel free to use it if you like, as per my usual regards.

I think you should use Concat instead of StringBuilder:

public static string ToPascalCase(this string s)
{
    var words = s.Split(new[] { '-', '_' }, StringSplitOptions.RemoveEmptyEntries)
                 .Select(word => word.Substring(0, 1).ToUpper() +
                                 word.Substring(1).ToLower());

    var result = String.Concat(words);
    return result;
}

This solution should not be as fast as manually looping through the original string, but I think it is much easier to read and follow. Unless you do those convertions like 1000000 times per second, you should not complicate things.


"Find the Min" challenge on Facebook Hacker Cup I was solving the "Find the Min" problem on Facebook Hacker Cup.

The code below works fine for the sample inputs given there, but for input size as big as 109, this takes hours to return the solution.

  Problem statement:

  After sending smileys, John decided to play with arrays. Did you know
  that hackers enjoy playing with arrays? John has a zero-based index
  array, m, which contains n non-negative integers. However, only
  the first k values of the array are known to him, and he wants to
  figure out the rest.

  John knows the following: for each index i, where k <= i < n,
  m[i] is the minimum non-negative integer which is not contained in
  the previous *k* values of m.

  For example, if k = 3, n = 4 and the known values of m are [2, 3, 0], he can figure out that m[3] = 1.

  John is very busy making the world more open and connected, as such,
  he doesn't have time to figure out the rest of the array. It is your
  task to help him.

  Given the first k values of m, calculate the nth value of this
  array. (i.e. m[n - 1]).

  Because the values of n and k can be very large, we use a
  pseudo-random number generator to calculate the first k values of
  m. Given positive integers a, b, c and r, the known values
  of m can be calculated as follows:

m[0] = a
m[i] = (b * m[i - 1] + c) % r, 0 < i < k

  Input

  The first line contains an integer T (T <= 20), the number of test cases.
  This is followed by T test cases, consisting of 2 lines each.
  The first line of each test case contains 2 space separated integers,
    n, k (\$1 <= k \le 10^5, k < n \le 10^9\$).
  The second line of each test case contains 4 space separated integers
  a, b, c, r (\$0 \le a, b, c \le 10^9, 1 \le r <= 10^9\$).

My solution:

import sys
cases=sys.stdin.readlines()
def func(line1,line2):
    n,k=map(int,line1.split())
    a,b,c,r =map(int,line2.split())
    m=[None]*n
    m[0]=a
    for i in xrange(1,k):
        m[i]= (b * m[i - 1] + c) % r
    #print m
    for j in range(0,n-k):

        temp=set(m[j:k+j])
        i=-1
        while True:
            i+=1
            if i not in temp:
                m[k+j]=i
                break

    return m[-1]

for ind,case in enumerate(xrange(1,len(cases),2)):
    ans=func(cases[case],cases[case+1])
    print "Case #{0}: {1}".format(ind+1,ans)

Sample input:

5
97 39
34 37 656 97
186 75
68 16 539 186
137 49
48 17 461 137
98 59
6 30 524 98
46 18
7 11 9 46

1. Improving your code

If you used if __name__ == '__main__': to guard the code that should be executed when your program is run as a script, then you'd be able to work on the code (for example, run timings) from the interactive interpreter.
The name func is not very informative. And there's no docstring or doctests. What does this function do? What arguments does it take? What does it return?
func has many different tasks: it reads input, it generates pseudo-random numbers, and it computes the sequence in the problem. The code would be easier to read, test and maintain if you split these tasks into separate functions.
Reading all the lines of a file into memory by calling the readlines method is usually not the best way to read a file in Python. It's better to read the lines one at a time by iterating over the file or using next, if possible.
When computing the result, you keep the whole sequence (all \$n\$ values) in memory. But this is not necessary: you only need to keep the last \$k\$ values (the ones that you use to compute the minimum excluded number, or mex). It will be convenient to use a collections.deque for this.
Similarly, it's not necessary to build the set of the last \$k\$ values every time you want to compute the next minimum excluded number. If you kept this set around, then at each stage, all you'd have to do is to add one new value to the set and remove up to one old value. It will be convenient to use a collections.Counter for this.

Applying all of these improvements results in the code shown below.

import sys
from itertools import count, islice
from collections import Counter, deque

def pseudorandom(a, b, c, r):
    """Generate pseudo-random numbers starting with a and proceeding
    according to the linear congruential recurrence
    a -> (b * a + c) % r.

    >>> list(islice(pseudorandom(34, 37, 656, 97), 10))
    [34, 71, 82, 4, 28, 43, 16, 84, 78, 50]

    """
    while True:
        yield a
        a = (b * a + c) % r

def mex(c):
    """Return the "mex" (Minimum EXcluded) number in the Counter c.

    >>> mex(Counter(range(10)))
    10
    >>> mex(Counter([2, 3, 0]))
    1

    """
    for i in count():
        if c[i] == 0:
            return i

def iter_mex(k, it):
    """Generate the sequence whose first k values are given by the
    iterable `it`, and whose ith value (for i > k) is the "mex"
    (minimum excluded number) of the previous k values of the
    sequence.

    """
    q = deque(islice(it, k))
    for m in q:
        yield m
    c = Counter(q)
    while True:
        m = mex(c)
        yield m
        q.append(m)
        c[m] += 1
        c[q.popleft()] -= 1

def nth_iter_mex(n, k, a, b, c, r):
    """Return the nth value of the sequence whose first k values are
    given by pseudorandom(a, b, c, r) and whose values thereafter
    are the mex of the previous k values of the sequence.

    """
    seq = iter_mex(k, pseudorandom(a, b, c, r))
    return next(islice(seq, n, None))

def main(f):
    cases = int(next(f))
    for i in xrange(cases):
        n, k = map(int, next(f).split())
        a, b, c, r = map(int, next(f).split())
        print('Case #{}: {}'.format(i, nth_iter_mex(n - 1, k, a, b, c, r)))

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

2. Timing

Python comes with the timeit module for timing code. Let's have a look at how long the program takes on some medium-sized test instances:

>>> from timeit import timeit
>>> timeit(lambda:nth_iter_mex(10**5, 10**2, 34, 37, 656, 97), number=1)
1.6787691116333008
>>> timeit(lambda:nth_iter_mex(10**5, 10**3, 34, 37, 656, 97), number=1)
16.67353105545044
>>> timeit(lambda:nth_iter_mex(10**5, 10**4, 34, 37, 656, 97), number=1)
143.31502509117126

The runtime of the program is approximately proportional to both \$n\$ and \$k\$, so extrapolating from the above timings, I expect that in the worst case, when \$n = 10^9\$ and \$k = 10^5\$, the computation will take about five months. So we've got quite a lot of improvement to make!

3. Speeding up the mex-finding

In the implementation above it takes \$Θ(k)\$ time to find the mex of the last \$k\$ numbers, which means that the whole runtime is \$Θ(nk)\$. We can improve the mex finding to \$O(\log k)\$ as follows.

First, note that the mex of the last \$k\$ numbers is always a number between \$0\$ and \$k\$ inclusive. So if we keep track of the set of excluded numbers in this range (that is, the numbers between \$0\$ and \$k\$ inclusive that do not appear in the last \$k\$ elements of the sequence), then the mex is the smallest number in this set. And if we keep this set of excluded numbers in a heap then we can find the smallest in \$O(\log k)\$.

Like this:

import heapq

def iter_mex(k, it):
    """Generate the sequence whose first k values are given by the
    iterable `it`, and whose ith value (for i > k) is the "mex"
    (minimum excluded number) of the previous k values of the
    sequence.

    """
    q = deque(islice(it, k))
    for m in q:
        yield m
    excluded = list(set(xrange(k + 1)).difference(q))
    heapq.heapify(excluded)
    c = Counter(q)
    while True:
        mex = heapq.heappop(excluded)
        yield mex
        q.append(mex)
        c[mex] += 1
        old = q.popleft()
        c[old] -= 1
        if c[old] == 0:
            heapq.heappush(excluded, old)

Now the timings are much more satisfactory:

>>> timeit(lambda:nth_iter_mex(10**5, 10**2, 34, 37, 656, 97), number=1)
0.2606780529022217
>>> timeit(lambda:nth_iter_mex(10**5, 10**3, 34, 37, 656, 97), number=1)
0.2634279727935791
>>> timeit(lambda:nth_iter_mex(10**5, 10**4, 34, 37, 656, 97), number=1)
0.32929110527038574
>>> timeit(lambda:nth_iter_mex(10**6, 10**5, 34, 37, 656, 97), number=1)
3.5652129650115967

However, we still expect that when \$n = 10^9\$ and \$k = 10^5\$, the computation will take around an hour, which is still far too long.

4. A better algorithm

Let's have a look at the actual numbers in the sequence generated by iter_mex and see if there's a clue as to a better way to calculate them. Here are the first hundred values from a sequence with \$k = 13\$ and \$r = 23\$:

>>> list(islice(iter_mex(13, pseudorandom(34, 37, 656, 23)), 100))
[34, 5, 13, 10, 14, 1, 3, 8, 9, 0, 12, 19, 2, 4, 6, 5, 7, 10, 11, 1,
 3, 8, 9, 0, 12, 13, 2, 4, 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13,
 2, 4, 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4, 6, 5, 7, 10,
 11, 1, 3, 8, 9, 0, 12, 13, 2, 4, 6, 5, 7, 10, 11, 1, 3, 8, 9, 0,
 12, 13, 2, 4, 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4, 6, 5]

You should have noticed a pattern there, but let's lay it out in rows of length \$k + 1 = 14\$ to make it obvious:

[34, 5, 13, 10, 14, 1, 3, 8, 9, 0, 12, 19, 2, 4,
 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4,
 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4,
 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4,
 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4,
 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4,
 6, 5, 7, 10, 11, 1, 3, 8, 9, 0, 12, 13, 2, 4,
 6, 5]

You can see that the first \$k + 1\$ values are different, but thereafter each group of \$k + 1\$ items from the sequence is identical (and moreover is a permutation of the numbers \$0, \ldots, k\$).

Let's check that this happens for some bigger examples:

>>> it = iter_mex(10**5, pseudorandom(34, 37, 656, 4294967291))
>>> next(islice(it, 10**5, None))
0
>>> s, t = [list(islice(it, 10**5 + 1)) for _ in xrange(2)]
>>> s == t
True
>>> sorted(s) == range(10**5 + 1)
True

Let's prove that this always happens. First, as noted above, the mex of \$k\$ numbers is always a number between \$0\$ and \$k\$ inclusive, so all numbers in the sequence after the first \$k\$ lie in this range. Each number in the sequence is different from the previous \$k\$, so each group of \$k + 1\$ numbers after the first \$k\$ must be a permutation of the numbers from \$0\$ to \$k\$ inclusive. So if the (\$k + 1\$)th last number is \$j\$, then the last \$k\$ numbers will be a permutation of the numbers from \$0\$ to \$k\$ inclusive, except for \$j\$. So \$j\$ is their mex, and that will be the next number. Hence the pattern repeats.

So now it's clear how to solve the problem. If \$n > k\$, element number \$n\$ in the sequence is the same as element number \$k + 1 + n \bmod (k + 1)\$.

The implementation is straightforward:

def nth_iter_mex(n, k, a, b, c, r):
    """Return the nth value of the sequence whose first k values are
    given by pseudorandom(a, b, c, r) and whose values thereafter
    are the mex of the previous k values of the sequence.

    """
    seq = iter_mex(k, pseudorandom(a, b, c, r))
    if n > k: n = k + 1 + n % (k + 1)
    return next(islice(seq, n, None))

and now the timings are acceptable:

>>> timeit(lambda:nth_iter_mex(10**9, 10**5, 34, 37, 656, 4294967291), number=1)
1.6802959442138672


Breakable MVC framework written in PHP I've written a framework in MVC. I call it Midget MVC, as it's so darn small. The reason I wrote it is because I wanted a lightweight and extensible framework to use in projects. It eventually got abandoned due to it's break-ability and my lack of experience in the subject. However, I've got a bigger project I'm working on and need a framework, so I want to start re-writing MMVC. Feel free to view the entire project here.

The main piece of code I want looked over is the router class. After talking a bit on the PHP I discovered that it was indeed breakable. One of the recommendations made was for me not to use global $config. What's the best way to do this?

<?php

class Route {
    public function load(){
        global $config;
        if(isset($_SERVER['PATH_INFO'])){
            $route = explode('/', substr($_SERVER['PATH_INFO'], 1));
            $controller = ucfirst(strtolower($route[0]));
            $method = isset($route[1]) ? strtolower($route[1]) : 'index';

            $path = APPPATH . "controllers/" . $route[0] . ".php";
            if(realpath($path)){
                require $path;
                $controller = new $controller;

                if($method != 'index'){
                    $variables = array();
                    $i = 0;
                    foreach($route as $vars){
                        if ($i >= 2)
                            $variables[] = $route[$i];
                        $i++;
                    }

                    call_user_func_array(array($controller, $method), $variables);
                }else{
                    $controller->index();
                }
            }
        }else{
            $controller = $config['deafult_controller'];
            require APPPATH . "controllers/" . $controller . ".php";
            $controller = ucfirst($controller);
            $controller = new $controller;
            $method = 'index';
            $controller->$method();
        }
    }
}

The second thing I want help with is the load class. Is there a better way to load models into the controller? Loading library files was quite difficult; you have to load it in each file. If you load the library in the controller, you'll have to load it again in the model if you want to access it. What's the best way to do this?

class Load{
    function __construct($controller){
        $this->controller = $controller;
    }

    // loads a specified model
    public function model($model){
        $model = strtolower($model);
        if(file_exists(APPPATH . "models/$model.php")){
            require(APPPATH . "models/$model.php");
            $modelclass = ucfirst($model . '_Model');
            // call function to include it in the controller
            $this->controller->$model = new $modelclass;
        }
    }

    // loads a veiw
    public function view($view, $data = NULL){
        // asign filepath
        $path = APPPATH . "views/" . $view . ".php";
        // asign variables if $data is set
        if($data != NULL)
            foreach($data as $var => $value)
                $$var = $value;

        if(file_exists($path)){
            ob_start();
            require($path);
            print(ob_get_clean());
        }else
            return false;
    }

    public function library($library){
        $library = strtolower($library);
        if(file_exists(LIBRARY . "$library.php")){
            require(LIBRARY . "$library.php");
            $libraryclass = ucfirst($library);

            $this->controller->$library = new $libraryclass;
        }
    }
}

And finally, how secure is the entire thing. What security flaws can you see? Are there any major points I should improve upon.

Nice initiative on a lightweight MVC framework!

Note wanted to add more reference links, but it won't allow me to do more thanb 2

Config

Pass the default controller setting to the function

or

Pass all routing related settings to the __constructor. and use them as class attributes

Loader class

The setup isn't all that bad. Nice and lightweight. A few things, i would do different;

Seperations of conserns (One of the SOLID principles). Let the loader just load an return the instances.

Don't bind the loaded elements directly to controller in you class, so in future you could also use it for different cases then just the controller class.
Don't render the views in a loader. Make a render class or such. If you way of rendering changes, you would have to change the loader. Which ultimately don't make sense.

Make APP_PATH and LIBRARY settable within the loader, and call them differently

A loader class attribute for each load path
Default the attributes to APP_PATH and LIBRARY
Getter and Setter for load paths
Now you would be able to integrate different paths / modules etc. Making it more flexible

Autoloader for library classes

By registering the loader as php autoloader you can just instantiate a class.
If not yet loaded into mem, it will be sent to the autoloader.
There you resolve the path it should be on and instantiate.

Security

The big thing in this form of autoloading is the concern for input. Make sure all input is either given by the application itself or validate it.

Validate user strings that will be autoloaded (eg. controller names)
You can do this by first doing a realpath() check on the computed path. And make sure the realpath() is within the locations you would expect.
This prevenst users from entering stuff like '......\uploads\mayhackscript' which would be executed otherwise.

This is a real danger.

General remarks

Take a look at te following;

SOLID principles
PSR-2 coding standards; note, it extends PSR-1 which extends PSR-0

Good luck on your project!


Python: count number of pairs in array(list) A solution for a small algorithmic problem. I believe tests are pretty descriptive:

def pairs(n):
    return n * (n - 1) / 2

def solution(lst):
    res = 0
    stack = lst[0:1]
    lst = sorted(lst[1:])
    while(len(lst)):
        nxt = lst.pop()
        if len(stack) and stack[0] == nxt:
            stack.append(nxt)
        else:
            res += pairs(len(stack))
            stack = [nxt]
    else:
        res += pairs(len(stack))
    return res

assert(pairs(3) == 3)
assert(pairs(2) == 1)
assert(pairs(0) == 0)
assert(solution([5, 3, 1, 5, 5, 3]) == 4)
assert(solution([5, 5]) == 1)
assert(solution([5]) == 0)
assert(solution([]) == 0)

UPDATE: there is a bug. I shouldn't take a first element before sorting an array

From what I see you are counting all the pairs of equal numbers that can be formed. (If not, please clarify)

It can be done in another way:

Take the first element of the array.
Count the number of elements equal to it in the array. It can be done via filter, storing the result as a new array, then len.
Call pairs over the length of the filtered array of step 2.
Add the result of pairs to an accumulator (previously initalized at 0).
Repeat steps 2--5 from the next element that is not equal to the one taken previously (for the first iteration, it would be the one taken in step 1). This can be done using filter, then assignment over the original array (if you don't want to preserve it).

The accumulator will be the result.

Quickly tested code snippet:

def countPairs(nums):
    count = 0

    while len(nums):
        probe = nums[0]
        count += pairs(len([x for x in nums if x == probe]))
        nums = [x for x in nums if x != probe]

    return count

EDIT.

Filtering can be thought of as an O(n) operation (iterating and adding elements that fulfill the condition). This is done twice inside the loop. Here, n = len(nums).

On the other hand, the external loop will run as many times as unique(nums), where unique is a function that retrieves the number of unique elements inside the array. Why unique? Because on each iteration, all the elements equal to the probe are taken out of the array (including the probe). They can be 1, 3, all of them. This is done for each iteration.

Nevertheless, unique(nums) <= len(nums), so it can be said the external loop will be a O(n) operation in the worst case.

Therefore, my proposed algorithm runs in O(n^2) time, due to traversing the whole array at most as many times as elements would be in the array.


Show/hide checkbox div on change and load I'm working on a show/hide div with checkbox on change and on load.  I've come up with this so far:

jsFiddle

$(document).ready(function() {

var $cbtextbook = $('#in-product_category-14'),
    $cbimod = $('#in-product_category-15'),
    $mb1 = $('#mbtextbook'),
    $mb2 = $('#mbimod');

function tbmb() {
    if ($cbtextbook.is(":checked")) $mb1.show();
    else $mb1.hide();
}

function immb() {
    if ($cbimod.is(":checked")) $mb2.show();
    else $mb2.hide();
}
tbmb();
immb();

$cbtextbook.change(tbmb);
$cbimod.change(immb);

})

At the moment, I'm not worrying about dynamically changing elements (although I might in the future as I learn more).

Is there a much cleaner way to do this?  I did get pretty simple toggle to work, but that didn't take into account if the box was already checked on page load, and the div I wanted to show/hide could get off cycle (i.e. show when unclicked, hide when clicked) if it was already checked, so I came up with this overly verbose solution.  How can I pare this down?

It could be done with:

$(document).ready(function() {
    $("input").change(function() {
        var index = $(this).closest("li").index();
        $(".metabox").eq(index)[this.checked ? "show" : "hide"]();
    }).change();
});

http://jsfiddle.net/bK8EC/118/

Remember to add proper qualifiers in your real code, for example add id="something" to your ul and then do $("#something input:checkbox") instead of just binding this event to every input on the page. The .metabox is also pretty fragile if a container isn't added.


Updating an item's status in MS SQL Server using ODBC Is the following query is vulnerable in terms of $itemstatus? I want to be sure about it. $yesorno is a user input which is sanitized and it's fine, however, $itemstatus is a not user input but is included in the query. Can $itemstatus be manipulated or overall if it's vulnerable?

    $username = $this->site->SanitizeIt($_POST['username'], 20);
    $yesorno = $this->site->SanitizeIt($_POST['yesorno'], 1);

    if (intval($yesorno) == 1)
    {
    $itemstatus = '1';
    }
    else if (intval($yesorno) == 2)
    {
    $itemstatus = '2';
    }
    else
    {
        $this->content = Template::Load('error', array('error_message' => Template::GetLangVar('INVALID_STATUS')));
        return;
    }

    $this->database[DBB]->doQuery('UPDATE INFO SET ItemStatus = ? WHERE UserName = ?', $itemstatus, $username);
    $this->content = Template::Load('error', array('error_message' => Template::GetLangVar('ITEM_STATUS_SUCCESS')));
}

And if you wanna look at the SanitizeIt .. (It's NOT required since I ask for $itemstatus but however let me include it as well) here it is: (BTW: I am using MSSQL Server 2005)

function SanitizeIt($intext, $lenght = 20)
{
    return substr(preg_replace("/[^a-zA-Z0-9_]/", "", htmlspecialchars(htmlentities(strip_tags($intext)))), 0, $lenght);
}

Here is my doQuery() which is part of my ODBC.class file:

function doQuery($query)
{
    if (!parent::isConnected())
    {
        if (!$this->doConnect())
            return -1;
    }

    if ($this->result)
        @odbc_free_result($this->result);

    $parameters = func_get_args();
    array_shift($parameters);

    parent::Query();
    if (sizeof($parameters) > 0)
    {
        $this->result = @odbc_prepare($this->conn, $query);
        if (!$this->result)
            return -1;

        $result = @odbc_execute($this->result, $parameters);
        if (!$result)
            return -1;
    }
    else
    {
        $this->result = @odbc_exec($this->conn, $query);
        if (!$this->result)
            return -1;
    }
    parent::Success();
    return @odbc_num_rows($this->result);
}

Security

$itemstatus

$itemstatus will either be 1 or 2. It cannot possibly be anything else, so it is not an attack vector.

doQuery

I don't know doQuery so I cannot say if it is secure. If it is this: MongoCursor::doQuery, I wouldn't use it. The documentation says this: Warning Please do not use me. It doesn't say why, but security concerns could be one reason.

SanitizeIt

I wouldn't use your SanitizeIt function. For yesorno you don't need it, and for username it is quite limiting.

htmlentities and htmlspecialchars are generally used to prevent XSS attacks when outputting data to the user, not to prevent SQL injection.

Limiting the input to alphanumeric characters is not only very limiting, it might also not be sufficient for multi-byte encodings. If you can ensure that usernames only ever contain alphanumeric characters[*] - and that this will never change - then you can leave it in (as part of a defense in depth strategy it's not bad)

The recommended approach to SQL injection is to use parametrized queries (see here for corner cases). If you post your doQuery function we could tell you if it does this correctly.

[*] This basically means that you have to use the exact same function when registering users and when writing the username to the database.

Other

Style

Your indentation is off, which makes you code hard to read
you can either use under_score or camelCase for variables, but if you use neither, variable names are hard to read (see yesorno, itemstatus).


Avoiding callback hell Currently, the code below looks awful because of all the callbacks involved. How can I reduce the amount of nesting?  Also, would you recommend the use of q or Async.js?

var mongoose = require('mongoose');
var User = require('../models/user.js');
var Genre = require('../models/genre.js');
var Album = require('../models/album.js');
var Band = require('../models/band.js');

exports.populate = function (req, res) {
  var bandNames = [
    'The Cool Group',
    'The Peeps',
    'Music Creation',
    'Number and Wumbers',
    'Testing For You',
    'What What',
    'Alrightly Then',
    'Is this the way it is?',
    'It hurts to know who you are',
    'There has been one or two of you',
    'Please and eyond'
  ];

  var userNames = [
    'syte',
    'koumarianos',
    'ericStas',
    'bloodCobia'
  ];

  albumId = Album.create({
      name: 'Test Album',
      biography: 'example'
    },
    function (err, album) {
      if (!err) {

        var albumId = album._id;

        Genre.create({ name: 'Rock' }, function (err, genre) {
            if (!err) {
              var genreId = genre._id;

              var users = [];

              for (var i = 0; i < userNames.length; i++) {
                users.push({
                  username: userNames[i]
                });
              }

              User.create(users, function (err) {
                var bands = [];
                var users = Array.prototype.slice.call(arguments, 1);
                var userIds = []

                for (var i = 0; i < users.length; i++) {
                  userIds.push(users[i]._id);
                }

                res.send(userIds);

                if (!err) {
                  for (var i = 0; i < bandNames.length; i++) {
                    bands.push({
                      name: bandNames[i],
                      biography: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean ac iaculis dui. Duis dignissim est ut ante cursus sodales. Duis et turpis ac nisl dictum mollis. Ut vel tempus metus. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Duis eu risus aliquet, mattis est vitae, tempus sem. In eget ante semper, adipiscing arcu eu, fringilla lectus. Sed non tortor vulputate magna sodales varius eu quis nulla. Sed dignissim sed magna sit amet sodales. Ut nisi tortor, dapibus quis nulla eu, consectetur ornare dolor. Morbi mollis diam ut vulputate consectetur. In tincidunt, elit quis ultricies auctor, dolor odio condimentum ligula, quis sodales magna dui at est. Integer a orci rutrum, fermentum sapien quis, facilisis purus.',
                      albums: [albumId],
                      genres: [genreId],
                      members: userIds
                    });
                  }

                  // save all bands
                  Band.create(bands, function (err) {
                      if (!err) {
                        res.send("Successfully made all test data");
                      } else {
                        res.send(err);
                      }
                  });
                }
              });
            } else {
              res.send(err);
            }
          });
      } else {
        res.send(err);
      }
    });
};

My 2 cents are:

This code is understandable, but I would still rewrite it since it is cookie cutter, and you will surely move on to more complex stuff
Q uses a Promise object, Mongoose does not, so that would not work without some overhauling ( I personally like Q better though )
Async works with 'last parameter must be a callback' which is exactly how Mongoose works, so use that


Proper use of timer in windows service I have following code to use timer (System.Timers.Timer) in windows service. The goal is that new time handler should not occur if previous one didn't finish its job. Here is how I achieve this:

protected override void OnStart(string[] args)
{

            try
            {
                //
                // Create and start a timer.
                //
                m_mainTimer = new System.Timers.Timer();
                m_mainTimer.Interval = 60000;   // every one min
                m_mainTimer.Elapsed += new System.Timers.ElapsedEventHandler(this.timer1_Tick);
                m_mainTimer.AutoReset = false;  // makes it fire only once
                m_mainTimer.Enabled = true;
            }
            catch (Exception ex)
            {
              // omitted
            }

}

And:

protected override void OnStop()
    {
        try
        {
            // Service stopped. Also stop the timer.
            m_mainTimer.Enabled = false;
            m_mainTimer = null;

        }
        catch (Exception ex)
        {

        }

    }

Also in handler:

private void timer1_Tick(object sender, ElapsedEventArgs e)
        {

            try
            {
            }catch(Exceptione x)
            {
            }
            finally
            {

            if (null != m_mainTimer)
            {
                m_mainTimer.Start(); // re - enable the timer
            }
            }
}

It works but my question is is this approach safe? Maybe I should make m_mainTimer volatile? Because of the null check inside finally?

Ok, had a few suggestions with the code for you to avoid some confusion and maintain some consistency. I'll start off with the code sample and add some comments after the code block:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Diagnostics;
using System.Linq;
using System.ServiceProcess;
using System.Text;
using System.Threading.Tasks;

namespace WindowsService1
{
    public partial class Service1 : ServiceBase
    {
        public Service1()
        {
            InitializeComponent();
        }

        private System.Timers.Timer m_mainTimer;
        private bool m_timerTaskSuccess;

        protected override void OnStart(string[] args)
        {
            try
            {
                //
                // Create and start a timer.
                //
                m_mainTimer = new System.Timers.Timer();
                m_mainTimer.Interval = 60000;   // every one min
                m_mainTimer.Elapsed += m_mainTimer_Elapsed;
                m_mainTimer.AutoReset = false;  // makes it fire only once
                m_mainTimer.Start(); // Start

                m_timerTaskSuccess = false;
            }
            catch (Exception ex)
            {
                // omitted
            }
        }

        protected override void OnStop()
        {
            try
            {
                // Service stopped. Also stop the timer.
                m_mainTimer.Stop();
                m_mainTimer.Dispose();
                m_mainTimer = null;
            }
            catch (Exception ex)
            {
                // omitted
            }
        }

        void m_mainTimer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
        {
            try
            {
                // do some work

                m_timerTaskSuccess = true;
            }
            catch (Exception ex)
            {
                m_timerTaskSuccess = false;
            }
            finally
            {
                if (m_timerTaskSuccess)
                {
                    m_mainTimer.Start();
                }
            }
        }
    }
}

System.Windows.Forms.Timer versus System.Timers.Timer

At one point in your code it is clear you dragged a Timer component into the design grid of the service, clicked it creating the Tick event, and then replaced with a System.Timers.Timer instead. I recommend sticking with either one or the other, and thus to maintain clarity I replaced your timer1_Tick event handler with m_mainTimer_Elapsed, since System.Timers.Timer utilize the Elapsed event instead of the Tick event.

Use of a boolean success indicator

Versus setting the timer to null when the Service is stopped, which at that point it is already too late to affect your running event, utilize a private boolean to hold whether success is achieved (dependent on how you define success in your project's domain). If an exception occurs during the running of the task, set it to false. Then utilize an if block to only restart the timer if the last run was successful.

You do NOT need volatile

You are only ever modifying the timer in one thread. volatile is utilized when multiple threads could potentially modify the same reference variable.


C++ Vector memory management in Game of Life I'm writing a simulator for Conway's Game of Life (http://en.wikipedia.org/wiki/Conway's_Game_of_Life) in C++ and I'm trying to make the code as idiomatic as possible.

The central object is a double array to represent the board:

typedef vector<vector<bool> > Board;

My main method has a Board b as a local variable, and I'm passing the board to several methods that update the board, draw the board, and so on.  Is it better (that is, more efficient and/or idiomatic) to use methods like

void update(Board *board);

int main() {
   Board b;
   //...
   update(&b);
   //...
}

or in this way:

void update(Board & board);

int main() {
   Board b;
   //...
   update(b);
   //...
}

Which is the proper choice for idiomatic C++?

One rules of thumb to write idiomatic C++ is:

  Use references where you can and pointers where you have to.

It's especially true for argument passing to a variable.
In your case, you have the choice, so use references, for all the reasons cited by Ilya and also just because you won't have any * to put everywhere in your function. Your code might be clearer that way.


Formatting XML strings in JavaScript for readability I've created a vanilla JS function to indent XML strings so they can be more easily read. It uses some pretty nasty regex...yes, I know it's a cardinal sin for XML/HTML, but it works. For instance, this string...

<?xml version='1.0' encoding='UTF-8'?><soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/"><soapenv:Body><ns:getCourseResponse xmlns:ns="http://course.ws.blackboard" xmlns:ax212="http://course.ws.blackboard/xsd" xmlns:ax211="http://ws.platform.blackboard/xsd" /></soapenv:Body></soapenv:Envelope>

...would look like this after being passed through the function:

<?xml version='1.0' encoding='UTF-8'?>
<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/">
    <soapenv:Body>
        <ns:getCourseResponse xmlns:ns="http://course.ws.blackboard" xmlns:ax212="http://course.ws.blackboard/xsd" xmlns:ax211="http://ws.platform.blackboard/xsd" />
    </soapenv:Body>
</soapenv:Envelope>

Here's the function itself. What can I do to simplify it?

function formatXML(input) {

    // PART 1: Add \n where necessary
    // A) add \n between sets of angled brackets without content between them
    // B) remove \n between opening and closing tags of the same node if no content is between them
    // C) add \n between a self-closing set of angled brackets and the next set
    // D) split it into an array

    xmlString = input.trim()
        .replace(/>\s*</g,'>\n<')
        .replace(/(<[^\/>].*>)\n(<[\/])/g,'$1$2')
        .replace(/(<\/[^>]+>|<[^>]+\/>)(<[^>]+>)/g,'$1\n$2');
    xmlArr = xmlString.split('\n');

    // PART 2: indent each line appropriately

    var tabs = '';          //store the current indentation
    var start = 0;          //starting line
    if (/^<[?]xml/.test(xmlArr[0])) start++;    //if the first line is a header, ignore it

    for (var i = start; i < xmlArr.length; i++) { //for each line
        var line = xmlArr[i].trim();    //trim it just in case
        if (/^<[/]/.test(line)) { // if the line is a closing tag
            // remove one tab from the store
            // add the tabs at the beginning of the line
            tabs = tabs.replace(/.$/, '');
            xmlArr[i] = tabs + line;
        } else if (/<.*>.*<\/.*>|<.*[^>]\/>/.test(line)) { // if the line contains an entire node
            // leave the store as is
            // add the tabs at the beginning of the line
            xmlArr[i] = tabs + line;
        } else { // if the line starts with an opening tag and does not contain an entire node
            // add the tabs at the beginning of the line
            // and add one tab to the store
            xmlArr[i] = tabs + line;
            tabs += '\t';
        }
    }

    //rejoin the array to a string and return it
    return xmlArr.join('\n');
}

I've been looking something like this, nice idea. But Your function couldn't parse text nodes. Try insert some text, example:

<?xml version="1.0" encoding="UTF-8"?><soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/"><soapenv:Body>textNode<soapenv:temp>innerText</soapenv:temp><ns:temp><ns:getCourseResponse xmlns:ns="http://course.ws.blackboard" /></ns:temp></soapenv:Body></soapenv:Envelope>

My proposition for this problem is:

1) change spliting code in this way:

.replace( /(<([a-zA-Z]+\b)[^>]*>)(?!<\/\2>|[\w\s])/g, "$1\n" ) //add \n after tag if not followed by the closing tag of pair or text node
.replace( /(<\/[a-zA-Z]+[^>]*>)/g, "$1\n") //add \n after closing tag
.replace( />\s+(.+?)\s+<(?!\/)/g, ">\n$1\n<") //add \n between sets of angled brackets and text node between them
.replace( />(.+?)<([a-zA-Z])/g, ">\n$1\n<$2") //add \n between angled brackets and text node between them
.replace(/\?></, "?>\n<") //detect a header of XML

2) little modify third statement else if and add one more:

 else if (/<.*>/.test(line)) //if the line starts with an opening tag and does not contain an entire node
 {
  xmlArr[i] = tabs + line;  //add the tabs at the beginning of the line
  tabs += indent;  //and add one indent to the store
 }
 else  //if the line contain a text node
 {
  xmlArr[i] = tabs + line;  // add the tabs at the beginning of the line
 }

So finaly, our function will be:

function formatXML(input,indent)
{
  indent = indent || '\t'; //you can set/define other ident than tabs

  //PART 1: Add \n where necessary
  xmlString = input.replace(/^\s+|\s+$/g, '');  //trim it (just in case) {method trim() not working in IE8}

  xmlString = input
                   .replace( /(<([a-zA-Z]+\b)[^>]*>)(?!<\/\2>|[\w\s])/g, "$1\n" ) //add \n after tag if not followed by the closing tag of pair or text node
                   .replace( /(<\/[a-zA-Z]+[^>]*>)/g, "$1\n") //add \n after closing tag
                   .replace( />\s+(.+?)\s+<(?!\/)/g, ">\n$1\n<") //add \n between sets of angled brackets and text node between them
                   .replace( />(.+?)<([a-zA-Z])/g, ">\n$1\n<$2") //add \n between angled brackets and text node between them
                   .replace(/\?></, "?>\n<") //detect a header of XML

  xmlArr = xmlString.split('\n');  //split it into an array (for analise each line separately)

  //PART 2: indent each line appropriately

  var tabs = '';  //store the current indentation
  var start = 0;  //starting line

  if (/^<[?]xml/.test(xmlArr[0]))  start++;  //if the first line is a header, ignore it

  for (var i = start; i < xmlArr.length; i++) //for each line
  {
    var line = xmlArr[i].replace(/^\s+|\s+$/g, '');  //trim it (just in case)

    if (/^<[/]/.test(line))  //if the line is a closing tag
     {
      tabs = tabs.replace(indent, '');  //remove one indent from the store
      xmlArr[i] = tabs + line;  //add the tabs at the beginning of the line
     }
     else if (/<.*>.*<\/.*>|<.*[^>]\/>/.test(line))  //if the line contains an entire node
     {
      //leave the store as is
      xmlArr[i] = tabs + line; //add the tabs at the beginning of the line
     }
     else if (/<.*>/.test(line)) //if the line starts with an opening tag and does not contain an entire node
     {
      xmlArr[i] = tabs + line;  //add the tabs at the beginning of the line
      tabs += indent;  //and add one indent to the store
     }
     else  //if the line contain a text node
     {
      xmlArr[i] = tabs + line;  // add the tabs at the beginning of the line
     }
  }

  //PART 3: return formatted string (source)
  return  xmlArr.join('\n');  //rejoin the array to a string and return it
}


Refactoring custom validator using lots of if's I have a custom validator that checks the min and max score of different sporting leagues -- e.g., MLB, NBA, NFL, NCAAB, etc.

Here's short version of what it looks like:

class ScoreValidator < ActiveModel::Validator

  def validate(event)

    if event.league_is_mlb?
      if event.total_score < 1
        event.errors.add :base, 'min total score for an MLB event is 1'
      elsif event.total_score > 49
        event.errors.add :base, 'max total score for an MLB event is 49'
      end
    elsif event.league_is_nba?
      if event.total_score < 119
        event.errors.add :base, 'min total score for an NBA event is 119'
      elsif event.total_score > 370
        event.errors.add :base, 'max total score for an NBA event is 370'
      end
    end

  end

end

Seems like this code could be much cleaner by making smarter use of ruby data structures and methods.  Any creative suggestions you could make would be educational and much appreciated.

You can refactor each condition using extract method

def validate_mlb_score
def validate_nba_score

Also you can methods to determine min/max score

def max_score_for(event)
  event.league_is_mlb? ? 49 : 370 # judging by your elseif without else I've suggested about only two leagues
end

I think my code would look like:

def validate(event)
  if event.total_score < event.min_score
    event.errors.add :base, "min total score for an #{event.name} event is #{event.min_score}"
  elsif event.total_score > event.max_score
    event.errors.add :base, "max total score for an #{event.name} event is #{event.max_score}"
  end
end

# I also suggest what you have this kind of model
class Event < ActiveRecord::Base

  def min_score
    self.league_is_mlb? ? 1 : 119
  end

  def max_score
    self.league_is_mlb? ? 49 : 370
  end
end


Sorting elements according to frequency of occurence in C++ I am sorting the elements in an array according to their frequency of occurrence in decreasing order and if two elements have same frequency then they are sorted in increasing order. I have used std::map to count the frequency and then copy the elements from std::map to std::vector<std::pair<K, V>>.

#include <iostream>
#include <vector>
#include <map>
#include <algorithm>

struct greater
{
   template<class T>
   bool operator()(T const &a, T const &b) const { return a.second > b.second; }
};

template<class T>
void sortByFreq(std::vector<T>& v)
{
  std::map<T, T> count;

  for (int i : v){
      count[i]++;
  }

  std::vector<std::pair<T, T>> mapcopy;

  for(auto itr = count.begin(); itr != count.end(); itr++)
  {
    mapcopy.push_back(*itr);
  }

  std::sort(mapcopy.begin(), mapcopy.end(), greater());

  std::cout << "Elements sorted according to Frequency \n";

  auto itr = mapcopy.begin();
  while(itr != mapcopy.end())
  {
    for(int i = 0; i < itr->second; i++)
    {
      std::cout << itr->first <<" ";
    }
  itr++;
  }

}

int main()
{
  std::vector<int> vec = {2, 3, 2, 4, 5, 12, 12, 3, 4, 3};
  sortByFreq(vec);
  std::vector<char> c ={ 'e', 't', 'r', 'e', 't', 't', 'f'};
  sortByFreq(c);
return 0;
}

The Output is

3 3 3 2 2 4 4 12 12 5
t t t e e f r

Review this code and help me to improve it.

Advice 1

Do not print in the algorithms. If your friend wants to reuse your implementation on large vectors, he will be overwhelmed with the plethora of output.

Advice 2

You can simplify your sorting algorithm via an appropriate comparator:

template<class T>
void sortByFreq2(std::vector<T>& v)
{
    std::unordered_map<T, size_t> count;

    for (T i : v) {
        count[i]++;
    }

    std::sort(
        v.begin(),
        v.end(),
        [&count](T const& a, T const& b) {
        if (a == b) {
            return false;
        }
        if (count[a] > count[b]) {
            return true;
        }
        else if (count[a] < count[b]) {
            return false;
        }
        return a < b;
    });
}

Note unordered_map<T, size_t> instead of map<T, T> since you don't need to keep the keys T in order (more efficient).

Hope that helps.


Number-guessing game I'm not really even sure if this counts as a game. There is so little code, but it works. It is a game where you have to guess the computers number, and it tells you how many times you took to get it right. I kind of want feedback, and not ways to improve the code. I want to know ways to improve my future projects and what to avoid again.

def main():
    # Guess My Number
    #
    # The computer picks a random number between 1 and 100
    # The player tries to guess it and the computer lets
    # the player know if the guess is too high, too low
    # or right on the money

    import random

    def ask_number(question, low, high, step):
        response = None
        while response not in range(low, high):
            response = int(input(question))
        return response
        tries += step

    #Opening Remarks
    print("Welcome to 'Guess My Number'!")
    print("I'm thinking of a number between 1 and 100.")
    print("Try to guess it in as few attempts as possible.")

    # set the initial values
    the_number = random.randint(1, 100)
    # Create the priming read here
    tries = 0
    guess= 0
    while int(guess) != int(the_number):

        guess = ask_number("Enter your guess:", 1, 100, 1)

        if int(guess) == int(the_number):
            print("Your right on the money!")
        elif int(guess) > int(the_number):
            print("To high!")
        elif int(guess) < int(the_number):
            print("To low!")

        tries += 1
    #Didnt know how to make it reloop to the start...
    print("You guessed it!  The number was", the_number)
    print("And it only took you", tries, "tries!")

    #Program Closing
    input("Press the enter key to exit.")

main()

import random should be at the top of the script.

The call to main should be guarded by if __name__ == "__main__":.

the_number and guess are already int, so the repeated calls to e.g. int(the_number) are redundant.

Docstrings should be formatted properly, as multi-line strings """ """ not comments #. Many of the actual comments are redundant - it's generally obvious what's going on (well done!)

The step argument to ask_number doesn't make much sense, and there's no error handling for non-numeric inputs.

The "magic numbers" 1 and 100 should be factored out.

Use str.format for combining strings with other objects.

Think about your logic more carefully - if guess isn't equal to or greater than the_number it must be less, you can just use else.

100 isn't in range(1, 100).

I would write it as:

import random

def main(low=1, high=100):
    """Guess My Number

    The computer picks a random number between low and high
    The player tries to guess it and the computer lets
    the player know if the guess is too high, too low
    or right on the money

    """
    print("Welcome to 'Guess My Number'!")
    print("I'm thinking of a number between {} and {}.".format(low, high))
    print("Try to guess it in as few attempts as possible.")

    the_number = random.randint(low, high)
    tries = 0

    while True:
        guess = ask_number("Enter your guess:", low, high)
        if guess == the_number:
            print("You're right on the money!")
            break
        elif guess > the_number:
            print("Too high!")
        else:
            print("Too low!")
        tries += 1

    print("You guessed it! The number was {}.".format(the_number))
    print("And it only took you {} tries!".format(tries))
    input("Press the enter key to exit.")

def ask_number(question, low, high):
    """Get the user to input a number in the appropriate range."""
    while True:
        try:
            response = int(input(question))
        except ValueError:
            print "Not an integer"
        else:
            if response in range(low, high+1):
                return response
            print "Out of range"

if __name__ == "__main__":
    main()

To play again, you could recursively call main, or have a loop at the end of the script:

if __name__ == "__main__":
    while True:
        main()
        again = input("Play again (y/n)? ").lower()
        if again not in {"y", "yes"}:
            break


List all direct and indirect employees of a manager This is the question:

  You have an Employee class:

class Employee {
    String name;
    Integer id,
    Employee manager
}

  Each employee has a manager and the manager of CEO is null. Find all
  direct and indirect employees of a manager.

  For example:

  Say Employee e1 reports to CEO
  e2 and e3 reports to e1
  e4 and e5 reports to e2
  e6 reports to e3

  Then by giving e1 as input, output should be e2, e3, e4, e5 and e6.

I know I am using the stupidest way to do it. But I wonder if there is any better way to do the if-statement?

function func() {
  function Employee(name, id, manager) {
    this.name = name;
    this.id = id;
    this.manager = manager;
  }

  const ceo = new Employee("ceo", 1, null);
  const e1 = new Employee("e1", 2, ceo);
  const e2 = new Employee("e2", 3, e1);
  const e3 = new Employee("e3", 4, e1);
  const e4 = new Employee("e4", 5, e2);
  const e5 = new Employee("e5", 6, e2);
  const e6 = new Employee("e6", 7, e3);

  var name = document.getElementById("txtName").value;

  var eArr = [];

  if (e1.manager.name === name.toLowerCase()) {
    eArr.push(e1.name);
  }
  if (e2.manager.name === name.toLowerCase() || (e2.manager.manager !== null && e2.manager.manager.name === name.toLowerCase())) {
    eArr.push(e2.name);
  }
  if (e3.manager.name === name.toLowerCase() || (e3.manager.manager !== null && e3.manager.manager.name === name.toLowerCase())) {
    eArr.push(e3.name);
  }
  if (e4.manager.name === name.toLowerCase() || (e4.manager.manager !== null && e4.manager.manager.name === name.toLowerCase()) || (e4.manager.manager.manager !== null && e4.manager.manager.manager.name === name.toLowerCase())) {
    eArr.push(e4.name);
  }
  if (e5.manager.name === name.toLowerCase() || (e5.manager.manager !== null && e5.manager.manager.name === name.toLowerCase()) || (e5.manager.manager.manager !== null && e5.manager.manager.manager.name === name.toLowerCase())) {
    eArr.push(e5.name);
  }
  if (e6.manager.name === name.toLowerCase() || (e6.manager.manager !== null && e6.manager.manager.name === name.toLowerCase()) || (e6.manager.manager.manager !== null && e6.manager.manager.manager.name === name.toLowerCase())) {
    eArr.push(e6.name);
  }

  if (eArr.length !== 0)
    document.getElementById("ans").innerHTML = "Ans: " + eArr;
  else
    document.getElementById("ans").innerHTML = "Ans: No reportees";
}
Manager Name = <input type="text" id="txtName">
<button onclick="func()">Result</button>

Review

You shouldn't put a function definition inside of another function like that. It means it has to recreate the function every time. It would be better to not get in a habit of that.
Same goes for the contants ceo, etc.
In addition, your code only works for one test case, as you manually check for each member.

Rewrite:

const [addEmployee, getEmplyeeBelows] = (function() {
   class Employee {
      constructor(name, id, manager) {
        this.name = name;
        this.id = id;
        this.manager = manager;
        this.manager = registry[manager];
    }
   }
   const registry = {};
   function addEmployee(name, number, parent) {
      if( parent === null ) {
         registry[name] = new Employee(name, number, parent);
      } else {
         parent.emplyees[number] = new Employee(name, number, parent);
         registry[name] = parent.emplyees[number];
      }
   }
   function getEmplyeeBelows(name) {
      let result = [];
      result.push(...Object.values(registry).map((emp) => {
        return emp.manager == name ? [emp, ...getEmplyeeBelows(emp)] : [];
      })
      return result;
   }
   return [addEmployee, getEmplyeeBelows]
})()
addEmployee('ceo', 1, null);
addEmployee('e1', 2, 'ceo');
addEmployee("e2", 3, 'e1');
addEmployee("e3", 4, 'e1');
addEmployee("e4", 5, 'e2');
addEmployee("e5", 6, 'e2');
addEmployee("e6", 7, 'e3');

const manager = () => {

  var name = document.getElementById("txtName").value;

  eArr = getEmplyeeBelows(name);

  if (eArr.length !== 0)
    document.getElementById("ans").innerHTML = "Ans: " + eArr;
  else
    document.getElementById("ans").innerHTML = "Ans: No reportees";
}


Clicking game in windows forms I've made a simple clicking game where you spam a button in order to get coins which you can use to buy upgrades to get even more coins.

I've only 2 classes one that is responsible for the game logic - MainGame and one for the items available in the shop - Item.

MainGame

public partial class MainGame : Form
{
    private readonly Item[] purchasableItems = new Item[20];

    private long _currentMoney = 0;
    private long currentMoney
    {
        get { return _currentMoney; }
        set
        {
            _currentMoney = value;
            lbCurrentMoney.Text = $@"{currentMoney} $";
        }
    }

    private long _moneyPerClick = 1;
    private long moneyPerClick
    {
        get { return _moneyPerClick; }
        set
        {
            _moneyPerClick = value;
            bGetMoney.Text = $@"{moneyPerClick} $";
        }
    }

    private Thread countThread = null;
    private bool stop = false;

    public MainGame()
    {
        InitializeComponent();
        Initialize();
    }

    private void Initialize()
    {
        lbCurrentMoney.Text = $@"{currentMoney} $";
        bGetMoney.Text = $@"{moneyPerClick} $";
        for (int i = 0; i < purchasableItems.Length; i++)
        {
            purchasableItems[i] = new Item((long) Math.Pow(2, i + 1), (long) (Math.Pow(2, i + 2)*10), flpShop,
                OnShopItemClickableMouseDown, OnShopItemMouseUp);
        }
        UpdateItemAccessibility();
    }

    private void OnShopItemMouseUp(object s, EventArgs e, Item item)
    {
        stop = true;
        UpdateItemAccessibility();
    }

    private void OnShopItemClickableMouseDown(object s, EventArgs e, Item item)
    {
        stop = false;
        countThread = new Thread(() =>
        {
            while (!stop && currentMoney >= item.RequiredMoney)
            {
                if (lbCurrentMoney.InvokeRequired && bGetMoney.InvokeRequired)
                {
                    Invoke(new MethodInvoker(() =>
                    {
                        currentMoney -= item.RequiredMoney;
                        moneyPerClick += item.DollarsPerClick;
                    }));
                }
                Thread.Sleep(100);
            }
        });
        countThread.Start();
    }

    private void bGetMoney_Click(object sender, EventArgs e)
    {
        currentMoney += moneyPerClick;
        UpdateItemAccessibility();
    }

    private void UpdateItemAccessibility()
    {
        foreach (Item item in purchasableItems)
        {
            item.UpdateAccessibility(currentMoney);
        }
    }
}

Item

public class Item
{
    public delegate void ShopItemEvent(object sender, EventArgs e, Item item);

    public long DollarsPerClick { get; }
    public long RequiredMoney { get; }

    private Button buttonActivator;

    public Item(long dollarsPerClick, long requiredMoney, Control parent, ShopItemEvent onMouseDown, ShopItemEvent onMouseUp)
    {
        DollarsPerClick = dollarsPerClick;
        RequiredMoney = requiredMoney;
        CreateBuyButton(parent, onMouseDown, onMouseUp);
    }

    private void CreateBuyButton(Control parent, ShopItemEvent onMouseDown, ShopItemEvent onMouseUp)
    {
        buttonActivator = new Button
        {
            Text = $@"{DollarsPerClick} $ per click." + Environment.NewLine +
                   $@"Cost : {RequiredMoney}",
            Size = new Size(parent.Width - 10, 40),
            Enabled = false
        };
        buttonActivator.MouseDown += (sender, EventArgs) => onMouseDown(sender, EventArgs, this);
        buttonActivator.MouseUp += (sender, EventArgs) => onMouseUp(sender, EventArgs, this);
        parent.Controls.Add(buttonActivator);
    }

    public void UpdateAccessibility(long currentMoney)
    {
        buttonActivator.Enabled = currentMoney >= RequiredMoney;
    }
}

The code is quite short but I think that there are plenty of stuff to improve.

private long _currentMoney = 0;
private long currentMoney
{
    get { return _currentMoney; }
    set
    {
        _currentMoney = value;
        lbCurrentMoney.Text = $@"{currentMoney} $";
    }
}

Looks like the only reason the private properties in this class exist is to update the text of some labels. While this isn't "wrong", it feels awkward, like you're only using the property construct for a convenient side effect. Personally I would rather create a method that updates the private field and sets the label's text, I feel that would be a more appropriate tool for this scenario.

  $@"{currentMoney} $"

You're using the @ symbol for every string. It just adds one more character for my brain to parse and wonder what it's escaping. Don't use it unless it actually makes a difference.

  private Thread countThread = null;

There's no reason (shown in the code, at least) for this to be a member field rather than a local in the single method that actually uses it.

private bool stop = false;
...
while (!stop && currentMoney >= item.RequiredMoney) // executed in another thread

stop should be marked volatile, otherwise the read of the stop variable in the loop could be hoisted out of the loop and only read once, rather than on each iteration.

public MainGame()
{
    InitializeComponent();
    Initialize();
}

Second method could use a better name; the first one is initializing the "component", the other is initializing... what? (It would be fine if only an Initializing method was being called, but with both I think it's confusing.)

OnShopItemMouseUp(...)
OnShopItemClickableMouseDown(...)

Those two methods could use better names too. Normally I'm fine with "OnXEvent" names, but these aren't actually being subscribed to the events... the Item class shows an anonymous function is subscribed to each event, which in turn calls these functions. Therefore I think these should be named for what they do, rather than for when they're called.

private void OnShopItemMouseUp(object s, EventArgs e, Item item)
private void OnShopItemClickableMouseDown(object s, EventArgs e, Item item)

You're not actually using the s or e parameters anywhere in these functions, and they don't need to fit any existing delegate signature (apart from your own which you can change) since they're not subscribed directly to the events, so it's pointless to have them. Remove both and leave just the Item.

private void OnShopItemClickableMouseDown(object s, EventArgs e, Item item)
{
    stop = false;
    countThread = new Thread(() =>
    {
        while (!stop && currentMoney >= item.RequiredMoney)
        {
            if (lbCurrentMoney.InvokeRequired && bGetMoney.InvokeRequired)
            {
                Invoke(new MethodInvoker(() =>
                {
                    currentMoney -= item.RequiredMoney;
                    moneyPerClick += item.DollarsPerClick;
                }));
            }
            Thread.Sleep(100);
        }
    });
    countThread.Start();
}

This is called every time some button is clicked, right? So you create a thread, then another, then another... each spends nearly 100% of its life sleeping. Use Tasks instead. And don't sleep on the thread - it means you'll have potentially hundreds of threads using memory but doing absolutely nothing. Use Timers instead, have them fire a Task on the thread pool every 100 ms, and probably a single thread pool thread can service all the requests.

  if (lbCurrentMoney.InvokeRequired && bGetMoney.InvokeRequired)

Never used that before but as far as I can tell, it tells you whether you can update the control on the current thread. Given that this code ALWAYS executes on a thread which cannot update the controls directly, that check is redundant. Also, if it turned out that you don't have to invoke on the UI thread, you're doing nothing - presumably you should be updating the controls from the current thread, right?

  public delegate void ShopItemEvent(object sender, EventArgs e, Item item);

As stated earlier, you may as well remove the object/EventArgs parameters since you don't use them. I know people tend to like having a sender and event args on events but rarely see them used outside actual UI-generated events, so don't see the point.

  public Item(long dollarsPerClick, long requiredMoney, ...

Is money in dollars? Then be consistent in naming: requiredDollars (or possibly costInDollars or dollarCost since it appears to relate to the cost?).

    buttonActivator.MouseDown += (sender, EventArgs) => onMouseDown(sender, EventArgs, this);
    buttonActivator.MouseUp += (sender, EventArgs) => onMouseUp(sender, EventArgs, this);

EventArgs should be camelCase since it's the name of a parameter.

Here is an example of using Timers (from System.Threading, not any other namespace where timers may exist) rather than Threads (not the best but I don't really know how everything is called and used, so a very basic example):

private List<Timer> timers = new List<Timer>();
private readonly object syncLock = new object();
...
private void OnX(Item item)
{
    Timer timer = null;
    timer = new Timer(x =>
    {
        if (!stop && currentMoney >= item.RequiredMoney)
        {
            Invoke(new MethodInvoker(() =>
            {
                currentMoney -= item.RequiredMoney;
                moneyPerClick += item.DollarsPerClick;
            }));
        }
        else
        {
            timer.Dispose();
            lock (syncLock)
                timers.Remove(timer);
        }
    });

    lock (syncLock)
        timers.Add(timer);

    timer.Change(0, 100);
}

Basically this creates a Timer, gives it a delegate to run, and starts it (Change(0 /*Start immediately*/, 100 /*recur every 100 ms*/)). The timer executes the delegate on a thread pool thread, i.e. one of the threads that are always running in the background, waiting for jobs to do. The timer is added to a list, because the timer would otherwise get garbage collected, which would stop it. Adding it to the list keeps it rooted. When the timer should no longer run the delegate, the code that the Timer started turns around and disposes the Timer, and removes it from the list so it can get GC'd. Both the add and remove are locked for the sake of thread safety.