ASP.NET Web Forms Singleton Is this way acceptable to do a C# singleton for use in ASP.NET webforms legacy app:

public class FileSingleton
{
    private static readonly Lazy<FileSingleton> lazy =
        new Lazy<FileSingleton>(() => new FileSingleton( new FileRepository()));

    public static FileRepository Repository { get; private set; }

    public static FileSingleton Instance { get { return lazy.Value; } }

    private FileSingleton(FileRepository repository)
    {
        Repository = repository;
    }
}

I created a repository for dealing with files in Azure. It has some overhead, so I don't want to instantiate it on every request. My design is based off of this post.

protected void Application_Start(object sender, EventArgs e)
{

    HttpContext.Current.Items.Add("fileRepository", FileSingleton.Instance);
}

Does that seem reasonable? Is there a better way to do this?

First of all a bug, Repository property has to be an instance property, not static. Not that big issue because you're working with a singleton but it defeats the purpose of its surrounding code. Don't go to fix this, wait for our refactoring...

Class itself should be sealed, I don't see any chance to extend it (and you probably want to avoid it). Again, wait little bit more...

Note that, according to C# version you're using, you may write read-only properties as:

public FileRepository Repository { get; }
public static FileSingleton Instance => lazy.Value;

About Application_Start. I do not see surrounding code then I'm guessing but I have few perplexities. HttpContext.Items is usually handy to share data between HTTP modules and/or HTTP handlers. In this case you're storing an instance of FileSingleton, it means that each user has to know that type. In this case they can directly access FileSingleton.Instance.

As last note I'd consider to merge FileSingleton and FileRepository (if you own that code and it can be changed). Consider that to access FileRepository methods you need to write something like this:

FileSingleton.Instance.Repository.DoSomething();

Isn't it too verbose? It should be:

FileSingleton.Repository.DoSomething();

Or:

FileRepository.Instance.DoSomething();

If possible I'd go with the last one to do not introduce another type with the only responsibility to keep an instance of something else. If not then I'd refactor your code to:

public static class FileSingleton
{
     private static readonly Lazy<FileRepository> _repository =
        new Lazy<FileRepository>(() => new FileRepository());

     public static FileRepository Instance => _repository.Value;
}


Deferred log file close My code works in that it compiles, and when executed writes out a log file using a buffered writer.

But I wonder whether:

I am correct in assuming that this actually winds up deferring a .Close that is always called
a package variable is the idiomatic way of doing this in Go

I realise that I could just have createLogger return the file pointer, and defer on that ... but that seems even weirder, to have createLogger set a package variable on log, then return the file pointer to be treated as a local ...

I'm still struggling with an idiom here.

package main

import (
    "bufio"
    "log"
    "os"
)

var logFile *os.File

func main() {
    createLogger()
    defer logFile.Close()

    // do some stuff
}

func createLogger() {
    logFile, err := os.OpenFile("log/app.log", os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
    if err != nil {
        log.Fatalf("error opening file: %v", err)
        os.Exit(-1)
    }
    log.SetOutput(bufio.NewWriter(logFile))
}

Yes, on both counts (although putting it in the main package is not).

Typically I would return a logFile from createLogger (calling it newLogger is idiomatic), and defer closing the result.

If you're building a logger to work across your whole app, I'd look at how the standard library does it.

Essentially, create a new package my_app_logger with an initializer which sets up the logger. That way, every package that includes my_app_logger gets the same logger instance.


Randomly select + place an image within a div I'm working on a proof of concept idea I want to use in a Brutalist styled web page.

This would involve having transparent image being placed randomly behind the content on load, I have this working with mostly vanilla JS, minus a few jQuery methods that I wasn't sure how I could translate back to vanilla. (I learned jQuery helper functions before having a concrete understanding of vanilla JS and now I regret it. lol)

Anyways, here's the JS Fiddle, for you to see the concept, and below is the code with comments.

Javascript

/* Set Variables at the start of the document *
*
* Frame = Parent of the image element (absolute pos within body content) *
* Top/Left Position = the top/left walls of the document *
* Tag = variable for Image element *
*/

var frame = document.getElementById("frame");
var top_position = (Math.random() * ($(document).width() - 500)).toFixed();
var left_position = (Math.random() * ($(document).width() - 500)).toFixed();
var tag = document.getElementById('random');

// Create array of images to randomly select one
var image = [];
image[0] = "https://via.placeholder.com/350x150";
image[1] = "https://via.placeholder.com/150x150";
image[2] = "https://via.placeholder.com/150x350";
image[3] = "https://via.placeholder.com/500x150";
var size = image.length

// randomly selects number of array 0 through size(array.length)
var x = Math.floor(size * Math.random());

//Apply image src/styles to image on load
tag.src = image[x];
tag.style.position = 'absolute';
tag.style.top = top_position + "px";
tag.style.left = left_position + "px";
top_position += 20;
left_position += 20

Is there anyways I can simplify this and combine some functions, while still keeping it readable?

Example of the effect I was trying to implement

multiple images, within grids

Yes, I love using functions to make code more readable.

First lets get rid of your jQeury, simply replace $(document) with document.body.clientWidth.

Lets create a function to get an image.

function getImage(src, top, left) {
  var imgElement = document.createElement("img");
  imgElement.src = src;
  imgElement.style.top = top + 'px';
  imgElement.style.left = left + 'px';
  imgElement.style.position = 'absolute';
  imgElement.style.opacity = '.3';

  return imgElement;
}

Notice we are creating a new image element, not modifying an existing one.  This has two advantages:

Avoid slow dom minipulaiton
Call multiple times to add multiple images.

Now lets use this function in random picture function

function randomPicture() {
  var randomIndex =  Math.floor((image.length) * Math.random());
  var left_position = (Math.random() * maxWidth).toFixed();
  //Assume you wanted height and not width
  var top_position = (Math.random() * maxHeight).toFixed();
  var imgEl = getImage(image[randomIndex], top_position, left_position);
  contentEl.appendChild(imgEl);
}

Notice we use appendChild

Other suggestions:

Avoid magic numbers, use constants
Avoid variable hoisting
Use meaningful variable names
Simplify calculations with constants

var MAX_IMAGE_WIDTH = 500; // repalce magic number with const
var MAX_IMAGE_HEIGHT = 350;   // repalce magic number with const
var docWidth = document.body.clientWidth;  //replace $(document).width()
var docHeight = document.body.clientHeight || 500;  //Note: document.body.clientHeight returns 0 in this envorment, in browser will be actaul height, setting defalult value of 500
// prevent negitve values that could place your image off screen
var maxWidth = Math.max(0, docWidth - MAX_IMAGE_WIDTH);
var maxHeight = Math.max(0, docHeight - MAX_IMAGE_HEIGHT);
var contentEl = document.getElementById('content');
var image = [ //set innital values and prevent variable hoisiting
  "https://via.placeholder.com/350x150",
  "https://via.placeholder.com/150x150",
  "https://via.placeholder.com/150x350",
  "https://via.placeholder.com/500x150"
];

function getImage(src, top, left) {
  var imgElement = document.createElement("img");
  imgElement.src = src;
  imgElement.style.top = top + 'px';
  imgElement.style.left = left + 'px';
  imgElement.style.position = 'absolute';
  imgElement.style.opacity = '.3';

  return imgElement;
}

function randomPicture() {
  var randomIndex =  Math.floor((image.length) * Math.random());
  var left_position = (Math.random() * maxWidth).toFixed();
  //Assume you wanted height and not width
  var top_position = (Math.random() * maxHeight).toFixed();
  var imgEl = getImage(image[randomIndex], top_position, left_position);
  contentEl.appendChild(imgEl);
}

randomPicture();
randomPicture();
randomPicture();
.left {
  width: 14%;
}

.right {
  width: 84%;
  border-left: none !important;
  overflow: hidden;
}

.left,
.right {
  display: block;
  float: left;
  border: 1px solid #000;
  margin: 0;
  height: 500px;
}

.content-body {
  position: relative;
}

#frame {
  display: block;
  position: absolute;
  top: 0;
  left: 0;
  height: 100vh;
  width: 100%;
}

#random {
  opacity: .3;
}

.content {
  padding: 30px 40px;
  overflow: scroll;
  display: block;
}

h1 {
  font-size: 90px;
  padding: 0;
  margin: 0;
}

h2 {
  margin: 3px 0;
}

Home

Inspo

Shop

Challenger

This is the race.

On my mark...

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin aliquet pharetra pharetra. Aliquam mauris leo, blandit vitae vulputate sit amet, imperdiet non magna. Pellentesque gravida gravida dapibus. Vivamus augue neque, dignissim sed bibendum
        non, malesuada sed orci. Nam turpis mi, dictum ac ullamcorper sit amet, ornare nec sapien. Pellentesque at nibh vel lectus porta sodales. Vestibulum malesuada, urna a egestas iaculis, velit neque vehicula nunc, quis tincidunt libero est eget arcu.
        Phasellus in nisl lectus. Curabitur tellus elit, lobortis eget viverra non, tempor eu quam. Duis facilisis dictum nulla, sit amet aliquet augue viverra eget. Curabitur elit tellus, luctus ac pretium et, tristique at enim.

 Morbi gravida vel orci et fringilla. Vestibulum id lorem vel ipsum tincidunt vehicula gravida eget ipsum. Donec feugiat nisl arcu, non consectetur lacus porttitor ac. Morbi et quam et risus suscipit maximus sit amet eu quam. Aliquam non felis dictum,
        porta dolor quis, suscipit nisl. Suspendisse id nulla et odio rutrum convallis. Pellentesque consequat quis ante id sodales. Nam porttitor nulla velit, non sodales diam dignissim ac.

 Sed quis pretium purus, sed tempor arcu. Duis non sapien sed sem posuere dignissim. Sed id dui ipsum. Nam aliquam, arcu at lacinia scelerisque, justo dui tristique odio, et sodales lorem ante in enim. Orci varius natoque penatibus et magnis dis
        parturient montes, nascetur ridiculus mus. Morbi dictum varius quam a congue. Nulla ornare elit quis ante ullamcorper, vitae vestibulum nunc condimentum. Sed dignissim erat vel lacus pulvinar, sit amet maximus lectus varius. Vestibulum ante ipsum
        primis in faucibus orci luctus et ultrices posuere cubilia Curae; Phasellus convallis, turpis ut molestie ultricies, lacus enim convallis mi, quis mollis arcu est vitae mauris. Donec quis tortor eu elit aliquam tincidunt. Sed vel sollicitudin
        justo.

References:

Keep DOM access to a minimum
createElement()
how-to-get-document-height-and-width-without-using-jquery


