Fibonacci sequence implementation This was too slow for my computer:

int f(unsigned int x)
{
    if(x <= 1) return 1;
    else return f(x-1)+f(x-2);
}

/* main */
int main()
{
    for(int i = 0; i < 50; i++) cout << f(i) << endl;
    return 0;
}

So I've made a faster implementation. It works, but is it well-written? Is there some way to improve it?

void f(unsigned long now)
{
    static int counter = 0;
    static unsigned long last = 0, tmp = last;
    if(counter++ == 50)
    {
        last = tmp = counter = 0;
        return;
    }
    std::cout << last + now << std::endl;
    tmp = last;
    last = now;
    f(now+tmp);
}

// calling f(1); in main

First of all: If you want execution speed, an iterative approach to fibonacci is faster. That said, implementing a recursive solution is a nice exercise.

Some other remarks:

Code that does something should not perform IO. Separate computations from UI. This will also allow you to output values only once, which is faster.
Your function is very specialized. What if I want to calculate only fib(5)? Or up o fib(65)? Strive to write reusable components.
Note that std::endl flushes the output buffer, which takes time. If you just want to output a newline, use std::cout << last + now << '\n'; instead.
Instead of

static unsigned long last = 0, tmp = last;

write

static unsigned long last = 0, tmp = 0;

It's clearer, and you won't have to change anything in case you remove last at some point.
I would pass state around, rather than use static variables.

Maybe something like this:

// Helper function
int fib_do(int max, int curr, int one_before, int two_before)
{
    if (curr == max-1) return one_before+two_before;

    return fib_do(max, curr+1, one_before+two_before, one_before);
}

int fib(int n)
{
    if (n <= 1) return 1;

    return fib_do(n, 1, 1, 0);
}

However, this snippet (and your second version) is basically just the iterative solution implemented using recursion.

Note that this snippet has not in any way been optimized for speed. However, it facilitates the Return Value Optimization.

Finally: Consider using some form of caching to increase the speed of your recursive function.


Lightweight TicTacToe in F# This is more of a 'model' of TicTacToe, as you'll see it's not "gameified" in the sense that it will allow for inputs and outputs nor will it record any "active" state of the game but those things are trivial to implement.

I'm cheating a little bit on the functional paradigm in my function to rotate the grid, but this is on purpose, it's just easier to rotate the grid by mutation than by going pure functional style, in the end, the state is only manipulated in a local function anyhow so I think it's neglible.

Also there are many times where I have ignored optimizations over conciseness. One might imagine that this would not be an area where optimal performance is key so I have prioritized getting as concise and readable code as possible.

module TicTacToe

type Mark = None | Cross | Nought
type Outcome = None | NoughtWon | CrossWon | Draw

let emptyGrid = (None, Array2D.init 3 3 (fun _ _ -> Mark.None))

