Event-triggering - Ready, On, Fire I have implemented a kind of thing that does event triggering in Golang. This is open-sourced here. Review and suggestions appreciated.

First thing I am trying to implement is adding event listeners to a trigger:

func (t *trigger) On(event string, task interface{}) error {
    if _, ok := t.functionMap[event]; ok {
        return errors.New("event already defined")
    }
    if reflect.ValueOf(task).Type().Kind() != reflect.Func {
        return errors.New("task is not a function")
    }
    t.mu.Lock()
    defer t.mu.Unlock()
    t.functionMap[event] = task
    return nil
 }

So that when Fire is called all the handler chain will be executed:

func (t *trigger) Fire(event string, params ...interface{}) ([]reflect.Value, error) {
    f, in, err := t.read(event, params...)
    if err != nil {
        return nil, err
    }
    result := f.Call(in)
    return result, nil
 }

But the thing is that don't want to maintain a whole param array for all the handlers' method parameters. How can it be done otherwise?

Your description of the problem indicates that each event type can have multiple listeners, but the code only implements one listener for each event. Is this intended?

The params awkwardness is a "feature" of go....

Because Go is

statically compiled
does not have function signature overloading (you can't have multiple functions with the same name, and differentiate them based on the signature)

You cannot avoid having the param input arguments to the Fire function. This is a "drawback" of Go. When you become more comfortable with the language you avoid constructs like you have, because they are hard to maintain (anything requiring reflect is probably a bad idea).

Additionally, your sync usage is broken - you need to manage the lock on both the Fire call, and the On call. Your On call is also broken because it has a race condition....

Note that reads on the map also need to be sync'd, not just writes.

So, this code:

func (t *trigger) On(event string, task interface{}) error {
    if _, ok := t.functionMap[event]; ok {
        return errors.New("event already defined")
    }
    if reflect.ValueOf(task).Type().Kind() != reflect.Func {
        return errors.New("task is not a function")
    }
    t.mu.Lock()
    defer t.mu.Unlock()
    t.functionMap[event] = task
    return nil
 }

should be:

func (t *trigger) On(event string, task interface{}) error {
    // Lock the mutex before reading it.
    t.mu.Lock()
    defer t.mu.Unlock()

    if _, ok := t.functionMap[event]; ok {
        return errors.New("event already defined")
    }
    if reflect.ValueOf(task).Type().Kind() != reflect.Func {
        return errors.New("task is not a function")
    }
    t.functionMap[event] = task
    return nil
 }

Additionally, the Fire should also have a mutex control (perhaps the t.read(...) function has a lock?)

func (t *trigger) Fire(event string, params ...interface{}) ([]reflect.Value, error) {
    f, in, err := t.read(event, params...)
    if err != nil {
        return nil, err
    }
    result := f.Call(in)
    return result, nil
 }


2D Collision Detection in C++ I'm attempting to rewrite the classic snake game in c++. What I am inquiring about is my implementation for a 2D collision detection function. I am utilizing SFML to create the window and shapes. The two objects that I am checking are squares of the same size. I would really appreciate any feedback that is given, as I'm still a beginner when it comes to c++.

This function works as intended. I'm looking for performance improvements, and general c++ practices that I should follow. If I need to add any more code, please let me know.

Game.h

#include <SFML/Graphics.hpp>
#include <SFML/System.hpp>
#include <SFML/Window.hpp>
#include <SFML/Audio.hpp>
#include <SFML/Network.hpp>

class Game {

    // Some code omitted for brevity //

    private:
        sf::RectangleShape enemy;
        sf::RectangleShape player;

    public:
        const bool isColliding(sf::RectangleShape player, sf::RectangleShape enemy) const;

    // Some code omitted for brevity //

};

Game.cpp

/*
    Determines if the player is colliding with the enemy.

    @param sf::RectangleShape player - Player object.
    @param sf::RectangleShape enemy - Enemy object.

    @return bool - True if colliding, False otherwise.
*/
const bool Game::isColliding(sf::RectangleShape player, sf::RectangleShape enemy) const {

    sf::Vector2f playerPosition = this->player.getPosition();
    sf::Vector2f enemyPosition = this->enemy.getPosition();

    float playerWidth = this->player.getSize().x;
    float playerHeight = this->player.getSize().y;

    return (
        ((playerPosition.x + playerWidth) > enemyPosition.x && (playerPosition.x - playerWidth) < enemyPosition.x) &&
        ((playerPosition.y + playerHeight) > enemyPosition.y && (playerPosition.y - playerHeight) < enemyPosition.y)
    );

}

const bool Game::isColliding(...

This indicates that the returned type is const, and should not be modified by any part of your code. But it's not a reference, so it doesn't make sense. It should compile, but it's a little confusing. Removing that const is clearer.

see this SO question for more information on that.

Your parameters to isColliding should be const references:

bool Game::isColliding(const sf::RectangleShape& player, const sf::RectangleShape& enemy) const

because you don't modify them, and references will prevent each object form being copied.

You never use the player parameter that you pass to the isColliding function. this->player is not the same as player in this context. It seems like you want to check the Game's current player object against collisions with arbitrary objects. You either

Make the function static (so it doesn't rely on a particular Game state) and pass two arbitrary objects to be checked for collision, or
Change the function name to something that indicates it will only be used with the player object.

One of:

static bool Game::isColliding(const sf::RectangleShape& a, const sf::RectangleShape& b)

or

bool Game::isPlayerColliding(const sf::RectangleShape& other) const

These Vector2d assignments can be made into const references to prevent copy construction.

    const sf::Vector2f& playerPosition = this->player.getPosition();
    const sf::Vector2f& enemyPosition = this->enemy.getPosition();


Random winner selector I am looking to improve and simplify the way I code. What is the best way to represent this:

let names = [];
let name;

while(true) {
 name = prompt('Enter a name:');
  names.push(name);

  if(name === ''){
   names.pop();
   break;
  }
}

let winner = function(){
 return names[Math.floor(Math.random() * names.length)];
}

console.log(winner());

There is not much to be done about the random selector.  However, I would make the function accept an explicit parameter.

That input loop, though, is too complicated. It also fails to handle the "Cancel" button in any way.

function winner(names) {
    return names[Math.floor(Math.random() * names.length)];
}

let names = [];
for (let name; name = prompt('Enter a name:'); ) {
    names.push(name);
}
console.log(winner(names));


Unit test for a method that populates a dictionary from an API call I am building a unit test for a method that populates a dictionary from an API call (responding with JSON):

<TestMethod()> Public Sub GetListOfBadKPI_EmptyResponse_ReturnsEmptyDict()
        'Arrange
        Const scheduleName As String = "testSchedule"

        Const emptyResponseJSON As String = "{""data"":[{""r"":""N""}]}"
        Dim fakeResult As Data = JsonConvert.DeserializeObject(Of Data)(emptyResponseJSON)
        Dim fakeServer As MockServer = New MockServer With {.ReturnData = fakeResult}

        Dim CalculationSchedule As CalculationSchedule = New CalculationSchedule(scheduleName, fakeServer)

        'Act
        Dim resultDictionary As Dictionary(Of String, Integer) =
        CalculationSchedule.GetDictionaryOfBadCalculation()

        'Assert
        Assert.IsTrue(resultDictionary.Count = 0)
    End Sub

The fake server is used to return any data I need for the test, without actually connecting with the API:

Public Class MockServer
    Implements IServer

    Public Property ReturnData As Data

    Public Function ExecuteQuery(query As String, Optional server As String = Nothing) As Data Implements IServer.ExecuteSQLQuery
        Return ReturnData
    End Function
End Class

This setup allows me to simulate API responses to test different cases.

However, I am not happy with the way it is done, mainly since JSON strings have a lot of quotes " and dealing with those in VB.NET is daunting, since you have to double them up, it makes copy-pasting difficult, and so on.

Moreover, it does not feel very 'professional' to do it like this.

How could I improve this unit test?

There is a better way than using a string, there is a Namespace that you can import called Json or System.Json

after you have imported this you have access to JsonValue Arrays which allows you to nest JavaScript Objects creating a JSON Document.

it actually looks like you are already using the Namespace to deserialize the string into an object, not sure why you aren't just creating the JSON Objects themselves. they are just collections of KeyValuePairs of Of String, JsonValue  (Dictionaries, Collections, or Enumerables)


Constructor for a Time object I am trying to write a constructor method that takes dd, hh, mm and ss as parameters and change it as a normal type of Time struct (e.g you enter 25 hours as the hh parameter, it changes it to 1 day and 1 hour).

struct Time
{
    public Time(int dd, int hh, int mm, int ss)
    {
        seconds= ss % 60;
        minutes = mm % 60;
        hours = hh % 24;
        days = dd % 7;
        if (ss > 60)
        {
            minutes++;
        }
        if (mm > 60)
        {
            hours++;
        }
        if (hh > 24)
        {
            days++;
        }
    }
    private int days, hours, minutes, seconds;
    public void Write()
    {
        Console.WriteLine("It has been " + days + " days, " + hours + " hours, " + minutes + " minutes, " + seconds + " seconds.");
    }
}
class Program
{
    static void Main(string[] args)
    {
        Time t = new Time(5, 28, 65, 55);
        t.Write();

        Console.ReadKey();
    }
}

The program works, but it feels very simple and amateurish. Is there anything I could do to make it more robust?

I can see several issues in your code:

Days
I guess the row with code days = dd % 7; is some historic rest of code when converting to weeks (?).

Boundaries
If you call Time(0, 0, 0, 60), the condition if (ss > 60) is not true and the addition minutes++; is not executed. You should use if (ss >= 60). The same error is repeated in other if statements.

Incorrect additions
Try to call your constructor with Time(0, 0, 0, 86400). If my understanding is correct, you should get 1 day, but you get only 1 minute.

The correct logic should be something like this:
public Time(int dd, int hh, int mm, int ss)
{
    seconds = ss % 60;
    minutes = mm % 60;
    hours = hh % 24;
    days = dd;

    if (ss >= 60)
    {
        minutes = mm + ss / 60;
    }
    if (minutes >= 60)
    {
        hours = hh + minutes / 60;
        minutes %= 60;
    }
    if (hours >= 24)
    {
        days = dd + hours / 24;
        hours %= 24;
    }
}

But, as Jamal mentioned, the constructor is for setting data members and other logic should be splitted into other method(s). That's why my example is not perfect from the design point of view. I also used your variable names in my example. I can imagine better names for them.


Attempting to make my properties an enum based singleton I was attempting to implement my first singleton using an enum, was a bit of a struggle for me and I'm not sure if its right and more importantly thread safe?

Property provider enum:

public enum PropertyProvider implements CanProvideProperties {

    INSTANCE;
    private final HashMap<String, String> properties;

    PropertyProvider() {
        properties = buildSettingsMap();
    }

    @Override
    public HashMap<String, String> getAllProperties() {
        return properties;
    }

    @Override
    public String getProperty(Property prop) {
        return properties.get(prop.toString());
    }

    @Override
    public HashMap<String, String> buildSettingsMap() {
        HashMap<String, String> temp = new HashMap<>();
        Properties prop = new Properties();
        try {
            prop.load(readFile("src/test/resources/framework.properties"));
        } catch (IOException ignore) {
            throw new RuntimeException("Framework properties file non accessible");
        }

        Enumeration<Object> KeyValues = prop.keys();
        while (KeyValues.hasMoreElements()) {
            String key = (String) KeyValues.nextElement();
            String value = prop.getProperty(key);
            temp.put(key, System.getProperty(key, value));
        }
        return temp;
    }

    @Override
    public FileInputStream readFile(String file) throws FileNotFoundException {
        return new FileInputStream(new File(file));
    }

}

Which implements the following interface:

public interface CanProvideProperties {
    String getProperty(Property prop);
    HashMap<String, String> getAllProperties();
    HashMap<String, String> buildSettingsMap();
    FileInputStream readFile(String file) throws FileNotFoundException;

}

and lastly another enum to hold some settings:

public enum Property {
    BROWSER {
        @Override
        public String toString() {
            return "browser";
        }
    },

    TESTENVIRONMENT {
        @Override
        public String toString() {
            return "env.baseurl";
        }
    },

    GRIDADDRESS {
        @Override
        public String toString() {
            return "grid.endpoint";
        }
    },

    LOCALORGRID    {
        @Override
        public String toString() {
        return "grid.or.local";
    }
    },

    PLATFORM {
        @Override
        public String toString() {
            return "platform";
        }
    },

    LANGUAGE {
        @Override
        public String toString() {
            return "application.language";
        }
    },

    PRODUCTNAME {
        @Override
        public String toString() {
            return "product.name";
        }
    },

    DATABASEURL {
        @Override
        public String toString() {
            return "database.url";
        }
    },

    BASEADMINACCOUNT {
        @Override
        public String toString() {
            return "base.admin.account";
        }
    },

    BASEADMINUSERNAME {
        @Override
        public String toString() {
            return "base.admin.username";
        }
    },

    BASEADMINPASSWORD {
        @Override
        public String toString() {
            return "base.admin.password";
        }
    },
}

Property-enum

An enum can have a constructor, allowing us to shorten the code to setup all the enum values.

public enum Property {
    BROWSER("browser"),
    TESTENVIRONMENT("env.baseurl"),
    ... // add more here.
    ;

    private final String key;

    Property(String key) {
        this.key = key;
    }

    public String getKey() {
        return this.key;
    }

    @Override public String toString() {
        return getKey();
    }
}

Do not have any logic based on toString. That method is mainly meant for debugging purposes. Using a getKey() instead (as above) allows you to use getKey() for your application logic.

Is a singleton really necessary?

Ask yourself if you really really NEED a singleton (Hint: Answer is most likely "no"). You might be able to use dependency injection to pass the object to the classes that needs it.

You might even not need a single instance, is there anything wrong with having multiple instances? All it does is read from a resource file, so worst case scenario is that you might spend a few nanoseconds reading a resource file from different places?


Secure RSA encryption with PyCrypto I'm coding a very simple reverse shell in python, and I want to encrypt the communication between the server and the client. The idea is to exchange an AES key using RSA then use it to encrypt everything sent on the network.

Right now I'm trying to use PyCrypto's Crypto.Cipher.PKCS1_v1_5 module for RSA and it seems to work, but since I'm no expert in cryptography (I know the maths behind RSA but thats about it) I have no idea whether this is secure or not. The test code:

from Crypto.PublicKey import RSA
from Crypto.Cipher import PKCS1_v1_5

msg = 'attack at dawn'
key = RSA.generate(4096)    # I know this is a huge overkill
pubkey = RSA.importKey(key.publickey().exportKey('DER'))
privkey = RSA.importKey(key.exportKey('DER'))
cipher = PKCS1_v1_5.new(pubkey)
ciphertext = cipher.encrypt(msg)
print ciphertext

dcipher = PKCS1_v1_5.new(privkey)
secret = dcipher.decrypt(ciphertext, 'thisIsForVerificationIfIAmRight')
print secret

The code works as expected. My questions:

Is this a correct way to use this module?
Is this secure?

I'm afraid that your second question ("Is this secure?") is one that you're not likely to get a good answer on here at Code Review. What you seem to be trying to do here is to design a cryptographic protocol, and cryptanalysis requires very specific expertise that I don't think any of the regulars here possess. I certainly don't. Also, to comment on the security of a protocol we have to see the whole protocol. The code in the question doesn't seem to have any defence against man-in-the-middle attacks, or any message authentication. But no doubt you plan to handle those problems elsewhere in your protocol.

However, I do have a couple of comments based on what you're shown us here:

The documentation for PKCS#1 v1.5 says, "If you are designing a new protocol, consider using the more robust PKCS#1 OAEP." Did you consider this?
The code says that the sentinel argument to PKCS115_Cipher.decrypt "is for verification", but in fact you are specifically warned in the documentation not to use it for verification: "you should not explicitly check if the returned value is the sentinel or not".

The usual advice when it comes to designing cryptographic protocols is "don't do it—use an industry standard instead". So in your use case ("encrypt the communication between the server and the client") I would use the TLS (Transport Layer Security) protocol via the built-in ssl module.


Python Document Downloader This is a Python document (PDF) downloader I made to download some question papers automatically. However, it is very slow. Any better way to do this?

try:
    import urllib2
    from bs4 import BeautifulSoup
    from time import sleep
    import os
    import sys
    from time import sleep
except ImportError:
    print "ImportError !"

url = raw_input('enter the url :')
c = 0

while True:
    path = raw_input("\nGive the directory Name where to save the files with full path :")
    if c == 3:
        if raw_input("you have tried 3 times do you want to create the folder ? (y/n)").lower().startswith('y'):
            os.mkdir(path)
            c = 0
            break
    else:
        if os.path.isdir(path):
            break
        else:
            print "\nEnter Path Carefully !\n"
            c+=1
            continue

headers = {'User-Agent': 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.106 Safari/537.36'}

request = urllib2.Request(url,None,headers)
html = urllib2.urlopen(request)
soup = BeautifulSoup(html.read(), "html.parser")
soup.prettify()

#modifying the url for concatonation with
url =  url.split('/')
del url[-1]
url.append('')
url = '/'.join(url)
urls = []
names = []

for anchor in soup.findAll('a', href=True): #Going inside links
    req = urllib2.Request(anchor['href'],None,headers)
    pag = urllib2.urlopen(req)
    pgf = pag.read()
    soup1 = BeautifulSoup(pgf, "html.parser")

    for i, link in enumerate(soup1.findAll('a')): # Download all pdf inside each link
        fullurl = url + str(link.get('href'))
        #print fullurl

        if fullurl.endswith('.pdf'):
            #print fullurl
            urls.append(fullurl)
            names.append(soup1.select('a')[i].attrs['href'])

names_urls = zip(names, urls)
os.chdir(path)

for name, url1 in names_urls:
    try:
        if os.path.isfile(name):
            print name, "Already exists skipping..."
            pass
        else:
            rq = urllib2.Request(url1)
            res = urllib2.urlopen(rq)
            with open(name, 'wb') as pdf:
                pdf.write(res.read())
            print name, "Downloaded"
    except:
        print name, "Failed to download"
        pass
    sleep(1)

When you encounter an ImportError it does not really help you debug the problem if you ignore it, just print the string "Import Error" and then fail at some random later point where that module is actually needed. This will generate a lot more error messages which are less clear than the normal ImportError!

Think about using argparse for your command line arguments. This will not allow you the shenanigans of asking three times for an existing directory name before offering to create it, but you can just add a flag for this:

import argparse

parser = argparse.ArgumentParser()
parser.add_argument("url", help="The base page where to search for PDF files.")
parser.add_argument("path", help="Where to save the files")
parser.add_argument("--create-directory", "-p", action="store_true", help="Create the directory if it does not exist")
locals().update(vars(parser.parse_args()))
if not os.path.isdir(path):
    if create_directory:
        os.makedirs(path)
    else:
        raise OSError("Directory {} does not exist. Use the '-p' flag to create it".format(path))

I used a trick here to update the locals dictionary, which is just the variables in the local scope. This way the variable url and path exist and have the right value. This magic works only outside of a function, inside you can use:

import argparse

parser = argparse.ArgumentParser()
parser.add_argument("url", help="The base page where to search for PDF files.")
parser.add_argument("path", help="Where to save the files")
parser.add_argument("--create-directory", "-p", action="store_true", help="Create the directory if it does not exist")
args = parser.parse_args()
if not os.path.isdir(args.path):
    if create_directory:
        os.makedirs(args.path)
    else:
        raise OSError("Directory {} does not exist. Use the '-p' flag to create it".format(args.path))

Use this argument parser by calling your script like:

$ python pdf_downloader.py http://url.to/pdfs.html path/to/save/files/to/

If you don't enter the two required arguments, an automatically generated usage message will be displayed:

$ python pdf_download.py
usage: pdf_download.py [-h] [-p] url path
pdf_download.py: error: too few arguments

$ python pdf_download.py --help
usage: pdf_download.py [-h] [-p] url path

positional arguments:
  url         The base page where to search for PDF files.
  path        Where to save the files

optional arguments:
  -h, --help              show this help message and exit
  -p, --create-directory  Create the directory if it does not exist

Interlude: Python has an official style guide, PEP8. One of the things it recommends is using a blank after a comma in a(n) (argument-)list, like (1, 2, 3). Another thing is to use lower_case for variable and function names. In all of the below code I fixed this.

Your url manipulation is probably better written as

base_url = '/'.join(url.split('/')[:-1])

And the separating trailing '/' can be added when actually building the full url. I would also put it into a new variable, just to make it clearer what this is.

Even better would be to just use the built-in os.path.dirname, since you already imported os:

base_url = os.path.dirname(url)

For the full url I would use str.format, which makes it a bit easier to read and which is also faster than multiple string additions (even though for one addition it does not matter).

full_url = "{}/{}".format(base_url, link.get('href'))

Instead of separately saving the urls to download and their names, only to zip it first and then iterate over the zipped container, just save the two together right away:

    if full_url.endswith('.pdf'):
        # print fullurl
        name = soup1.select('a')[i].attrs['href']
        urls.append((name, full_url))

pass is not what you think it is. It is a no-op that does nothing (you can use it as placeholder when you need some code block there because of indentation but don't have the code yet). What you mean is continue which directly goes to the next iteration of the loop.

You should never have a bare except clause. This will also catch e.g. CTRL-C if you run into an infinite loop and want to abort it! Use at least except Exception or better, figure out the actual exceptions you code would raise.

Also try to limit the scope of the except to the one call that might fail, if possible. At leas use except Exception as e and print the exception afterwards. Otherwise this information is lost.

Figuring out which urls to download and actually downloading them are two distinct tasks, which are ideal for functions. In addition I would add a main function for the rest and guard it with a if __name__ == "__main__": clause. The latter allows you to do in another script to just do from pdf_download import get_urls and reuse that function.

This results in the final code:

import urllib2
from bs4 import BeautifulSoup
from time import sleep
import os
import sys
import argparse

HEADERS = {'User-Agent': 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.106 Safari/537.36'}

def get_urls(url):
    request = urllib2.Request(url, None, HEADERS)
    soup = BeautifulSoup(urllib2.urlopen(request).read(), "html.parser")
    soup.prettify()

    urls = []
    base_url = os.path.dirname(url)
    for anchor in soup.findAll('a', href=True): #Going inside links
        req = urllib2.Request(anchor['href'], None, HEADERS)
        soup1 = BeautifulSoup(urllib2.urlopen(req).read(), "html.parser")

        for i, link in enumerate(soup1.findAll('a')): # Download all pdf inside each link
            full_url = "{}/{}".format(base_url, link.get('href'))
            if full_url.endswith('.pdf'):
                name = soup1.select('a')[i].attrs['href']
                urls.append((name, full_url))
    return urls

def download(urls, path):
    old_dir = os.getcwd()
    os.chdir(path)
    for name, url in urls:
        if os.path.isfile(name):
            print name, "already exists, skipping..."
            continue
        try:
            request = urllib2.Request(url)
            res = urllib2.urlopen(request).read()
            with open(name, 'wb') as pdf:
                pdf.write(res)
            print "Downloaded", name
        except Exception as e:
            print "Failed to download", name, ", because of", e
    os.chdir(old_dir)

def main():
    parser = argparse.ArgumentParser()
    parser.add_argument("url", help="The base page where to search for PDF files.")
    parser.add_argument("path", help="Where to save the files")
    parser.add_argument("--create-directory", "-p", action="store_true", help="Create the directory if it does not exist")
    args = parser.parse_args()

    if not os.path.isdir(args.path):
        if args.create_directory:
            os.makedirs(args.path)
        else:
            raise OSError("Directory {} does not exist. Use the '-p' flag to create it".format(args.path))

    download(get_urls(args.url), args.path)

if __name__ == "__main__":
    main()

Here I made some more cosmetic changes, like using proper grammar in the notifications ("Downloaded file_name" instead of "file_name Downloaded"), reverting back to the original working directory after downloading all files, removing unnecessary comments and made headers a constant, because it is used in multiple functions now. I also deleted the sleep because it is usually not necessary (unless there is some flood protection on the website, maybe).


Threadsafe get method on queue that draws values from other queues? I have a class that implements Queue and draws values from other queues which may still be referenced outwith it. I want my method to draw values from the contained queues, using synchronized locks on them to ensure thread safety with other code that uses synchronized locks on the queues.

The way I've tried to achieve this is by having my method loop through all values indefinitely, storing the value if it's the next one, updating the stored values if it reaches the same queue again and the variables the queues are being measured by have changed since the list iteration, and returning the next value of the queue if it's unchanged since the last time it was checked and evaluated to have the next value - my logic being that at that point, all queues have been checked and the currently stored value/queue was checked earliest and has been reconfirmed to be the next value.

Is this the ideal way to create a thread-safe version of this method? Or would there be a better way?

http://pastebin.com/TSkiJFh0

Collection<Queue<T>> memberQueues;
final Lambda<T, Comparable> keyGetter;

public T get(boolean remove)
{
    // workaround to ensure thread-safety when synchronized locks can't extend past the block they're declared in.

    // Work on a copy of memberQueues
    List<Queue<T>> memberQueuesCopy;

    synchronized(memberQueues)
    { memberQueuesCopy = new ArrayList<Queue<T>>(memberQueues); }

    // Declare variables and initialise with last member of memberQueuesCopy.
    // If it checks every variable and the last one is next, then I don't think I need to check again.
    Queue<T> nextQueue = memberQueuesCopy.get(memberQueuesCopy.size() - 1);
    T nextValue = nextQueue.peek();
    Comparable nextValueComparable = keyGetter.getMember(nextValue);

    // Check all members of memberQueuesCopy. Find the lowest and hold the value until it gets to it again incase
    // any values have changed. When it gets back to the current lowest value, check whether the value it's being
    // sorted by has changed - and if it has, use its new values and run through all members again. If it hasn't,
    // return it.

    // I want a less contrived method. Suggestions that maintain thread safety?
    for(;;)
    {
        for(Queue<T> i : memberQueuesCopy)
        {
            synchronized(i)
            {
                T iValue = i.peek();
                Comparable iComparable = keyGetter.getMember(iValue);

                if(i == nextQueue)
                {
                    if(nextValue == iValue && nextValueComparable.equals(iComparable))
                    {
                        if(remove)
                            return nextQueue.remove();
                        else
                            return iValue;
                    }

                    nextValue = nextQueue.peek();
                    nextValueComparable = keyGetter.getMember(nextValue);
                }
                else
                {
                    if(iComparable.compareTo(nextValueComparable) < 0)
                    {
                        nextQueue = i;
                        nextValue = iValue;
                        nextValueComparable = iComparable;
                    }
                }
            }
        }
    }
}

Code Style

Java Code Style puts the open-brace at the end of the line, not the start of the next line. For example, you have:

            if(i == nextQueue)
            {

but that should be:

            if(i == nextQueue) {

Variable conventions

i as a variable name is a great idea, if the variable is the control integer in a for loop. In your case, I presume it is short for 'item', or something, but, a Queue, being called i is unconventional.

As it happens, the letter q is perfect as a substitute....

Now, your nextQueue variable is actually the lastQueue odd.

Function extraction

With synchronization, return-balues from methods are often a great help for readibility. Consider this code you have:

// Work on a copy of memberQueues
List<Queue<T>> memberQueuesCopy;

synchronized(memberQueues)
{ memberQueuesCopy = new ArrayList<Queue<T>>(memberQueues); }

Which should really be written as:

// Work on a copy of memberQueues
List<Queue<T>> memberQueuesCopy;

synchronized(memberQueues) {
    memberQueuesCopy = new ArrayList<Queue<T>>(memberQueues);
}

would be even better if written as:

private final List<Queue<T>> copyQueues() {
    synchronized(memberQueues) {
        return new ArrayList<Queue<T>>(memberQueues);
    }
}

and then:

// Work on a copy of memberQueues
List<Queue<T>> memberQueuesCopy = copyQueues();

Bugs

There are three bugs I should point out:

NoSuchElementException if memberQeues is empty:

  Queue nextQueue = memberQueuesCopy.get(memberQueuesCopy.size() - 1);

(and bug 3) NullPointerException if any of the queues are empty (in some combinations) (one bug on iComparable, the other on nextValueComparable):

T nextValue = nextQueue.peek();
Comparable nextValueComparable = keyGetter.getMember(nextValue);

  ....

    T iValue = i.peek();
    Comparable iComparable = keyGetter.getMember(iValue);

  ....

    if(nextValue == iValue && nextValueComparable.equals(iComparable))

  ....

    if(iComparable.compareTo(nextValueComparable) < 0)


Calculations to reposition a logo I have this block of code which clearly looks like duplicate, but I don't know how to refactor.

    float offsetDifference;
            nfloat newLogoHeight;
            nfloat newHeaderHeight;
            nfloat newLogoWidth;

            if (offset > _previousOffset)
            {
                offsetDifference = offset - _previousOffset;

                newHeaderHeight = (HeaderViewHeightConstraint.Constant - offsetDifference);
                newLogoHeight = (LogoImageViewHeightConstraint.Constant - offsetDifference);
                newLogoWidth = (LogoImageViewWidthConstraint.Constant - offsetDifference);

                HeaderViewHeightConstraint.Constant = newHeaderHeight < headerViewMinHeight ? headerViewMinHeight : newHeaderHeight;
                LogoImageViewHeightConstraint.Constant = newLogoHeight < logoViewMinHeight ? logoViewMinHeight : newLogoHeight;
                LogoImageViewWidthConstraint.Constant = newLogoWidth < logoViewMinWidth ? logoViewMinWidth : newLogoWidth;
            }
            else
            {
                offsetDifference = _previousOffset - offset;

                newHeaderHeight = (HeaderViewHeightConstraint.Constant + offsetDifference);
                newLogoHeight = (LogoImageViewHeightConstraint.Constant + offsetDifference);
                newLogoWidth = (LogoImageViewWidthConstraint.Constant + offsetDifference);

                HeaderViewHeightConstraint.Constant = newHeaderHeight > headerViewMaxHeight ? headerViewMaxHeight : newHeaderHeight;
                LogoImageViewHeightConstraint.Constant = newLogoHeight > logoViewMaxHeight ? logoViewMaxHeight : newLogoHeight;
                LogoImageViewWidthConstraint.Constant = newLogoWidth > logoViewMaxWidth ? logoViewMaxWidth : newLogoWidth;
            }

            //HeaderViewHeightConstraint.Constant = (headerViewMaxHeight - offset) < logoViewMaxHeight ? logoViewMaxHeight : (headerViewMaxHeight - offset);
            //LogoImageViewHeightConstraint.Constant = (logoViewMaxHeight - offset) < 0 ? 0 : (logoViewMaxHeight - offset);
            _previousOffset = offset;

Basically, I receive an offset, and according to the previous offset (wether is smaller or bigger), I reorganize some other values.

The thing is, the calculation is almost the same, and I'd love a method that just does the calculation in which I can pass the parameters, but I don't know how to write the "common code" since all the calculations varies in the + and -, or < and >.

Anyway, you're seeing this as well as I am, but you probably know how to refactor it better than I do. How would you do it ? :)

Note : I've rewritten most of this from my phone, sry for the terrible code formatting, it's really impractical. Feel free to edit

If I take newHeaderHeight = HeaderViewHeightConstraint.Constant - offsetDifference and expand offsetDifference I get: newHeaderHeight = HeaderViewHeightConstraint.Constant - (offset - _previousOffset) which is newHeaderHeight = HeaderViewHeightConstraint.Constant - offset + _previousOffset

If I then go to the else statement and expand newHeaderHeight = (HeaderViewHeightConstraint.Constant + offsetDifference); I get newHeaderHeight = HeaderViewHeightConstraint.Constant + (_previousOffset - offset); which is newHeaderHeight = HeaderViewHeightConstraint.Constant + _previousOffset - offset;

They're the same thing. So you can basically remove them from the if/else statement and only write them once.

The second part is a bit more tricky to refactor because you compare it with min values in one condition and max values in the other. It would however still refactor each ternary operator statement with Math.Min() or Math.Max() (careful not to get them the other way round).


Looking cleaner code of Python Combining Image with PIL Please review and help me write better codes.
Combining Pictures
We can use a conditional to copy just the non-white pixels from one picture to another picture. We can take this tiny image of the women and put her by the Eiffel tower in Paris, France. I have modified the code so that the code automatically swaps the image1 and image2; hence, always using the larger image as the canvas and the smaller picture as the insert.
from PIL import Image

image1 = Image.open("lady_tiny.jpg")
image2 = Image.open("eiffel.jpg")

width = min(image1.size[0], image2.size[0])  # (x)
length = min(image1.size[1], image2.size[1])  # (y)

# check if smaller image then use it as insert
if width == image2.size[0] and length == image2.size[1]:
    canvas = image1.load()  # larger pic
    insert = image2.load()  # smaller pic
else:
    insert = image1.load()
    canvas = image2.load()

for col in range(width):
    for row in range(length):
        r, g, b = insert[col, row]  # read each pixel of insert(small)

        # Check if the pixel isn't white
        if r < 250 and g< 250 and b< 250:
            # Copy the color of insert (smaller) onto the canvas(larger)
            canvas[col, row + 130] = r, g, b

if (
    width == image2.size[0] and length == image2.size[1]
):  # if image2 is insert then image 1 is canvas
    image1.show()
else:
    image2.show()

You can simplify the checks that find the smallest image. If you assume that if the image is smaller in one dimension, it will be smaller in both (which you seem to be already), you can find the smallest image using min's key parameter:
small_image = min(image1, image2, key=lambda image: image.size[0])
large_image = image1 if image2 == small_image else image2

canvas = large_image.load()
insert = small_image.load()

The key function is applied to each image before they're compared. In this case, I'm comparing each of the image's size[0] values. To get the larger image, you could get creative with sets if images are hashable, but I think a conditional expression is probably easiest.
This also reduces your check at the bottom to just
large_image.show()

This part:
r, g, b = insert[col, row]

if r < 250 and g< 250 and b< 250:
        canvas[col, row + 130] = r, g, b

Can be simplified a bit if you use all with a generator expression:
color = insert[col, row]

if all(c < 250 for c in color):
    canvas[col, row + 130] = color

This frees you up from needing to create, compare, then recombine three separate variables.

Final code:
from PIL import Image

image1 = Image.open("lady_tiny.jpg")
image2 = Image.open("eiffel.jpg")

small_image = min(image1, image2, key=lambda image: image.size[0])
large_image = image1 if image2 == small_image else image2

canvas = large_image.load()
insert = small_image.load()

for col in range(width):
    for row in range(length):
        color = insert[col, row]

        if all(c < 250 for c in color):
            canvas[col, row + 130] = color

large_image.show()


Find common characters (LeetCode) The task
is taken from leetcode

  Given an array A of strings made only from lowercase letters, return a
  list of all characters that show up in all strings within the list
  (including duplicates).  For example, if a character occurs 3 times in
  all strings but not 4 times, you need to include that character three
  times in the final answer.

  You may return the answer in any order.

  Example 1:

  Input: ["bella","label","roller"]

  Output: ["e","l","l"]

  Example 2:

  Input: ["cool","lock","cook"]

  Output: ["c","o"]

  Note:

  1 <= A.length <= 100

  1 <= A[i].length <= 100

  A[i][j] is a lowercase letter

My imperative solution:

function commonChars(A) {
  const [first, ...rest] = A.sort((a,b) => -(a.length - b.length));
  const duplicates = [];
  [...first].forEach(e => {
    let isDuplicate = true;
    for (let x = 0, len = rest.length; x < len; x++) {
      let letters = rest[x];
      const i = letters.search(e);
      if (i !== -1) {
        letters = letters.slice(0, i) + letters.slice(i + 1);
        rest[x] = letters;
      } else {
        isDuplicate = false;
      }
    }
    if (isDuplicate) {
      duplicates.push(e);
    }
  });
  return duplicates;
}

const arr = ["cool","lock","cook"];
console.log(commonChars(arr));

My declarative solution

const commonChars2 = A => {
  const [first, ...rest] = A.sort((a,b) => b.length - a.length));
  return [...first].reduce((duplicates, e) => {
    let isDuplicate = true;
    for (let x = 0, len = rest.length; x < len; x++) {
      const letters = rest[x];
      const i = letters.search(e);
      if (i !== -1) {
        rest[x] = letters.slice(0, i) + letters.slice(i + 1);
      } else {
        isDuplicate = false;
      }
    }
    if (isDuplicate) { duplicates.push(e); }
    return duplicates;
  }, []);
};

const arr = ["cool","lock","cook"];
console.log(commonChars2(arr));

Is there also a good functional approach?

Opportunities missed

You have essentially the same algorithm in both examples. When I first looked at the solutions the sort at the start was a worry. The logic is sound, to work from the smallest word as there will be no more characters to test then is contained in that word.

However you have the sort AoT and the result is you set first to be the longest word in the array.

Changing the sort to be the other direction works great improving the performance near 3-4 times if there is a much shorter word in the array. However there is no real benefit with the sort if all the words are of similar length.

Another missed optimization is breaking out of the inner loop when you set isDuplicates to false. Adding a break saves having to do any further iteration and gives another 10% performance. BTW the name  duplicates would have just as much meaning.

String.search(arg) and String.indexOf(arg)

The real bottle neck is in the inner loop where you locate the current character e in the current word. letters.search(e)

It may look innocent but this is where knowing the language well come into play. String.search(arg) takes as the first argument a regExp but also works with just a string. The catch is that the string is first converted to a regExp before the search starts. This is a huge overhead.

If you use String.indexOf instead your code is almost twice as fast. I tried to write something that could beat it using Maps to reduce time complexity, and as such only starts to outperform when the word lengths get much longer 50+ characters.

Rewrite

So for small words your solution with slight modification is very fast.

Sort now from smallest to largest,
Breaks from inner loop when no duplicates found.
Uses faster String.indexOf to do inner character search.

And some name changes..

function commonCharsNo(words) {
    const [first, ...rest] = words.sort((a, b) => (a.length - b.length));
    const duplicates = [];
    [...first].forEach(e => {
        let duplicate = true;
        for (let x = 0, len = rest.length; x < len; x++) {
            let word = rest[x];
            const i = word.indexOf(e);
            if (i !== -1) {
                rest[x] = word.slice(0, i) + word.slice(i + 1);
            } else {
                duplicate = false;
                break;
            }
        }
        if (duplicate) {
            duplicates.push(e);
        }
    });
    return duplicates;
}

Reduce complexity

The search (now indexOf) is where the code gets poor complexity \$O(m)\$. Its \$O(n.m)\$ where \$n\$ is the number of characters in the smallest word and \$m\$ is the mean number of characters per word.

You can reduce this to approx \$O(w.m)\$ where \$w\$ is the number of words and \$m\$ is the mean number of characters per word.

The improvement is small and only pays out for longer words.

function findCommonChars(words) {
    var chars;
    const result = [];
    words = [...words.sort((a,b)=>b.length - a.length)];
    const countChars = (word, res = new Map()) => {
        for (const char of word) {
            res.has(char) && (!chars || chars.has(char)) ?
                res.get(char)[0]++ : res.set(char, [1]);
        }
        return res;
    }
    chars = countChars(words.pop());
    const charCounts = words.map(word => countChars(word));
    for (let [char, count] of chars.entries()) {
        for (const word of charCounts) {
            if (word.has(char)) { count = Math.min(count, word.get(char)[0]) }
            else {
                count = 0;
                break;
            }
        }
        while (count--) { result.push(char) }
    }
    return result;
}


Different implementation of an Interface along with additional parameters I am designing an interface for 2 distinct implementations as follows:

Perform operations without client credentials ( for my local applications)
Perform operations using security token. (for web applications)

Interfaces are as follows :

interface IContactService
{
    void Add(Contact c);
    void Update(Contact c);
}

interface IProxy
{
     string Token { get; set; }
     void SetToken(string token);
}

and below are the implementations :

public class ContactBusinessImpl : IContactService
{

    private IContactService _contactService;

    public ContactBusinessImpl(IContactService contactService) {
        _contactService = contactService;
    }

    public void Add(Contact c)
    {
        _contactService.Add(c);
    }

    public void Update(Contact c)
    {
        _contactService.Update(c);
    }
}

public class ContactBusinessProxyImpl : IContactService , IProxy
{
    public string Token { get; set; }

    public void SetToken(string token)
    {
        Token = token;
    }
    public void Add(Contact c)
    {
        var client = new HttpClient();
        client.SetBearerToken(Token);
        var stringContent = new StringContent(JsonConvert.SerializeObject(c), Encoding.UTF8, "application/json");
        client.PostAsync("api/add", stringContent);
    }

    public void Update(Contact c)
    {
        var client = new HttpClient();
        client.SetBearerToken(Token);
        var stringContent = new StringContent(JsonConvert.SerializeObject(c), Encoding.UTF8, "application/json");
        client.PostAsync("api/update", stringContent);
    }
}

Earlier I was using token as a parameter for each method (Add(Contact c,string token)) which was not acceptable.

Now I am okay with above approach, but it seems like without SetToken I cant do anything in proxy implementation.

Since I am using DI to instantiate classes for can't go for Adapter pattern like ContactBusinessProxyImpl(string token).

Is there any better approach or pattern can be used here?

Since I am using DI to instantiate classes

I have to disappoint you... there is no DI in your code. The only service you are using is instantiated inside the class using it:

private ContactService _contactService = new ContactService();

DI would be if you passed it to ContactBusinessImpl via constructor.

public string Token { get; set; }

public void SetToken(string token)
{
    Token = token;
}

We don't usually do that. A value is either a mandatory one, then we use a constructor so the user must provide it or it's optional so we create a get/set property for it and don't enforce it via constructor. Having a method that sets a property that has a setter is pointless.

public void Add(Contact c)
{
    var client = new HttpClient();
    client.SetBearerToken(Token);
    var stringContent = new StringContent(JsonConvert.SerializeObject(c), Encoding.UTF8,"application/json");
    client.PostAsync("api/add", stringContent);
}

Creating HttpClient on each call is a bad idea. At some point you'll run out of sockets.


Method for parsing floats and adding totals My intention is to create a nice DRY method (add) which will take care of parsing floats and adding totals internally and will be called by other methods.

I feel like I'm jumping through unnecessary complex hoops here. Can anyone help me out?

PS: the intention is to be able to specify different objects with add, so if I had a label object called 'food' with a credit property I could create a function called addFoodCredit(amount) which would call add( amount, 'credit', 'labels', 'food').

The full code can be seen here.

var dateObject = function() {
    var totals = {
        totalDebit: 0,
        totalCredit: 0,
        labels: {}
   }

    var add = function add( amount ) {
        var args = Array.prototype.slice.call(arguments)
        var objectReference = totals;
        var floatAmount = parseFloat (args.shift());
        var objectKey = args.shift();
        while(args.length > 0) {
          objectReference = objectReference[args.shift()];
        }
        var currentTotal = parseFloat( objectReference[objectKey] );
        objectReference[objectKey] = currentTotal + floatAmount;
        console.log(objectReference, objectKey);
        console.log(totals.totalDebit)
    };

    this.addCredit = function addCredit( amount ) {
        add( amount, 'totalCredit' );
    };

    this.addDebit = function addDebit ( amount ) {
        add( amount, 'totalDebit');
    };

    this.addLabel = function ( label, amount, type ) {
        var labelName = label + '-' + type;
        if ( ! totals.labels.labelName ) {
            totals.labels[labelName] = 0;
        }

        add(amount, labelName, 'labels')
    }

  this.print = function() {console.log(totals)};

};

var x = new dateObject();
x.addCredit(15);
x.addCredit(21);
x.addDebit(12.42);
x.addDebit(122);
x.addLabel('food', 20, 'dr')
x.print();

The add method is overly complex for no real benefit. It is hitting a thumbtack with a sledge hammer. The add method is used for waaaay to much. It's not clear what it does. I had to spend 5 minutes dissecting what it did. For an 11 line method, that's pretty long.

I understand you are trying to DRY up your code, but you have sacrificed clarity for perceived duplication of code. Adding to the credit and debit totals appears to be duplicated code, when actually it is not. You are better off removing the add method and going for something explicit. What you do want to DRY up is the conversions of amount values to numbers, and perhaps some error checking.

Defining this inside your dateObject class makes ensuring arguments are numbers a quick one-liner.

var toNumber = function(x) {
    var n = Number(x);

    if (isNaN(n)) {
        throw new Error("Invalid number: " + amount);
    }

    return n;
};

Now, your addCredit and addDebit methods become explicit and easy to read:

this.addCredit = function(amount) {
    totals.totalCredit += toNumber(amount);
};

this.addDebit = function(amount) {
    totals.totalDebit += toNumber(amount);
};

The addLabel method flat out confused me for a good while because it delegated to the add method. As a general rule, avoid turning methods into a "utility belt" where they perform more than one task. It makes for confusing code that is hard to maintain and debug.

The addLabel method (as far as I can tell) becomes much easier to visualize and debug as well:

this.addLabel = function(label, amount, type) {
    var key = label + "-" + type;
    totals.labels[key] = totals.labels[key] || 0;
    total.labels[key] += toNumber(amount);
};

And now on to every programmers' favorite subject: naming things :)

You have a "class" called dateObject. Naming conventions in JavaScript dictate that "classes", or more accurately constructor functions, should capitalize their first letter. Secondly, your dateObject class does nothing at all with dates, and is confusing because JavaScript has a Date class. Crediting and debiting amounts has nothing to do with dates. I realize you omitted some code but consider renaming this class to something like Account:

function Account() {
    var totals = {
        totalDebit: 0,
        totalCredit: 0,
        labels: {}
    };

    var toNumber = function(x) {
        var n = Number(x);

        if (isNaN(n)) {
            throw new Error("Invalid number: " + amount);
        }

        return n;
    };

    this.addCredit = function(amount) {
        totals.totalCredit += toNumber(amount);
    };

    this.addDebit = function(amount) {
        totals.totalDebit += toNumber(amount);
    };

    this.addLabel = function(label, amount, type) {
        var key = label + "-" + type;
        totals.labels[key] = totals.labels[key] || 0;
        total.labels[key] += toNumber(amount);
    };
}


Print a tic-tac-toe board I am currently learning Haskell and as an exercise, I am building a simple tic-tac-toe program. I start with
import Data.Vector

data Cell = Empty | X | O
type Board = Vector Cell

and then, to render the board:
render :: Board -> IO ()
render b = do
    renderSep
    renderRow 0 b
    renderSep
    renderRow 1 b
    renderSep
    renderRow 2 b
    renderSep
  where
    renderSep :: IO ()
    renderSep = putStrLn "+---+---+---+"
    renderRow :: Int -> Board -> IO ()
    renderRow r b = do
        putStr "| " >> putStr (cellRepr $ getCell r 0 b)
        putStr " | " >> putStr (cellRepr $ getCell r 1 b)
        putStr " | " >> putStr (cellRepr $ getCell r 2 b)
        putStrLn " |"

How could I abstract my implementation so that there's less repetition? Especially in the main body of the render function. I thought of using sequence together with the list fmap (\x -> renderRow x b >> renderSep) [0, 1, 2]. However, in this case I get a value of type IO [()] and not IO ().

I think a simple way to address the problem is by building the board first then printing it:
empty_row :: String
empty_row = "+---+---+---+"

board_repr :: type of b -> String
board_repr b = empty_row + concat ( map (\x -> (build_row x b) + empty_row ) [0, 1, 2] )

main = print (board_repr b)

Take this as pseudo-code pointing in an interesting direction by separating the concerns of building a representation and printing it.


Python List of Dictionaries to XLS How may I optimize the following function to write a xls file from a list of dictionaries using the openpyxl library in Python?

The current logic behind feels very very wrong

The dict list:

things = [
    {
        "Fruit": "Orange",
        "Flavour": "Good",
        "Expiration": "21May20"
    },
    {
        "Fruit": "Apple",
        "Flavour": "Good",
        "Expiration": "19May20"
    },
    {
        "Fruit": "Banana",
        "Flavour": "Regular",
        "Expiration": "16May20"
    }
]

The functions that I have:

from openpyxl import Workbook, load_workbook

def create_xls(filepath):
    wb = Workbook()
    wb.save(filepath)

def write_xls(filepath, dictionary):
    wb = load_workbook(filepath)
    sheet = wb.active

    headers = [x for x in dictionary[0]]

    for index, value in enumerate(headers):
        sheet.cell(row=1, column=index+1).value = value

    for i, x in enumerate(dictionary):
        for idx,value in enumerate(x.values()):
            sheet.cell(row=i+2, column=idx+1).value = value

    wb.save(filepath)

Thank you!

From the documentation, you can see that a worksheet object has an .append method that let you write a row from an iterable at the bottom of said sheet. Documentation from the builtin help is reproduced here:

Help on method append in module openpyxl.worksheet.worksheet:

append(iterable) method of openpyxl.worksheet.worksheet.Worksheet instance
    Appends a group of values at the bottom of the current sheet.

    * If it's a list: all values are added in order, starting from the first column
    * If it's a dict: values are assigned to the columns indicated by the keys (numbers or letters)

    :param iterable: list, range or generator, or dict containing values to append
    :type iterable: list|tuple|range|generator or dict

    Usage:

    * append(['This is A1', 'This is B1', 'This is C1'])
    * **or** append({'A' : 'This is A1', 'C' : 'This is C1'})
    * **or** append({1 : 'This is A1', 3 : 'This is C1'})

    :raise: TypeError when iterable is neither a list/tuple nor a dict

This means that your can sheet.append(headers) instead of your ugly loop. Similarly, using .values() on your dictionnaries, you can simplify your write_xls function to:

def write_xls(filepath, dictionary):
    wb = load_workbook(filepath)
    sheet = wb.active

    headers = list(dictionary[0])
    sheet.append(headers)

    for x in dictionary:
        sheet.append(list(x.values()))

    wb.save(filepath)

Now, a few more things to consider.

First off, since you are only interested in creating the file and writing in it, you may be interested in the write-only mode provided by openpyxl. This mean you will simplify your code to a single function:

def write_xls(filepath, dictionary):
    wb = Workbook(write_only=True)
    sheet = wb.create_sheet()

    headers = list(dictionary[0])
    sheet.append(headers)

    for x in dictionary:
        sheet.append(list(x.values()))

    wb.save(filepath)

Second, you relly very much on your data being presented well ordered and without flaws. This might bite you at some point. I would:

find all possible headers in your dictionnaries and order them;
use them to recreate each row using the same ordering each time.

This will allow you to have a coherent output, even with inputs such as:

things = [
    {
        "Fruit": "Orange",
        "Flavour": "Good",
        "Expiration": "21May20",
    },
    {
        "Flavour": "Good",
        "Fruit": "Apple",
        "Expiration": "19May20",
    },
    {
        "Flavour": "Regular",
        "Expiration": "16May20",
        "Fruit": "Banana",
    }
]

or even:

things = [
    {
        "Fruit": "Orange",
        "Flavour": "Good",
        "Expiration": "21May20"
    },
    {
        "Fruit": "Apple",
        "Flavour": "Good",
        "Junk": "Spam",
        "Expiration": "19May20"
    },
    {
        "Fruit": "Banana",
        "Flavour": "Regular",
        "Expiration": "16May20"
    }
]

Proposed improvements:

import itertools

from openpyxl import Workbook

def write_xls(filename, data):
    wb = Workbook(write_only=True)
    ws = wb.create_sheet()

    headers = list(set(itertools.chain.from_iterable(data)))
    ws.append(headers)

    for elements in data:
        ws.append([elements.get(h) for h in headers])

    wb.save(filename)


Creating MarkDown documents in ruby I'm trying to dynamically generate some MarkDown formatted documents, with header fields of arbitrary lengths.

I want to have a title of arbitrary length underlined like this
---------------------------------------------------------------

This should work to
-------------------

The code I came up with to do this in my ERB views.

<%= @title %>
<%= @title.length.times.map{"-"}.join %>

Is there a more efficient way to make this happen?

One way is to use multiplication. Multiplying a String by a Fixnum will produce a new String where the original String appears the Fixnum of times.

"-" * @title.length #=> "---------------"

Documentation for String#*

Warning: Make sure you put the String first or else you will get a TypeError: String can't be coerced into Fixnum since that would be calling Fixnum#* instead of String#*.


Finding overlapping time intervals You have two lists with meetings scheduling (start time, end time). Meetings in single list don't intersect. Find all intersecting meetings across the two lists.

Is my complexity \$O(N M)\$? If I use binary search I might get \$O(N \log M)\$. Can you think of a better algorithm?

For simplicity, I didn't use DateTime. I just assumed meetings are in round hours.

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

namespace ConsoleApplication2
{

    public class IntersectingMeetings
    {
        public IntersectingMeetings()
        {
            MeetingList meetingList1 = new MeetingList();
            MeetingList meetingList2 = new MeetingList();
            meetingList1.AddMeeting(2,3);
            meetingList1.AddMeeting(1,2);

            meetingList1.AddMeeting(4,6);

            meetingList2.AddMeeting(2,3);
            meetingList2.AddMeeting(3,4);
            meetingList2.AddMeeting(5,6);
            List<Meeting> intersectionsList = FindInterSections(meetingList1, meetingList2);
        }

        private List<Meeting> FindInterSections(MeetingList meetingList1,MeetingList meetingList2)
        {
            meetingList1.list.Sort();
            meetingList2.list.Sort();
            List<Meeting> intersectingList = new List<Meeting>();
            foreach (var item in meetingList1.list)
            {
                foreach (var other in meetingList2.list)
                {
                    if ((item.StartTime >= other.StartTime && item.EndTime <= other.EndTime) ||
                        (other.StartTime >=item.StartTime && other.EndTime <=item.EndTime))
                    {
                        intersectingList.Add(item);
                        intersectingList.Add(other);
                    }
                }
            }
            return intersectingList;
        }
    }

    public class MeetingList
    {
        public List<Meeting> list;
        public MeetingList()
        {
            list = new List<Meeting>();
        }
        public void AddMeeting(int start, int end)
        {
            list.Add(new Meeting(start,end));
        }
    }

    public class Meeting : IComparable<Meeting>
    {
        public int StartTime { set; get; }
        public int EndTime { set; get; }

        public Meeting(int start, int end)
        {
            StartTime = start;
            EndTime = end;
        }

        public int CompareTo(Meeting other)
        {
            if (StartTime > other.StartTime)
            {
                return 1;
            }
            else if (StartTime < other.StartTime)
            {
                return -1;
            }
            return 0;
        }
    }
}

This is one solution of the problem I have found.

An efficient approach is to first sort the intervals according to starting time. Once we have the sorted intervals, we can combine all intervals in a linear traversal. The idea is, in sorted array of intervals, if interval[i] doesn't overlap with interval[i-1], then interval[i+1] cannot overlap with interval[i-1] because starting time of interval[i+1] must be greater than or equal to interval[i]. Following is the detailed step-by-step algorithm.

You could make the Meeting class generic with the T : IComparable<T> constraint. Thus you'll be able to use any type as a time value:

[DebuggerDisplay("{Start} .. {End}")]
public sealed class Meeting<T>
    where T : IComparable<T>
{
    // You can replace these readonly fields with the auto-properties { get; private set; }:
    public readonly T Start;
    public readonly T End;

    public Meeting(T start, T end)
    {
        Start = start;
        End = end;
    }
}

Next you need a comparer:

public sealed class MeetingComparer<T> : IEqualityComparer<Meeting<T>>
    where T : IComparable<T>
{
    public bool Equals(Meeting<T> x, Meeting<T> y)
    {
        // We can use only 2 sequential comparisons to check for the intersection:
        return x.Start.CompareTo(y.Start) >= 0
            ? x.Start.CompareTo(y.End) <= 0
            : y.Start.CompareTo(x.End) <= 0;
    }

    public int GetHashCode(Meeting<T> obj)
    {
        // Make all meetings have the same hash code.
        // In this case the Equals method will be called for each object.
        return 1;
    }
}

And finally you could use LINQ to find the intersecting meetings:

private static IEnumerable<Meeting<T>> FindIntersections<T>(IEnumerable<Meeting<T>> list1,
    IEnumerable<Meeting<T>> list2)
    where T : IComparable<T>
{
    return list1.Join(list2, p => p, p => p,
        (a, b) => new[] { a, b }, new MeetingComparer<T>())
            .SelectMany(p => p);
}

Sample:

var list1 = new List<Meeting<int>>
                {
                    new Meeting<int>(0, 1),
                    new Meeting<int>(100, 111),
                    new Meeting<int>(20, 41),
                    new Meeting<int>(10, 11)
                };
var list2 = new List<Meeting<int>>
                {
                    new Meeting<int>(2, 3),
                    new Meeting<int>(105, 106),
                    new Meeting<int>(40, 41),
                    new Meeting<int>(15, 16)
                };

var intersection = FindIntersections(list1, list2);

EDIT
Try this code to find intersections of the sorted sets. It has complexity \$O(N+M)\$:

private static IEnumerable<Meeting<T>> FindIntersections<T>(IEnumerable<Meeting<T>> listX,
    IEnumerable<Meeting<T>> listY)
    where T : IComparable<T>
{
    var iteratorX = listX.GetEnumerator();
    var iteratorY = listY.GetEnumerator();
    Meeting<T> lastX = null;
    Meeting<T> lastY = null;
    iteratorX.MoveNext();
    iteratorY.MoveNext();
    while (iteratorX.Current != null && iteratorY.Current != null)
    {
        Meeting<T> x = iteratorX.Current;
        Meeting<T> y = iteratorY.Current;

        // Move iterators if needed:
        if (x.End.CompareTo(y.Start) < 0)
        {
            iteratorX.MoveNext();
            continue;
        }
        if (y.End.CompareTo(x.Start) < 0)
        {
            iteratorY.MoveNext();
            continue;
        }

        // Add items to the resulting set, if the items aren't already there:
        if (lastX != x)
        {
            yield return x;
            lastX = x;
        }
        if (lastY != y)
        {
            yield return y;
            lastY = y;
        }

        // Determine which iterator should be moved first:
        if (y.End.CompareTo(x.End) >= 0)
        {
            iteratorX.MoveNext();
        }
        else
        {
            iteratorY.MoveNext();
        }
    }
}

The method above:

Returns a set of the unique Meeting<T>s.
Supports case if one range overlaps several ranges in another set.


Avoiding redundant code in MVC view page Based on the true condition, I am making some operations.  But I want to simplify the below HTML code in a better way.

@{
    bool savingHtml = (Request.QueryString["savehtml"] == "1");
    string activeTab = Request.QueryString["from"];
}

@if (savingHtml)
{
    if (activeTab.ToLower() == "index")
    {

Heat Map

Table

        @Html.Partial("~/Views/Shared.MultiQuery/_resultset.cshtml", Model)

    }
    else{

Heat Map

Table

        @Html.Partial("~/Views/Shared.MultiQuery/_resultset.cshtml", Model)

    }
}
else
{

Heat Map

Table

        @Html.Partial("~/Views/Shared.MultiQuery/_resultset.cshtml", Model)

}

Most of your code is duplicated, what I did is extract the logic to the first code block, and left only the placeholders in the HTML.

@{
    bool savingHtml = (Request.QueryString["savehtml"] == "1");
    string activeTab = Request.QueryString["from"];
    string tabContent = Html.Partial("~/Views/Shared.MultiQuery/_resultset.cshtml", Model);
    string heatClass, tableClass;
    string tableId = "table";
    string heatId = "heat";
    string heatTabContent, tableTabConten;

    if (savingHtml && activeTab.ToLower() != "index") {
        heatClass = "active";
        heatTabContent = tabContent;
        tableId += "2";
        heatId += "2";
    } else {
        tableClass = "active";
        tableTabContent = tabContent;
        if (savingHtml) {
            tableId += "1";
            heatId += "1";
        }
    }
}

Heat Map

Table

        @tableTabContent

        @heatTabContent

Note: I have no experience with ASP.Net, so I apologize if the code is not as clean as possible, or has any syntax errors... The gist of the matter is still that if you extract the logic, you get shorter and cleaner code, with much less copy+paste...


Newtonsoft.Json.JsonConvert.SerializeObject Like many ASP.NET developers, I use the Newtonsoft library to convert C# objects into JSON. Is this use of Newtonsoft.Json.JsonConvert.SerializeObject secure?

Here is how I use it in a Razor view:

<script type="text/javascript">
    var json = @Html.Raw(Newtonsoft.Json.JsonConvert.SerializeObject(Model))
</script>

In Web Forms I might have this in my ASPX:

<script type="text/javascript">
    var json = <%= Newtonsoft.Json.JsonConvert.SerializeObject(Model)) %>
</script>

Take the following result :

<script type="text/javascript">
    var json = { "key" : "</script>" }
</script>

Unless you HtmlEncoded your data somewhere, the "</script>" will break your code. With Html Encoding, the result should look like this :

<script type="text/javascript">
    var json = { "key" : "&lt;/script&gt;" }
</script>

To easily encode all string, you can check this solution : http://pastebin.com/R2PWZkUq

Note that this should happen only with object declare inline. JSon coming from Ajax don't have this problem.


Efficiently write in a file in C++ I've made a program which reads from the console and writes it in a .txt file. This programm should write in a file with \n.
#include <iostream>
#include <fstream>
#include <string>

using namespace std;

int main() {
    ofstream fich("file.txt");

    if (!fich)
        cerr << "Error!\n";

    char fst = 'l';
    string c;
    while (fst != '\0') {
        getline(cin, c);
        fst = c[0];
        fich << c << endl;
    }

    fich.close();

    return 0;
}

This program ends when the user inputs nothing and then a \n, because, doing that, the string c will only have a \0 end of string element.
I was wondering if there is any way of doing this more efficiently, maybe without needing a "void \n" line to finish the program.

Error Handling
You check whether the output file was opened correctly, but all you do is output a message (you don't even say what's wrong!) and then you keep on ignoring it. What if file.txt is a directory or read-only? I guess you haven't tested these cases. By default, throw an exception if you can't continue at some point. In this case, throw std::runtime_error("failed to open output file").
Loop Condition
You are reading until the line is empty. This is okay-ish, but why don't you just check whether c.empty() and then break from the loop? Another case is that the end of the input is reached. In that case, the streamstate is set to failed. I'm not sure what happens in getline() then (read the docs at cppreference.com), but if it just doesn't change the string passed by reference, you may end up in an endless loop.
As an alternative I would use getline(cin, c) as loop condition. If this succeeds, you have received a line which can then write to the output file. Otherwise, you either had an error or you reached the end of the input.
Closing Filestreams
If you didn't call fich.close(), when would it be closed? However, consider the possibility that the file can't be written fully. You'll never notice this, neither with your code nor if you let the destructor do its work. So, what I'd rather do there is a call to fiche.flush() followed by a check of the streamstate.
Efficiency
I wouldn't bother with that at the moment. Point is, you're still learning to walk, so don't try to run yet. Still, what you're doing is inefficient. Firstly, writing in size of lines is useless, because you're just moving bytes from one file to the other. Secondly, endl implies a newline and a flush. So this is not really a cheap operation! In addition, there is a lot of stuff going on in C++ IOStreams that there's a whole book about their internals. Lastly, copying files can sometimes be done more efficiently using OS features which will avoid looking at the data more than necessary.


Refactoring a LINQ query that (sometimes) returns null I have the following LINQ query that should return a result for each Request. I'd prefer to have the query translated to SQL by EntityFramework.

private static Expression<Func<Request, RequestResultModel>> requestResultExpression = request =>
    request.Applicant
            .ApplicantAddresses
            .OrderBy(address => address.IsPreferred)
            .Select(address => new RequestResultModel
            {
                Id = request.Id,
                ApplicantName = request.Applicant.FullName,
                ReviewerName = request.Reviewer.FullName,
                RefferrerName = request.Refferrer.Name,
                City = address.Address.City,
                Province = address.Address.Province.Code,
                DisciplineCode = request.Discipline.Code,
                Event = request.Event,
                StatusName = request.Status.Name,
                Submitted = request.Submitted
            })
            .FirstOrDefault();

The problem is that when the Applicant has no ApplicantAddresses, then the expression returns null for that specific Request. The correct behavior should be that a RequestResultModel should still be returned but with empty address information.

Unfortunately, I can only think of doing this in two steps like this:

private static Expression<Func<Request, RequestResultModel>> requestResultExpression = request =>
{
    var address = request.Applicant
                         .ApplicantAddresses
                         .OrderBy(address => address.IsPreferred)
                         .FirstOrDefault();

    return new RequestResultModel
            {
                Id = request.Id,
                ApplicantName = request.Applicant.FullName,
                ReviewerName = request.Reviewer.FullName,
                RefferrerName = request.Refferrer.Name,
                City = address == null ? null : address.Address.City,
                Province = address == null ? null : address.Address.Province.Code,
                DisciplineCode = request.Discipline.Code,
                Event = request.Event,
                StatusName = request.Status.Name,
                Submitted = request.Submitted
            };
});

Which has he following drawbacks:

It will not be translated to SQL by EntityFramework
It will be executed as multiple queries to the database

Any ideas how I can achieve the desired result?

All assistance will be greatly appreciated.

EDIT: Here is the Request class

public class Request
{
    [Key]
    public int Id { get; set; }

    public virtual Applicant Applicant { get; set; }

    public virtual Reviewer Reviewer { get; set; }

    public virtual Refferrer Refferrer { get; set; }

    [Required]
    public virtual Department Department { get; set; }

    [Required]
    public virtual Discipline Discipline {get; set; }

    [Required]
    public string Event { get; set; }

    public virtual Status Status { get; set; }

    public DateTime Submitted { get; set; }
}

Your current code generates an INNER JOIN, which as you've noticed, excludes any Applicant without an ApplicantAddress.

You want to select from Applicant and generate a LEFT JOIN on ApplicantAddress, this SO answer shows how to use DefaultIfEmpty() to do that:

var query = from u in usergroups
            join p in UsergroupPrices on u equals p.UsergroupID into gj
            from x in gj.DefaultIfEmpty()
            select new {
                UsergroupID = u.UsergroupID,
                UsergroupName = u.UsergroupName,
                Price = (x == null ? String.Empty : x.Price)
            };

It's hard to tell exactly what/how to change in your code to make it work, because we're not seeing the DbContext and it's not clear how Request turns out hitting the database; I'd rather not say anything than assume what's going on.

In this specific case:

private static Expression<Func<Request, RequestResultModel>> requestResultExpression = request =>
    request.Applicant
            .ApplicantAddresses
            .DefaultIfEmpty()
            .OrderBy(address => address.IsPreferred)
            .Select(address => new RequestResultModel
            {
                Id = request.Id,
                ApplicantName = request.Applicant.FullName,
                ReviewerName = request.Reviewer.FullName,
                RefferrerName = request.Refferrer.Name,
                City = address.Address.City,
                Province = address.Address.Province.Code,
                DisciplineCode = request.Discipline.Code,
                Event = request.Event,
                StatusName = request.Status.Name,
                Submitted = request.Submitted
            })
            .FirstOrDefault();


Regular Expression for parsing PHP annotations containing multiple lines and markup I am writing a regular expression for parsing PHP annotations in a flexible way.
I'd like to accomplish all the goals listed above with one single expression. So I would appreciate any suggestions about the quality of it (in terms of corner cases, performance, best practices and correctness).

PCRE expression:

/[\*\s]*@(?P<name>\w+[\\\w]*?)(?:\s|\()
(?P<value>
  (?:[\/\w\s\"\<\>\_\#\=\-\.\'\{\}:;,\*\(\)\[\]]*[^\R\*\s\/\)])
)?
(?:\s | $|\))/gsxmu

Goals of the regular expression:

List all annotations with their values
Values can be multi-lined and have markup (html, json or markdown)
The initial space + * of each line should be removed from the value
Many annotations can be in the same line
Annotation names can be namespaced

Sample PHPDoc string:

/**
 * Description
 *
 * @Tag name name @annotation beee @aaf dsfsd fgdg
 * @Tag name name {"json":"dfsf"}
 * @Tag asdasd <html> #markdown ==markdown== __markdown__
 * - markdown
 * > mark 1
.mark
"string"
'string'
 * @Annotation()
 * @Tag name name @annotation beee @aaf dsfsd fgdg <markdown> #markdown ==markdown==
 * @a() @b("name") @c()
 * @Annotation\Name("var1()", "var2") @n("name()_name")
 * @Annotation(["var1", "var2"], "var3")
 * @Annotation\Filter\Name(["var1", "var2"], "var3", {"var4": "var5"})
 * @Annotation(
 *    ["GET", "POST"]
   ) @Name({"name": "Tomas"})
 * @Tag name name
 */

Expected result:

<?php

array(
  [
      'name' => 'Tag',
      'value' => 'name name'
  ],
  [
      'name' => 'annotation',
      'value' => 'bee'
  ],
  [
      'name' => 'aaf',
      'value' => 'dsfsd fgdg'
  ]
  // ... and so on... (see live example)
);

A live demo can be found here.

If you're using it in a PHP code, you are not restricted to using / as the pattern delimiter. Using any other character, such as ~ or % frees you of using \/ everywhere, thereby shortening it.
I don't know what \R is meant to do there, but I think it was supposed to be \r. If so, you do not need it at all.
For the annotation name parameter match, you can just specify that it start with a \w character, followed by a lazy match on [\w\\] character set. Even using [\w\\]+ as the name parameter would not be wrong.
When inside a character set, you absolutely do not need to escape any characters other than the closing square bracket (]) and the exponent (^) if it is the first character. You can move the hyphen (-) to either the beginning or the end. So, the whole value group reduces to

(?P<value>(?:[-/\w\s"<>_#=.'{}:;,*()[\]]*[^*\s/)]))?

PS: You might need to escape at most one of ' or " depending on how you are using the pattern in your code.
In the above, it appears you want to match everything except the next @ character, so; [^@]* would be my next suggestion.
Towards the end of you pattern, you have: (?:\s | $|\)) which could become: (?:\s+|$|\)) or simply (?:\s+|\)) since you would not be needing the anchor at all.
You are not counting on a string like:

@name    (something)

where, you receive name and \s\s\s(something as resulting value (\s is space literal). This can be kept under check by setting \s*\(? instead of (?:\s|\(). This might also not be the intended behaviour from the user, which is why I kept this suggestion at the end.

[*\s]* # multiple space or asterisk characters
@ # followed by the @ sign which
(?P<name>[\w\\]+) # has a string of \w or \ characters following it
\s*\(? # separated by spaces and maybe opening parenthesis
  (?P<value> # store the value attached to the named annotation
    (?:
      [^@]* # value is made of enlisted characters
      [^*\s/)] # but does not end with
    )
  )? # the value is optional parameter
(?:\s|$|\)) # succeded by one of these


Number of words in a string I wrote this pretty simple program (it's an exercise from a book). It counts the words in a simple string. The book is a very old one (I think it's from 2003) and uses char arrays. Because of this, I don't know if this solution is a proper one or not.

Can you tell me whether it is okay or not?

#include <iostream>

int main()
{
    int count_words(std::string);

    std::string input_text;
    std::cout<< "Enter a text: ";
    std::getline(std::cin,input_text);

    std::cout << "Number of words: " << count_words(input_text) << std::endl;
    return 0;
}
int count_words(std::string input_text)
{
    int number_of_words = 1;
    for(int i = 0; i < input_text.length();i++)
        if(input_text[i] == ' ')
            number_of_words++;
    return number_of_words;
}

Prefer std::size_t for the word count and count_words()'s return type.  Also prefer std::string::size_type for the loop counter (when using indices is needed).
count_words()'s parameter should be passed by const& since it's not being modified:

std::size_t count_words(std::string const& input_text) {}

count_words() has a problem.  If I press ENTER without typing anything, it gives me 1.  To fix this, return 0 if the string is empty:

if (input_text.empty()) return 0;

In the for-loop, prefer std::isspace for detecting whitespace.
Prefer iterators over indices for std::string:

for (auto iter = input_text.cbegin(); iter != input_text.cend(); ++iter)
{
    if (*iter == isspace)
    {
        number_of_words++;
    }
}

If you have C++11, use this range-based for-loop instead:

for (auto& iter : input_text)
{
    if (iter == isspace)
    {
        number_of_words++;
    }
}

As @r-s mentions, this just counts whitespace.  Here's another solution from this answer:

unsigned int countWordsInString(std::string const& str)
{
    std::stringstream stream(str);
    return std::distance(std::istream_iterator<std::string>(stream), std::istream_iterator<std::string>());
}

I realize this may be difficult to understand.  I'd still recommend holding onto the answer for future reference.  If you want to try it out, include <sstream> and <iterator>.


Create a simple hangman game using OOP and Javascript I had a task to create a simple hangman game using O.O.P. with Javascript. It had to render the puzzle and remaining guesses to the DOM but didn't require any C.S.S.
This is what I created.
This is my index.html
<!DOCTYPE html>

    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>

This is my hangman.js which is used to create a hangman object
"strict";
const Hangman = function (word, remainingGuesses) {
  this.word = word.toLowerCase().split("");
  this.remainingGuesses = remainingGuesses;
  this.guessedWords = new Set();
  this.status = "Playing";
};

Hangman.prototype.getPuzzle = function () {
  let puzzle = [];
  this.word.forEach((char) => {
    this.guessedWords.has(char) || char === " "
      ? puzzle.push(char)
      : puzzle.push("*");
  });
  return puzzle.join("");
};

Hangman.prototype.makeGuess = function (guess) {
  guess = guess.toLowerCase();
  const isUnique = !this.guessedWords.has(guess);
  const isBadGuess = !this.word.includes(guess);

  if (isUnique) {
    this.guessedWords.add(guess);
  }

  if (isUnique && isBadGuess && this.status === "Playing") {
    this.remainingGuesses--;
  }

  this.calculateStatus();
};

Hangman.prototype.calculateStatus = function () {
  const finsished = this.word.every((letter) => this.guessedWords.has(letter));

  if (this.remainingGuesses === 0) {
    this.status = "Failed";
  } else if (finsished) {
    this.status = "Finished";
  } else {
    this.status = "Playing";
  }
};

Hangman.prototype.getStatus = function(){

  let message = "";

  if (this.status === "Playing") {
    message = `Remaining Guesses: ${this.remainingGuesses}`
  } else if(this.status === "Failed") {
    message = `Nice try! The word was ${this.word.join("")}`;
  } else {
    message = `Great Work! You guessed the word!!!`
  }

  return message;
}

and this is the app.js where I create an instance of the hangman game
"strict";
const puzzle = document.querySelector("#render-puzzle");
const guesses = document.querySelector("#guesses");

const hangman = new Hangman("Cat", 2);
generatePuzzleDom();

window.addEventListener("keypress", (e) => {
  hangman.makeGuess(e.key);
  generatePuzzleDom();
  console.log(hangman.status);
});

function generatePuzzleDom() {
  puzzle.innerHTML = hangman.getPuzzle();
  guesses.innerHTML = hangman.getStatus();
}

Good things

The code appears to function properly.
The code makes good use of strict equality operators and functional techniques like using .every().
const is used for most variables.
Prototypes are used properly.

Suggested changes
Strict mode flaw
Strict mode is enabled with the string literal:
"use strict";

Yet the first line of the two JS files is

"strict";

Variable declarations
Some variables declared with let could be declared with const since they are not re-assigned - e.g. puzzle since it is never re-assigned. Using const instead of let helps avoid accidental re-assignment and other bugs.
status values
The values for status could be stored in constants - e.g.
const STATUS_FAILED = 'Failed';
const STATUS_PLAYING = 'Playing';
// ... etc...

Then those can be used in the code- e.g. instead of

this.status = "Playing";

it can use the constant:
this.status = STATUS_PLAYING;

Those could also be stored in "an enum"
const STATUS_VALUES = Object.freeze({ FAILED: 0, PLAYING: 1, FINISHED: 2});

With this approach, there is no risk of mistyping the values throughout the code, and if a value needs to be updated, it can be done in one place.
Selecting elements
It isn't wrong to use querySelector to get elements by id but using getElementById() "is definitely faster" 1 (see this jsPerf test for comparison).
ES-6 class syntax
The code could be converted to the newer ES6 class syntax - bear in mind that it is "primarily syntactical sugar over JavaScript's existing prototype-based inheritance". If there were many subclasses then it would help simplify setting up the prototypal inheritance.


Demonstration of processing a queue of no-op threads I am creating a script, that will SSH to several hosts, get stuff, parse it and return. Because I wait for server response, I'm using threading.

Real script is more complex, so I created this simple program to demonstrate logic.

In real application 'timer' function does ssh with paramiko and parsing, 'main' is pretty much same.

Do I need to use threading.Lock() in this program?

from threading import Thread
import time
import random
import queue

'''
This function demonstrates usage of threads with queue
'''

que = queue.Queue()

def timer(name, delay, que):  # timer's name, time.sleep() delay, queue.Queue() object
    time.sleep(delay)  # sleep

    que.put(name + " is done after " + str(delay) + " seconds")  # add items to queue, used when threaded
    return name + " is done after " + str(delay) + " seconds"  # this is used if fuction is called directly

def main(repeat):
    threads = []  # stores threads after they are started. needed to join threads
    out = []      # used for return
    for i in range(1, repeat):  # Will loop few times, executing threads
        t = Thread(target=timer, args=(('timer' + str(i)), random.choice([2, 3, 2]), que))
        t.start()
        threads.append(t)

    for t in threads:
        t.join()        # Join is needed so that main function waits for threads. We need this to return out list

    while not que.empty():      # At this point threads are completed and joined. so we can dequeue que variable
        out.append(que.get())

    print("main completed")
    return out

start_time = time.time()        # To measure time
if __name__ == '__main__':
    print('\n'.join(main(30)),)
print("--- Done in %s seconds ---" % (time.time() - start_time))

You could skip the join loop here

for t in threads:
    t.join()

while not que.empty():
    out.append(que.get())

and let Queue.get wait instead:

for _ in range(len(threads)):
    out.append(que.get())

And no, you don't need an explicit threading.Lock in this program because Queue handles the synchronization.

Note that for i in range(1, repeat) loops repeat-1 times, which is probably not what you meant.


Number of Occurrences of a word in a text file or group of text files The question given to me was as follows :
WAP to take the input from the user a path and a word. If the path is a text file, search the word in the file and print the number of occurrences. If the path is a folder, read the files in the folder and print the number of occurrences in all the text files of the word.

Is what I did an effecient way of going about it? Please ignore indentations and lack of comments.

This was my code :

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;

public class TestOccurances {

public static void main(String[] args) {
Scanner sc1 = new Scanner(System.in);
String path, word;
System.out.println("Enter a path");
path = sc1.nextLine();
System.out.println("Enter the word");
word = sc1.nextLine();
sc1.close();
File f = new File(path);
BufferedReader br = null;
List<String> list = new ArrayList<String>();
try {
//Checking to see if its a .txt file
if(f.exists() && f.isFile() && f.getName().endsWith(".txt")){
    br  = new BufferedReader(new FileReader(path));
    String line;
    while((line=br.readLine())!=null) {
        String[] arr = line.split(" ");
        for(String str : arr)
        list.add(str);
        }
    System.out.println("Single text file, Frequency = "+Collections.frequency(list, word));
    }
//Checking to see if its a folder
if(f.exists() && f.isDirectory()) {
    File[] allfiles = f.listFiles();
    List<File>  files= new ArrayList<File>();
    for(File f1 : allfiles) {
        if(f1.getName().endsWith(".txt"))
            files.add(f1);
    }
    //Loop through each text file and do the same as above
    for(File file : files) {
        List<String> occurances = new ArrayList<String>();
        br = new BufferedReader(new FileReader(file.getPath()));
        String line;
        while((line=br.readLine())!=null) {
            String[] arr = line.split(" ");
            for(String str : arr)
            occurances.add(str);
            }
        System.out.println("File path :"+file.getName()+" , Frequency = "+Collections.frequency(occurances, word));
        }
    }
}catch(IOException e) {
    e.printStackTrace();
    }
finally {
    if(br!=null) {
        try {
            br.close();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        }
    }
}

}

While the functional approach may not always be the best solution, I find using Streams on files very convenient.

If you need to extend the second requirement on directories even further to include all .txt files in subdirectories, you can easily recurse through and flatmap all of the subdirectories until you are left with just a single stream of .txt files.

Code:

public static void main(String[] args) throws IOException {

    Scanner inputReader = new Scanner(System.in);

    System.out.println("Enter a path");
    String inputPath = inputReader.nextLine();
    System.out.println("Enter the word");
    String targetWord = inputReader.nextLine();

    File file = new File(inputPath);

    long numOccurances = 0;

    if(file.isFile() && file.getName().endsWith(".txt")){

        numOccurances = toLineStreamFromFile(file)
                            .flatMap(str -> Arrays.stream(str.split("\\s")))
                            .filter(str -> str.equals(targetWord))
                            .count();

    }else if(file.isDirectory()){

        numOccurances = Arrays.stream(file.listFiles(pathname -> pathname.toString().endsWith(".txt")))
                              .flatMap(Main::toLineStreamFromFile)
                              .flatMap(str -> Arrays.stream(str.split("\\s")))
                              .filter(str -> str.equals(targetWord))
                              .count();
    }

    System.out.println(numOccurances);
}

public static Stream<String> toLineStreamFromFile(File file){
    try {
        return Files.lines(file.toPath());
    } catch (IOException e) {
        e.printStackTrace();
    }
    return Stream.empty();
}


Method for formatting a decimal number in Python using the given maximum number of digits and decimal places The question itself should be fairly self-explanatory. My method takes in value, max_digits, and decimal_places, and returns a formatted decimal number string. The code:

from decimal import Decimal, ROUND_DOWN

def format_to_decimal_string(value, max_digits, decimal_places):
    """Formats a decimal number to the given number of decimal places.

    Notes:
        - If the decimal number is equal to zero, the absolute of the decimal
        number is formatted and returned to remove the minus sign. See the
        first doctest for an example.
        - If the number of digits in the rounded up decimal number exceeds the
        `max_digits`, the method tries to return the rounded down version. See
        the second doctest for an example.

    Doctests:
        >>> format_to_decimal_string('-0.000', 4, 2)
        '0.00'
        >>> format_to_decimal_string('99.999999', 4, 2)
        '99.99'
        >>> format_to_decimal_string('99.999999', 5, 2)
        '100.00'
        >>> format_to_decimal_string('420.2108', 5, 0)
        '420'
        >>> format_to_decimal_string('foo', 4, 2)
        Traceback (most recent call last):
            ...
        TypeError: The value 'foo' is not a valid decimal number.
        >>> format_to_decimal_string('120.411', 4, 2)
        Traceback (most recent call last):
            ...
        ValueError: After formatting, the number of digits in the value
        '120.41' is 5, which exceeds the maximum number of digits of 4.
    """
    try:
        decimal_number = Decimal(str(value))
    except:
        raise TypeError(
            'The value \'{}\' is not a valid decimal number.'.format(value)
        )

    if decimal_number == 0:
        return str(abs(decimal_number).quantize(
            Decimal(10) ** -decimal_places)
        )

    rounded_up = decimal_number.quantize(Decimal(10) ** -decimal_places)
    if not len(rounded_up.as_tuple().digits) > max_digits:
        return str(rounded_up)

    rounded_down = decimal_number.quantize(
        Decimal(10) ** -decimal_places,
        rounding=ROUND_DOWN
    )
    rounded_down_digits_length = len(rounded_down.as_tuple().digits)
    if not rounded_down_digits_length > max_digits:
        return str(rounded_down)
    else:
        raise ValueError(
            'After formatting, the number of digits in the value \'{}\' is ' \
            '{}, which exceeds the maximum number of digits of {}.'.format(
                rounded_down, rounded_down_digits_length, max_digits
            )
        )

Please assume that max_digits, and decimal_places will always be positive integers, and max_digits is greater than decimal_places, because these are coming from my database, so no validation is required for these arguments.

The comments should also be self explanatory. We only want to round up if the rounding up does not exceed the given maximum number of digits. Would really appreciate a review of this method.

Your code looks quite good to me. Good structure, consistent naming, appropriate documentation.

There are just some small ideas I would like to offer to you. Since your question is specifically tagged for Python 3, you might make use of the new f-string synthax instead of format. So your error messages would become something like

raise TypeError(
    f"The value '{value}' is not a valid decimal number."
)

which I think looks even nicer. I also replaced the outer ' with " so one does not have to escape the inner ' with backslashes.

The last error message could use the same tricks and could also utilize implicit line continuation in function parenthesis instead of manual line continuation triggered by \. This would ultimately lead to the following snippet:

raise ValueError(
    f"After formatting, the number of digits in the value '{rounded_down}' "
    f"is {rounded_down_digits_length}, which exceeds the maximum number of "
    f"digits of {max_digits}."
)

I was thinking about to address the idea of adding doctests to your code, but you have added this feature in an edit that took place while is was writing this up. So consider this to be done.