Implementing permuted index in C++ I have implemented a program that produces a permuted index.
Actually it is this exercise:
https://stackoverflow.com/questions/4015016/what-is-a-permuted-index

Could anyone tell me if I did something wrong, and if it could be done better?

An input is for example:

The quick brown fox

And result:

      The quick     brown fox
The quick brown     fox
            The     quick brown fox
                    The quick brown fox

main.cpp

#include "myImplementation.cpp"

using namespace std;

vector<string> split(const string &s) {
    vector<string> ret;
    string::size_type i = 0;
    while (i != s.size()) {
        while (i != s.size() && isspace(s[i]))
            ++i;
        string::size_type j = i;
        while (j != s.size() && !isspace(s[j]))
            j++;
        if (i != j) {
            ret.push_back(s.substr(i, j - i));
            i = j;
        }
    }
    return ret;
}

int main() {
    string phrase;
    cout << "Please give me some phrase" << endl;
    getline(cin, phrase);

    vector <string> splitted = split(phrase);

    vector<string> permuted = makePermutedIndex(splitted);

    for (const auto i : permuted) {
        cout << i << endl;
    }

    return 0;
}

myImplementation.cpp

#include <vector>
#include <sstream>
#include <algorithm>

using namespace std;

vector<string> concatLeftAndRight(const vector<string> &left, const vector<string> &right,
                                  const string::size_type maxLengthOfLeftLine) {
    vector<string> ret;
    for (vector<string>::size_type i = 0; i != left.size(); ++i) {
        std::stringstream ss;
        ss << string(maxLengthOfLeftLine - left[i].size(), ' ')
        << left[i] << "   " << right[i];
        ret.push_back(ss.str());
    }
    return ret;
}

vector<string> makePermutedIndex(const vector<string> &splitted) {

    vector<string> sorted(splitted);

    std::sort(sorted.begin(), sorted.end());

    string::size_type maxLengthOfLeftLine = 0;

    vector<string> left;
    vector<string> right;

    for (auto index : sorted) {
        string rightLine, leftLine;
        bool indexEncountered = false;

        for (auto word : splitted) {
            if ((index != word) && !indexEncountered) {
                leftLine += word;
                leftLine += " ";
            }
            else {
                indexEncountered = true;
                rightLine += word;
                rightLine += " ";
            }
        }
        maxLengthOfLeftLine = max(maxLengthOfLeftLine, leftLine.size());
        left.push_back(leftLine);
        right.push_back(rightLine);
    }
    return concatLeftAndRight(left, right, maxLengthOfLeftLine);
}

The code does not implement the permuted index. Instead of

      The quick     brown fox
The quick brown     fox
            The     quick brown fox
                    The quick brown fox

it produces

                   The quick brown fox
      The quick    brown fox
The quick brown    fox
            The    quick brown fox

Notice that the order is wrong. The permuted index shall be sorted by the second column.
using namespace std; is considered bad practice.
Never #include a .cpp file. See for example this discussion for details.
Try not use so many strings. It hurts both memory usage and performance. In fact there is no need to break the initial string into words and reconstruct left and right strings. A vector of whitespace locations in the original string suffices.


Generating Julia set fractal I am a total novice to Python and have been testing my skills recently. I found this particular challenge and wanted to give it a shot. I made this code and it functions well, but very slowly. I was looking for feedback on how to optimize in Python, as well as general Python best practices!

Main.py

from PIL import Image

IMGHeight = 2000
IMGWidth  = 2000

xmin = -2.0
xmax =  2.0

ymin = -2.0
ymax =  2.0

def Function(z):
    return z*z + complex(-0.221, -0.713)

def Pixel(point):
    #set up how the complex number will create a pixel

    limit = 255
    thresh = 2

    val = Function(point)
    for I in range(1, limit):
        if(abs(val) > thresh):
            return I

        val = Function(val)

    return limit

def main():
    myImg = Image.new('L', (IMGWidth+1,IMGHeight+1))

    for x in range(IMGWidth):
        zx = x * (xmax - xmin)/(IMGWidth - 1) + xmin
        for y in range(IMGHeight):
            zy = y * (ymax - ymin)/(IMGHeight - 1) + ymin
            z = complex(zx, zy)
            #pixels.append(Pixel(z))
            myImg.putpixel((x,y), Pixel(z))

    myImg.save('Fractals.png')

if __name__ == "__main__":
    main();

PEP 8

Please follow PEP 8, you ca use tools like pylint to find PEP 8 issues immediately. But for instance:

Function -> function
Pixel    -> pixel

There are other small related issues.

Docstrings

Python has docstrings which are ways to document your functions. It isn't exactly clear what Function or Pixel do at the moment.

You have a comment describing Pixel. Two things about this comment:

I don't understand it, what do you mean by "create a pixel"?
It is not, as of now a docstring, to make it one, use the triple quoted string and put right below the function declaration, so:

def Pixel(point):
    #set up how the complex number will create a pixel

Should be:

def Pixel(point):
    """Set up how the complex number will create a pixel"""

But, again you should probably reword the comment, at the moment the meaning is not so clear, possibly:

def Pixel(point):
    """Calculate the color threshold for a particular pixel"""

Generalization

I would argue that you should add an additional parameter to Pixel. Function is one of the many functions you could pass into the set so allow someone else to change it and add f as a parameter to Pixel and pass the function in.

Optimization

What would I change to optimize the code?*

Instead, I would first switch to PyPy (which uses JIT compilation to make the code, in this case, much faster) and run the program that way, install PyPy, then (this worked for me):

pypy -m pip install Pillow

to install Pillow on pypy run it. Here are the time comparisons on my computer:

➜  codereview time pypy julia.py
pypy julia.py  3.10s user 0.07s system 99% cpu 3.198 total
➜  codereview time python julia.py
python julia.py  33.53s user 0.30s system 96% cpu 35.224 total

Just to be clear though, PyPy isn't always the best solution. It's great in this case though because you are repeatedly calling the same two functions over and over again, something that PyPy is generally much better than CPython at doing.

For a lot of, often considered, "slower" languages simply changing the implementation can do wonders.

* Well there are probably other things you can do, but switching the language is sooo easy, and helps with sooo much!


Determining if the mouse is on top of a box I have 4 picture boxes and a collision detection to see if my mouse is on top of any of them, but I have to run the same code 4 different times, but just changing the name. Also, the else if statements can be regular if statements.

PictureBox display = sender as PictureBox;
Point drop = PointToClient(Cursor.Position);

if (((displayCollision(drop, pbDisplay1)) == true) && (display != pbDisplay1))
{
    display.Location = pbDisplay1.Location;
    display.Size = pbDisplay1.Size;

    pbDisplay1.Location = defaultLocation;
    pbDisplay1.Size = defaultSize;
}
else if (((displayCollision(drop, pbDisplay2)) == true) && (display != pbDisplay2))
{
    display.Location = pbDisplay2.Location;
    display.Size = pbDisplay2.Size;

    pbDisplay2.Location = defaultLocation;
    pbDisplay2.Size = defaultSize;
}
else if (((displayCollision(drop, pbDisplay3)) == true) && (display != pbDisplay3))
{
    display.Location = pbDisplay3.Location;
    display.Size = pbDisplay3.Size;

    pbDisplay3.Location = defaultLocation;
    pbDisplay3.Size = defaultSize;
}
else if (((displayCollision(drop, pbDisplay4)) == true) && (display != pbDisplay4))
{
    display.Location = pbDisplay4.Location;
    display.Size = pbDisplay4.Size;

    pbDisplay4.Location = defaultLocation;
    pbDisplay4.Size = defaultSize;
}
else
{
    display.Location = defaultLocation;
}

I don't see a convenient way to do a for loop, but you could extract common functionality into a single function:

bool CheckAndModify(PictureBox pb)
{
    if (displayCollision(drop, pb) && (display != pb))
    {
        display.Location = pb.Location;
        display.Size = pb.Size;
        pb.Location = defaultLocation;
        pb.Size = defaultSize;
        return true;
    }
    return false;
}

And then:

bool modified = CheckAndModify(pbDisplay1) ||
                CheckAndModify(pbDisplay2) ||
                CheckAndModify(pbDisplay3) ||
                CheckAndMovify(pbDisplay4);

Or, I suppose a loop would be possible:

foreach (var pbDisplay in new[]{pbDisplay1, pbDisplay2, pbDisplay3, pbDisplay4})
{
    if (CheckAndModify(pbDisplay))
    {
        break;
    }
}


Simple JDBC Query By Example (QBE) for Spring JDBC Template This is a quick and dirty QBE method that covers about 60% of my use cases in my DAO. I just wanted to see if someone would take a quick look at it to see if there are any issues.

public List<Order> orderQuery(Order order) {

    StringBuilder sql = new StringBuilder();
    sql.append("select * from orders ");

    List<String> clauses = new ArrayList<String>();
    List<Object> values = new ArrayList<>();

    if (order.getOrderID() != null) {
        clauses.add("order_id = ?" + order.getOrderID());
        values.add(order.getOrderID());
    }

    if (order.getAccountCode() != null) {
        clauses.add("acct_nmbr = ?");
        values.add(order.getAccountCode());
    }

    // TODO: Once I switch this to the AccountType enum, use order.getAccountType().getCode()
    if (order.getAccountType() != null) {
        clauses.add("acct_typ = ?");
        values.add(order.getAccountType());
    }

    if (order.getRsid() != null) {
        clauses.add("rsid = ?");
        values.add(order.getRsid());
    }

    if (order.getUserID() != null) {
        clauses.add("user_id = ?");
        values.add(order.getUserID());
    }

    if (order.getCurrentStatus() != null) {
        values.add(order.getCurrentStatus());
    }

    if (!clauses.isEmpty()) {
        sql.append("where ");
    }

    String whereClause = clauses.stream().collect(Collectors.joining(" and "));
    sql.append(whereClause);

    List<Order> result = jdbcTemplate.query(sql.toString(), orderRowMapper, values.toArray());
    return result;
}

Generally it's simple:
1. Put the SQL clause in a StringBuilder.
2. Check the various fields I care about and if they aren't null, add a clause for them.
3. Turn the list of SQL where clauses into a single string
4. Build the entire SQL statement.
5. Execute it with Spring JDBC Template.

Any external input?

Be careful of SQL injections.

Assuming that the Order object is sanitized, then the only suggestion I'd make is to make a class for your order query and extract the logic from your method into that class.

A quick example would be that:

public class OrderQuery {

    private List<String> clauses = new ArrayList<>();
    private List<Object> values = new ArrayList<>();

    public OrderQuery(Order order) {
        setOrderId(order.getOrderID());
        // ...
    }