let flatten (x: 'a [,]) = x |> Seq.cast<'a>

let rotateCw (grid: 'a [,]) =
    let out = Array2D.zeroCreate 3 3
    for y in 0..2 do
        for x in 0..2 do
            out.[x,y] <- grid.[0 + y, 2 - x]
    out

let checkOutcome grid =
    let checkSeg seg =
        seq { for i in 0..2 do
                let count =
                    seg i |> Array.sumBy
                        (function Cross -> 1 | Nought -> 2 | _ -> 0)
                if count = 6 || count = 3 then
                    yield (if count = 6 then NoughtWon else CrossWon) }
        |> Seq.tryFind ((<>)None)
    let vertical =
        let rot = grid |> rotateCw
        (fun x -> rot.[x, 0..2])
    let horizontal = (fun y -> grid.[0..2, y])
    let diagonal (m:_[,]) = (fun _ -> [| for i in 0..2 -> m.[i,i] |])
    let diagonal2 = grid |> rotateCw |> diagonal
    let checks =
        [checkSeg vertical; checkSeg horizontal;
         checkSeg (diagonal grid); checkSeg diagonal2]
    if checks |> List.exists (Option.isSome) then
        checks |> List.find (Option.isSome) |> Option.get
    elif grid |> flatten |> Seq.forall ((<>)Mark.None) then Draw
    else None

let placeMark pos mark (_, grid: Mark [,]) =
    let res =
        Array2D.init 3 3 (fun x y ->
            if (x,y) = pos then mark else grid.[x,y])
    (checkOutcome res, res)

let emptyGrid = (None, Array2D.init 3 3 (fun _ _ -> Mark.None))

This can be simplified using Array2D.create:

let emptyGrid = None, Array2D.create 3 3 Mark.None

In rotateCw and placeMark, x is used as the first index and y as the second. This is confusing, as the usual order is row then column. For instance,

let test = Array2D.create 3 3 Mark.None
test.[2, 1] <- Cross
printfn "%A" test

[[None; None; None]
 [None; None; None]
 [None; Cross; None]]

rotateCw can be simplified (assumes the indexing changes in the previous paragraph)

let rotateCw (grid : 'a[,]) = Array2D.init 3 3 (fun y x -> grid.[2 - x, y])

I find this version of placeMark a bit more readable

let placeMark (x, y) mark (_, grid : Mark[,]) =
    let res = Array2D.copy grid
    res.[y, x] <- mark
    (checkOutcome res, res)


Messages with status codes I need to refactor following class:

public class Message
{
    public Guid ID { get; set; }
    public string MessageIn { get; set; }
    public string MessageOut { get; set; }
    public int StatusCode { get; set; } //EDIT could be changed during message lifecycle

    public bool IsDeletable
    {
        get
        {
            switch (this.StatusCode)
            {
                case 12:
                case 13:
                case 22:
                case 120:
                    return true;
                default:
                    return false;
            }
        }
    }

    public bool IsEditable
    {
        get
        {
            switch (this.StatusCode)
            {
                case 12:
                case 13:
                case 22:
                case 120:
                    return true;
                default:
                    return false;
            }
        }
    }

    public string Message
    {
        get
        {
            switch (this.StatusCode)
            {
                case 11:
                case 110:
                    return this.MessageIn;
                case 12:
                case 13:
                case 22:
                case 120:
                    return this.MessageOut;
                default:
                    return string.Empty;
            }
        }
    }
}

I would like to remove the business rules IsDeletable and IsEditable
I would like to remove these switch statements at the same time

I am not sure if it's worth knowing that I am mapping entity to database table through Entity Framework.

One more problem that I have is that fields MessageIn and MessageOut are dependent on StatusCode. One of them are always populated. I could create new property but still the switch case is there:

public string Message
{
    get
    {
        switch (this.StatusCode)
        {
            case 10:
            case 12:
            case 13:
            case :
              return this.MessageIn;
           default:
             return this.MessageOut;
        }
    }
    set { // switch again}
}

I would like to remove these switch statments in the same time

[Flags]
public enum StatusCode{
    codeX = 1,
    codeY = 2,
    codeZ = 4,
    codeA = 8,
    editable = codeX | codeZ,
    deleteable = codeY | codeZ
}

Key points

Use the Flags attribute
enum values are powers of 2 - NOT multiples of 2
bit-wise AND, OR provides the magic!

Edit

addressing the comments:

public static class StatusCodes {
    private Dictionary<StatusCode, int> values;
    private Dictionary<int,StatusCode> keys;

    static StatusCodes() {

        values = new Dictionary<StatusCode, string> {
            {StatusCode.A, 10},
            {StatusCode.B, 20},
            // and so on
        }

        keys = new Dictionary<in, StatusCode> {
            {10, StatusCode.A},
            {20, StatusCode.B},
        }
    }

    public static int GetValue(StatusCode theStatusCodeKey) {} // don't forget error trapping!
    public static StatusCode GetKey(int theIntValue) {}  // ditto

    public static bool Editable(StatusCode thisCode) {
        return (StatusCode.editable & thisCode) == thisCode;
    }

    public static bool Editable(int thisValue) {
        return Editable( GetKey(thisValue));
    }
}

The definition of the codes is all in one place - that's SOLID (D = DRY, don't repeat yourself)
The definitions are in its own class - that's SOLID (S = single responsibility)
editable and deleteable can be removed from Message - that's very SOLID (S = single responsibility)
We know what all those integers mean - thats.. well, just good programming.
The StatusCodes are available anywhere and everywhere in the application - That's SOLID, an attribute of being DRY.
I'm not sure what "status code values might not be fixed" in the comments means. Surely the set of status codes is finite.

Edit

Define "editability" in StatusCodes
Use above to express Message is editable
Address issue of exposing StatusCode.editable "as if it were a valid code"
Adhere to Single Responsibility
Adhere to DRY principle

...

public static class StatusCodes
{
    private static Dictionary<StatusCode, int> values;
    private static Dictionary<int,StatusCode> keys;

    static StatusCodes() {

        values = new Dictionary<StatusCode, int> {
            {StatusCode.A, 10},
            {StatusCode.B, 20},
            {StatusCode.C, 30},
            {StatusCode.D, 40}
            // and so on
        };

        keys = new Dictionary<int, StatusCode> {
            {10, StatusCode.A},
            {20, StatusCode.B},
            {30, StatusCode.C},
            {40, StatusCode.D}
        };
    }

    [Flags]
    enum Fungability
    {
        Editable = StatusCode.A | StatusCode.B,
        Deleteable = StatusCode.B | StatusCode.D
    }

    public static int GetValue( StatusCode theStatusCodeKey ) {
        int retVal;

        values.TryGetValue( theStatusCodeKey, out retVal );
        return retVal;
    } // don't forget error trapping!

    public static StatusCode GetKey( int theIntValue ) {
        StatusCode retVal;
        keys.TryGetValue( theIntValue, out retVal );
        return retVal;
    }  // ditto

    public static bool Editable( StatusCode thisCode )
    {
        return ( (StatusCode)Fungability.Editable & thisCode ) == thisCode;
    }

    public static bool Editable( int thisValue )
    {
        return Editable( GetKey( thisValue ) );
    }

}

public class Message
{
    public StatusCode myStatus;

    public Message( int statusCode = 20 ) { myStatus = StatusCodes.GetKey(statusCode); }
    public Message( StatusCode statusCode = StatusCode.A ) { myStatus = statusCode; }

    public bool Editable
    {
        get { return StatusCodes.Editable( myStatus ); }
    }

    public bool Deleteable
    {
        get { return StatusCodes.Deleteable( myStatus ); }
    }
}

Take Away

Structure data in an OO way
Expose the data adhering to the Single Responsibility principle
You get DRY as a side effect
Structure yields simplicity, coherence, clarity. Editable is implemented with only one line of code!
Message.Editable went from originally "calculating" if the status code was editable, to simply asking the StatusCode "are you editable?"


Counting adenine and cytosine bases I've started a little challenge on a website, and the first one was about counting different DNA letters. I've done it, but I found my method very brutal. I have a little experience, and I know that hard-coded information is not good. Any way to improve this code?

chaine = "AAAACCCGGT"

nA = 0
nT = 0
nG = 0
nC = 0

for i in range(len(chaine)):
   if chaine[i] == "A":
       nA = nA + 1
   if chaine[i] == "C":
       nC = nC + 1
print nA, nC

for i in range(len(...)): is almost never the right way to go in Python. Instead, in this case, you should iterate directly over the characters:

for char in chaine:
    if char == 'A':
        ...

Rather than separate names nA, nC, etc., use a dictionary to map each character to its count:

counts = {'A': 0, 'C': 0, ...}

Now you don't need the ifs at all:

for char in chaine:
    counts[char] += 1
print counts['A'], counts['C']

In fact, Python already implements collections.Counter, a dictionary subclass that makes this task trivial:

from collections import Counter

counts = Counter(chaine)
print counts['A'], counts['C']


Sort array of hashes with nil values last I wrote a method which sorts an array of hashes by given hash keys. The method should put nil values at the end.

def sort(records, *attrs)
  records.sort do |a,b|
    result = 0
    attrs.each do |attr|
      unless a[attr] == b[attr]
        result = if a[attr].nil?
                   1
                 elsif b[attr].nil?
                   -1
                 else
                  a[attr] <=> b[attr]
                 end
        break
      end
    end
    result
  end
end

p sort([{:a => 1},{:a => nil},{:a => 2}], :a)
#=> [{:a=>1}, {:a=>2}, {:a=>nil}]
p sort([{:a => nil},{:a => 'x'},{:a => 'a'}], :a)
#=> [{:a=>"a"}, {:a=>"x"}, {:a=>nil}]

My solution looks quite complex. Is there a better way to achieve the ordering in Ruby?

You can create temporary sorting columns:

def sort records, *attrs
  records.sort_by do |h|
    h.values_at(*attrs).map do |v|
      v.nil? ? [2] : [1, v]
    end
  end
end

Here I added columns with values 1 or 2 to the left for higher priority -- could add to the right or even in between for more complex sorting.


2D OpenGL tile atlas fragment shader I am currently using the following glsl code to retrieve separate textures from a tile atlas:

#version 330 core
in vec2 TexCoord;
out vec4 color;

uniform sampler2D image;

void main()
{
    //width and height in tiles of atlas
    int width = 8;
    int height = 8;
    //x and y position of a tile in the atlas
    int x = 1;
    int y = 4;

    float scalarX = 1.0 / width;
    float scalarY = 1.0 / height;

    color = vec4(1.0) * texture(image, vec2((TexCoord.x + x) * scalarX,  (TexCoord.y * scalarY) + y * scalarY));
}

The tile atlas is made up of: 8*8 sprites each with dimensions of: 16*16 pixels

Is this the best method to use when carrying out this task and could my code be improved / made more efficient?

Why don't you simply set the texture coordinates properly and have a "pass-thru" shader that just samples the texture and nothing else? For example, if you're using glVertexAttribPointer() with an array of vertices and texture coords, make sure that the texture coordinates you send have the proper tile offset. Something like this:

const int numXTiles = 8;
const int numYTiles = 8;
const int numXPixelsPerTile = 16;
const int numYPixelsPerTile = 16;
const int atlasWidth = numXTiles * numXPixelsPerTile;
const int atlasHeight = numYTiles * numYPixelsPerTile;
const double pixelXDelta = 1.0 / atlasWidth;
const double pixelYDelta = 1.0 / atlasHeight;
typedef struct vertex {
    Point3D position;
    Point2D texCoord;
} vertex;
// Assume you're drawing a square and want to texture it with tile 3,4
vertex square[] = {
    { {-1.0, -1.0, 0.0 }, { 3.0 * numXPixelsPerTile * pixelXDelta, 4.0 * numYPixelsPerTile * pixelYDelta } },
    { { 1.0, -1.0, 0.0 }, { 4.0 * numXPixelsPerTile * pixelXDelta, 4.0 * numYPixelPerTile * pixelYDelta } },
    { { 1.0, 1.0, 0.0 }, { 4.0 * numXPixelsPerTile * pixelXDelta, 5.0 * numYPixelPerTile * pixelYDelta } },
    { { -1.0, 1.0, 0.0 }, { 3.0 * numXPixelsPerTile * pixelXDelta, 5.0 * numYPixelPerTile * pixelYDelta } }
};
glBufferData(GL_ARRAY_BUFFER, 4, square, GL_STATIC_DRAW);
glVertexAttribPointer(0, 3, GL_FLOAT, sizeof(vertex), 0); // Position
glVertexAttribPointer(1, 2, GL_FLOAT, sizeof(vertex), (GLvoid*)sizeof(Point3D)); // Texture Coordinate

This assumes your vertex shader has attribute 0 as position and attribute 1 as texture coordinates. Then your fragment shader just becomes:

#version 330 core
in vec2 TexCoord;
out vec4 color;

uniform sampler2D image;

void main()
{
    color = texture(image, TexCoord);
}

NOTE: I may be off by 1 on the right and top side. You might need to subtract 1 pixelXDelta and 1 pixelYDelta from those coordinates - I can never remember.


C-style `switch` statement implementation using decorators Yes I know context managers are a much better way to do this, but I spent four hours working on implementing a switch construct using generic decorators, and I'd like to hear feedback on how I did. I'll do it with context managers tomorrow.

I don't think I qualify as a beginner anymore (I'm looking to start getting paid to write Python soon), and I would appreciate guidance on best practices.

My Code

"""Module to implement C style switch functionality using decorators.

  Functions:
    `switch_construct`: Returns functions that provide the required functionality and provides a closure for said functions.
"""

__all__ = ["switch", "case", "break_"]

import operator as op

def switch_construct():
  """Forms a closure to store the meta data used by the switch construct.

    Vars:
      `closure` (list): List to store dictionaries containing metadata for each `switch` instance.
        * The dictionary contains two keys: "control" and "halt".
          - "control": the value governing the current switch instance. `case` blocks are tested against this value.
          - `halt`: value is used to implement break functionality.
            > `True`: break after execution of current function.
            > Default: `False`.

    Returns:
      tuple: A tuple of functions (`switch`, `case`, `break_`) defined in the function's scope.
  """
  closure = []

  def switch(input_):
    """Sets the metadata for the current `switch` block.

      Args:
        `input` (object): The value governing the switch construct.

      Returns:
        `function`: A decorator for the function enclosing the switch block.
    """
    nonlocal closure
    def decorator(func):
      closure.append({"control": input_, "halt": False})  # Add the metadata to closure.
      # Sets `control` to `input_` and `halt` to `False` for each `switch` block.
      def wrapper(*args, **kwargs):
        return func(*args, **kwargs)
      wrapper.switch_index = len(closure)-1
      # Assign the `switch_index` attribute to the decorated function for using when retrieving switch construct metadata.
      return wrapper
    return decorator

  def case(value, enclosing, test = op.eq):
    """Returns a decorator which executes decorated function only if its `value` argument satisfies some test.

      Args:
        `value` (object): value used to test whether the decorated function should be executed.
        `enclosing` (function): enclosing function corresponding to the current switch block.
        `test` (function): test that should be satisfied before decorated function is executed.
          - Default value is `op.eq`.
          - Returns `bool`.

      Returns:
        function: A decorator for the function corresponding to the case block.
    """
    def decorator(func):
      nonlocal closure
      index = enclosing.switch_index
      return_value = None
      if test(value, closure[index]["control"]) and not closure[index]["halt"]:
        return_value = func()
      return return_value
    return decorator

  def break_(enclosing):
    """Used to indicate that the switch construct should break after execution of the current function."""
    nonlocal closure
    closure[enclosing.switch_index]["halt"] = True
    return None

  return case, switch, break_

case, switch, break_ = switch_construct()

@switch(2)
def func():
  """Sample use case of the switch construct."""
  @case(1, func)
  def _(num=10):
    print(num + 1)

  @case(2, func)
  def _(num=10):
    print(num + 2)
    break_(func)

  @case(3, func, op.gt)
  def _(num=10):
    print(num + 3)

func()

I wish I had enough practice with decorators to be able to comment on most of this. I've never really needed to play with them, so I've kind of neglected them beyond simple memoization toys. This looks pretty cool, but I think all I can comment on are a few things along with the low-hanging PEP fruit since no-ones jumped on that yet :)

len(closure)-1

This should have spaces in there:

  Always surround these binary operators with a single space on either side: assignment (=), augmented assignment (+=, -= etc.), comparisons (==, <, >, !=, <>, <=, >=, in, not in, is, is not), Booleans (and, or, not).

It doesn't explicitly mention -, but the "No:" examples after that bit above suggest that it should use spaces as well.

And on the other end,

test = op.eq

Shouldn't use spaces (from the link above again):

  Don't use spaces around the = sign when used to indicate a keyword argument, or when used to indicate a default value for an unannotated function parameter.

break_ doesn't need a return None in there. That's implicit.

Some of what you're documenting with doc-strings could be documented in other arguably cleaner ways.

For example, in switch_construct you have:

* The dictionary contains two keys: "control" and "halt".
          - "control": the value governing the current switch instance. `case` blocks are tested against this value.
          - `halt`: value is used to implement break functionality.
            > `True`: break after execution of current function.
            > Default: `False`.

You have a dictionary that should only take on certain keys. This sounds more like a job for a dataclass instead of a dictionary:

from dataclasses import dataclass

@dataclass
class SwitchMetadata:
    """- "control": the value governing the current switch instance. `case` blocks are tested against this value.
       - `halt`: value is used to implement break functionality.
    """

    control: str
    halt: bool = False  # Makes the constructor allow halt to default to False

. . .

closure.append(SwitchMetadata(input_))

. . .

if test(value, closure[index].control) and not closure[index].halt

. . .

closure[enclosing.switch_index].halt = True

This gets rid of the need for string dictionary lookups and gives some type safety. Both of these decrease the chance you'll make silly typos since both allow for static checking by the IDE. The class definition also clearly communicates to the reader what the structure holds and what types the data should be.


php preg_replace kills cpu I am reading some HTML files and returning them upon request, but when they go through my codes I need to filter out some stuff from these HTMLs, everything is working fine and I'm using about 20 or more str_replace and preg_replace to do that which works pretty fine.

Until I used this

$responseBody = preg_replace('/<script>((?!<)[\s\S])*googletagmanager\.com[\s\S]*?<\/script>/', '', $responseBody);

which is supposed to remove this part from html

<script>
(function (w, d, s, l, i) {
    w[l] = w[l] || []; w[l].push({
        'gtm.start':
        new Date().getTime(), event: 'gtm.js'
    }); var f = d.getElementsByTagName(s)[0],
    j = d.createElement(s), dl = l != 'dataLayer' ? '&l=' + l : ''; j.async = true; j.src =
    '//www.googletagmanager.com/gtm.js?id=' + i + dl; f.parentNode.insertBefore(j, f);
})(window, document, 'script', 'dataLayer', 'GTM');</script>

and it does that very well in a regex tester => http://regexr.com/3e8nb
but on the server cpu just goes up and the server halts.
why is this happening? and how can I improve this regex?

Simple as that:

/<script>[^<]*?googletagmanager\.com.*?<\/script>/s

What's killing performance for you, is most likely the backtracking followed by a look-ahead on the first padding. Using a character class instead of a look-ahead, and making it non-greedy should solve that issue.

The /s modifier allows you to use . instead of explicit [\s\S].


Scale an image to fit some maximum dimension using vanilla JavaScript code Fill in the ? with a Javascript expression to set the scale ratio for
  an image having a given height and width so that it fit inside a
  maxdim-by-maxdim square area (touching at least two edges).

function scaleImage(width, height, maxdim) {
  var scale = ?;
  return [scale * width, scale * height];
}

Here is my solution:

function scaleImage(width, height, maxdim) {
  var scale = (width > height ? maxdim/width : maxdim/height);
  return [scale * width, scale * height];
}

Is there a cleaner way of doing this?

I would probably use Math.max(), as that seems more readable to me and would make what is happening more clear. However, I don't think it will affect efficiency in any meaningful way since it's still making a comparison to determine the max.

var scale = maxdim / Math.max(width, height);


Atomically incrementing a value in a map in Clojure I am working out a bit of Clojure code that will take a ref to a map and increment a key value pair in the map.  I think I am using ref correctly, but Im not sure about atom.  Do I need to use swap! to be more idiomatic?  I am new to STM and Clojure, does this look thread-safe / sane?  What am I missing?

(defn increment-key [ref key]
    (dosync
        (if (= (get @ref key) nil)
            (alter ref assoc key (atom 1))
            (alter ref assoc key (atom (inc @(get @ref key)))))))

(defn -main [& args]
    (def my-map (ref {}))
    (increment-key my-map "yellow")
    (println my-map)
    (increment-key my-map "yellow")
    (println my-map))

Prints

$ lein run
#<Ref@494eaec9: {yellow #<Atom@191410e5: 1>}>
#<Ref@494eaec9: {yellow #<Atom@7461373f: 2>}>

Don't always create new atoms

My first comment is that your use of atom seems quite wrong to me: atoms are mutable entities which you're supposed to mutate via e.g. swap! or reset!, which alter the value the atom points to without changing the atom reference itself. What your code is doing is to create new atoms at every call, i.e. you change the atom reference completely. A more proper approach would be:

(defn increment-key [ref key]
  (dosync
    (if-let [current (@ref key)]        ; maps are functions of their keys, no need for 'get'
      (do (swap! current inc) @ref)     ; 'do' here is only used to return the current map
      (alter ref assoc key (atom 1))))) ; change the atom in place

Do you need an atom?

My second comment is that you most likely don't need atom at all, unless you want to also use it directly as shared state among different threads. If you only plan to use the ref map as the shared state, you can just drop the atom and make your code easier to handle. One of the benefits of Clojure immutable data structures is that values are inherently thread safe. The following code would be perfectly thread safe:

(defn increment-key [ref key]
    (dosync
        (if-let [current (@ref key)]
            (alter ref assoc key (inc current))
            (alter ref assoc key 1))))

Do you need a ref?

A last comment is about choosing the right tool for the job: ref is intended for coordinated state change. This means that it helps you in case you have several mutable state references that need to mutate all in a single transaction, which you create with dosync. Your code doesn't seem to need transactions at all, it just needs a single mutable state reference to change atomically, i.e. in a thread safe manner. This is better achieved with a straight atom, which relieves you from the need to establish a transaction:

(defn increment-key [atm key]
  (if-let [current (@atm key)]
    (swap! atm update key inc)
    (swap! atm assoc key 1)))

A next step might be to let the change to be asynchronous by using an agent instead of an atom, but that's slightly beyond the scope here IMO.


How do I put $db in a central location, but maintain variable scope I have that $db, that I will use again and again to connect to the database. How do I put this in a central location so if I change my db I will not have to rewrite a bunch of lines of code? Everything I've tried so far just gives me an error concerning variable scope, the variable is not declared in my functions, so I am forced to simply connect and use the $db object everytime I need to access the database.

TLDR: How do I create the $db object once, and then use it again and again. Just looking for a general idea here.

function input_registration ($email, $password) {

$email_clean = htmlspecialchars($email);
$password_clean = htmlspecialchars($password);

$hash = md5($password_clean);

$db = new PDO("mysql:host=localhost;dbname=users", "root", "");

try {

$statement = $db->prepare("INSERT into userinfo(email, hash) VALUES (:email, :hash)");

$array_parameters = array(

    'email' => $email_clean,
    'hash'  => $hash

    );

$statement->execute($array_parameters);

  }

catch (Exception $error) {

    echo "Database error: ". $error->getMessage();
  }

}

You can't find the correct answer until your code is just a bunch simple PHP scripts. The ultimate solution would be is to use a dependency injection container and resolver where sou can say for example a PDO instance to keep in use until the request ends (in PHP this would be a singleton behavior).

If you can't rewrite your whole system to have an object oriented design to have the ability to use a DI container then the only thing you could do is to create a class loader in your bootstrap section to load a factory.

A Factory is a simple class which can build object instances like a DatabaseFactory::CreateNewConnection would return a new PDO instance. You can create a static factory but that would be as flexible as the pig-iron. A recommend you to have a static DatabaseBuilder class there you can register a DatabaseFactory class instance as the default database factory and your factory can hold a database class instance until the request ends. Here is the skeleton:

<?php

class DatabaseFactoryBuilder {

    private function __construct() {}

    public static function SetFactory(IDatabaseFactory $factory) {
        $this->_factory = $factory;
    }

    public static function GetFactory() {
        return $this->_factory;
    }

}

interface IDatabaseFactory {
    function GetDatabase();
}

class DatabaseFactory implements IDatabaseFactory {

    /* ... */

    private $_db;

    public function __construct($dbServer, $dbName, $dbUser, $dbPassword) {
        /* ... */
    }

    public function GetDatabase() {
        if ($this->_db == NULL) {
            $this->_db = new PDO(/* ... */);
        }

        return $this->_db;
    }

}

Ofcourse as you can se you have to create a decent way to set the connection properties for your database.


Find size for elements to fit in certain width This is a follow-up on Change size of elements until parent reaches certain height

The goal of the function is to find a size for the elements inside a parent, so they all fit in a single row. I tried it with a loop (check previous question), and now I changed it to if statements. However I would like to simplify it, but I can't find how. The Size of the elements don't have to be exactly how they are now. The default size is width: 10px; height: 10px; margin: 5px 7px, which is a total of 24px wide.

function _AdjustHeightOwlDots(id) {
    const thisElementId = `owl-carousel-${id}`;
    const dotRowWidth = $(`#${thisElementId} .owl-dots`).width();
    const amountOfDots = document.querySelector(`#${thisElementId} .owl-dots`).childElementCount;
    const singleDotWidth = $(`#${thisElementId} .owl-dots > button`).width();

    if ( (singleDotWidth * amountOfDots) < dotRowWidth ) { // dots fit in single row
      return;
    }
      let maxDotWidth = Math.floor(dotRowWidth / amountOfDots);
      const setNewStyle = function(hw, margin) {
        $(`#${thisElementId} div.owl-dots > button > span`).css('width', `${hw}px`);
        $(`#${thisElementId} div.owl-dots > button > span`).css('height', `${hw}px`);
        $(`#${thisElementId} div.owl-dots > button > span`).css('margin', `5px ${margin}px`);
      }
      if (maxDotWidth % 2 === 1) {
        maxDotWidth -= 1;
      }

      // A/B
      // width = Apx
      // height = Apx
      // margin = 5px Bpx

      if (maxDotWidth <= 3) {
        // set to 1/1
        setNewStyle(1, 1);
      } else if (maxDotWidth === 4) {
        // set to 2/1
        setNewStyle(2, 1);

      } else if (maxDotWidth === 6) {
        // set to 2/2
        setNewStyle(2, 2);

      } else if (maxDotWidth === 8) {
        // set to 4/2
        setNewStyle(4, 2);

      } else if (maxDotWidth === 10) {
        // set to 4/3
        setNewStyle(4, 3);

      } else if (maxDotWidth === 12) {
        // set to 6/3
        setNewStyle(6, 3);

      } else if (maxDotWidth === 14) {
        // set to 6/4
        setNewStyle(6, 4);

      } else if (maxDotWidth === 16) {
        // set to 8/4
        setNewStyle(8, 4);

      } else if (maxDotWidth === 18) {
        // set to 8/5
        setNewStyle(8, 5);

      } else if (maxDotWidth === 20) {
        // set to 10/5
        setNewStyle(10, 5);

      } else if (maxDotWidth === 22) {
        // set to 10/6
        setNewStyle(10, 6);
      }
  };

Using a template variable in the script means you have this script in the HTML document and repeat it for each owl carousel. That is a vary bad idea, mostly because that way you may have multiple functions with the same name.

Generally your HTML shouldn't "know about" (reference) your JavaScript. Instead the JavaScript should find the elements it wants to attach itself to.

Put the function in a separate script file (together with all of your other scripts, wrapped in an IIFE) and include it once with <script src="..."> at the end of the HTML.

Then place a class on all owl carousels you want to apply the function to and use that to call the script for all and any elements with this class. For example, with jQuery:

(function() {

    $('.owl-carousel--adjust-height').each(function () {
        _AdjustHeightOwlDots($(this));
    };

    function _AdjustHeightOwlDots(owlCarousel) {
        // ...
     }

})();

Now, inside "find" the sub elements (and save references to avoid duplicate queries of the same elements):

const dots = owlCarousel.find('.owl-dots');
const dotRowWidth = dots.width();
const amountOfDots = dots[0].childElementCount;
const dotButtons = dots.find('> button');
const singleDotWidth = dotButtons.width();

(You learn new things every day. Never heard of childElementCount before this - and it's not even a new thing.)

In setNewStyle the query is also repeated and can be replaced with chaining (or the alternative syntax for .css() using an object) and also can be moved outside the function:

const dotButtonSpans = dotButtons.find('> span');

const setNewStyle = function(hw, margin) {
    dotButtonSpans
        .css('width', `${hw}px`)
        .css('height', `${hw}px`)
        .css('margin', `5px ${margin}px`);
}

Or

const setNewStyle = function(hw, margin) {
    dotButtonSpans.css({
        width: `${hw}px`,
        height: `${hw}px`,
        margin: `5px ${margin}px`
    });
}

Finally the big if block can be replaced with a simple calculation (including removing the part that makes maxDotWidth even, instead using | 0 as a short cut to round numbers to integers):

const size = ((maxDotWidth / 4) | 0) + 1;
const margin = maxDotWidth < 2 ? 1 : (((maxDotWidth - 2) / 4) | 0) + 1;

setNewStyle(size , margin);


Print all nodes that are at distance k from a leaf node Given a binary tree and a positive integer \$k\$, print all nodes that
  are distance \$k\$ from a leaf node.

  Here, the meaning of distance is different from the previous post.
  Here, \$k\$ is the distance from a leaf means \$k\$ levels higher than
  a leaf node. For example, if \$k\$ is more than height of binary tree,
  then nothing should be printed. Expected time complexity is \$O(n)\$
  where \$n\$ is the number nodes in the given binary tree.

I'm looking for code review, optimizations and best practices.  I'm verifying \$O(n)\$ to be both the time and space complexity.

public class KDistanceFromLeaves<T> {

    private TreeNode<T> root;

    public KDistanceFromLeaves(List<T> items) {
        create(items);
    }

    private void create (List<T> items) {
        root = new TreeNode<T>(items.get(0));

        final Queue<TreeNode<T>> queue = new LinkedList<TreeNode<T>>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode<T> current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode<T>(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode<T>(items.get(right));
                    queue.add(current.right);
                }
            }
        }
    }

    public static class TreeNode<T> {
        private TreeNode<T> left;
        private T item;
        private TreeNode<T> right;

        TreeNode(T item) {
            this.item = item;
        }
    }

    public List<T> kDistanceFromLeaf(int k) {
        final List<T> list = new ArrayList<>();
        final List<Boolean> booleanList = new ArrayList<>();
        recurse(root, k, new ArrayList<T>(), list, booleanList);
        return list;
    }

    private void recurse(TreeNode<T> node, int k, List<T> items, List<T> list, List<Boolean> visited) {

        if (node == null) return;

        if (node.left == null && node.right == null && k <= items.size() && !visited.get(items.size() - k)) {
                list.add(items.get(items.size() - k));
                visited.set(items.size() - k, true);
        }

        items.add(node.item);
        visited.add(false);

        recurse(node.left, k, items, list, visited);
        recurse(node.right, k, items, list, visited);

        items.remove(items.size() - 1);
        visited.remove(visited.size() - 1);
    }
}

public class KDistanceFromLeavesTest {

    @Test
    public void testCompleteTree() {
        KDistanceFromLeaves<Integer> kd = new KDistanceFromLeaves<Integer>(Arrays.asList(1, 2, 3, 4, 5, 6, 7));
        assertEquals(Arrays.asList(2, 3), kd.kDistanceFromLeaf(1));
        assertEquals(Arrays.asList(1), kd.kDistanceFromLeaf(2));
        assertEquals(Collections.EMPTY_LIST, kd.kDistanceFromLeaf(3));
        assertEquals(Collections.EMPTY_LIST, kd.kDistanceFromLeaf(4));
    }

    @Test
    public void testInCompleteTree() {
        KDistanceFromLeaves<Integer> kd1 = new KDistanceFromLeaves<Integer>(Arrays.asList(1, 2, 3, 4, 5, null, null, null, null, 6, 7));
        assertEquals(Arrays.asList(2, 5, 1), kd1.kDistanceFromLeaf(1));
        assertEquals(Arrays.asList(1, 2), kd1.kDistanceFromLeaf(2));
        assertEquals(Arrays.asList(1), kd1.kDistanceFromLeaf(3));
        assertEquals(Collections.EMPTY_LIST, kd1.kDistanceFromLeaf(4));
    }
}

Your algorithm seems fine; it is indeed \$O(n)\$ time and \$O(n)\$ space. If you adjusted your code somewhat, you could reduce it to \$O(h)\$ space (ignoring input), where \$h\$ is the height of the tree; I'll come back to this later. One nit: it doesn't handle k == 0.

The major issue with this code is that it doesn't help the reader understand it. Your names describe the mechanics of the named item, not its purpose (recurse, list, items -- what purpose do these have in your algorithm?). The classes are divided oddly -- encapsulation is broken (e.g. almost all of your code has to know the precise representation of the tree).

Encapsulation of the tree representation

KDistanceFromLeaves<T>'s constructor takes a List<T>. Here you're making assumptions about the representation of the tree that have nothing to do with its type. The easiest solution is probably to take a TreeNode<T> instead (and make TreeNode<T> a public class instead of a nested class of KDistanceFromLeaves<T>. Alternatively, you could implement something like

interface IterableTree<T> {
    interface Iterator {
        T getValue();
        boolean hasLeft();
        /** The behavior of getLeft() is undefined when !hasLeft() */
        Iterator getLeft();
        /** The behavior of getRight() is undefined when !hasRight() */
        boolean hasRight();
        Iterator hasRight();
    }
    boolean isEmpty();
    /** The behavior of getRoot() is undefined when isEmpty(). */
    Iterator getRoot();
}

Two implementations arise immediately to mind:

class TreeNode<T> implements IterableTree<T>, IterableTree<T>.Iterator {
    private final T value;
    private TreeNode<T> left = null;
    private TreeNode<T> right = null;

    public TreeNode(T value) { this.value = value; }
    public void setLeft(TreeNode<T> left) { this.left = left; }
    public void setRight(TreeNode<T> right) { this.right = right; }

    /** A non-null TreeNode<T> is never an empty tree */
    @Override
    public boolean isEmpty() { return false; }

    @Override
    public TreeNode<T> getRoot() { return this; }

    @Override
    public T getValue() { return value; }

    @Override
    public TreeNode<T> getLeft() { return left; }

    @Override
    public boolean hasRight() { return right != null; }

    @Override
    public TreeNode<T> getRight() { return right; }
}

public ListBackedTree<T> implements IterableTree<T> {
    private final List<T> nodes;

    public ListBackedTree(List<T> nodes) { this.nodes = nodes }

    private class Iterator implements IterableTree<T>.Iterator {
        private int index;

        public Iterator(int index) { this.index = index; }

        @Override
        public T getValue() { return nodes.get(index); }

        @Override
        public boolean hasLeft() { return 2 * index + 1 < nodes.size(); }

        @Override
        public Iterator getLeft() {
            return new Iterator(2 * index + 1);
        }

        @Override
        public boolean hasRight() { return 2 * index + 2 < nodes.size(); }

        @Override
        public Iterator getRight() {
            return new Iterator(2 * index + 2);
        }
    }

    @Override
    public boolean isEmpty() { return nodes.isEmpty(); }

    @Override
    public Iterator getRoot() { return new Iterator(0); }
}

If you use IterableTree<T> instead of List<T>, your users don't have to convert between tree representations just to use your algorithm -- they can use whatever tree they've already got lying around.

As a side note, from an encapsulation point of view, with the code you had, create should be a static method on TreeNode<T> (or even be renamed to TreeNode<T>.TreeNode(List<T>)), not a private method on KDistanceFromTreeLeaves<T>. Classes should be responsible for understanding their own representation.

Rewriting the algorithm

As I mentioned above, your algorithm is fine. The problem comes in the way it is presented.

I'd start by making kDistanceFromLeaf a static method that takes a tree as an argument rather than a method on a class whose only member is a tree. It sort of made sense with your code, since you transformed the tree passed to the constructor and so wanted to save the effort of transformation each time you called your algorithm. However, your transformation was simply a change in representation -- you don't do any additional computation. Thus, it makes more sense to abstract the algorithm away from the representation than to transform the data and make your algorithm depend on a single representation.

How about the return value? A List is ordered and may contain multiple copies of a given value. That doesn't really model our return value. What we really want to return is a Set -- this is an unordered collection that contains at most one copy of each value.

Next up is naming. What is items? Well, it's a list of the current node's ancestors. So let's call it ancestors. What about list? This is a list of the nodes that satisfy our condition. We could call it found or targets. visited is obviated by using a Set instead of a List.

Finally, comments. First, it is sensible to document the purpose of classes and methods using doc comments. Second, it is sometimes very useful to add comments that explain why code is written a certain way.

With all that in mind, here's another whack at implementing the algorithm:

/** Collects implementations of interview questions */
class InterviewProblems {
    /**
     * Given a tree, determines which nodes are exactly k distance from
     * a leaf node (that is, nodes that are k levels above a leaf node).
     */
    public static <T> Set<T> kDistanceFromLeaf(
            IterableTree<T> tree, int k) {
        Set<T> found = new HashSet<>();
        if (tree == null || tree.isEmpty()) return found;
        computeKDistanceFromLeaf(tree.getRoot(), k, new ArrayList<T>(), found);
        return found;
    }

    private static <T> boolean isLeafNode(IterableTree<T>.Iterator node) {
        return !node.hasLeft() && !node.hasRight();
    }

    /** Recursive helper function for {@link #kDistanceFromLeaf}. */
    private static <T> void computeKDistanceFromLeaf(
            IterableTree<T>.Iterator node, int k,
            List<T> ancestors, Set<T> found) {
        ancestors.add(node.getValue());
        // If this is a leaf node, its kth ancestor should be added to found.
        if (isLeafNode(node) && ancestors.size() > k) {
            found.add(ancestors.get(ancestors.size() - 1 - k));
        }
        if (node.hasLeft())
            computeKDistanceFromLeaf(node.getLeft(), k, ancestors, found);
        if (node.hasRight())
            computeKDistanceFromLeaf(node.getRight(), k, ancestors, found);
        ancestors.remove(ancestors.size() - 1);
    }
}

Minor comments

You use final a lot for local variables. That's fine (and as someone who uses C++ a lot, I certainly understand the instinct). However, a more common Java style is only to mark local variables final when they are to be used by inner classes. final is not nearly as useful as C++'s const and the cost-benefit tradeoff (cost: final is sprinkled liberally through the code; benefit: you know that the reference won't be reassigned, but not that the objects won't be modified) is less in favor of adding final in Java than const in C++.

Yay unit tests! One question: You've given an example in the problem description -- why not write a unit test that confirms that the code correctly works the example?

A final comment: I haven't compiled, much less tested, the code in this example, so use it with care and let me know if you have any questions!


Ext JS: Avoid duplication I have ext_scaffold (question.js). When I row click I get answers on this question, so

'rowselect': function(sm, row, rec) {
    store = new Ext.data.Store({
        proxy: new Ext.data.HttpProxy({
        url: '/answers/index_all_for_question_id/' + rec.data.id + '?format=ext_json',
        method: 'GET'}),
        reader: new Ext.data.JsonReader({
        root: 'answers',
        id: 'id',
        totalProperty: 'results'
      }, [
        {name: 'text', mapping: 'answer.text'},
        {name: 'email', mapping: 'answer.respondent.email'}
          ])
    });

    grid = Ext.getCmp('answers_grid');
    grid.reconfigure(store, new Ext.grid.ColumnModel([
                {id: 'text', header: "Answer &darr;", width: 500, sortable: true, dataIndex: 'text'},
                {id: 'email', header: "Email", width: 300, sortable: true, dataIndex: 'email'}
          ]));
    grid.store.load();

    scaffoldPanel.selectedRecordId = rec.data.id;
    scaffoldPanel.getFormPanel().getForm().loadRecord(rec);
}

and I have another admin file where grids (answer):

 var answers_datastore = new Ext.data.Store({
     autoLoad: true,
    proxy: new Ext.data.HttpProxy({
        url: '/answers/index_all_for_question_id/<%= @questions.first.id %>?format=ext_json',
        method: 'GET'}),
    reader: new Ext.data.JsonReader({
        root: 'answers',
        id: 'id',
        totalProperty: 'results'
      },[
      {name: 'text', mapping: 'answer.text'},
      {name: 'email', mapping: 'answer.respondent.email'}
      ])
    });

    var answersGrid = new Ext.grid.GridPanel({
    id: 'answers_grid',
    store: answers_datastore,
            frame: true,
        columns: [
          {id: 'text', header: "Answer &darr;", width: 500, sortable: true, dataIndex: 'text'},
          {id: 'email', header: "Email", width: 300, sortable: true, dataIndex: 'email'}
        ],
         listeners: {
             rowdblclick: function(answersGrid, rowI, event)   {
            Ext.MessageBox.show({
                title:            "Answer" ,
                   msg:             "A",
                closable:         true,
                   autowidth:        true
           });
             }
         },

        stripeRows: true,
        title:'Answers'
    });

How to avoid duplication?

The trouble with your question is that the code resembles more config than coding and there is nothing wrong with it except that it is not DRY between those 2 files.

It seems that Ext JS supports require as of version 4, so you simply need to extract the common logic/config into a file that you will require into both places.


Count complete trips to the basement and back to ground floor An elevator starts on the ground floor 0 and moves up or down by one floor at a time as described by an input string travel. Here's an example travel string udddudduuuuddu where u is a move up 1 floor and d is a move down 1 floor.
count-basement-trips takes a travel string and counts how many times the elevator returns back to the ground floor from the dark and mysterious underground levels.
(defun count-basement-trips (travel)
  (flet ((finished-basement-trip-p (old-floor new-floor)
           (and (= -1 old-floor)
                (=  0 new-floor)))
         (change (c)
           (if (char= c #\u) 1 -1)))
    (let ((floor 0)
          (count 0))
      (loop for c across travel
            do (let ((new-floor (+ floor (change c))))
                 (if (finished-basement-trip-p floor new-floor)
                     (incf count))
                 (setf floor new-floor)))
      count)))

(count-basement-trips "udddudduuuuddu")

The last expression evaluates to 2.
What changes would make this more idiomatic Common Lisp? My goal is to speak more like a native. Thanks!

A few points.

if with only the “then” case is usually written with when, which is in general more handy since it allows several forms inside its body. So you could write (when (finished-basement-trip-p floor new-floor) (incf count)).

In a loop you can define variables that change at each iteration with the clause for variable = expression, and this in many cases avoid the use of nested let clauses, that make more difficult to read the program. Another useful iteration clause is for variable = expression1 then expression2, which assigns to variabile the value of expression1 during the first iteration, and in all the others the value of expression2.

In a loop, if you need to count something and then return its value, you can use the “numeric accumulation” clause count, analogous to the sum clause, in which you count how many times a boolean espression is true.

Here is a proposed simplification. Note that in my version I do not use local functions since I think the program is easy enough to read without them. This is just a personal opinion.
(defun count-basement-trips (travel)
  (let ((floor 0))
    (loop for c across travel
          for up = (char= c #\u)
          do (incf floor (if up 1 -1))
          count (and (zerop floor) up))))

Here's the buggy version suggested by @user235906 in a comment that got deleted. @Renzo points out it gives an incorrect answer for the test string "ud":
(defun count-basement-trips (travel)
  (loop for c across travel
        for up? = (char= c #\u)
        for floor = 0 then (incf floor (if up? 1 -1))
        for completed-trip? = (and up? (zerop floor))
        count completed-trip?))

And here's the suggested fix:
(defun count-basement-trips (travel)
  (loop for c across travel
        for up? = (char= c #\u)
        for change = (if up? 1 -1)
        for floor = change then (incf floor change)
        for completed-trip? = (and up? (zerop floor))
        count completed-trip?))


Biggest Binary Gap Solution Inspired by this question I've come up with a solution of my own. The task is to find longest sequence of zeroes surrounded by ones in a binary representations of an integer (e.g. 1041 == 0b10000010001, thus the answer is 5). The task also requires the complexity of \$O(log(N))\$. As far as I understand it means that it means linear complexity in terms of digits, whose number is \$log(N)\$. Am I correct in this conclusion?

private int biggestBinaryGap(int n) {
    while (endsWithZero(n))
        n >>= 1;
    return biggestBinaryGapRec(n >> 1);
}

private int biggestBinaryGapRec(int n) {
    if (n == 0)
        return 0;

    int gap = 0;
    for (; endsWithZero(n); n >>= 1, gap++);
    return Math.max(gap, biggestBinaryGapRec(n >> 1));

}

private boolean endsWithZero(int n) {
    return n > 0 && (n & 1) == 0;
}

Working at a bit-wise level for this problem is a superior option than converting it to a string like the linked question (your inspiration) did.

There may be some really tricky magic that can be done with complicated AND/OR/XOR/NOT operations to identify the gaps in a more efficient way, but your solution of "walking" the bits is adequate from an algorithmic perspective.

Note, in your code you have special handling for negative values, but that's not needed if you use the >>> operator for right shifting. Read up on the difference between signed and unsigned right shifts: http://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.19

The recursion bothers me, though. The overhead for creating a stack frame for each level of recursion, calling the function, etc. seems overkill for something that would be "trivial" with a while-loop.

public static int maxGap(int n) {
    // get rid of right-hand zeros
    while (n != 0 && (n & 1) == 0) {
        n >>>= 1;
    }

    int max = 0;
    int gap = 0;
    while (n != 0) {
        if ((n & 1) == 0) {
            gap++;
            max = Math.max(gap, max);
        } else {
            gap = 0;
        }
        n >>>= 1;
    }
    return max;
}

Now, that runs in \$O(log(n))\$ time where n is the integer being tested.... but, can it be a bit quicker?

I don't think the time complexity can be significantly improved (i.e. I can't see an \$O(1)\$ solution, but I can see some grunt performance improvements out there... assuming that Integer.numberOfTrailingZeros() function is better than O(N) performance:

public static int maxGapX(int n) {
    if (n == 0) {
        return 0;
    }
    int rbits = Integer.numberOfTrailingZeros(n);
    int max = 0;

    // shift off any initial right-most zeros in the init step.
    // then treat any subsequent 1's as the end of a span of zeros.
    for (n >>>= (rbits + 1); n != 0; n >>>= (rbits + 1)) {
        rbits = Integer.numberOfTrailingZeros(n);
        max = Math.max(max, rbits);
    }
    return max;
}

I put this up in an ideone here: https://ideone.com/hllRUa


Selector targets overly obtuse - recommendations? This presumably simple task resulted in much more scripting logic than I thought would be necessary.

Goal: when the user clicks a button inside one of the divs, it affects said div.

Issues: I did not wish to have my function target specific elements in my DOM. (ex. targeting <a> by ID). Also, because my function is traversing the DOM and being called in multiple instances, I had to use $(this) format so as not to be specific to one ID.

Anyways, here is my code:

$(".button").toggle(function(){
    $(this).parent().children(".innerBox").fadeIn();
}, function(){
    $(this).parent().children(".innerBox").fadeOut();
});

It works fine. However, can someone with a little more jQuery savvy please let me how I could execute this function in the future, without having to parse through siblings and making it more convoluted than need be?

You can certainly cache the lookups, abusing of the scopes.

$(".button").each(function(){
  var $btn = $(this);
  var $box = $btn.parent().children(".innerBox");
  $btn.toggle(function(){
    $box.fadeIn();
  }, function(){
    $box.fadeOut();
  });
});

Using .fadeToggle() instead of .fadeIn() and .fadeOut().

$(".button").each(function(){
  var $btn = $(this);
  var $box = $btn.parent().children(".innerBox");
  $btn.click(function(){
    $box.fadeToggle();
  });
});

And last but not least, .siblings().

$(".button").each(function(){
  var $btn = $(this);
  var $box = $btn.siblings(".innerBox");
  $btn.click(function(){
    $box.fadeToggle();
  });
});


Creating my own flow control statements in C++. Is that acceptable? I always considered switch statement as somehow defective:

works only on integral types and enumerations.
isn't an readability improvement over traditional if/else chain.
forget a break - you have a bug.
variable declaration spills over neighbouring cases.
is essentially a computed goto

Because of said reasons, and also as an exercise on lambdas and variadic templates I created my own flow control function.

#include <functional>
#include <tuple>

template<typename V>
bool switch_on(const V& value)
{
    return false;
}

template<typename V, typename P, typename... Args>
bool switch_on(const V& value, const P& p, Args... args)
{
    if(std::get<0>(p)(value, std::get<1>(p)))
    {
        std::get<2>(p)();
        return true;
    }
    else
    {
        return switch_on(value, args...);
    }
}

template<template <typename> class P, typename F, typename V >
auto case_of(const V& v, const F& p) -> decltype( std::make_tuple(P<V>(), v, p) )
{
    return std::make_tuple(P<V>(), v, p);
}

template<typename P, typename F, typename V >
auto case_of(const V& v, const F& p) -> decltype( std::make_tuple(P(), v, p) )
{
    return std::make_tuple(P(), v, p);
}

template<typename F, typename V >
auto case_of(const V& v, const F& p) -> decltype( std::make_tuple(std::equal_to<V>(), v, p) )
{
    return std::make_tuple(std::equal_to<V>(), v, p);
}

so I can use:

using std::less;
using std::greater;

int main()
{
    int a = 42;
    switch_on(a,
        case_of<less>(0, [&]{
            std::cout << "LESS THAN ZEROOOOOOOOOOOOOO.";
        }),
        case_of(42, [&]{
            std::cout << "Yes";
        }),
        case_of<greater>(9000, [&]{
            std::cout << "IT'S OVER NINE THOUSAAAAAAAAND!!!";
        })
    );
}

While in Lisp, it is encouraged to create new forms of flow control, what about C++? I would also like to see some opinions about template usage, some pointers on how to improve the code and possible corner cases when this code will break.

Lisp has an uniform syntax, so it’s possible to create user defined flow control constructs that look exactly like the built in constructs. In C++, that’s generally not possible.

Your code is certainly clever, and it solves the problem of omitted breaks (I consider the other drawbacks you list less important). However:

Despite naming this a “switch”, you’ve essentially recreated an if-else chain. Notably, a compiler may have a bit more difficulty optimizing this than a traditional switch.
Your implementation does not seem to support range comparisons or a default case, although both of these seem to be doable in your conceptual framework.
When used in a disciplined way, falling through from one switch case to the next is not always devoid of merit.
I’m not sure that returning this boolean has a great benefit. It might be preferable to design a construct that ensures that one of the cases is always executed.
Be honest: do you really find this more readable than a traditional switch statement?


Is this a reasonable Scala monad? I found myself writing code of the form...

val foo : Foo = ???

val step1 : Foo = {
  if ( someCharacteristic( foo ) )
    someTransformation( foo )
  else
    foo
}
val step2 : Foo = {
  if ( otherCharacteristic( foo ) )
    otherTransformation( foo )
  else
    foo
}

val result = step2;

That seemed awkward, and it seemed like this sort of ugly stepwise processing could be monadified. I've used but not written Scala monads; I thought I'd give it a shot. What I have seems to work, but I found proving the monad laws surprisingly challenging. I'm not sure I've got a valid monad, and wonder whether there is some subtle problem that would bite me someday if I haven't. FWIW, here's the code, any comments would be very welcome.

package dumbmonad;

import scala.language.implicitConversions;

object If {
  implicit def unwrap[T]( ift : If[T] ) = ift.value;

  def evaluate[T]( ift : If[T] ) : T = if ( ift.condition( ift.value ) ) ift.transformation( ift.value ) else ift.value;

  def flatten[T]( outer : If[If[T]] ) : If[T] = evaluate( outer );

  def apply[T]( value : T ) : If[T] = apply( value, _ => false, identity );
}

case class If[T]( val value : T, val condition : (T)=>Boolean, val transformation : (T)=>T ) {

  lazy val evaluate : T = If.evaluate( this );

  def map[S]( f : (T) => S ) : If[S] = If( f( this.evaluate ) )
  def flatMap[S]( f : (T) => If[S] ) : If[S] = If.flatten( this.map( f ) )
}

I use it like this:

import dumbmonad._;
import dumbmonad.If._;

val foo : Foo = ???

for {
  a <- If( foo, someCharacteristic, someTransformation )
  b <- If( a, otherCharacteristic, otherTransformation )
} yield b;

val result : Foo = b; // implicit call to If.unwrap( ... )

Is this terrible?

What you are describing are simply functions.

To take a related example from mathematics,

          /   x, if x >= 0
abs(x) =  |
          \  -x, otherwise

And you are just composing functions: f(g(x)).

You can somewhat make the correspondence to monads, but it is a bit of a stretch.  I think it is more that monads are extensions of functions rather than the other way around.

flatMap would be the composition operator: o  , ie (f o g)(x) = f(g(x)).  And unit would just be the identity operation: I(x) = x.

The three monad laws:

Associativity: f o (g o h) = (f o g) o h
Left unit: I o f = f
Right unit: f o I = f

So instead of defining your If class, you can just stick to def, the usual function definition.  Functional programming!!!  In your case you can compose all your functions in any order since they always input/output Foo type.  You can compose functions in Scala with val h = f _ compose h _, or using andThen.


Demonstration of the Largest Square Submatrix Challenge

  Using the C++ language, have the function MaximalSquare(strArr) take the strArr parameter being passed which will be a 2D matrix of 0 and 1's, and determine the area of the largest square submatrix that contains all 1's. A square submatrix is one of equal width and height, and your program should return the area of the largest submatrix that contains only 1's. For example: if strArr is ["10100", "10111", "11111", "10010"] then this looks like the following matrix:

1 0 1 0 0
1 0 1 1 1
1 1 1 1 1
1 0 0 1 0

  For the input above, you can see the bolded 1's create the largest square submatrix of size 2x2, so your program should return the area which is 4. You can assume the input will not be empty.

  Sample Test Cases

Input:"0111", "1111", "1111", "1111"
Output:9

Input:"0111", "1101", "0111"
Output:1

How can I make improvements to my code? Is there any STL that I might not know of that might make the code simpler and/or efficient?

#include <algorithm>
#include <iostream>
#include <string>
#include <vector>

 bool check_Same_Length(std::vector< std::vector<int> > maximalSquareInput)
{
    bool state;

    size_t length = maximalSquareInput[0].size();

    for (int i = 0; i < maximalSquareInput.size(); ++i)
    {
        if (length == maximalSquareInput[i].size())
        {
            state = true;
        }
        else
        {
            state = false;

            return state;
        }
    }

    return state;
}

bool only_1_And_0(std::vector< std::vector<int> > maximalSquareInput)
{
    for (int i = 0; i < maximalSquareInput.size(); ++i)
    {
        if (!(std::all_of(maximalSquareInput[i].begin(), maximalSquareInput[i].end(),
                          [] (int digit) { return digit == 1 || digit == 0; })))
        {
            return false;
        }
    }

    return true;
}

int findBiggestElement(std::vector< std::vector<int> > maximalSquareOutput)
{
    int biggestElement = 0;

    for (int i = 0; i < maximalSquareOutput.size(); ++i)
    {
        auto itr = std::max_element(maximalSquareOutput[i].begin(), maximalSquareOutput[i].end());

        if (*itr > biggestElement)
        {
            biggestElement = *itr;
        }
    }

    return biggestElement;
}

std::string findLargestSubmatrix(const std::vector< std::vector<int> > maximalSquareInput)
{
    std::vector< std::vector<int> > maximalSquareOutput (maximalSquareInput.size(), std::vector<int>(maximalSquareInput[0].size()) );

    if(check_Same_Length(maximalSquareInput) && only_1_And_0(maximalSquareInput))
    {
        for (int row = 0; row < maximalSquareInput.size(); ++row)
        {
            for (int column = 0; column < maximalSquareInput[row].size(); ++column)
            {
                if (row == 0 || column == 0 || maximalSquareInput[row][column] == 0)
                {
                    maximalSquareOutput[row][column] = maximalSquareInput[row][column];
                }
                else
                {
                    int minElement = std::min({maximalSquareOutput[row-1][column], maximalSquareOutput[row-1][column-1], maximalSquareOutput[row][column-1]});

                    maximalSquareOutput[row][column] = maximalSquareInput[row][column] + minElement;
                }
            }
        }
    }

    unsigned long long int biggestElement = findBiggestElement(maximalSquareOutput);

    return std::to_string(biggestElement * biggestElement);
}

#include <algorithm>
#include <iostream>
#include <string>
#include <vector>

Do you need <iostream>?

bool check_Same_Length(std::vector< std::vector<int> > maximalSquareInput)

Space in the typename isn't required since c++11.

Not thrilled with the naming.  The function could be better named to signal that you are checking the column counts.  The parameter doesn't have to be your maximal input, but any 2D container.

You copy the vector of vectors every time you call a function.  Pass your matrix by reference (and to const if immutable).

bool has_equal_column_counts(const std::vector<std::vector<int>>& matrix)

    size_t length = maximalSquareInput[0].size();

size_t is missing its std:: namespace prefix.  Consider auto.

What happens if maximalSquareInput is empty?  If you are not going to check the preconditions yourself, you should at least document them.

    for (int i = 0; i < maximalSquareInput.size(); ++i) {
        if (length == maximalSquareInput[i].size()) {
            state = true;
        } else {
            state = false;
            return state;
        }
    }
    return state;

When you are accessing elements by an index and you are not using any index arithmetic, consider using range-based for.

    for (const auto& row : maximalSquareInput) {
        if (length == row.size()) {
            state = true;
        } else {
            state = false;
            return state;
        }
    }
    return state;

If you are returning false the first time you encounter an invalid length, just return false.

    for (const auto& row : maximalSquareInput) {
        if (length != row.size()) {
            return false;
        }
    }
    return true;

It turns out, the standard library has two functions that will do what you want here. You could use std::find or std::all_of, but std::adjacent_find works as well.

    auto not_equal_size = [](const auto& v1, const auto& v2) {
        return v1.size() != v2.size();
    };
    return matrix.end()
        == std::adjacent_find(maximalSquareInput.begin(),
                              maximalSquareInput.end(),
                              not_equal_size)

bool only_1_And_0(std::vector< std::vector<int> > maximalSquareInput)

Function should be named all_are_one_or_zero.

    for (int i = 0; i < maximalSquareInput.size(); ++i) {
        if (!(std::all_of(maximalSquareInput[i].begin(), maximalSquareInput[i].end(),
                          [] (int digit) { return digit == 1 || digit == 0; })))
        {
            return false;
        }
    }
    return true;

Again, range-based for would be better than index-based for.

Consider adding more tools to your toolbox to reduce repetition (DRY - Don't repeat yourself).  You have a couple functions iterating over collections of iterables.  An iterator that chains those collections (essentially "flattening" the collection) would allow you to simplify your function.  Consider the following code (exposition-only):

bool all_are_one_or_zero(const std::vector<std::vector<int>>& matrix) {
    return std::all_of(make_flatten_iterator{matrix},
                       make_flatten_iterator{},
                       [](int digit) { return digit == 1 || digit == 0; });
}

auto find_biggest_element(const std::vector<std::vector<int>>& matrix) {
    return std::max_element(make_flatten_iterator{matrix},
                            make_flatten_iterator{});
}

int findBiggestElement(std::vector< std::vector<int> > maximalSquareOutput)

Be consistent with your formatting/style.  You're function names have mixed snake_Camel_Case and camelCase.  Pick one, stick with it.

If this was meant to be a generalized max element finder for a matrix, you should consider (a) what happens if the matrix is empty, and (b) is 0 the minimum element value for type int?

std::string findLargestSubmatrix(const std::vector< std::vector<int> > maximalSquareInput)

Why return a string? Consider just returning either the square length or the square area and let the caller convert it to a string if they want it formatted.

You've used "Biggest" and "Largest" but you've always called the input parameter "maximal" (for maximum).  Just call it maximum and you can cut down on the parameter name.

std::size_t max_length_of_square_submatrix(const std::vector<std::vector<int>>& matrix)

    if(check_Same_Length(maximalSquareInput) && only_1_And_0(maximalSquareInput))

Back to being consistent, you have spaces to distinguish between language constructs (if, for) and functions and in this instance you don't.

Avoid unnecessary code nesting by returning early.

    if (!check_Same_Length(maximalSquareInput) || !only_1_And_0(maximalSquareInput))
    {
        return 0;
    }

    for (...)

Do you even need to check? Should that be the responsibility of some other function?

        for (int row = 0; row < maximalSquareInput.size(); ++row)
        {
            for (int column = 0; column < maximalSquareInput[row].size(); ++column)
            {
                if (row == 0 || column == 0 || maximalSquareInput[row][column] == 0)
                {

You check to see if you are on either the first row or first column.  When in either the first row or first column, you already know you can't make a square towards the northwest.  Therefore, you can just copy those values into your matrix of sums before calculating the entire matrix.  Then you check from [1,row][1,col].

std::size_t max_area_of_square_submatrix(const std::vector<std::vector<int>>& matrix)
{
    auto sums = matrix;

    for (std::size_t row = 1; row < matrix.size(); ++row)
    {
        for (std::size_t col = 1; col < matrix.size(); ++col)
        {
            if (matrix[row][col]) {
                sum[row][col] = 1 + std::min({sum[row-1][col],
                                              sum[row][col-1],
                                              sum[row-1][col-1]});
            }
            else
            {
                sum[row][col] = 0;
            }
        }
    }
    auto max_length = find_maximum_element(sums);
    return max_length * max_length;
}

std::string MaximalSquare(std::string strArr)
{
    auto matrix = to_matrix2i(strArr);
    if (!has_equal_column_counts(matrix) ||
        !all_are_one_or_zero(matrix))
    {
        return "0";
    }
    return std::to_string(max_area_of_square_submatrix(matrix));
}


File Output given repeated small operations I want to write a file which contains values for each cell of a CFD computation. This will be an input file to the program which runs the computation.

Currently, the most dubious section of my code looks like:

for (int y = 0; y < ycells; y++)
{
    double yPos = yMin + (cellToY * (y + 0.5));
        for (int x=0; x<xcells; x++)
        {
            double deltaY = yProfileHeight[x] - yPos;
            if (deltaY < -cellToY)
            {
                file << "1\n";
            }
            else if (deltaY > cellToY)
            {
                file << "0\n";
            }
            else
            {
                double value = (0.5 + (-0.5 * (deltaY / cellToY)));
                file << value << "\n";
            }
        }
    }
}

For most of the file these if checks are going to be slow/useless - since depending on the value of yPos, they will all evaluate to true/false for large regions.

I can rewrite it like this (with appropriate min/max functions):

for (int y = 0; y < ycells; y++)
{
    double yPos = yMin + (cellToY * (y + 0.5));
    if (yPos < yHeightMin)
    {
        for (int x=0; x<xcells; x++)
        {
            file << "0\n";
        }
    }
    else if (yPos > yHeightMax)
    {
        for (int x=0; x<xcells; x++)
        {
            file << "1\n";
        }
    }
    else
    {
        for (int x=0; x<xcells; x++)
        {
            double deltaY = yProfileHeight[x] - yPos;
            if (deltaY < -cellToY)
            {
                file << "1\n";
            }
            else if (deltaY > cellToY)
            {
                file << "0\n";
            }
            else
            {
                double value = (0.5 + (-0.5 * (deltaY / cellToY)));
                file << value << "\n";
            }
        }
    }
}

Now my if conditions are evaluated less frequently, but I'm still left with segments like

for (int x=0; x<xcells; x++) // where xcells is ~ 1-10k
{
    file << "1\n";
}

Can I rewrite this in a way that doesn't require several thousand separate (identical) writes to a single file?

Measuring is good, but it will only tell you what those data will result in, so you will need to reason on the code too and then measure that it isn't totally wrong.

The reason to change the code is the suspicion that the repeated calls to file output would be a large overhead.

// ToDo: add sanity checks.

string Duplicate(string org, size_t maxSize) {
  string dup;
  dup.reserve(org.length() * maxSize);
  for (auto i = 0; i < maxSize; ++i)
    dup.append(org);

  return dup; // hope for NVRO
}

string Ones = "1\n";
size_t numLens = Ones.length();
string Ones = Duplicate(Ones, MaxNumberOfRepeats);
string Zeroes = Duplicate("0\n", MaxNumberOfRepeats);

Usage, dups is the number of duplicates

inline void Writer(string text, size_t dups) {
  file << text.substr(0, dups*numLens); // potentially makes a copy
}

There is a method using char[] which doesn't make any copy. (not shown)

 double value = (0.5 + (-0.5 * (deltaY / cellToY)));

Saving on the math operations as '/' is expensive and '-0.5 *' also is a constant that might as well be calculated outside the loop.
WARNING this may result in some subtle differences in final values due to float pointiness.

Outside the loop or if cellToY is a global const make it a constexpr

constexpr double invCellToY = -0.5 / cellToY;

This leaves

double value = (0.5 + ( (deltaY * invCellToY)));

Saving a divide.

The ifs are a problem

            if (deltaY < -cellToY) {
                file << "1\n";
            }
            else if (deltaY > cellToY) {
                file << "0\n";

If they are true at a near random occurrence you have a serious performance problem. If they come in streaks you can do something like this

double deltaY = yProfileHeight[x] - yPos;
if (deltaY < -cellToY) {
    int count = 1;
    while (++x < xcells) {
        deltaY = yProfileHeight[x] - yPos;
        if (!(deltaY < -cellToY))
            break;

        ++count;
    }
    Writer(Ones, count);
} else
    ...

This guarantees one mispredict for the while so this might be a loss if they don't come in streaks. (branch prediction is somewhat dependend on global branch history so some of the lost might be redeemed again as the next x itereation should have an increased probability of predict the first if not taken after the loop exit).

This would also mean that the original code would get lots of mispredicts, you could try this

// these bool calculations should be branchless
bool LT = (deltaY < -cellToY);
bool GT = (deltaY > cellToY);
// ??? LT and GT are mutual exclusive
bool LTGT = LT | GT;

if (LTGT)
    res = LT?Ones:Zeroes;
else
    // very costly operation of turning double to string ...
    // should the result be 0 or 1 also?
    res = string(0.5 + ( (deltaY * invCellToY)))+"\n";

fill << res;

as in some cases (trinaries) some compilers (gcc+?) can find out to generate conditional moves.

But my guess would be the while implementation would be faster as the domain suggest some form of streaks. Merge your final decision with this code.

string res;
for (int y = 0; y < ycells; y++) {
    double yPos = yMin + (cellToY * (y + 0.5));
    if (yPos < yHeightMin)  {
        Writer(Zeroes, xcells);
    }
    else if (yPos > yHeightMax) {
        Writer(Ones, xcells);
    } else {
        for (int x=0; x<xcells; x++) {
            double deltaY = yProfileHeight[x] - yPos;

            // exchange this with the wanted code
            if (deltaY < -cellToY) {
                file << "1\n";
            }
            else if (deltaY > cellToY) {
                file << "0\n";
            } else {
                double value = (0.5 + ( (deltaY * invCellToY)));
                file << value << "\n";
            }
        }
    }
}


Making sure all elements are processed in a JQuery call I have a piece code similar to below:

But I keep having this nagging feeling that there has to be an easier way!
Any ideas?

var synch = false
var indexArray = new Array();
var rowCount = theTable.find('tr').length;
$(".views").each(function(index) {
    indexArray.push( $(this).val()); // Not the real code but something to this effect
    if (rowCount <= (index+1)) synch = true;
});
while (!synch);
DO something with indexArray.....

There is absolutely no need for the synch part - JavaScript is synchronous, and you should not get into a situation where the processing of that function (the one you're showing) will not be completed before the next line runs.

(By the way, that function you have there can be rewritten for greater efficiency)

var indexArray = $('.views').map(function(){
    return this.value;
}).get();

Let's assume that you are doing something async in that each loop, for instance, ajax calls. Your best bet is to keep a count of how many of the calls are completed in the callback function, then execute the rest of your code when the last callback function is called:

var views = $('.views'),
    completed = 0,
    total = views.length;

views.each(function(){
    someAsyncFunction (function(){
        // Process data
        if(++completed === total) {
            // We're done! Let's move on..
        }
    });
});

This can be abstracted away into another function, though doing that is left as an exercise for the reader.


AngularJS directive for common form fields I have created a directive for common form fields shared across all the forms in a large AngularJS app I am re-factoring.

The app has lots of forms for various data models (user, owners, victims, suppliers, etc). A lot of the forms for these data models contain an address section which is the same format throughout the app.

Address Line 1
Address Line 2
Town / City
County
Postcode
Country

Here is the markup for the directive:

 <address-dir
     address-line-one="user.address_line_1"
     address-line-two="user.address_line_2"
     address-town-city="user.address_town_city"
     address-county="user.county"
     address-postcode="user.postcode"
     address-country="user.address_country"
 >
 </address-dir>

The actual angular directive code is:

(function () {
    var app = angular.module('userDirectives', []);

    app.directive('addressDir', function () {
        return {
            restrict: "AE",
            require: '^form',
            scope: {
                addressLineOne: '=',
                addressLineTwo: '=',
                addressTownCity: '=',
                addressCounty: '=',
                addressPostcode: '=',
                addressCountry: '='
            },
            templateUrl: 'address.html',
            link: function (scope, element, attributes, form) {
                scope.addressForm = form;
            }
        };
    });
}());

Here is my address.html template which is the template used in the directive (I have removed all but the first field to simplify the example):

        Address Line 1

    <input
        type="text"
        id="address_1"
        name="address_1"
        ng-model="addressLineOne"
        required
        placeholder="address line 1"
    >

You did not enter first line of address

Here's a simple controller:

(function () {
    var app = angular.module('userControllers', []);

    app.controller('UserController', ['$scope', function ($scope) {

        $scope.user = {};

        // This is called when the form is sumitted

        $scope.continue = function () {

            console.log($scope.user);

            // save here

        };

        // get user
        User.get().then(function (user) {
            $scope.user = user;
        });

    }]);

}());

I'm working on the project solo for a while and I couldn't find any other directives that do something like this on the net (or at least for common fields in forms).

The reason I have made this a directive instead of a basic include is because I next want to add some custom DOM manipulations to the address section on some of the forms. Probably some third-party address lookup / validation integration.

It is quite basic at moment but I am looking for some external feedback on what I have so far and the direction I'm going in.

address-dir

What is "dir"? Is there something special about "dir"? It would be best to provide a very common and understandable name to directives. That way, new devs wouldn't be scratching their heads and waste the next 20 mins figuring out what it does. I would name it <address-field> or <address-input>.

address-*

The prefix is redundant. We already know it's an address field.

address-town-city

Sounds a bit redundant. I'd go with either one, but "city" would be more common in some cases.

address-postcode

Would "zip-code" or just "zip" be a better name? Not sure what it's called in other places, but that's what I normally see in shopping sites.

scope.addressForm = form;

I would prefer that you not let the directive know about the form or anything from the outside world for that matter. This limits the directive's portability. Additionally, it's an implicit dependency which nobody will find out until one actually looks into the implementation - which defeats the purpose of an interface.

It's best that a directive should not make assumptions of the outside world and only know about the things inside it. For it to know something from the outside world, it's best that you explicitly define an interface, like a scope variable. In this case, you want to know if the form was submitted. Why not design a scope variable has-submitted.

Now for suggestions:

You might also want to add in your own required property to require all fields, or several require-* to require separate fields. That's because address line 2 and county are usually optional.
You might also want to add in show-* properties, so that you can conditionally hide fields.
Built-in values for countries and cities. That way, it's not a free text input, which should limit chances of error. There's a lot of libraries you could use for this one.
Built-in validation. The usual checks like whitespace/blank checks, value and length checks. I think ng-pattern covers most cases, but you could pull in a validation library if you want.


Find disjoint integer intervals Given a list of numeric intervals in the form of (a,b), where a and b are integers, it finds a set of disjoint intervals that cover the same numbers by splitting the original intervals at the points where they intersect.

For example: disjoint [(1,2),(1,3),(1,3),(30,31)] gives [(1,2),(2,3),(30,31)].

Please help me make this code less ugly.

{-# LANGUAGE TupleSections #-}

import Data.List (group, sortBy)

-- [(97, 99), (97, 100), (98, 108)]
-- [97, 97] [98, 99], [100, 100], [101, 108]

disjoint :: (Ord a, Num a) => [(a, a)] -> [(a, a)]
disjoint [] = []
disjoint ts = disjoint' 0 x xs
    where
      disjoint' _ _ []          = []
      disjoint' c (k, p) (x:xs) = let c' = c + k
                                      h  = if c' > 0 then [(p, snd x)] else []
                                  in h ++ (disjoint' c' x xs)
      eitherValue               = either id id
      endpoints                 = foldr (\(a, b) m -> (Left a):(Right b):m) []
      pcompare x y              = compare (eitherValue x) (eitherValue y)
      runs                      = let f xs@(x:_) = either (length xs,) (-length xs,) x
                                  in map f . group
      (x:xs)                    = runs $ (sortBy pcompare) $ endpoints ts

The compactness and efficiency of the code is quite impressive.  The downside is that it's rather convoluted and difficult to understand.

Types

Annotations

Haskell types are very helpful, both when writing and reading code.  Type declarations should not be limited to top-level functions.  They can, and should, be added to local variables as well.  Even though it's not very complicated, the purpose of endpoints becomes much clearer when its type is explicitly stated:

endpoints :: [(a, b)] -> [Either a b]
endpoints = foldr (\(a, b) m -> (Left a):(Right b):m) []

It converts tuples into eithers.  It's obvious from the type that it works almost like a map, which we can make more explicit by using concatMap:

endpoints = concatMap (\(a,b) -> [Left a, Right b])

Defining types

Another way in which types improve readability is that they hint at the semantics of a value.  Either is a very generic type, defining a newtype for interval endpoints might make things easier to understand.

Code organization

Related code should usually be grouped together.  The order, in which the local functions are defined, seems rather random.  Reorganizing those functions could benefit readability.

Variable naming

The variables (x:xs) a are shadowed in some functions.  That's confusing and will cause compiler warnings when compiled with -Wall.  I'd suggest renaming the outer (x:xs) to (y:ys).

This is just personal taste, but I'd also rename eitherValue to fromEither (in analogy to fromMaybe and fromJust).  This would make even more sense if a custom type was used (e.g. fromBorder).

Opinionated cleanup

Below is what I perceive as a cleaner version of this function.  It doesn't have the nice compactness of the original code, but uses the same basic algorithm.  The local functions are quite general and can easily be promoted to top level functions.  Reusable code is usually to be preferred.

import Data.Function (on)
import Data.List (group, sortBy)
type Border a = Either a a

disjoint' :: (Ord a) => [(a, a)] -> [(a, a)]
disjoint' = concatMap consecutiveBordersToIntervals
            . groupBalancedBorders
            . sortBy (compare `on` fromBorder)
            . intervalBorders
    where
      intervalBorders :: [(a, a)] -> [Border a]
      intervalBorders = concatMap $ \(x, y) -> [Left x, Right y]

      groupBalancedBorders :: [Border a] -> [[Border a]]
      groupBalancedBorders [] = []
      groupBalancedBorders xs = let (ys, zs) = spanBalancedBorders 0 xs
                                in ys : groupBalancedBorders zs

      spanBalancedBorders :: Int -> [Border a] -> ([Border a], [Border a])
      spanBalancedBorders _       []     = ([], [])
      spanBalancedBorders balance (x:xs) =
          let balance' = either (const $ balance + 1) (const $ balance - 1) x
          in case balance' of
               0 -> ([x],xs)
               _ -> let (ys,zs) = spanBalancedBorders balance' xs in (x:ys,zs)

      consecutiveBordersToIntervals :: (Eq a) => [Border a] -> [(a, a)]
      consecutiveBordersToIntervals =
          (\xs -> zip xs (tail xs)) . dedupSortedList . map fromBorder

      dedupSortedList :: (Eq a) => [a] -> [a]
      dedupSortedList = map head . group

      fromBorder :: Border a -> a
      fromBorder = either id id

This is as fast as the original, but arguably easier to read.  It also fixes a bug which caused empty intervals to be included.

>>> disjoint [(1,5), (5,10), (1,10)]
[(1,5),(5,5),(5,10)]
>>> disjoint' [(1,5), (5,10), (1,10)]
[(1,5),(5,10)]

As a final note, one should be aware that the algorithm makes some implicit assumptions on the topology of the intervals in a.  It would break down when intervals were allowed to have start points which are greater than their endpoint (think modular arithmetic).


Calculation caching experiment There are a lot of numerical properties to be invoked many times with an expensive calculation. Let’s say for the sake of example, they expose a Factorial:

public class Analysis
{
    public double n10 => F(10);
    public double n100 => F(100);
    double F(int n) =>
        n <= 1 ? 1 : n * F(n-1);
}

I was looking for a way to cache them while keeping syntax clean, so property calculation logic will not intermix with caching. A proposed solution is:

public class Analysis
{
    public double n10 => this.Cached() || F(10);
    public double n100 => this.Cached() || F(100);
    double F(int n) =>
        n <= 1 ? 1 : n * F(n-1);
}

Where library code is:

public static class Caching
{
    static ConditionalWeakTable<object, Dictionary<string, double>> Values { get; } =
        new ConditionalWeakTable<object, Dictionary<string, double>>();

    public static Value Cached(this object target, [CallerMemberName] string name = null) =>
        Values.GetOrCreateValue(target).ContainsKey(name) ?
            new Value(() => Values.GetOrCreateValue(target)[name], null) :
            new Value(null, v => Values.GetOrCreateValue(target)[name] = v);
}

And:

public class Value
{
    public static implicit operator Value(double value) => new Value(() => value, null);
    public static implicit operator double(Value value) => value.Getter();
    public static bool operator true(Value value) => value.Getter != null;
    public static bool operator false(Value value) => value.Getter == null;

    public static Value operator |(Value cached, Value computed)
    {
        cached.Setter(computed);
        return computed;
    }

    public Value(Func<double> getter, Action<double> setter)
    {
        Getter = getter;
        Setter = setter;
    }

    Func<double> Getter { get; }
    Action<double> Setter { get; }
}

Operator-free version (answer to...your answer):

public static class Analysis
{
    public static readonly Lazy<double> n10 = new Lazy<double>(() => F(10));

    private double static F(int n)
    {
        return n <= 1 ? 1 : n * F(n - 1);
    }
}

Do not look at marginal style differences (I cannot get used/appreciate expression bodied functions but it's just my own problem), point I'd like to highlight is:

You do not need Once class, Framework already has a class for lazy creation of values: Lazy<T>. It even nicely handle thread-safety issues (safe creation and/or publication) if field is not static (in that case C# itself helps us.)

Few minor notes about current code:

Analysis class has no need to be instantiable and all methods can be static.
Class itself can then be static.
IMO you do not need a dictionary, first of all because of the performance impact (even if it's a concurrent collection without locks) but also because it limits its usage to static methods (you use MethodInfo as key).

I understand your point of view (C# syntax isn't so clean to be used for business logic) but instead of forcing syntax and change operators semantic I'd prefer to use another language. Boo was first that came to my mind (especially because to change and extend the language to have a DSL is incredibly easy). I used it few years ago then I'm not up-to-date with its evolution. There are also other tools (second thought if for Microsoft Modeling SDK if you like visual editors instead of plain text code)


Callback system for events in a window I have written a simple window event capture wrapper that gets events that happen to the window, and can be checked anywhere in the program through a global class. I wanted to be able to create a callback anywhere in the program with any event that I have asked to be checked. To hold the data that is returned from the events, I have used a pointer that is allocated whenever an event occurs, so I can grab the data returned later on in the program.

I don't want to use templates to hold the data since that would require a lot of objects to be created, or a lot of vectors for each specific event that I want. I find this to be too clunky, and I wanted a clean, simple solution.

Data that is captured

struct windowEventData
    {
        void *data = nullptr;
        std::string id;
        sf::Event::EventType event;

        bool polled = false;
    };

Window Event Manager

class windowEventManager
    {
        private:
            std::vector<windowEventData> _subscribedEvents;

        public:
            void subscribe(const std::string &id, sf::Event::EventType event);
            void pollEvent(sf::Event event);
            void clearEvents();

            template<typename T>
            T* hasEventPolled(const std::string &id);

            ~windowEventManager();
    };

Grabbing and setting the pointer

void windowEventManager::pollEvent(sf::Event event)
    {
        for (auto &eventCheck : _subscribedEvents)
            {
                if (eventCheck.event == event.type)
                    {
                        switch (event.type)
                            {
                                case sf::Event::KeyPressed:
                                case sf::Event::KeyReleased:
                                    eventCheck.data = new sf::Event::KeyEvent(event.key);
                                    break;
                                case sf::Event::MouseButtonPressed:
                                case sf::Event::MouseButtonReleased:
                                    eventCheck.data = new sf::Mouse::Button(event.mouseButton.button);
                                    break;
                                case sf::Event::MouseWheelMoved:
                                case sf::Event::MouseWheelScrolled:
                                    eventCheck.data = new bool(true);
                                    break;
                                case sf::Event::TextEntered:
                                    eventCheck.data = new sf::String(event.text.unicode);
                                    break;
                                default:
                                    break;
                            }
                        eventCheck.polled = true;
                    }
            }
    }

I clear events before each poll, and in the destructor so I don't have memory leaks/too much on the heap

void windowEventManager::clearEvents()
    {
        for (auto &eventCheck : _subscribedEvents)
            {
                if (eventCheck.data)
                    {
                        delete eventCheck.data;
                        eventCheck.data = nullptr;
                    }
                eventCheck.polled = false;
            }
    }

Finding out if an event has occured

template<typename T>
inline T *windowEventManager::hasEventPolled(const std::string &id)
    {
        auto it = std::find_if(_subscribedEvents.begin(), _subscribedEvents.end(), [id] (windowEventData &dat) { return dat.id == id && dat.polled; });
        if (it != _subscribedEvents.end())
            {
                return static_cast<T*>((*it).data);
            }

        return nullptr;
    }

Is my use of a void* proper? I figured I could use a union as well for this, but I would rather see if using a pointer would be a good solution.

delete eventCheck.data;

This line invokes undefined behavior, because you aren't allowed to call delete on an incomplete type (and certainly not on void). Both GCC and Clang warn on this line:

(clang++)
prog.cc:4:5: warning: cannot delete expression with pointer-to-'void' type 'void *' [-Wdelete-incomplete]
    delete p;
    ^      ~

(g++)
prog.cc:4:12: warning: deleting 'void*' is undefined
     delete p;
            ^

Anyway, it won't do what you want (whatever that might be).

For an open set of types, the type-safe way to do what you're trying to do is to use type erasure, as seen in library containers such as std::function<void(void)>. (Shameless plug for my CppCon 2014 talk — type erasure is covered starting at 23m00s.) This kind of type-erasure is easy to implement yourself from scratch in about 20 lines of code.

For a closed set of types, you might see whether your compiler of choice supports C++17 std::variant and/or Boost boost::variant; you can use the visit function to perform an operation on a variant in a type-safe way, and of course you wouldn't even need delete at that point because you wouldn't be using pointers, just plain old value semantics. If you can't use Boost, then I don't recommend trying to implement variant yourself.