    private void setOrderId(String orderId) {
        if (orderId != null) {
            clauses.add("order_id = ?");
            values.add(orderId);
        }
    }

    // ...

    public List<Object> getValues() {
        return values;
    }

    @Override
    public String toString() {
        StringBuilder sql = new StringBuilder();
        sql.append("select * from orders ");
        if (!clauses.isEmpty()) {
            sql.append("where ");
        }
        String whereClause = clauses.stream().collect(Collectors.joining(" and "));
        sql.append(whereClause);
        return sql.toString()
    }
}

And to call it from your now much less clustered method:

public List<Order> orderQuery(Order order) {
    OrderQuery orderQuery = new OrderQuery(order)
    List<Order> result = jdbcTemplate.query(orderQuery.toString(), orderRowMapper, orderQuery.getValues().toArray());
    return result;
}

But it could be further optimized by using something like builder pattern.


Checking string length limitation and add items to it in a shorter way with Linq This method has three parameters. If the extension is not null, for each item in list try to add extension to main text in a way that length of result doesn't exceed 20.

I wrote this code, but I don't like its style. I like to use Linq and newer styles for doing this, in order to make it shorter.

public string AddExtension(IList<string> extension, bool needDefaultAdd, string text )
{
    if (needDefaultAdd)
    {
        if (extension==null)
        {
            string tempText = text + "deafult extension txt";
            if (text.Length >20)
            {
                return text;
            }
            else
            {
                return tempText;
            }
        }
        else
        {
             string finalResult = null;
            foreach (var e in extension)
            {

                string tempText = text + "deafult extension txt";
                if (text.Length > 20)
                {
                    finalResult=text;
                }

                var temp2Text = tempText +e;
                if (text.Length > 20)
                {
                    finalResult=tempText;
                }
                else
                {
                    finalResult = finalResult + temp2Text;
                }
            }
            return finalResult;
        }

    }
    return text;
}

Reduce nesting by putting a negated condition first:

if (! needDefaultAdd)
{
    return text;
}


ES5 utility functions implementations I have a small library for utility functions for ES5 mainly aimed at OO code organization.

Are there any recommendations for the API or for the code quality / organization?

github page
styleGuide it complies to

Note the library has a few tricks in it for aggressive minification.

Code inlined

/*
    pd(obj) -> propertyDescriptorsOfObject {
        bindAll: function that binds all the methods of an object to the object,
        extend: function that extends the first argument with the rest
        Name: returns a namespace(anyKey) -> uniqueObject function
    }

    pd requires ES5. Uses the shimmable subset of ES5.
*/
(function (Object, slice) {
    "use strict";

    extend(pd, {
        bindAll: bindAll,
        extend: extend,
        Name: Name
    });

    typeof window !== "undefined" ? window.pd = pd : module.exports = pd;

    /*
        pd will return all the own propertydescriptors of the object

        @param Object object - object to get pds from.

        @return Object - A hash of key/propertyDescriptors
    */
    function pd(obj, retObj) {
        retObj = {};
        Object.getOwnPropertyNames(obj).forEach(function(key) {
            var pd = Object.getOwnPropertyDescriptor(obj, key);
            retObj[key] = pd;
        });
        return retObj;
    }

    /*
        Extend will extend the firat parameter with any other parameters
        passed in. Only the own property names will be extended into
        the object

        @param Object target - target to be extended
        @arguments Array [target, ...] - the rest of the objects passed
            in will extended into the target

        @return Object - the target
    */
    function extend(target) {
        slice.call(arguments, 1).forEach(function(source) {
            Object.defineProperties(target, pd(source));
        });
        return target;
    }

    /*
        defines a namespace object. This hides a "privates" object on object
        under the "key" namespace

        @param Object object - object to hide a privates object on
        @param Object key - key to hide it under

        @author Gozala : https://gist.github.com/1269991

        @return Object privates
    */
    function namespace(object, key) {
        var privates = Object.create(object),
            valueOf = object.valueOf;

        Object.defineProperty(object, "valueOf", {
            value: function(value) {
                return value !== key ? valueOf.apply(this, arguments) : privates;
            },
            writable: true
        });

        return privates;
    }

    /*
        Constructs a Name function, when given an object it will return a
        privates object.

        @author Gozala : https://gist.github.com/1269991

        @return Function name
    */
    function Name(key) {
        key = {};
        return name;

        function name(object) {
            var privates = object.valueOf(key);
            return privates !== object ? privates : namespace(object, key)
        }
    }

    /*
        bindAll binds all methods to have their context set to the object

        @param Object obj - the object to bind methods on
        @param Array whitelist - optional whitelist of methods to bind

        @return Object - the bound object
    */
    function bindAll(obj, whitelist) {
        (whitelist || Object.keys(obj).filter(getMethods)).forEach(bindMethods);

        return obj;

        function getMethods(key) {
            return obj[key].call;
        }

        function bindMethods(name) {
            obj[name] = obj[name].bind(obj)
        }
    }
})(Object, [].slice);

There isn't anything majorly wrong, so these will be mostly minor things.

getMethods doesn't really get methods, isMethod (or isBindable) is a more descriptive name for it.

You'd also want to check that .call is in fact the .call inherited from Function
prototype, or since you are doing the filtering in preparation for .bind, just
check that obj[key].bind === Function.prototype.bind or similar. Also check for truthiness, so you don't try to access null.bind:

return obj[key] && obj[key].bind ===

If Name is a normal function, it shouldn't be capitalized and be a named like a noun,
this is convention for functions that are meant to be invoked as constructors using new.
Something like makeName or makeNamespace, a verb that implies a factory, would
be a better fit.


Multithreading extensions I created a following class to manage multithreading without extra overhead, which exist when I use Parallel TPL class. It is also useful for systems without TPL.

Can it be enchanced?

    public static void Foreach<T>(this ICollection<T> source, Action<T> action)
    {
        var allDone = new ManualResetEventSlim(false);
        int completed = 0;
        foreach (var item in source)
        {
            ThreadPool.QueueUserWorkItem(state =>
            {
                var closure = (T)state;
                action(closure);
                if (Interlocked.Increment(ref completed) == source.Count)
                    allDone.Set();
            }, item);
        }
        allDone.Wait();
    }

    public static void Do(Action action1, Action action2)
    {
        var firstTask = QueueWaitableTask(action1);
        action2();
        firstTask.Wait();
    }

    public static ManualResetEventSlim QueueWaitableTask(Action action)
    {
        var result = new ManualResetEventSlim(false);
        ThreadPool.QueueUserWorkItem(mse =>
        {
            action();
            ((ManualResetEventSlim) mse).Set();
        }, result);
        return result;
    }

You should consider what happens when a user passes an action which can error:

new List<string> { "a", "b", "c" }.Foreach(_ => { throw new Exception(); });

In this case, your manual reset event will never be signalled and the code will never finish executing. Eeek!

As ManualResetEventSlim was introduced in .Net 4 you may as well use another class that was new at the same time CountdownEvent.

public static void Foreach<T>(this ICollection<T> source, Action<T> action)
{
    var allDone = new CountdownEvent(source.Count);
    foreach (var item in source)
    {
        ThreadPool.QueueUserWorkItem(state =>
        {
            try
            {
                action((T)state);
            }
            finally
            {
                allDone.Signal();
            }
        }, item);
    }
    allDone.Wait();
}

Having said that Foreach is a bad name, it's too similar to List.ForEach and doesn't even hint at the internal behaviour.

You should be checking that source != null and action != null.

Update:
If you want to catch exceptions and rethrow them as an AggregateException, add them to a concurrent collection e.g. ConcurrentBag so you don't need to worry about synchronisation.


C - manually parsing char* into integer I'm trying to parse a char* into an int without using atoi(). I walk through the string, check if it's a valid digit, then add that digit to my integer by multiplying by 10 and adding the digit. I am not accepting negative integers.

Here's the code I'm using:

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

#define TRUE  1
#define FALSE 0

int main(int argc, char *argv[]) {
    if (argc < 2)
        exit(1);

    char *p = argv[1];

    int amount = 0;
    int len = strlen((const char*) argv[1]); // calculate len beforehand
    int contin = TRUE;
    for (int i = 0; contin && i < len; ++i, ++p) {
        switch (*p) {
        case '0':
            amount = (amount * 10);
            break;
        case '1':
            amount = (amount * 10) + 1;
            break;
        case '2':
            amount = (amount * 10) + 2;
            break;
        case '3':
            amount = (amount * 10) + 3;
            break;
        case '4':
            amount = (amount * 10) + 4;
            break;
        case '5':
            amount = (amount * 10) + 5;
            break;
        case '6':
            amount = (amount * 10) + 6;
            break;
        case '7':
            amount = (amount * 10) + 7;
            break;
        case '8':
            amount = (amount * 10) + 8;
            break;
        case '9':
            amount = (amount * 10) + 9;
            break;
        default:
            contin = FALSE;
            break;
        }
    }

    fprintf(stdout, "amount: %i\n", amount);
    return 0;
}

...which works nicely. But, is there a better/more idiomatic way to do this?

EDIT: Thanks to Groo, I'm able to remove the giant switch statement:

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

#define TRUE  1
#define FALSE 0

int main(int argc, char *argv[]) {
    if (argc < 2)
        exit(1);

    char *p = argv[1];

    int amount = 0;
    int len = strlen((const char*) argv[1]); // calculate len beforehand
    int contin = TRUE;
    for (int i = 0; contin && i < len; ++i, ++p) {
        /* handle negative integers */
        if (*p == '-' && i == 0) {
            fprintf(stderr, "negative integers are invalid.\n");
            exit(1);
        }

        if (*p > 0x2F && *p < 0x3A)
            amount = (amount * 10) + (*p - '0');
        else {
            contin = FALSE;
            --p;
        }
    }

    fprintf(stdout, "amount: %i\n", amount);
    return 0;
}
```

Since command line arguments are null-terminated, we can avoid strlen() entirely by just running through the string until we hit '\0'; this also gets rid of contin and i
(*p > 0x2F && *p < 0x3A) isn't very obvious, I'd just use '0' and '9' to make it easier to understand the intention and to be consistent with the rest of the code
You could use EXIT_SUCCESS and EXIT_FAILURE instead of 0 and 1 respectively
For consistency, I'd replace your call to exit() with a simple return
I'd remove the amount: part of the print; it should be clear from the context (running this program) what the output actually is; this would also make processing the output easier

Putting it all together, we end up with this short piece:

int main(int argc, char *argv[]) {
    if (argc < 2)
        return EXIT_FAILURE;

    char *p = argv[1];
    int amount = 0;

    while (*p) {
        if (*p < '0')
            return EXIT_FAILURE;
        if (*p > '9')
            return EXIT_FAILURE;

        amount = (amount * 10) + (*p++ - '0');
    }

    fprintf(stdout, "%i\n", amount);
    return EXIT_SUCCESS;
}

Note, however, that this doesn't account for:

negative numbers (starting with a -)
positive numbers starting with a +
any leading whitespace

Furthermore:

If the input starts with a number and has a letter following sometimes later, it will break out and print the number it has found until then; but if the first character found is a letter, the program prints 0 and reports success, which could be by design, but could also be considered a bug
The use of *p++ can be a bit dangerous if this was code shared with less experienced programmers, as simply changing it to ++*p would increase the value pointed to instead of the pointer itself; changing it to *(p++) might help
While this comes down to taste, I would argue to always use curly braces, even for one-liners: otherwise, adding another line later would break the code; also consistency is a plus
Adding support for leading whitespace is easy and also supports my previous point, as missing brackets would break the program:

    if (*p == ' ') {
        *p++;
        continue;
    }

If you don't want to implement support for negative numbers, it might be reasonable to change the type of amount to unsigned or - even better - unsigned long long, which would give you a possible range of [0, +18446744073709551615]
Naive handling of negative integers would be quite easy to implement (but note Roland's answer for caveats with this):

    int sign = 1;

    if (*p == '-') {
        sign = -1;
        *p++;
    }

    /* loop */

    amount *= sign;


List of path constants I have a file called paths.php which is the first file that loads on every page request. I read that I should make a constant of the root folder path but not for every file, but the main point why I did constants for commonly used paths: when the path changes I don't need to go around different files changing require(ROOT . "/path/to/file"); instead all I need is to change the constant value. The path constant list is building up fast and every time I open that file I feel that something is not right and something needs to be changed so I decided to ask my first question here. I heard about autoloading, but by my (small) knowledge it's only for OOP projects (please fix me if I'm wrong)? This project is done without OOP only for practice purposes, I want to make at least one non-oop page and then move on to OOP.
So here is the code, I hope to get criticism, tips, and what should I change or do differently with arguments so I can learn.
This is not a full file, just a part of it - modules/templates paths
// 04.
defined("MODULES") or define("MODULES", RESOURCES . "/modules");
defined("FUNCTIONS") or define("FUNCTIONS", MODULES . "/functions.php");
defined("AUTH") or define("AUTH", MODULES . "/authentication");
defined("AUTH_CONTROL") or define("AUTH_CONTROL", AUTH . "/auth-control.php");
defined("SIGN_UP") or define("SIGN_UP", AUTH . "/sign-up.php");
defined("SIGN_IN") or define("SIGN_IN", AUTH . "/sign-in.php");
defined("FORMS") or define("FORMS", MODULES . "/forms");
defined("FORM_CONTROL") or define("FORM_CONTROL", FORMS . "/form-control.php");
defined("FORM_ERRORS") or define("FORM_ERRORS", FORMS . "/form-errors.php");
defined("FORM_VALIDATION_FUNCTIONS") or define("FORM_VALIDATION_FUNCTIONS", FORMS . "/form-validation-functions.php");
defined("UPLOAD_IMG") or define("UPLOAD_IMG", MODULES . "/upload-image.php");

// 05.
// Templates Folder
defined("TEMPLATES") or define("TEMPLATES", RESOURCES . "/templates");
// Main Templates
defined("MAIN_TEMPLATES") or define("MAIN_TEMPLATES", TEMPLATES . "/main-templates");
defined("HEAD") or define("HEAD", MAIN_TEMPLATES . "/head.php");
defined("HEADER") or define("HEADER", MAIN_TEMPLATES . "/header.php");
defined("BANNER") or define("BANNER", MAIN_TEMPLATES . "/banner.php");
defined("FOOTER") or define("FOOTER", MAIN_TEMPLATES . "/footer.php");
defined("JS_FILES") or define("JS_FILES", MAIN_TEMPLATES . "/js-files.php");
// Content Templates
defined("CONTENT_TEMPLATES") or define("CONTENT_TEMPLATES", TEMPLATES . "/content-templates/");
// Form Templates
defined("FORM_TEMPLATES") or define("FORM_TEMPLATES", TEMPLATES . "/form-tmp");

If this is the first thing that loads on every page, then the chance of any of these constants being already defined is very small. Why then define them conditionally? These are constants, they should always have the same value. Therefore you can change:
defined("CONSTANT2") or define("CONSTANT2", CONSTANT1 . "/constant2");

to:
define("CONSTANT2", CONSTANT1 . "/constant2");

One of the reasons to define constants conditionally is that you don't want to overwrite constants that are part of PHP. In my opinion there is a better way to prevent this: Choosing longer constant names. So instead of calling a constant HEAD, you could call it TEMPLATE_HEAD. That way the chance of a naming collision is reduced.
Since PHP 5.3 from 2009 is has been possible to define global constants this way:
const CONSTANT2 = CONSTANT1 . "/constant2";

I know it is purely subjective, but I prefer this syntax. Later, when you start using OOP this will be the only syntax for class constants.
Given the above your code could look like this:
// 04.
const FOLDER_MODULES            = FOLDER_RESOURCES . '/modules';
const MODULE_FUNCTIONS          = FOLDER_MODULES . '/functions.php';
const FOLDER_AUTH               = FOLDER_MODULES . '/authentication';
const AUTH_CONTROL              = FOLDER_AUTH . '/auth-control.php';
const AUTH_SIGN_UP              = FOLDER_AUTH . '/sign-up.php';
const AUTH_SIGN_IN              = FOLDER_AUTH . '/sign-in.php';
const FOLDER_FORMS              = FOLDER_MODULES . '/forms';
const FORM_CONTROL              = FOLDER_FORMS . '/form-control.php';
const FORM_ERRORS               = FOLDER_FORMS . '/form-errors.php';
const FORM_VALIDATION_FUNCTIONS = FOLDER_FORMS . '/form-validation-functions.php';
const IMAGE_UPLOAD              = FOLDER_MODULES . '/upload-image.php';

// 05.
// Templates Folder
const FOLDER_TEMPLATES          = FOLDER_RESOURCES . '/templates';
// Main Templates
const FOLDER_MAIN_TEMPLATES     = FOLDER_TEMPLATES . '/main-templates';
const TEMPLATE_HEAD             = FOLDER_MAIN_TEMPLATES . '/head.php';
const TEMPLATE_HEADER           = FOLDER_MAIN_TEMPLATES . '/header.php';
const TEMPLATE_BANNER           = FOLDER_MAIN_TEMPLATES . '/banner.php';
const TEMPLATE_FOOTER           = FOLDER_MAIN_TEMPLATES . '/footer.php';
const TEMPLATE_JAVASCRIPT       = FOLDER_MAIN_TEMPLATES . '/js-files.php';
// Content Templates
const FOLDER_CONTENT_TEMPLATES  = FOLDER_TEMPLATES . '/content-templates/';
// Form Templates
const TEMPLATE_FORM             = FOLDER_TEMPLATES . '/form-tmp';

Somehow I find this much easier on the eyes.


Finding the max subset of non-overlapping intervals How could this code become cleaner?  I think that the way I handle the interfaces and binary search could be improved.  I am trying to understand how to structure such a code (and usage of APIs) in a cleaner and more efficient manner.

This code solves the problem of finding the max subset of non-overlapping intervals.  I especially believe the way I handle the binary search result is error-prone.

private boolean nonOverlapping(Pair interval, SortedSet<Pair> selectedIntervals) {
    if(selectedIntervals.isEmpty())
        return true;
    if(selectedIntervals.contains(interval)){
        return true;
    }
    Pair[] sortedSelections = selectedIntervals.toArray(new Pair[0]);
    int pos = Arrays.binarySearch(sortedSelections, interval, new Comparator<Pair>() {

        @Override
        public int compare(Pair o1, Pair o2) {
            return o1.getStart() - o2.getEnd();
        }
    });
    pos = (-pos) -1;
    if(pos == sortedSelections.length){
        if(sortedSelections[pos - 1].getEnd() < interval.getStart()){
            return true;
        }

    }
    else if(sortedSelections[pos].getEnd() > interval.getStart()){
        if(pos + 1 < sortedSelections.length){
            if(sortedSelections[pos + 1].getEnd() < interval.getStart()){
                return false;
            }
        }
        if(pos - 1 >= 0){
            if(sortedSelections[pos - 1].getEnd() < interval.getStart()){
                return false;
            }
        }
        return true;
    }

    return false;
}

I see three points of improvement:

The isEmpty check is redundant can be removed. The contains will handle the scenario your are checking for.
The Comparator really clutters things up, and since you already have a SortedSet coming in, I know you have another compareTo method somewhere else.  If Pair is one of your classes, go ahead and make it implement Comparable so you don't have to worry about rewriting comparison logic everywhere.
Your conditional block could be simplified if you only look for your failure cases. In cases like this, I usually step back and start by writing our pseudo code for these complex boolean constructs and let loose with DeMorgan's law.

Here's a pastebin with my changes. It takes you down from 35 loc to 19 loc.  http://pastebin.com/NpSHemqP

--edit-- Wrong sleep deprived academic reference.


Finding two movies whose lengths add up to a given length Here's some code which, given a list of \$N\$ movie lengths, checks if there are two movies that equal the exact length of a flight. This solution runs in \$\mathcal{O}(N)\$ time since I used a hash table.

#include <iostream>
#include <vector>
#include <unordered_map>
using namespace std;

int main() {
    const int LENGTH = 170;
    vector<int> movies = {60, 120, 50, 150, 30, 45};
    unordered_map<int, bool> secondMovies;

    for (int movie: movies)
        secondMovies[movie] = true;

    for (int firstMovie: movies)
        if (secondMovies[LENGTH-firstMovie]) {
            cout << firstMovie << " and " << LENGTH-firstMovie;
            return 0;
        }

    cout << "No movies found.";
    return 0;
}

In the second for loop, I immediately return from main if two valid movies are found. Is this considered bad practice?

Returning from the body of the loop isn't necessary a problem in itself. Nonetheless, your code does have a fairly significant problem.

When you search for a second movie of a particular length, you use the subscript operator. The problem arises from the fact that when this doesn't find an existing entry for a particular length, it inserts a new entry for that length. The value associated with the key will be value-initialized, will will give false in this case.

That means the code will work, but because you're potentially inserting a lot of unnecessary items into your map, you could waste a lot of time. As long as you're only dealing with half a dozen movies, it's probably irrelevant, but if you deal with a lot it could get ugly.

In fact, there doesn't seem to be any reason to use a map here at all. It looks like an unordered_set will work perfectly fine for the task at hand.

There is something else here that could probably be viewed as a bug: if you have a movie whose length is exactly half the total length of the flight, it appears that your code will suggest watching the same movie twice in succession, which probably isn't something anybody would actually want.


Extracting pattern and simplifying testing I observe the same pattern in a number of my classes but can't extract/abstract it due the tight coupling inside each particular implementation.

Having the class accepting an optional number of dependencies onto its constructor:

public Repository(IBuilder, IFormatter, ILoader, ISelector)

With the single method passing its arguments to the first dependency. Passing result to the second. And so on. Returning result of the last dependency:

public XElement Do(int i)
{
    string q = _builder.Build(i);
    string f = _formatter.Format(q);
    XDocument d = _loader.Load(f);
    XElement r = _selector.Select(d);
    return r;
}

(names of the classes, methods and variables are random, just to illustrate the pattern)

Can it be refactored to decouple the flow from the types of dependencies, arguments and return values?

It also will simplify the (unit) testing because currently I make sure that a dependency N passes its result to a dependency N+1:

[TestMethod]
public void Do_Should_Pass_Result_Of_Build_To_Format()

One option would be to create a generic method that accepts delegates to the methods:

var combined = Combine(_builder.Build, _formatter.Format, _loader.Load, _selector.Select);
return combined(i);

The implementation would look like this:

public static Func<T1, T5> Combine<T1, T2, T3, T4, T5>(
    Func<T1, T2> f1, Func<T2, T3> f2, Func<T3, T4> f3, Func<T4, T5> f4)
{
    return input =>
    {
        var r1 = f1(input);
        var r2 = f2(r1);
        var r3 = f3(r2);
        var r4 = f4(r3);
        return r4;
    };
}

The implementation would need to have overloads for different number of delegates.

Also, I'm not completely sure whether this actually improves the situation. Your original code is very straightforward, this code is harder to understand, because it's not immediatelly clear what does Combine() do.


Pagination object oriented This a little pagination script that I am writing "Object Oriented" and I have no idea how to set the current page equal to total page, if the current page is greater.  I also would really like it if anyone can tell me how to improve my code.

class pagination extends Db_connection {

    public $per_page;
    public $current_page;
    public $sql;

    //put your code here
    function __construct($per = 3, $current = 1) {
        parent::__construct();
        $this->per_page = $per;
        $this->current_page = $current;
    }

    function execute_query($sql) {
        $this->sql = $sql;
        $q = $this->query($sql);
        return $q;
    }

    function total_pages() {
        $total = ceil($this->count_rows() / $this->per_page);
        return $total;
    }

    function current_page() {
        if (isset($_GET['page']) && is_numeric($_GET['page']))
            $this->current_page = intval($_GET['page']);

        return $this->current_page;
    }

    function offset() {
        $off = ($this->current_page() - 1) * $this->per_page;
        return $off;
    }
    public function previous_page(){
        //move to previous record by subtracting one into the current record
        return  $this->current_page - 1;
    }
    public function next_page(){
        //mvove to next record by incrementing the current page by one
        return  $this->current_page + 1;

    }

    function count_rows() {
        $nums = mysqli_fetch_row($this->execute_query("select count(post_id) from Post"));
        return $nums[0];
    }

    function results() {
        $off = $this->offset();
        $per = $this->per_page;
        $query = $this->execute_query("Select * from Post limit $off, $this->per_page");
        while ($result = mysqli_fetch_array($query)) {
            $data[] = $result;
        }
        return $data;
    }
}

If you have to identify the main objects here, we could conclude that you have at least 3

-- The connection to the DataBase

-- The pagination object that knows how to paginate a query

-- The query it self

so ie this is a possible solution:

abstract class DBConnection{
    //Logic
    public function executeQuery(Query $query)
    {
        $sql = $query->getSql();

        //Logic and return results
    }
}

class Query{
    private $sql;
    private $limit;
    private $offset;

    //getters and setters for each private property ... getSql ... setSql ...

    public function __construct($sql)
    {
        $this->sql = $sql;
    }

    public function getSql()
    {
        return $this->sql += "offset $this->getOffset() limit $this->getLimit()";
    }
}

class Paginator
{
    private $currentPage;
    private $perPage;
    private $query;
    private $connection;

    //Getters and setters...

    public function __construct(Query $query, $currentPage = 0, $perPage = 10)
    {
        $this->query = $query;
        $this->connection = new DBConnection();
    }

    public function nextPage()
    {
        $this->currentPage++;
    }

    public function prevPage()
    {
        $this->currentPage--; //check if 0 not substract
    }

    public function execute()
    {
        $this->query->setOffset($this->currentPage * $this->perPage);
        $this->query->setLimit($this->perPage);
        return $this->connection->executeQuery($this->query);
    }
}


Get rows from a table and add rows to google charts The Code

PHP:

<?php
session_start();

if ($_SESSION['Username'] == "")
{
    header('Location: Login.php');
}
?>

Java Script:

<script type="text/javascript" src="https://www.gstatic.com/charts/loader.js"></script>
<script type="text/javascript">
    //Google Stuff
    google.charts.load('current', {packages: ['corechart']});
    google.charts.setOnLoadCallback(drawChart);

    function drawChart() {
        // Define the chart to be drawn.
        var data = new google.visualization.DataTable();
        data.addColumn('string', 'Element');
        data.addColumn('number', 'Percentage');
        data.addRows([
            <?php

            include 'php/dbh.php';

            $Rows = array();

            $sql = "SELECT * FROM Sales";

            $Result = mysqli_query($conn, $sql);

            if ($Result)
            {
                while ($row = mysqli_fetch_assoc($Result)) {
                    $Rows[] = $row;
                }
            }
                $ListArray = array();
                foreach ($Rows as $row)
                {
                    asort($row);
                    $row = explode(",", $row['Products']);
                    $ListArray = $row;

                }

                $numItems = count($ListArray);
                $i = 0;
                foreach ($ListArray as $item)
                {
                    $i2 = 0;
                    $Newitem = ltrim($item);
                    $sql2 = "SELECT * FROM submenus WHERE Name='".$Newitem."'";
                    $Result = mysqli_query($conn, $sql2);
                    if (!$Result)
                    {
                        print_r(mysqli_error($conn));
                    }
                    if ($Result)
                    {
                        while ($row = mysqli_fetch_assoc($Result))
                        {
                            $i2 = $row['Sales'];

                        }
                    }

                    if(++$i === $numItems)
                    {
                        echo "['".$item."',".$i2."]";
                    }
                    else
                    {
                        echo "['".$item."',".$i2."],";
                    }
                }

                ?>
            ]);

        // Set chart options
    var options = {
    title:'Sales',
    titleTextStyle: {color: 'white', bold: true},
    height:300,
    backgroundColor: '#3a3939',
    color: '#fff',
    legend: {textStyle: {color: 'white'}},
    hAxis: {gridlines: {color: 'black'}}};

  // Instantiate and draw the chart.
  var chart = new google.visualization.PieChart(document.getElementById('SalesChar'));
  chart.draw(data, options );
    }

   </script>

HTML:

    <fieldset class="Sales">
        <legend style="color: white;">Sales</legend>
        <div id="SalesChar"/>
    </fieldset>

How it works

The PHP Code just start a session and if the Username is == "" it will redirect to the login page.

The Java Script is Located in the header of a webpage and it simply load and draw rows, inside the data.addRows([]); There is a PHP code that first gets a table called Sales, then for each row it will explode the column called products so it gets  multiple string and then add it to a array then for each string it will get from another table called submenus then if everything goes right it will echo something like this ['Test', 2]

The Html is located in the body and it is only there because it is indicating where the chart should be drawn

What i want to know

i want to know if there is any way possible to make the PHP that is located inside the javascript more compact and more readable (i mean easier to understand) and take a less space.

I am open to any other suggestion

Is the javascript part of the same file as the PHP?  If so, you should show them together as a single file, not as separate pieces of code.

Typically, you should have already made a call to the DB to get records before you ever begin rendering things to the browser like you are doing for your javascript.  Keep these pieces of logic separated.  Your page might look more like:

<?php
// not-shown: file bootstrapping logic, if any
$rows = [];
// not shown: your DB logic to retrieve records

if(empty($rows)) {
    // you have no results to work with so perhaps handle this as error condition and exit/redirect.
}
?>
<script type="text/javascript" src="https://www.gstatic.com/charts/loader.js"></script>
<script type="text/javascript">
    // use json_encode to pass data from PHP to javascript literal
    var rows = <?php echo json_encode($rows); ?>;

    //Google Stuff
    google.charts.load('current', {packages: ['corechart']});
    google.charts.setOnLoadCallback(drawChart);

    function drawChart() {
        // Define the chart to be drawn.
        var data = new google.visualization.DataTable();
        data.addColumn('string', 'Element');
        data.addColumn('number', 'Percentage');
        data.addRows(rows);

        // Set chart options
        var options = {
            title:'Sales',
            titleTextStyle: {color: 'white', bold: true},
            height:300,
            backgroundColor: '#3a3939',
            color: '#fff',
            legend: {textStyle: {color: 'white'}},
            hAxis: {gridlines: {color: 'black'}}
        };

        // Instantiate and draw the chart.
        var chart = new google.visualization.PieChart(document.getElementById('SalesChar'));
        chart.draw(data, options );
    }
</script>

Note my use of json_encode() to serialize the PHP data structure into javascript in the code above. This is the #1 most useful way to interchange data between PHP and javascript. Embrace the power of JSON serialization to simplify data transfer. Dynamically building javascript data structures via PHP string concatenation is a really bad approach.

I would encourage to look at working with mysqli in an object-oriented manner.  There are a lot of legacy procedural PHP code examples out there, and while it is good to understand procedural style, I would venture to guess that you will likely fall into good development habits more easily if you bias yourself towards object-oriented programming for PHP.

Your SQL has some problems:

Don't use SELECT *. It is lazy and a potential point for application fragility.  Why make a developer go look at a database table in order to understand what fields are available to a particular section of code?  Why waste extra bandwidth sending across fields that are not needed in a certain area of code?
You have fallen into a querying-in-a-loop anti-pattern. Almost always, when you see code like this, it should be a red flag to you that perhaps a join is a better option than sending a bunch of individual queries against a database.
You may have a schema problem.  Why work with Sales/Products as a comma-separated list?  Should this be normalized into its own table (potentially with aggregation into comma-separated values if needed for purposes of a given query)?


Web API with IoC and database integration tests I've made integration tests for a major part of my application. I test my dependency injection is set up correctly, creating controllers with it's dependencies going all the way to the database (using localDB) and returning the response. I'm looking for what could be improved.

Here is the general concept of my tests:

public class TestDbInitializer : DropCreateAlways<MyContext>()
{
    public static List<Item> Items;

    public override Seed(DbContext context)
    {
        Items = new List<Item>();

        // Adding items
        // ..

        Items.ForEach(x => context.Add(x));

        context.SaveChanges();
    }
}

public class BaseTransactionsTests
{
    private TransactionScope _scope

    [TestInitialize]
    public void Initialize()
    {
        _scope = new TransactionScope();
    }

    [TestCleanup]
    public void Cleanup()
    {
        _scope.Dispose();
    }
}

[TestClass]
public class IntegrationTests : BaseTransactionsTests {

    private IDependenciesContainer _container;

    public static void AssemblyInit(TestContext context)
    {
        Database.SetInitializer(new TestDbInitializer());

        _container = new DependenciesContainer();

        // Registers all my application's dependencies
        _container.RegisterAll();
    }

    [TestInitialize]
    public void Initialize()
    {
        using (var context = new MyContext("TestsDatabase"))
        {
            context.Initialize(true);
        }
    }

    [TestMethod]
    public void TestAddItem()
    {
        var controller = _container.Resolve<MyController>();

        var result = controller.AddItem(new Item({Name = "Test"}))

        var goodResult = result as OkNegotiatedResult<string>();

        if (result == null)
            Assert.Fail("Bad result")

        using (var context = new MyContext("TestsDatabase"))
        {
            Assert.AreEqual(context.Items.Count, TestDbInitializer.Items.Count + 1)
        }
    }
}

I use my dependency injector in my tests, registering all dependencies once (AssemblyInitialize).

I created a DB instance for testings, and a specific DropCreateAlways initializer with a fake data Seed method, which I set as the initializer in the AssemblyInitialize as well.

I want to reseed the database with my fake data before each test run. For that case I implemented the base class which holds a transaction scope.

When I run my tests, the following exception is thrown when seeding the database in the TestInitialize:

  DROP DATABASE statement cannot be used inside a user transaction

How should I deal with it?

Is there a way I can stimulate a real Http request, so that my ModelState.IsValidvalidation will function as it does when running the real thing? Are there any more ways I can improve those testings?

I eneded up initializing the database of my tests in the AssemblyInitialize instead of in TestInitialize. This way my database is created once at the beginning of all tests, and after each test it is rolled back to contain only thr content of the Seeding


TCP Client-Server communications I have developed this simple TCP Server/Client communication program, and am looking for ways of lowering the code footprint where possible.Side note, Both Classes are Main as they run independently, though interact.

TCPClient

package socketUse;
import java.io.BufferedReader;
import java.io.DataOutputStream;
import java.io.InputStreamReader;
import java.net.Socket;
import java.net.SocketTimeoutException;

class TCPClient
{
    public static void main(String[] args) throws Exception
    {
        try
        {
        String input;
        String modedInput;
        System.out.println("You are connected to the TCPCLient;" + "\n" + "Please enter a message:");
        BufferedReader inFromUser = new BufferedReader(new InputStreamReader(System.in));
        @SuppressWarnings("resource")
        Socket client = new Socket("localhost", 22003);
        client.setSoTimeout(3000);
        while(true)
        {
            DataOutputStream outToServer = new DataOutputStream(client.getOutputStream());
            BufferedReader inFromServer = new BufferedReader(new InputStreamReader(client.getInputStream()));
            input = inFromUser.readLine();
            outToServer.writeBytes(input + '\n');
            modedInput = inFromServer.readLine();
            System.out.println("You Sent: " + modedInput);
            try
            {
                Thread.sleep(2000);
            }
            catch(InterruptedException e)
            {
                Thread.currentThread().interrupt();
            }
        }
        }
        catch(SocketTimeoutException e)
        {
            System.out.println("Timed Out Waiting for a Response from the Server");
        }

    }
}

TCPServer

package socketUse;
import java.io.BufferedReader;
import java.io.DataOutputStream;
import java.io.InputStreamReader;
import java.net.ServerSocket;
import java.net.Socket;

class TCPServer
{
    public static void main(String[] args) throws Exception
    {
        try
        {
            String TCPclientMess;
            String returnMess;
            @SuppressWarnings("resource")
            ServerSocket input = new ServerSocket(22003);

            while(true)
            {
                Socket connectionSocket = input.accept();
                BufferedReader iptFromClient = new BufferedReader(new InputStreamReader(connectionSocket.getInputStream()));
                DataOutputStream optToClient = new DataOutputStream(connectionSocket.getOutputStream());
                TCPclientMess = iptFromClient.readLine();
                System.out.println("You Sent: " + TCPclientMess);
                returnMess = TCPclientMess;
                optToClient.writeBytes(returnMess);
            }
        }

        catch(Exception e)
        {
            e.getStackTrace();
        }
    }
}

Efficiency / logic

In the while loop in the client class,
you recreate a DataOutputStream and a BufferedReader from the socket in every single iteration.
This is just silly.
It would be better to create these objects before the loop.

Closing resources

You didn't close any of the resources you open.
As of Java 7,
an easy way to properly close everything is using the try-with-resources idiom,
for example:

try (
    Socket connectionSocket = input.accept();
    BufferedReader iptFromClient = new BufferedReader(new InputStreamReader(connectionSocket.getInputStream()));
    DataOutputStream optToClient = new DataOutputStream(connectionSocket.getOutputStream())
) {
    String TCPclientMess = iptFromClient.readLine();
    System.out.println("You Sent: " + TCPclientMess);
    String returnMess = TCPclientMess;
    optToClient.writeBytes(returnMess);
}

Limit variables to the smallest necessary scope

In both implementation you declare some variables at the top,
when that's really not needed and may invite mistakes.
It's best to declare variables at the smallest scope where you really need them.
For example, instead of:

    String input;
    // ...
    while(true)
    {
        BufferedReader inFromServer = new BufferedReader(new InputStreamReader(client.getInputStream()));
        input = inFromUser.readLine();
        // ...

This is better:

    while(true)
    {
        BufferedReader inFromServer = new BufferedReader(new InputStreamReader(client.getInputStream()));
        String input = inFromUser.readLine();
        // ...

Hardcoded values

Instead of hardcoding "localhost", 22003, 3000,
it would be better to put these values in constants with descriptive names,
for example:

private static final int TIMEOUT_MILLIS = 3000;
private static final String SERVER_HOST = "localhost";
private static final int SERVER_PORT = 22003;

Redundant variable

In TCPServer,
the message read from the client input stream is stored in the TCPclientMess variable,
and then assigned to the returnMess variable,
which is not used for anything else.
You don't need both variables, one of them would be enough.

Naming

You have a String named TCPclientMess.
The convention is to use camelCase for variable names.


Solve Project Euler #5 in Javascript 2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder. What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?

Feel like I went about this the wrong way, but my gut said to check if a number divided evenly by each number up to 20 and when it did, return it.

function isDivisible(number) {
    for (d = 1; d <=20; d++) {
        if (number % d === 0) {
            continue;
        } else {
            return false;
        }
    }
    return true;
}

var number = 1;
do {
    number++;
} while (! isDivisible(number));

console.log(number);

Well, it works. Better algorithms will be given in the discussion list on the Project Euler site, so I'll just look at JavaScript style.

    for (d = 1; d <=20; d++) {

For consistency, there should be a space in <= 20.

        if (number % d === 0) {
            continue;
        } else {
            return false;
        }

When the code skipped by continue is so short, it's more readable to invert the condition and remove the continue:

        if (number % d !== 0) {
            return false;
        }


Prim's Algorithm using heapq module in python Implementing Prim's with a \$O(m\log n)\$ run time using heaps, similar to Dijkstra heap implementation. Where \$n\$ is the number of nodes, and \$m\$ is the number of edges. Using the heapq library I tried to create a delete function for my heap ('unexplored') so that this heap only stores crossing edges at every iteration of outer While loop. That didn't work, I got amazing runtime but wrong result, so I just cut corners and used heapify and inf. But now the run time is around \$O(nm)\$, but I do get the correct answer now though.

graph is a dict, which contains adjacency lists of an undirected graph as:

{v1: [[v2, cost2], [v3, cost3],... ]... }

from math import inf
from heapq import heappush, heappop, _siftup, _siftdown, heapify
from time import time

def prims(graph):
    start = list(graph)[0]
    explored, treeCosts = set([start]), []

    unexplored = []
    for v in graph[start]:
        heappush(unexplored, [v[1], v[0]])

    while explored != set(graph):
        winner = heappop(unexplored)

        explored |= set([winner[1]])
        treeCosts.append(winner[0])

        #deleting edges which have head at 'winner'
        for v in unexplored:
            if v[1] == winner[1]:
                v[0] = inf
                '''i = unexplored.index(v)
                unexplored[i], unexplored[-1] = unexplored[-1], unexplored[i]
                unexplored.pop()
                if i < len(unexplored):
                    _siftup(unexplored, i)
                    _siftdown(unexplored, 0, i)'''
        heapify(unexplored)

        #adding the new crossing edges to heap
        for v in graph[winner[1]]:
            if v[0] not in explored:
                heappush(unexplored, [v[1], v[0]])

    return sum(treeCosts)

if __name__ == '__main__':
    draft, graph = open('undirectedGraph (weighted, 500).txt').read().splitlines(), {}

    for line in draft[1:]:
        edge = list(map(int, line.split()))
        if edge[0] not in graph:
            graph[edge[0]] = []
        if edge[1] not in graph:
            graph[edge[1]] = []

        graph[edge[0]].append(edge[1:])
        graph[edge[1]].append([edge[0], edge[2]])

    startTime = time()
    print('Good prims: ' + str(prims(graph)) + ', ' + 'Time: ' + str(time() - startTime))

1. Review

There is no docstring. What does prims do? What argument does it take? What does it return? The text in the post would be a good starting point for a docstring.
The code has a lot of sequence lookups [1] and [0]. This makes it hard for the reader to follow (what does [1] mean?). It's clearer to use Python's sequence unpacking to provide names for these elements. For example, instead of:

for v in graph[start]:
    heappush(unexplored, [v[1], v[0]])

write something like:

for neighbour, cost in graph[start]:
    heappush(unexplored, (cost, neighbour))

This makes the data structures easier for the reader to follow.
To pick an arbitrary vertex from graph, the code makes a list of all the vertices, which is then thrown away because only the first vertex in the list is needed:

start = list(graph)[0]

It would be better not to make the list at all:

start = next(iter(graph))   # Arbitrary starting vertex

Instead of keeping a list of costs and summing them at the end:

treeCosts = []
# ...
treeCosts.append(winner[0])
# ...
return sum(treeCosts)

it would be simpler to maintain a running total:

total = 0
# ...
total += winner[0]
# ...
return total

Every iteration, the code tests that all vertices have been explored:

while explored != set(graph):

This takes time proportional to the number of vertices in the graph. It would be better to test whether there are any unexplored edges remaining:

while unexplored:

which can be tested in constant time.
Once a winning vertex has been found and added to the tree, all other edges in the heap that end at that vertex are now invalid. The code tries to delete them from the heap by setting their cost to infinity, but this violates the heap invariant. I can see that you attempted to write some code for trying to restore the heap invariant, but this didn't work and you ended up calling heapify instead, but this takes linear time and so leads to unacceptable (quadratic) overall runtime complexity.

This is a problem that often arises when using a heap: how do you efficiently delete an item? Well, there is a way to do it, but it is somewhat complicated (see this answer on Stack Overflow for a description of the method).

However, there is a much simpler solution to this problem: leave the bad item in the heap and postpone dealing with it until it is popped. (See the fourth bullet under "Priority Queue Implementation Notes" in the heapq documentation.) In the minimum spanning tree algorithm this is easy because you can just check against the explored set, like this:

while unexplored:
    cost, winner = heappop(unexplored)
    if winner in explored:
        continue # Already found via a shorter path

You should check that leaving these items in the heap does not affect the runtime complexity (hint: \$O(\log {n^2}) = O(\log n)\$).
The heap is initialized with all the neighbours of the starting vertex:

unexplored = []
for v in graph[start]:
    heappush(unexplored, [v[1], v[0]])

But this duplicates the code for adding the neighbours of the winning vertex:

#adding the new crossing edges to heap
for v in graph[winner[1]]:
    if v[0] not in explored:
        heappush(unexplored, [v[1], v[0]])

It would be simpler to start with just the starting vertex in the heap (at zero cost), and the explored set empty, like this:

explored = set()
unexplored = [(0, start)]

2. Revised code

from heapq import heappush, heappop

def minimum_spanning_tree_cost(graph):
    """Return the sum of the costs of the edges in the minimum spanning
    tree for the given graph, which must be a mapping from nodes to an
    iterable of (neighbour, edge-cost) pairs.

    """
    total = 0                   # Total cost of edges in tree
    explored = set()            # Set of vertices in tree
    start = next(iter(graph))   # Arbitrary starting vertex
    unexplored = [(0, start)]   # Unexplored edges ordered by cost
    while unexplored:
        cost, winner = heappop(unexplored)
        if winner not in explored:
            explored.add(winner)
            total += cost
            for neighbour, cost in graph[winner]:
                if neighbour not in explored:
                    heappush(unexplored, (cost, neighbour))
    return total

3. What if vertices can't be compared?

The heap contains pairs (cost, vertex) which get compared by the heap algorithm to see which is smallest. If two cost values are the same then the heap algorithm will have to compare the vertices to break the tie. This is fine if vertices can be compared (for example if vertices are represented by numbers or strings) but if they cannot be compared then the code will fail with:

TypeError: unorderable types

This is a common problem: see the second bullet under "Priority Queue Implementation Notes" in the heapq documentation. Again, there is a general solution: add a tie-breaking value between the cost and the vertex. itertools.count is a good source of tie-breaking values.

from heapq import heappush, heappop
from itertools import count

def minimum_spanning_tree_cost(graph):
    """Return the sum of the costs of the edges in the minimum spanning
    tree for the given graph, which must be a mapping from nodes to an
    iterable of (neighbour, edge-cost) pairs.

    """
    tiebreak = count().__next__ # Factory for tie-breaking values
    total = 0                   # Total cost of edges in tree
    explored = set()            # Set of vertices in tree
    start = next(iter(graph))   # Arbitrary starting vertex
    unexplored = [(0, tiebreak(), start)] # Unexplored edges ordered by cost
    while unexplored:
        cost, _, winner = heappop(unexplored)
        if winner not in explored:
            explored.add(winner)
            total += cost
            for neighbour, cost in graph[winner]:
                if neighbour not in explored:
                    heappush(unexplored, (cost, tiebreak(), neighbour))
    return total


Any criticism on my JavaScript micro-library? I made this little library for personal use to make javascripting faster and easier.
I would like to know if there are any inefficiencies in it (in size and performance) and possibly if there is a feature you think is missing that would be useful.

//PUT ALL CODE AT THE END OF THE PAGE!!!!!! THIS IS UBER IMPORTANT

//document shorthand
doc=document

//querySelectorAll shorthand
function $q(a){return doc.querySelectorAll(a)}

//returns specific element
function $(a,b){return $q(a)[b||0]}

//function to set properties in a chainable manner
Element.prototype.$p=function(a){for(b in a)this[b]=a[b];return this}

//creates a class that hides elements (notice the use of the function above)
$("head").appendChild(doc.createElement("style").$p({"innerHTML":".wqh{display:none}"}))

Element.prototype.$p({
"removeClass":function(a){this.classList.remove(a)},
"addClass":function(a){this.classList.add(a)},
"hide":function(){this.addClass("wqh")},
"show":function(){this.removeClass("wqh")},
//loads content of a file inside the element, example: element.load("random file.txt")
"load":function(a,b,c){c=this;b=new XMLHttpRequest;b.open("GET",a,!0);b.send();b.onreadystatechange=function(){c.innerHTML=b.responseText}}
})

Example of its uses:

$(".test").addClass("red")
$(".test",1).removeClass("big")
$(".test",2).hide()

for(a in b=$q("p"))
b[a].style.fontFamily="serif",
b[a].style.color="blue",
b[a].style.fontSize="20px";

More readable version of the $p() function:

//function to set properties in a chainable manner
Element.prototype.$p = function(proplist){
    var property;
    for(property in proplist){this[property] = proplist[property]}
    return this;
}

Problems with it :

in the global namespace
use of innerHTML
no cross-compatibility
messing with object prototypes
quite useless


SQL structure to save coordinates for spigot plugin I'm writing a spigot plugin where I give the user the possibility to send a message to the admins of the server. One of the requirements is that the coordinates of the player are stored, when the message is sent, and that it is stored in a sql table, so that admins on multiple spigot servers can access them independently of where the message was sent originally.
I have got a simple sql table, which stores the message, the player's uuid, etc in a table, but I really dislike the idea that I have to store all three values of the Vec3 in separate columns. I googled for a possibility to simply store a Vec3 via SQL, but had no luck.
Here's an overview of my current table:

I just have this one table with the data in it, which I create like this:
CREATE TABLE IF NOT EXISTS Meldung (
    id INT AUTO_INCREMENT NOT NULL,
    UUID VARCHAR(64),
    NAME VARCHAR(64),
    MESSAGE TEXT,
    XCOORD INT,
    YCOORD INT,
    ZCOORD INT,
    ISREAD TINYINT(1),
    primary key(id));

I'd like to know how I can improve in this matter, or the general way how my data is stored. I'm particularly interested in alternative methods to store these coordinates, I don't know why, I find my current way very unsatisfying.

One of the requirements is that the coordinates of the player are stored, when the message is sent, and that it is stored in a sql table, so that admins on multiple spigot servers can access them independently of where the message was sent originally.

Which raises questions about transactions and concurrent access.

CREATE TABLE IF NOT EXISTS Meldung (
    id INT AUTO_INCREMENT NOT NULL,
    UUID VARCHAR(64),
    NAME VARCHAR(64),
    MESSAGE TEXT,
    XCOORD INT,
    YCOORD INT,
    ZCOORD INT,
    ISREAD TINYINT(1),
    primary key(id));

Your table structure raises many questions. Let's start with the obvious that MySQL table names might or might not be case-sensitive, depending on whether it runs on a filesystem that is case-sensitive or not.
The second is, do you need all columns to be nullable?
Third, I believe that at least UUID and ISREAD should have an index assigned, but that depends on the use-case mostly.
    id INT AUTO_INCREMENT NOT NULL,

Why is the casing different to everything else?
    UUID VARCHAR(64),

That's a bad column name, what UUID? Of what? PLAYER_UUID oder rather PLAYER_ID would be a better choice.
    NAME VARCHAR(64),

Same here, name of what?
    XCOORD INT,
    YCOORD INT,
    ZCOORD INT,

Unless the database provides a fitting datatype, storing the single values of a vector is the best thing you can do. Do not give into the idea "I will store this as string and parse it later", that's going to bite you or someone else down the road.

What I'm missing in this table is a CREATED_AT and READ_AT datetime field. The later could even double as flag whether it was read or not, by having it nullable.
What I'd also rather do is have a separate Player table, which allows to not store the name multiple times:
CREATE TABLE Player (
    -- Note that I've skipped a column-name prefix here,
    -- as it is clear what is meant because of the table.
    UUID VARCHAR(64) PRIMARY KEY,
    NAME TEXT NOT NULL
)

CREATE TABLE Meldung (
    -- Primary Keys are not nullable by default.
    ID INT AUTO_INCREMENT PRIMARY KEY,
    PLAYER_UUID VARCHAR(64) FOREIGN KEY REFERENCES Player(UUID),
    MESSAGE TEXT NOT NULL,
    PLAYER_LOCATION_X INT NOT NULL,
    PLAYER_LOCATION_Y INT NOT NULL,
    PLAYER_LOCATION_Z INT NOT NULL,
    -- Could also be SENT_AT.
    CREATED_AT DATETIME NOT NULL DEFAULT NOW(),
    READ_AT DATETIME
)

On another note, I like to use lowercase SQL with all columns and table names uppercase, like this:
create table PLAYER (
    -- Note that I've skipped a column-name prefix here,
    -- as it is clear what is meant because of the table.
    UUID varchar(64) primary key,
    NAME text not null
)

create table MELDUNG (
    -- Primary Keys are not nullable by default.
    ID int auto_increment primary key,
    PLAYER_UUID varchar(64) foreign key references PLAYER(UUID),
    MESSAGE text not null,
    PLAYER_LOCATION_X int not null,
    PLAYER_LOCATION_Y int not null,
    PLAYER_LOCATION_Z int not null,
    CREATED_AT datetime not null default NOW(),
    READ_AT datetime
)

As it is quite easier to type, and anything that is upper-case can be easily identified as table or column. It also removes any of the ambiguity regarding case-sensitivity in MySQL.


Function invocation every second time Recently I got test task at the project, it is fairly simple:

const _ = require('underscore');

// Challenge:
//
// Write a function that accepts as argument a "function of one variable", and returns a new
// function.  The returned function should invoke the original parameter function on every "odd"
// invocation, returning undefined on even invocations.
//
// Test case:
//  function to wrap via alternate(): doubleIt, which takes a number and returns twice the input.
//  input to returned function: 1,2,3,4,9,9,9,10,10,10
//  expected output: 2, undefined, 6, undefined, 18, undefined, 18, undefined, 20, undefined

const input = [1,2,3,4,9,9,9,10,10,10];
const doubleIt = x => x * 2;

const alternate = (fn) => {
  // Implement me!
  //
  // The returned function should only invoke fn on every
  // other invocation, returning undefined the other times.
}

var wrapped = alternate(doubleIt)

_.forEach(input, (x) => console.log(wrapped(x)))
// expected output: 2, undefined, 6, undefined, 18, undefined, 18, undefined, 20, undefined

And my solution was:

const alternate = (fn) => {
  let odd = false;

  return (x) => {
    odd = !odd;

    if (odd) {
      return fn(x);
    }

    return undefined;
  };
};

// An alternate solution if ternary operator (?) is allowed according to coding standards used on the project.
// Sometimes it's treated as bad practise.
const alternateShort = (fn) => {
  let odd = false;

  return (x) => (odd = !odd) ? fn(x) : undefined;
};

And I got the reply that tech lead didn't like my solution at all and I'm not hired to the project.
I'm really confused, do you have any idea what else he could expect?
Do you see here any alternative solutions?

Code style is not the problem. The expectation is that a new coder uses the in house style guides when writing.

I think the problem is that you missed an important assumption.

The test case passes one argument, however that does not mean that the function you are given will have only one argument.

Thus maybe (I am just guessing) if you wrote...

const alternate = fn => {
    let odd = false;
    return (...args) => (odd = !odd) ? fn(...args) : undefined;
};

Also though not stated there could have been an expectation that it not throw on bad input. Thus first vet the input function.

const alternate = fn => {
    let odd = false;
    return typeof fn === "function" ?
        (...args) => odd = !odd ? fn(...args) : undefined :
        () => {};
}

or

function alternate (fn) {
    if (typeof fn === "function") {
        let odd = false;
        return (...args) => odd = !odd ? fn(...args) : undefined;
    }
    return () => {};
}

As the documentation does not say anything about throwing an error the safer bet is to silently deal with bad input


Make a MySQL SELECT and INNER JOIN faster I have the following query which works as it should, but I'd like to know how I can make it run faster.

For info, tables cl, jo and mi all have column ref as the auto_inc primary key and ID as the main identification number.  Therefore, I use the refs to keep a history of changes for ID.  So, I use SELECT MAX(ref) FROM [xx] GROUP BY ID to get the most recent cl jo and mi.

SELECT mi.ref, jo.ID, mi.ID AS mID, mt.mi_type, jo.ref, jt.jo_type, cl.cl_name
FROM mi
INNER JOIN mt ON mt.ref = mi.mi_type
INNER JOIN jo ON jo.ID = mi.job
INNER JOIN cl ON cl.ID = jo.cl
INNER JOIN jt ON jt.ref = jo.jo_type
WHERE mi.ref IN (SELECT MAX(ref)
                    FROM mi
                    GROUP BY ID)
AND jo.ref IN (SELECT MAX(ref)
                    FROM jo
                    GROUP BY ID)
AND cl.ref IN (SELECT MAX(ref)
                    FROM cl
                    GROUP BY ID)
ORDER BY cl.cl_name, jt.jo_type, mt.mi_type ASC

Moving your subqueries to the from clause may improve performance.

Additional indexes may help too. Add a clustered index on ID or (ID, ref) for tables mi, jo and cl to speed up the group by and max operations. You may also benefit from a clustered index on each of your order by columns.

SELECT mi.ref, jo.ID, mi.ID AS mID, mt.mi_type, jo.ref, jt.jo_type, cl.cl_name
FROM mi
INNER JOIN mt ON mt.ref = mi.mi_type
INNER JOIN jo ON jo.ID = mi.job
INNER JOIN cl ON cl.ID = jo.cl
INNER JOIN jt ON jt.ref = jo.jo_type
INNER JOIN (SELECT MAX(ref) AS ref FROM mi GROUP BY ID) mi2 ON mi2.ref = mi.ref
INNER JOIN (SELECT MAX(ref) AS ref FROM jo GROUP BY ID) jo2 ON jo2.ref = jo.ref
INNER JOIN (SELECT MAX(ref) AS ref FROM cl GROUP BY ID) cl2 ON cl2.ref = cl.ref
ORDER BY cl.cl_name, jt.jo_type, mt.mi_type


Given a string S and a set of words D, find the longest word in D that is a subsequence of S I'm trying to improve my code and algorithm to find out the longest word from a dictionary occurring as the sub-sequence in the given string.

Example : For example, given the input of S = "abppplee" and D = {"able", "ale", "apple", "bale", "kangaroo"} the correct output would be "apple".

Currently my algorithm compares each character of the dictionary-word with the given-word and advances to next char in dictionary-word on finding a match and so on till either the dictionary-word is iterated over completely(which means the word is a valid sub-sequence) or the given word is iterated over completely(which means not a valid string).

Being a beginner to c++ and algorithms in general, my methods and logic could be further improved. So kindly suggest if any improvements in the algorithm or code which can improve the performance. Thanks.

#include <iostream>
#include <string>
#include <tuple>
#include <vector>

class StringManipulater {
  public:
  StringManipulater(const std::string& p) : given_string_{p} {}
  std::string given_string_;
  std::tuple<std::string, typename std::string::size_type> current_longest_ = {
      "", 0};

  const std::string&& FindLongestSubSeq(const std::vector<std::string>& dict)
  {
    // flag determines whether current word is present in given_string input
    bool flag;

    // iterate through each of the dict-words
    for (const std::string& word : dict) {
      flag = false;
      auto word_len = word.size();

      // fair optimization : only consider current word if its
      // length is greater than previous one
      if (word_len > std::get<1>(this->current_longest_)) {
        // iterate through the letters in both given string and current word
        // to be compared
        for (typename std::string::size_type i = 0, j = 0; j < word_len; ++i) {
          // no need to go further checking if we have iterated over given
          // string
          if (i == this->given_string_.size()) break;

          // compare each letter of both words
          if (word[j] == given_string_[i]) {
            flag = true;
            // advance to next charcter in the dictionary word
            ++j;
          }
          else {
            // if the char couldn't be found
            flag = false;
          }
        }  // end of comparison loop
        if (flag) {
          std::get<0>(this->current_longest_) = word;
          std::get<1>(this->current_longest_) = word_len;
        }
      }  // top if
    }    // end of iteration of dictionary words
    return std::move(std::get<0>(this->current_longest_));
  }
};

int main()
{
  StringManipulater s_manip{"abppplee"};
  std::cout << "Longest subsequence = "
            << s_manip.FindLongestSubSeq(
                   {"able", "ale", "apple", "bale", "kangaroo"})
            << "\n";
  return 0;
}

You know, if you want to improve performance, the first thing to do is get a high-level overview of what your code does to achieve its aim. Currently, you do this:

Iterate over all words in D.

Iterate over the whole string S trying to prove it a sub-sequence.

That's trivially proven to be an \$O(\#D * \#S)\$ algorithm.

You can do better, by using the proper data structure:

Build a prefix-tree from the words, noting the longest word which can be found from there.
Iterate over the string.

Continue if the current character doesn't fit.
Update the best found if it is a terminal node.
recurse with the sub-tree.

Regarding your code:

Your use of a class is completely unmotivated. There are no invariants to protect, as evidenced by you marking it all public, and you only create it to call one single function anyway. Just make it a free function. Ok, if you change the algorithm, you might want to break it up it into building the trie, using a pre-computed trie, and putting it all together.
auto would allow you to always have the proper type, without cluttering your code with long and obscure names. Read "Almost Always Auto".
I'm not sure why you use a std::tuple. Maybe for obfuscation? If so, I confirm you are on the right way.
Always define variables in the smallest scope possible, unless there are significant advantages (most likely in performance) for a slightly longer lifetime.
return 0; is implicit for main().


Factory Design Pattern in Java I am currently attempting to understand the fundamentals of factory methods in Java and I found a lot of exampels where a pattern like this has been used.
I got some exercises about sorting algorithms where one task is to write a factory class for all existing algorithms but with the requirement that newly added algorithmes can be integrated via "plug and play". That phrase itched me.By using the switch case solution I would be unable to add Algorithms without rewriting the factory class so I came up with this:

SorterFactory.java

public final class SorterFactory
{

  private SorterFactory()
  {
  }

  public static Sortierer.Sorter getNewSorter( Sorts sorts )
  {
    try
    {
      return (Sortierer.Sorter) Class.forName( sorts.getName() ).getConstructor().newInstance();
    }
    catch ( ClassNotFoundException | InstantiationException | IllegalAccessException
        | IllegalArgumentException | InvocationTargetException | NoSuchMethodException
        | SecurityException e )
    {
      e.printStackTrace();
      // further exception handling
    }
  }
}

Sorts.java

public final class Sorts
{

  private String name;

  // hardcoded for test purpose.
  // will be imported from XML-file
  public static final Sorts HEAPSORT = new Sorts( HeapSort.class.getName() );
  public static final Sorts MERGESORT = new Sorts( MergeSort.class.getName() );
  public static final Sorts BUBBLESORT = new Sorts( BubbleSort.class.getName() );
  public static final Sorts INSERTSORT = new Sorts( InsertSort.class.getName() );
  public static final Sorts QUICKSORT = new Sorts( QuickSort.class.getName() );
  public static final Sorts TRUEHEAPSORT = new Sorts( TrueHeapSort.class.getName() );

  private Sorts( String name )
  {
    this.name = name;
  }

  public String getName()
  {
    return name;
  }
}

So why is the switch case approach much more favored above this kind of solution in terms of safety,security and performance? Also is it useful to box the names in a class to prevent the misusage of the factory method instead of allow Strings as arguments?

Thanks for any advice and criticism.

In general reflection is slow (relative), makes code harder to understand and near impossible to debug if things go wrong.

So if you already know the exact class it's far better to let the compiler check against typo's for you at compile time.

A second issue is that the way it's written now, you just shifted the problem of "rewriting the factory class" to "rewriting the Sorts class". You can't add any other public constant to that class based on an XML file.

This isn't entirely clear from the limited problem statement you've given, but perhaps you've misunderstood what kind of factory they expect.

Another interpretation of the factory method pattern expects you to provide a factory interface. This interface defines one or more methods that create some Sorter. That way you can "plug" in any other factory to use a different Sorter. (Which, compared to just plugging in a different Sorter directly, is only useful if certain Sorters require something extra to create them. For example an extra input parameter.)

Note that this is still usually defined at compile time.