Leetcode Count of Smaller Numbers After Self solution Question link

  You are given an integer array nums and you have to return a new
  counts array. The counts array has the property where counts[i] is the
  number of smaller elements to the right of nums[i].

Solution is done using customized BST where count stores the total number of elements lesser than current element found from left to right.

class Solution:
    def countSmaller(self, nums):
        class Node:
            def __init__(self, val):
                self.val = val
                self.left = None
                self.right = None
                self.count = 0

        def bst_insert(root, val, result, count):
            if not root:
                root = Node(val)
                root.left = None
                root.right = None
                result.append(count)
                return root
            if val > root.val:
                root.right = bst_insert(root.right, val, result, root.count + 1 + count)
            else:
                root.count += 1
                root.left = bst_insert(root.left, val, result, count)
            return root

        root = None
        result = []
        for num in nums[::-1]:
            if not root:
                root = Node(num)
                result.append(0)
            else:
                bst_insert(root, num, result, 0)
        return result[::-1]

The first advice is to remove the class. In python you should use classes when it makes sense to do so, but you don't need to.

Next, your bst_insert has redundant code. root.left = None and root.right = None are already done in your constructor, so you don't need to duplicate them.

The last piece of advice is to use reversed(nums) instead of num[::-1] as the former does not make a copy, which should be faster and use less memory.

Other than that this looks pretty good.


Checks size of and number of files in each subdirectory The code snippet here checks in every folder for subfolders, and stores the sub-sub folders
This is my sample directory structure. I want to read through every sub-folders to find the number of files and total size of the sub folder.

--Parent
    |---FolderA
    |   |__subFolder1
    |   |__subFolder2
    |
    |---FolderB
        |__subFolder3
        |__subFolder4
        |__subFolder5

import glob
import os
from IPython import embed
import subprocess
import humanize

list_of_files = os.listdir(os.getcwd())
total_stats =[]
path = os.getcwd()

for files in list_of_files:  #check FolderA, Folder B etc.,
        print files
        while (os.walk(path+"/"+files).next()):
            curr_path ,dirs,_ = os.walk(path+"/"+files).next()
            data_dict = {}
            flag = "_hd"

            for d in dirs:  #check subFolder1,2,3,4...
                 sub_p,sub_d,_ = os.walk(curr_path+"/"+d).next()
                 for d  in sub_d:         #reads through files inside subfolder
                   p,_,f =os.walk(sub_p+"/"+d).next()
                   total_size = 0
                   for fn in f:          #Finds total size of all files
                       total_size += os.path.getsize(p+"/"+fn)
                   data_dict[d+flag] =  [len(f),humaize.naturalsize(total_size)] #prints file size in human readable form Ex: '2.3MB'
                 flag = "_rd"
            break
    total_stats.append([files,data_dict])        #updates no. of files and sizes

How can I optimize this code to avoid these many ridiculous amounts of for loops?

edit: I just realized, that you're using python2. I'm very unfamiliar with that and if you'll be able to use yield from, pathlib and format string literals. If you like the solution, I think I could rewrite it to be usable for python2.

Solution for python 3.6 and up (of that, I am very confident that it will produce no error):

Instead of creating all your information within one function, you can reduce the nesting depth by pulling up some of the functionality.

from pathlib import Path

def folders_in_path(path):
    if not Path.is_dir(path):
        raise ValueError("argument is not a directory")
    yield from filter(Path.is_dir, path.iterdir())

def folders_in_depth(path, depth):
    if 0 > depth:
        raise ValueError("depth smaller 0")
    if 0 == depth:
        yield from folders_in_path(path)
    else:
        for folder in folders_in_path(path):
            yield from folders_in_depth(folder, depth-1)

def files_in_path(path):
    if not Path.is_dir(path):
        raise ValueError("argument is not a directory")
    yield from filter(Path.is_file, path.iterdir())

def sum_file_size(filepaths):
    return sum([filep.stat().st_size for filep in filepaths])

if __name__ == '__main__':
    for folder in folders_in_depth(Path.cwd(),1):
        #       vvvv quick hack to to use len(), does not perform well
        files = list(files_in_path(folder))
        total_size = sum_file_size(files)
        print(f'{folder}: filecount:{len(files)}, total size:{total_size}')

which produces:

/tmp/Parent/FolderB/subfolder3: filecount:10, total size:50
/tmp/Parent/FolderB/subfolder2: filecount:10, total size:50
/tmp/Parent/FolderB/subfolder1: filecount:10, total size:50
/tmp/Parent/FolderA/subfolder3: filecount:10, total size:50
/tmp/Parent/FolderA/subfolder2: filecount:10, total size:50
/tmp/Parent/FolderA/subfolder1: filecount:10, total size:50

if your cwd is Parent.
This will only count files in that are in a subdirectory at depth == 1 and nothing else.

There might be some neater solutions, that's just what I came up with in a few minutes.

edit2: I was curious, so I did a 2.7 version. os.walk is using depth first search. From what I gather, it could be done like this:

import os

def folders_in_depth(path, depth, walk_iter=None):
    if walk_iter is None:
        walk_iter = os.walk(path)
    if 0 > depth:
        raise ValueError("depth smaller 0")
    if 0 == depth:
        dirpath, dirnames, filenames = next(walk_iter)
        for i in range(len(dirnames)):
            dirpath, dirnames, filenames = next(walk_iter)
            yield dirpath, filenames
    else:
        dirpath, dirnames, filenames = next(walk_iter)
        for i in range(len(dirnames)):
            for result in folders_in_depth(path, depth-1, walk_iter=walk_iter):
                yield result

def sum_file_size(folder, filepaths):
    return sum([os.path.getsize(folder + '/' + filep) for filep in filepaths])

if __name__ == '__main__':
    for folder in folders_in_depth(os.getcwd(),1):
        foldername, files = folder
        print foldername,': filecount:', len(files), ', total size:', sum_file_size(foldername,files)

which results in

/tmp/Parent/FolderB/subFolderB3 : filecount: 10 , total size: 50
/tmp/Parent/FolderB/subFolderB2 : filecount: 10 , total size: 50
/tmp/Parent/FolderB/subFolderB1 : filecount: 10 , total size: 50
/tmp/Parent/FolderA/subFolderA3 : filecount: 10 , total size: 50
/tmp/Parent/FolderA/subFolderA2 : filecount: 10 , total size: 50
/tmp/Parent/FolderA/subFolderA1 : filecount: 10 , total size: 50

Not sure, where the extra spaces come from.

Here, the iterator is passed down in the recursion and has to be forwarded with next to produce the correct result.

Caveat: I think, this 2.7 solution will not work if the 'subFolder*' contain more folders on their own. Judging from how os.walk is implemented, modifying that solution might be the best option, so you don't have to worry about that. (sadly I was looking at how 3.6 implements os.walk, which is a little more complicated)

So for this to work correctly, you'd have to forward the iterator through the complete sub-tree, which is completely unneccesary.
os.walk allows to modify the returned dirname to control where it should recurse via side effects. So clearing that might lead to the wanted result, you'd need to test that.


Optimize a zscore algorithm I wrote a zscore algorithm in Ruby that runs fine, but is incredibly slow when I have 8000+ entries to score. Can anyone help me figure out a way to improve my code, please?

module Enumerable
  def mean
    reduce(:+).to_f / length
  end

  def sample_variance
    sum = inject(0){ |acc, i| acc + (i - mean)**2 }
    1 / length.to_f * sum
  end

  def standard_deviation
    Math.sqrt(sample_variance)
  end

  def zscore
    if standard_deviation.zero?
      Array.new(length, 0)
    else
      collect { |v| (v - mean) / standard_deviation }
    end
  end
end

The float is giving every score an accuracy of up to 17 decimal places.  Would making it only 8 decimal places speed things up?

EDIT:
Here is an updated version of the algorithm given the advice received in this thread.

class Array
  def mean(len=self.length)
    reduce(:+).to_f / len
  end

  def sample_variance
    len = length
    m = mean(len)
    sum = reduce { |acc, i| acc + (i - m)**2 }
    sum.to_f / len
  end

  def standard_deviation
    Math.sqrt(sample_variance)
  end

  def zscore
    stdev = standard_deviation
    m = mean
    stdev.zero? ? Array.new(length, 0) : collect { |v| (v - m) / stdev }
  end
end

The problem is here: collect { |v| (v - mean) / standard_deviation }. standard_deviation is constant but, being inside a block, it is called on each iteration. Set the value to a local variable before. As noted by Flambino, the same principle applies to sample_variance (which uses mean inside a block).

In a functional language (where immutability is honored) the compiler would be able to do the right thing, but not in an imperative language plagued with side-effects like Ruby.

Some additional notes to your code:

module Enumerable: But you call .length, which is not a method that an enumerable is required to implement. Consider adding them to Array (which includes Enumerable).
reduce and then inject. I'd use just one of the alias.


Javascript speed of an animated object (planet orbiting sun) So I am pretty new to JavaScript and I thought I might make an animation demonstrating a planet orbiting  a sun in an elliptical orbit using Kepler's laws.
I am struggling to get the speed right. So I want the speed of the planet to be slower further away from the sun, and faster closer to the sun.
Within this method I use Keplers laws to calculate the x and y coordinate. I have also calculated the (real) velocity at each point in its trajectory (this may need to be scaled appropriately as I hope to use real astronomical data).
I tried to use a scaled version of the velocity. The velocity values produced were in the range of 10,000 to 80,000 - so I thought of using that to update theta using the following:
  this.speed = function(){
    this.theta += this.velocity/10000 ;
  }

However this didn't work and the planet disappears.
I've got full code is below:
HTML:
<canvas id="mybox"></canvas>

CSS:
canvas {
  border: 2px solid #000;
  background-color: #102233;
}

JS:
var canvas = document.getElementById('mybox');
canvas.width = window.innerWidth ;
canvas.height = window.innerHeight ;

var ctx = canvas.getContext('2d');

function sun(){
// Draws Sun at center
  ctx.beginPath() ;
  ctx.fillStyle = "orange";
  ctx.arc(canvas.width/2, canvas.height/2, 40, 0, Math.PI*2);
  ctx.fill();
};

function planet(M, G, T, e) {
  this.x = 0 ;
  this.y = 0 ;
  this.radius = 20 ;
  this.theta = 0 ;
  this.dtheta = 0.5 ;
  this.vel = 0 ;

  this.updateTheta = function() {
    if (this.theta > Math.PI){
      this.theta += this.dtheta ;
      this.dtheta += 0.001 ;

    }
    else if (this.theta < Math.PI){
      this.theta +=this.dtheta ;
      this.dtheta -= 0.001 ;
    }
    if(this.theta>=2*Math.PI || this.theta < 0) {
      this.theta = 0;
    }
  }

  this.position = function(theta) {
// Calculates position
    var R = Math.pow(((Math.pow(T,2))/((4*Math.pow(Math.PI, 2))/(G*M))),(1/3)) ;
    var a = R/(1-e) ;
    var radius = a*(1-Math.pow(e,2))/(1+e*Math.cos(theta)) ;
    this.x = radius*Math.cos(theta)*10e-10 + canvas.width/2 ;
    this.y = radius*Math.sin(theta)*10e-10 + canvas.height/2;
    this.vel = Math.sqrt(G*M*((2/radius)-(1/a))) ;
  }

  this.draw = function() {
    ctx.beginPath();
    ctx.fillStyle = 'green';
    ctx.arc(this.x, this.y, this.radius, 0, Math.PI*2);
    ctx.fill();

    this.position(this.theta) ;
    this.updateTheta() ;
  }

}

// Values to display elliptical orbit
var G = 6.e-11;
var M = 5e+30;
var T = 1e+7;
var e = 0.6;

function update(){
  ctx.clearRect(0, 0, canvas.width, canvas.height);
  sun();
  earth.draw();
}

let earth = new planet(M, G, T, e)
setInterval(update, 50);

Any help or comments will be appreciated. Thanks!

Review
Orbit
You are incorrectly calculating the position of the planet.
The eccentric anomaly which you attempt to calculate with a linear, integrated 2nd derivative, in the function this.updateTheta will never work as the solution in non linear. That said the example below uses a linear approximation.
There is no easy way to change your code such that time steps can be scaled while maintaining the same orbit. It is best to use the standard orbital solution.
Animation
Never use 'setInterval' for animation
For the best result when animating via JS in the browser use the animation callback requestAnimationFrame (rAF)
It will call the main render loop function at 60FPS (If your render function takes no more than ~80% of 1/60th second (12 - 13ms)) Please read the documentation for rAF as there are many caveats.
Example of basic animation loop
    requestAnimationFrame(renderLoop);     // request first frame

    function renderLoop() {
        ctx.clearRect(0, 0, W, H);         // clear canvas ready for next frame render

        ... // call render function

        requestAnimationFrame(renderLoop); // Request Next frame
    }

Readable
Your calculations are very hard to read and you are using some outdated calls.

Use the ** operator rather than Math.pow or Math.sqrt. Eg 2 ** 2 is two squared and 2 ** 0.5 is the square root of two.

The main reason that your calcs are hard to read is because you don't space out the operators. Always use spaces between operators.
The line...

var R = Math.pow(((Math.pow(T,2))/((4*Math.pow(Math.PI, 2))/(G*M))),(1/3)) ;

is far easier to read as...
 const R = (T ** 2 / (4 * Math.PI ** 2 / (G * M))) ** (1 / 3);

Note that variables that do not change should be constants.

Orbits and controlling their animation periods.
Note I use radians
Kepler's laws modify a circular orbit into an ellipse such that the line from the planet (Earth) to the parent (Sun) sweeps equal areas in equal time.
We define the ellipse by the semi-major axis, the eccentricity, and either the closest (periapsis) or furthermost (apoapsis) points in the orbit. Both points are on the semi-major axis
We want to crate a function that gives the planets position at a given time. We can then scale that time to control the rate that the animation plays out.

For a circular orbit the object must travel 2 * MATH.PI.

Using ms we can define the period of the orbit. eg 1 second is 1000ms

We can then scale the orbit position as a function of time.
Example

The orbit position scale is scale = (period) => 2 * MATH.PI / period
With an orbit of 1 second the position is  pos = (ms) => scale(1000) * ms where ms is the current time. The pos returned is the mean anomaly M

Kepler's law

The orbital position is called the mean anomaly M in radian (range -PI to PI)

When traveling on a non circular orbit the mean anomaly M is advanced or retarded depending on the orbital distance. That value is the eccentric anomaly E in radians

The ellipse is defined by eccentricity e range  0 <= e < 1

Kepler states that M = E - e * Math.sin(E)

We know M and e, but not E so we must solve the above equation in terms of M. This is non trivial so we must approximate.
To do this we use Newtons method to solve M = E - e * Math.sin(E) for M with a linear approximation of the non linear part e * sin(E) see function eccentricAnomaly in Demo

With E we can then calculate the 2D position of the body using e and the distance to apoapsis r
  const x = r * (Math.cos(E) - e);
  const y = r * Math.sin(E) * (1 - e ** 2) ** 0.5;

This 2D position can then easily be aligned to the semi-major axis, and if needed rotated into 3D to match the inclination, position of ascending node and argument of periapsis to fully define any 2 body orbit (Newtonian).

DEMO
All that above was clear as mud I am sure, thus a Demo.
The code below animates 3 objects (two 2 body solutions) at a specified orbital period of 1 and 2 seconds. The time is given by rAF as first argument given to the callback renderLoop.
Time 0 is set to apoapsis which is defined by the start positions of the planet relative to the parent body. The orbit is rotated in 2D to align to the apoapsis.
The eccentricity of planets e is 0.5

requestAnimationFrame(renderLoop);
const ctx = canvas.getContext("2d");
ctx.lineCap = "round";
var frameCount = 0, startTime, prevTimeScale = timeScale.value, lastTime;
const W = canvas.width, H = canvas.height, CX = W * 0.25, CY = H * 0.5;

const SUN_RADIUS = 20, SR = SUN_RADIUS;               // in pixels
const PLANET_SCALE = 0.4, PS = PLANET_SCALE;          // in sun radi
const FIRST_PLANET_DIST = 4, FPD = FIRST_PLANET_DIST; // in sun radi
const ECCENTRICITY = 0.5, E = ECCENTRICITY;           // 0 circlular
const KEPLER_PERIOD = 1000, KP = KEPLER_PERIOD;       // ms per orbit

const MBLUR_LINE_GRAD = 8; // Steps to grade blur line
const FRAME_RATE = 60; // Max 60 Frames per second. Only use valid values
                       // Valid vals 60, 30, 20, 15, 12, 10, 6, 5, 4, 3, 2, 1
const FRAME_SKIP = 60 / FRAME_RATE;
Math.TAU = Math.PI * 2;
Math.R90 = Math.PI * 0.5;

const calcVectors = (A, B) => {
    const dx = B.x - A.x;
    const dy = B.y - A.y;
    const rSqr = dx * dx + dy * dy, r = rSqr ** 0.5;
    const nx = dx / r;
    const ny = dy / r;
    return {nx, ny, rSqr, r, axis: Math.atan2(ny, nx)};
}
const eccentricAnomaly = (M, e) => {  // newtons method to solve M = E-e sin(E) for E
    var d, E = M; // guess E
    do {
        d = (E - e * Math.sin(E) - M) / (1 - e * Math.cos(E));
        E -= d;
    } while (d > 1e-6);
    return E;
}

const Planet = {
    x:  0, y:  0, vx: 0, vy: 0, ox: 0, oy: 0,
    init() { return this },
    update() { },
    draw(ctx, lastSubFrame) {
        const A = this;
        const dx = A.ox - A.x;
        const dy = A.oy - A.y;
        const dist = (dx * dx + dy * dy) ** 0.5;
        ctx.setTransform(1, 0, 0, 1, CX + A.x, CY + A.y);
        ctx.fillStyle = ctx.strokeStyle = A.col;
        if (dist > 0.1 && MBLUR_LINE_GRAD > 0) {
            ctx.globalAlpha = (1 / (dist - 0.1)) ** 1.4;
            ctx.beginPath();
            ctx.strokeStyle = A.col;
            const r = A.radius * 2;
            ctx.moveTo(dx, dy);
            ctx.lineTo(0, 0);
            const MBLG = MBLUR_LINE_GRAD;
            let i = MBLG;
            while (i--) {
                ctx.lineWidth = Math.cos(((i - 1) / MBLG) * Math.R90) * r;
                ctx.stroke();
            }
        } else if (lastSubFrame) {
            ctx.beginPath();
            ctx.arc(dx / 2, dy / 2, A.radius, 0, Math.TAU);
            ctx.fill();
        }
        ctx.globalAlpha = 1;
        A.ox = A.x;
        A.oy = A.y;
    },
};
const KeplerPlanet = {
    period: 1,
    init(e, B) {  // B at relative position at time 0. A is at apoapsis
        const A = this;
        A.period =  Math.TAU / A.period;
        A.e = e;
        Object.assign(A, calcVectors(A, B));
        A.px = Math.cos(A.axis); // Transform to Rotate semi major axis
        A.py = Math.sin(A.axis);
        return A;
    },
    update(time) {
        const A = this;
        const E = eccentricAnomaly(A.period * time, A.e);
        const x = A.r * (Math.cos(E) - A.e);
        const y = A.r * Math.sin(E) * (1 - A.e ** 2) ** 0.5;
        A.x = x * A.px - y * A.py;
        A.y = x * A.py + y * A.px;
    },
};
const FixedPlanet = {
    radius: 0,
    col: "#EC8",
};

function createOrbitObj(type, x, y, radius, col, period, e, orbits) {
    return  ({...Planet, ...type, x, y, ox: x, oy: y, period, radius, col }).init(e, orbits);
}
const sun = createOrbitObj(FixedPlanet , 0, 0, SR, "#FF8");
const p1 = createOrbitObj(KeplerPlanet,  FPD * SR, 0, SR * PS, "#6C1", KP, E, sun);
const p9 = createOrbitObj(KeplerPlanet,  FPD * SR * 2, 0, SR * PS, "#C61", KP * 2, E, sun);
const system = Object.assign([sun ,p1, p9], {
    update(time) {
        var i = this.length - 1;
        while (i) { this[i--].update(time) }
    },
    draw(ctx, lastSubFrame) {
        for (const A of this) { A.draw(ctx, lastSubFrame) }
    },
});

function renderLoop(time) {
    var tT, i, pTS = prevTimeScale; // tT is totalTime
    if (frameCount++ % FRAME_SKIP === 0) {
        startTime = startTime ?? time;
        tT = time - startTime;

        ctx.setTransform(1, 0, 0, 1, 0, 0);
        ctx.clearRect(0, 0, W, H);

        const inputTimeScale = timeScale.value;
        if (inputTimeScale !== pTS) {
            startTime = time - tT * pTS / inputTimeScale;
            pTS = prevTimeScale = inputTimeScale;
            tT = time - startTime;
        }
        const dT = tT * pTS - lastTime; // delta time
        i = 0;
        while (i < MBLUR_LINE_GRAD && lastTime) {
             const current = lastTime + dT * (i++ / MBLUR_LINE_GRAD);
             system.update(current);
             system.draw(ctx, false);
        }

        system.update(tT * pTS);
        system.draw(ctx, true);
        lastTime = tT * pTS;
    }
    requestAnimationFrame(renderLoop);
}
canvas { background: black; }
input { position: fixed; top: 10px; left: 10px; color: white; width: 340px }
<canvas id="canvas" width="350" height="300"></canvas>
<input type="range" id="timeScale" min="0.01" max="4" step ="0.01" value="1">

Why retro? The view is up from under the ecliptic (to matche canvas coords), orbits are thus retrograde to the solar system's direct (prograde)
Extras
I have added some additional features to demo.

Time Scale. The range bar changes the time scale that lets you change the rate of time. Time is a value based on a start time. When scaling time that start time must be change so that the planet does not skip time.

Frame Rate. There is a value called FRAME_RATE which sets the frame rate. Max 60. This rate is a requested rate, the actual rate will vary depending on device and CPU/GPU load, but over time it will average the requested rate.

Motion Blur. To improve the look when the time scale is large there is a simple motion blur applied to the moving planets.
The blur is just a linear interpolation between sub time steps and thus only works when movement per sub step is less than about 0.5 radians.
MBLUR_LINE_GRAD controls how the blur is drawn and the number of sub time steps are used. A value of 0 will turn off motion blur. Values 1 or greater will increase the persistence. Current value is 8 and is tuned for the current frame rate of 60FPS.


Translate datetime format from one standard to another In my python code, date formats are stored as "strftime and strptime syntax". For instance %d/%m/%Y %H:%M.

On the client side, I use pickadate.js which format is rather dd/mm/yyyy HH:i.

So I have to translate %d/%m/%Y %H:%M to dd/mm/yyyy HH:i before sending a datetime format to the client.

I first thought str.translate was the right tool for this. But actually, this is only for characters translation.

Then I thought about str.replace but it looks awful and inefficient to loop over str.replace:

def format_to_pickadate_format(fmt):
    trans_table = [
        ('%d', 'dd'),
        ('%a', 'ddd'),
        ('%A', 'dddd'),
        ('%m', 'mm'),
        ('%b', 'mmm'),
        ('%B', 'mmmm'),
        ('%y', 'yy'),
        ('%Y', 'yyyy'),
        ('%H', 'HH'),
        ('%M', 'i'),
        ('%p', 'A'),
    ]
    for trans in trans_table:
        fmt = fmt.replace(*trans)
    return fmt

So, to make it more elegant I used functools.reduce:

from functools import reduce

def format_to_pickadate_format(fmt):
    trans_table = [
        ('%d', 'dd'),
        ('%a', 'ddd'),
        ('%A', 'dddd'),
        ('%m', 'mm'),
        ('%b', 'mmm'),
        ('%B', 'mmmm'),
        ('%y', 'yy'),
        ('%Y', 'yyyy'),
        ('%H', 'HH'),
        ('%M', 'i'),
        ('%p', 'A'),
    ]
    return reduce(lambda x, y: x.replace(*y), trans_table, fmt)

But I think it still screams inefficiency. The string is browsed once for each sequence to replace if found.

Is there a way make it more efficient (ie. make the translation in only one iteration)?

  Note: This question is mainly for the sake of curiosity and learning. This is probably one of the least important thing I have to care about on my project, especially concerning performance.

  Obviously in this case, making it more efficient would be ridiculously imperceptible. If this helps you find an interest in this question, imagine another use case where I'd have a string of the size of an entire book, and a translation table of 10,000 or 100,000 entries. Doesn't it look dumb to read the book 10,000 or 100,000 times to make the translation?

The strftime function is relatively well known, even if the formatting directives are not completely standardized across platforms.  On the other hand, pickadate.js is a relatively unknown library.  It would therefore be helpful to cite its specification, preferably in a docstring.  The use of doctests would also be a good idea here.

According to the pickadate.js documentation, you should escape any “rule” characters with an exclamation mark (!).

Doing string substitutions in multiple passes is, in general, a bad idea.  Not only is it more work to traverse the string several times, there is also the possibility that operating on the output leads to incorrect results.  Fortunately, you seem to be safe here, since the pickadate.js directives don't start with % like the strftime directives.  Still, a one-pass solution is preferable, and Python gives you a way to do that with re.sub():

  re.sub(pattern, repl, string, count=0, flags=0)

    …

    If repl is a function, it is called for every non-overlapping occurrence of pattern. The function takes a single match object argument, and returns the replacement string.

Using re.sub(), and handling special cases where strings that appear to be strftime and pickadate.js directives should actually be treated literally…

import re

def format_to_pickadate_format(fmt):
    """
    Convert a strftime format string to a pickadate.js template.
    http://amsul.ca/pickadate.js/date/#formats
    http://amsul.ca/pickadate.js/time/#formats

    >>> format_to_pickadate_format('%d/%m/%Y %H:%M')
    'dd/mm/yyyy HH:i'
    >>> format_to_pickadate_format('%Y %% d')
    'yyyy % !d'
    """
    TRANS = {
        '%d': 'dd',
        '%a': 'ddd',
        '%A': 'dddd',
        '%m': 'mm',
        '%b': 'mmm',
        '%B': 'mmmm',
        '%y': 'yy',
        '%Y': 'yyyy',
        '%H': 'HH',
        '%M': 'i',
        '%p': 'A',
    }
    return re.sub(
        r'%(.)|([^dmyhHiaA])|(.)',
        lambda m: TRANS.get(m.group(), m.group(1)) or m.group(2) or '!' + m.group(3),
        fmt
    )


Creating cover traffic by calling random urls - Javascript loop to decrement float This is a for-loop which runs once for each step with i>1 and maybe for 0 <= i <= 1.
var i;
var prob_ = 3.5;
for ( i = prob_; i >= 0; i -= 1 ) {
    if ( i > 1 || withProbability(i) ) {
        loadHttp(randomUrl());
    }
}

with
function withProbability(chance) {
    return Math.random() < chance;
}

JsLint gave me a for-loop error. Is this code ok despite that warning, or should this for-loop be changed to some other looping construct?
Background
prob_ is a ratio between fake actions and real actions. The loop is triggered once for each real action. If prob_ is greater than 1, the fake action is done. If it is in between 0 and 1, it is considered a probability and thus maybe a fake action follows.
This is a case where it was not clear how to replace the for-loop with a forEach, as is often recommended. Maybe ES6 tail recursion could be used...
More context
function loadHttp(toLoad) {
    require("sdk/page-worker").Page({
        contentURL: toLoad
    });
}

The implementation of randomUrl() is rather lengthy and does not relate to the question. It determines a statistically probable length of a HTML (or embedded object like js, img, css, ...) site, then does a lookup for the next bigger URL in a hardcoded list. It is supposed that the randomness will confuse someone who watches for traffic patterns.
Full class coverTraffic.js
This is initialized via
const coverTraffic = require('./coverTraffic.js');
coverTraffic.setLoader(require('./load.js'));

with load.js containing similar to loadHttp as above. It is triggered via
function loads(URL) {
    if ( _.contains(activeHosts, URL.host) ) { // has already started
        coverTraffic.loadNext();
    } else {
        activeHosts.push(URL.host);
        coverTraffic.start();
    }
}
exports.loads = URL => loads(URL);

which is called whenever the browser loads a new URL.
"use strict";

exports.DOC = 'creates cover traffic, up to predetermined parameters';

const { setTimeout } = require("sdk/timers");

const coverUrl = require("./coverUrl.js");
const stats = require("./stats.js");

/** overhead of dummy traffic -1; 1.5 has overhead of 50% */
const FACTOR = 1.5;

var load;

var site_ = {};
var pad_ = {};
var prob_;

function setLoader(load_param) {
    load = load_param;
}
exports.setLoader = (load_param) => setLoader(load_param);

/** a website is loaded by the user. covertraffic determines a target
 * feature vector and adds to the load to approximate the target.
 */
function start() {
    site_ = {};
    pad_ = {};

    site_.html = stats.htmlSize(1); // td: buffer known sites in bloomfilter
    pad_.html = stats.htmlSize(FACTOR) - site_.html;
    site_.num_embedded = stats.numberEmbeddedObjects(1); // td: see above
    pad_.num_embedded = stats.numberEmbeddedObjects(FACTOR) - site_.num_embedded;
    prob_ = pad_.num_embedded / site_.num_embedded;

    //    setTimeout(loadNext, stats.parsingTime());
    load.http(coverUrl.sized(pad_.html));
}
exports.start = start;

function loadNext() {
    var i;
    if ( pad_.num_embedded > 0 ) {
    for ( i = prob_; i >= 0; i -= 1 ) {
        if ( i > 1 || stats.withProbability(i) ) {
        load.http(coverUrl.sized(stats.embeddedObjectSize()));
        pad_.num_embedded -= 1;
        }
    }
    } else {
    console.log("cover traffic empty, num: " + pad_.num_embedded);
    }
}
exports.loadNext = loadNext;

The stats module provides statistical distributions, the load-module just loading a URL as described above.

The problem JSLint is pointing at is that you usually iterate with an index that is an integer. If you were to have an array, you couldn't use i to access the array elements, because if i is big enough, rounding issues may cause you to skip elements or see elements twice!

What I'd recommend is that you just use i as integer, and then once the for loop is done, check if there is an additional chance to roll for. That way, you don't have to worry about floating point rounding errors accumulating.

Another way you could do it is first roll, and then if you roll high enough, up i by 1. That way you only have one place where you run your code.

So like this:

var chance = i % 1;
if (chance !== 0 && withProbability(chance)){
    i = Math.ceil(i);
} else {
    i = Math.floor(i);
}
//for loop goes here


Connect to MySQL and fetch a list of customers asynchronously using C# I am connecting with a database and returning values from a Web API controller.  For better performance, I am doing it completely asynchronously with a new task.

Am I doing it the correct way? Am I using correct the using statements? Is the await correct in Open, Close connection and sda.FillAsync?

public async Task<HttpResponseMessage> Get()
{
    return await Task.Run(() => GetAllCustomers());
}

private async Task <HttpResponseMessage> GetAllCustomers()
{
    DataTable Customers = new DataTable();

    using (MySqlConnection con = new MySqlConnection(""))
    using (MySqlCommand cmd = new MySqlCommand("SELECT * FROM Customers", con))
    {
        try
        {
            if (con.State == ConnectionState.Closed)
            {
                await con.OpenAsync();
                cmd.CommandType = CommandType.Text;
                MySqlDataAdapter sda = new MySqlDataAdapter(cmd);
                await sda.FillAsync(Customers);
            }
        }

        catch (MySqlException ex)
        {
            ex.Message.ToString();
        }
        finally
        {
            await con.CloseAsync();
        }
    }
    return ControllerContext.Request
             .CreateResponse(HttpStatusCode.OK, new { Customers });
}

In Get() you do not need to wrap the call to GetAllCustomers() into a Task because it's already asynchronous:

public async Task<HttpResponseMessage> Get()
    => await GetAllCustomers();

There is, however, something else to do:

GetAllCustomers() should be renamed GetAllCustomersAsync() to inform that it's an asynchronous method.
GetAllCustomersAync() should not be responsible to create the HttpResponseMessage because you want to test DB logic separately from controller logic. More on this later.
You do not need to return an HttpResponseMessage, simply return the strongly typed model/view model you're interested to.

In code:

public async Task<DataTable> Get()
    => await GetAllCustomersAsync();

Now you have to change GetAllCustomersAsync() to match the above.

Outside a fictional example (or an extremely simple application) I'd avoid to directly return DataTable and I'd move to a proper data layer. You usually do not want to handle SQL code directly in your business logic, especially because:

It's error-prone.
UI requirements may change over time but database may be stable.
Database may change over time but UI may be stable.
You do not have any compile-time check.
It's hard to test business logic without a database.

According to usual C# naming conventions local variables are camelCase, Customers should be customers.

You do not need to close a connection if it's wrapped within a using statement, disposing the connection will also close it.

Even assuming you do not return the model type, for example:

public IHttpActionResult Get()

You do not need to go through the ControllerContext.Request.CreateResponse() thing, simply:

public IHttpActionResult Get()
{
    return Ok(new { Customers });
}

In your catch clause ex.Message.ToString(); is doing nothing. If you properly had DB logic in a separate layer (let's say in a class DatabaseServices just to give a name) you may want to:

catch (MySqlException e)
{
    throw new DatabaseException("Cannot read customers", e);
}

Where DatabaseException is a custom exception you use through your data layer. Your controller (simplified without async stuff):

public IHttpActionResult Get()
{
    try
    {
        using (var db = new DatabaseService())
            return Ok(new { Customers = GetAllCustomers() });
    }
    catch (DatabaseException e)
    {
        return InternalServerError(e);
    }
}

Of course in real world there will also be some logging here and there and some logic to recover from known errors...

Jumping back to your query method, it might be simplified little bit:

using (var con = new MySqlConnection("..."))
using (var cmd = new MySqlCommand("SELECT * FROM Customers", con))
{
    try
    {
        await con.OpenAsync();

        var sda = new MySqlDataAdapter(cmd);
        var customers = new DataTable();
        await sda.FillAsync(customers);

        return customers;
    }
    catch (MySqlException ex)
    {
        // ...
    }
}

You do not need to check if connection is closed for a newly created MySqlConnectionObject(), it can't happen and your code wil always work as expected but if it was not the case then you will return an empty DataTable.

Note that when code will grow and you will probably need a better way to build SQL commands (assuming you won't use an ORM). Very first step is to, at least, move them outside in private const string fields.


Shorthand and nicer python syntax for four options clicking a button I have a var af_button which is a button on UI. It can be either selected or deselected, so in that case I have four possibilities, which I have implemented. I don't like this implementation, so what would be a more pythonic way of implementing this?

af_button = squishtest.waitForObject("{text='AF' type='QPushButton' unnamed='1' visible='1'}")
if state == 'click':
    if af_button.checked == 1:
        pass
    else:
        squishtest.clickButton(squishtest.waitForObject(af_button))
else:
    if af_button.checked == 0:
        pass
    else:
        squishtest.clickButton(squishtest.waitForObject(af_button))

If 0 and 1 are the only possible values of af_button.checked, you could condense this to just one if:

if (state == 'click') != af_button.checked:
    squishtest.clickButton(squishtest.waitForObject(af_button))

The parentheses are important because otherwise Python would treat this as a chained comparison. (state == 'click') evaluates to True or False, and True equals 1 and False equals 0.


Simple logging library in C This header implements a very simple set of C (only) functions for logging.
This is part of a larger collection of utility functions aimed to be used during the development process, meaning that they are intended to provide quick, easy to use solutions that could be replaced in the production code if needed.

To ease with reading the code, I've removed the (rather verbose) comments with the documentation. You can find it in the README.md file here.

Beside the printf()-like functions, there are a couple of functions that may be used for testing and debugging purpose.

I'm very interested in any feedback you may have, especially in the area of usability.

#ifndef UTL_H
#define UTL_H

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
#include <stdarg.h>
#include <stddef.h>
#include <time.h>
#include <ctype.h>

#ifndef UTL_NOLOG

#define logprintf(...)  utl_log_printf(__VA_ARGS__)

#define logclose()      utl_log_close("LOG STOP")
#define logopen(f,m)    utl_log_open(f,m)

#ifndef NDEBUG
#define logcheck(e)     utl_log_check(!!(e),#e,__FILE__,__LINE__)
#define logassert(e)    utl_log_assert(!!(e),#e,__FILE__,__LINE__)
#define logdebug        logprintf
#else
#define logcheck(e)     utl_log_one()
#define logassert(e)    ((void)0)
#define logdebug(...)   ((void)0)
#endif

#define _logprintf(...) ((void)0)
#define _logdebug(...)  ((void)0)
#define _logcheck(...)  utl_log_one()
#define _logassert(...) ((void)0)
#define _logopen(f,m)   ((void)0)
#define _logclose()     ((void)0)

void utl_log_close(char *msg);
void utl_log_open(char *fname, char *mode);
int  utl_log_check(int res, char *test, char *file, int line);
void utl_log_assert(int res, char *test, char *file, int line);
void utl_log_printf(char *format, ...);
int  utl_log_one(void);

#ifdef UTL_MAIN

static FILE *utl_log_file = NULL;

void utl_log_close(char *msg)
{
  if (msg) logprintf(msg);
  if (utl_log_file && utl_log_file != stderr) fclose(utl_log_file);
  utl_log_file = NULL;
}

void utl_log_open(char *fname, char *mode)
{
  char md[2];
  md[0] = (mode && *mode == 'w')? 'w' : 'a'; md[1] = '\0';
  utl_log_close(NULL);
  utl_log_file = fopen(fname,md);
  logprintf("LOG START");
}

void utl_log_printf(char *format, ...)
{
  va_list  args;
  char     log_tstr[32];
  time_t   log_time;

  if (!utl_log_file) utl_log_file = stderr;
  time(&log_time);
  strftime(log_tstr,32,"%Y-%m-%d %X",localtime(&log_time));
  fprintf(utl_log_file,"%s ",log_tstr);
  va_start(args, format);
  vfprintf(utl_log_file, format, args);
  va_end(args);
  fputc('\n',utl_log_file);
  fflush(utl_log_file);
}

int utl_log_check(int res, char *test, char *file, int line)
{
  logprintf("CHK %s (%s) %s:%d", (res?"PASS":"FAIL"), test, file, line);
  return res;
}

void utl_log_assert(int res, char *test, char *file, int line)
{
  if (!utl_log_check(res,test,file,line)) {
    logprintf("CHK EXITING ON FAIL");
    logclose();
    exit(1);
  }
}

int utl_log_one() {return 1;} /* to avoid warnings */

#endif /* UTL_MAIN */
#endif /* UTL_NOLOG */
#endif /* UTL_H */

Exit
There should never be a call to exit() in a library. If for example this code was included in a daemon or some other part of an operating system you would be shutting down the system without meaning to. The best you could do here is to require that a setjmp() command is used in the linking program and do a longjump() from the assert function. This allows the calling program to clean up and perform the correct actions in this case. A library will never know what other actions are necessary.

Incorrect Usage of Header Files
Header files should generally not have executable functions in them. Part of the reason is that if multiple files include the header then at link time the linker will complain about multiple definitions of the functions.

You seem to have worked around the multiple definition problem with complex #ifdef statements. It would be much simpler to have both a utl.h and and a utl.c. The utl.c file contains the executable functions and the utl.h file contains the function prototypes. This is a very common practice and could be
considered a standard. This would also remove the requirement for most of the header files included by this header file.

If you are creating a library that may be used by multiple programs you should probably turn utl.c into either an archive library (libmylog.a) or a dynamically linked library (libmylog.so or libmylog.dll based on the operating system).


Using PHP goto Labels for code folding I've been using PHP labels in my Controllers for code folding. PHP labels are actually used with goto statements. But I'm using labels only because it's so much easier to fold my code in different IDEs using labels + curly braces, and I can use labels as titles for different sections of the code.

Here's an example. Take a look at SettingCartFields label.

<?php

namespace App\Http\Controllers;

use App\Cart;
use Illuminate\Http\Request;
use App\Http\Controllers\Controller;

class CartController extends Controller
{
    public function create(Request $request)
    {
        $cart = new Cart;

        SettingCartFields: {
            $cart->field1 = $request->field1;
            $cart->field2 = $request->field2;
            $cart->field3 = $request->field3;
            $cart->field4 = $request->field4;
            $cart->field5 = $request->field5;
            $cart->field6 = $request->field6;
        }

        $cart->save();
    }
}

I have a few questions, are there any performance issues with this? Am I introducing some kind of overhead by adding a lot of labels in my code? Is there any better way to do this?

The main issue I have with your approach is that it is non-standard and purely for the way that you work.  Others looking at your code may be a bit confused as to what you are doing and why.  In your example, there is (IMHO) little benefit in being able to fold the code as there is not really enough code to need to fold the code up, but in larger code segments there may be a use for it.

For me - it looks as though you are in the initial stages of identifying logically grouped functionality within your code, something that could then be extended by extracting these logical groups into new class methods.

The exact approach I would take depends on how much other processing you have with the cart either pre or post this main chunk of code.

If there is a lot of other processing around what the cart does, then you could just pass the cart and the request to a new method and get something like...

class CartController extends Controller
{
    public function create(Request $request)
    {
        $cart = new Cart;

        $this->setCartFields ( $cart, $request );

        $cart->save();
    }

    private function setCartFields ( Cart $cart, Request $request ) {
        $cart->field1 = $request->field1;
        $cart->field2 = $request->field2;
        $cart->field3 = $request->field3;
        $cart->field4 = $request->field4;
        $cart->field5 = $request->field5;
        $cart->field6 = $request->field6;
    }
}

If the data from the request forms the basis of the data for the cart, then this could instead create it's own cart, initialise the data and return the newly created cart for further processing...

class CartController extends Controller
{
    public function create(Request $request)
    {

        $cart = createCart ( $request );

        $cart->save();
    }

    private function createCart ( Request $request ) : Cart {
        $cart = new Cart;
        $cart->field1 = $request->field1;
        $cart->field2 = $request->field2;
        $cart->field3 = $request->field3;
        $cart->field4 = $request->field4;
        $cart->field5 = $request->field5;
        $cart->field6 = $request->field6;

        return $cart;
    }

}

The problem being that you could eventually end up with all of the processing in the newly created method and rather than relieve the problem you have just moved it.  This is only something you can decide on a per instance basis.


Testing mapping code Obviously, you have to test mapping code somehow, even (or especially) if you use AutoMapper. Is there any way to make it less verbose?

[Test]
public void Map_Always_SetsSimpleProperties()
{
  var auctionPlace = fixture.Create<string>();
  var submissionCloseDateTime = fixture.Create<DateTime>();
  var quotationForm = fixture.Create<string>();
  var quotationExaminationDateTime = fixture.Create<DateTime>();
  var envelopeOpeningTime = fixture.Create<DateTime>();
  var envelopeOpeningPlace = fixture.Create<string>();
  var auctionDateTime = fixture.Create<DateTime>();
  var applExamPeriodDateTime = fixture.Create<DateTime>();
  var considerationSecondPartDate = fixture.Create<DateTime>();

  var doc = fixture.Create<Notification223>();

  doc.AuctionPlace = auctionPlace;
  doc.SubmissionCloseDateTime = submissionCloseDateTime;
  doc.QuotationForm = quotationForm;
  doc.QuotationExaminationTime = quotationExaminationDateTime;
  doc.EnvelopeOpeningTime = envelopeOpeningTime;
  doc.EnvelopeOpeningPlace = envelopeOpeningPlace;
  doc.AuctionTime = auctionDateTime;
  doc.ApplExamPeriodTime = applExamPeriodDateTime;
  doc.ConsiderationSecondPartDate = considerationSecondPartDate;

  var sut = CreateSut();

  var actual = sut.Map(doc);

  Assert.That(actual.AuctionPlace, Is.EqualTo(auctionPlace));
  Assert.That(actual.SubmissionCloseDateTime, Is.EqualTo(submissionCloseDateTime));
  Assert.That(actual.QuotationForm, Is.EqualTo(quotationForm));
  Assert.That(actual.QuotationExaminationDateTime, Is.EqualTo(quotationExaminationDateTime));
  Assert.That(actual.EnvelopeOpeningTime, Is.EqualTo(envelopeOpeningTime));
  Assert.That(actual.EnvelopeOpeningPlace, Is.EqualTo(envelopeOpeningPlace));
  Assert.That(actual.AuctionDateTime, Is.EqualTo(auctionDateTime));
  Assert.That(actual.ApplExamPeriodDateTime, Is.EqualTo(applExamPeriodDateTime));
  Assert.That(actual.ConsiderationSecondPartDate, Is.EqualTo(considerationSecondPartDate));
}

Yes, you can make it less verbose.

I assume you are using AutoFixture in you call to Create()?

If so, this will create random values out of the box. No need to specify the content and assign each property. Create the source object for mapping, map it, compare the mapped values with the properties on the source object:

    [Test]
    public void Map_Always_SetsSimpleProperties()
    {
        var doc = fixture.Create<Notification223>(); // Autofixture will fill properties with random data.
        var sut = CreateSut();

        var actual = sut.Map(doc);

        Assert.That(actual.AuctionPlace, Is.EqualTo(doc.AuctionPlace));
        Assert.That(actual.SubmissionCloseDateTime, Is.EqualTo(doc.SubmissionCloseDateTime));
        Assert.That(actual.QuotationForm, Is.EqualTo(doc.QuotationForm));
        Assert.That(actual.QuotationExaminationDateTime, Is.EqualTo(doc.QuotationExaminationDateTime));
        Assert.That(actual.EnvelopeOpeningTime, Is.EqualTo(doc.EnvelopeOpeningTime));
        Assert.That(actual.EnvelopeOpeningPlace, Is.EqualTo(doc.EnvelopeOpeningPlace));
        Assert.That(actual.AuctionDateTime, Is.EqualTo(doc.AuctionDateTime));
        Assert.That(actual.ApplExamPeriodDateTime, Is.EqualTo(doc.ApplExamPeriodDateTime));
        Assert.That(actual.ConsiderationSecondPartDate, Is.EqualTo(doc.ConsiderationSecondPartDate));
    }


Finding possible misconceptions in ColumnChart I've written this to accomplish a specific problem, but it's too long. I've sure that it can be reduce too fit it.

I have the following model:

public class ColumnChart
{
    public virtual string type { get; set; }
    public virtual string name { get; set; }
    public virtual List<double> data { get; set; }
}

The idea of List<double> is to keep 12 values (1 per month if exists)
All these 12 values has to belong someone, so the name will keep the legend.
The type is column (not always, but in this case will make it simple).

Big Logic

// Create a List of ColumnChart's
var list = new List<ColumnChart>();

// Instantiate one object of type ColumnChart
var serie = new ColumnChart();

// Instantiate the List<double> (like my model says)
serie.data = new List<double>();

// Control the loop
int i = 0;

foreach (var element in MyDataSource)
{
   // If the serie.name don't have the current Group Name that came from my DataSource
   if (serie.name != element.GroupName)
   {
       // Isn't the first loop ?
       if (i != 0)
       {
           // Return a new empty instance of ChartColumn
           serie = NewInstance();

           // Set the legend that belong all data
           serie.name = element.GroupName;

           // Set the type of the chart
           serie.type = "column";

           // Instantiate a new List<double> (like my model says)
           serie.data = new List<double>();

           if (serie.data.Count == element.Month - 1)
           {
               serie.data.Insert(element.Month - 1, element.ValueRevenue);
           }

           // If the number of elements in my List<double> is lower then the Month.
           else
           {
               while (serie.data.Count < element.Month - 1)
               {
                   serie.data.Add(0);
               }

              serie.data.Insert(element.Month - 1, element.ValueRevenue);
            }
       }

       // Is the first loop interaction
       else
       {
            serie.name = element.GroupName;
            serie.type = "column";
            serie.data.Insert(element.Month - 1, elemento.ValueRevenue);
            i++;
        }
   }

   // The serie.name have the same current Group Name that came from my DataSource
   else
   {
       serie.data.Insert(element.Month - 1, element.ValueRevenue);
       i++;
   }

   // My i variable that control the loop is the last interaction ?
   if (i == MyDataSource.Count() - 1)
   {
       list.Add(serie);
   }
}

i = 0;

Data Source

MyDataSource
      [0]
           Month: 1
           GroupName: 'Music'
           ValueRevenue: 700.0
      [1]
           Month: 2
           GroupName: 'Music'
           ValueRevenue: 700.0
      [2]
           Month: 3
           GroupName: 'Music'
           ValueRevenue: 700.0
      [3]
           Month: 4
           GroupName: 'Music'
           ValueRevenue: 700.0
      [4]
           Month: 5
           GroupName: 'Music'
           ValueRevenue: 700.0
      [5]
           Month: 6
           GroupName: 'Music'
           ValueRevenue: 700.0
      [6]
           Month: 7
           GroupName: 'Music'
           ValueRevenue: 700.0
      [7]
           Month: 8
           GroupName: 'Music'
           ValueRevenue: 700.0
      [8]
           Month: 9
           GroupName: 'Car'
           ValueRevenue: 700.0

It looks as if this can be solved using Linq.

If your datasource consists an IEnumerable of records each containing a MonthNumber, GroupName and ValueRevenue

Say,

public class DataRecord {

    public int MonthNbr { get; set; }
    public string GroupName { get; set; }
    public double ValueRevenue { get; set; }

}

then a GroupBy() should populate your ChartColumn

e.g.

private static ColumnChart GroupAndSort(IEnumerable<DataRecord> input) {

    return input.GroupBy(n => n.GroupName).Select(g => new ColumnChart {
            Type = "column",
            Name = g.Key,
            Data = g.OrderBy(d => d.MonthNbr).Select(d => d.ValueRevenue).ToList()
        }).First();

}

Note:

This was set up to deal with one set of records and only one ChartColumn being generated. If you have many groups in the input then remove the 'First()' and return an 'IEnumerable'


Composite GetHashCode function I've built a small library for comparing objects based on their members here (related SO question).

There's a need to compute hash function in order to conform to IEqualityComparer<T> interface. In my case this function has to be computed based on the hash functions of members. So the main problem is how to compose them.

I use the following approach currently:

public int GetHashCode(T obj)
{
    VerifyHaveMembersSetup();

    if (TypeTester<T>.IsNull(obj))
    {
        return 0;
    }

    if (getHashCodeBehavior == GetHashCodeBehavior.Cache && cachedHashCode.HasValue)
    {
        return cachedHashCode.Value;
    }

    int hashCode = 0;

    for (int i = 0; i < memberSetups.Count; ++i)
    {
        int leftShift = i % 32;
        int rightShift = 32 - leftShift;

        int memberHashCode = memberSetups[i].GetMemberHashCode(obj);

        hashCode = hashCode ^ ((memberHashCode << leftShift) | (memberHashCode >> rightShift));
    }

    if (getHashCodeBehavior == GetHashCodeBehavior.ImmutableCheck
        && cachedHashCode.HasValue && cachedHashCode.Value != hashCode)
    {
        throw new InvalidOperationException("Hash code value changed");
    }

    cachedHashCode = hashCode;

    return hashCode;
}

Are there any problems with this code?
I don't like the way many people implement composite functions. With multiplication and addition, significant bits are shifted away and are lost.

That's why I use bitshift operators << and >>. Still I'm not completely sure how good would be a hash function generated such way. Especially when it's computed over small collection of memberSetups of bools for example (since bool has only 2 hash values in .NET: 1 and 0).

Shifting the values before xor:ing them is somewhat better than the worst thinkable ways of combining the hash codes, but it's not very good. You will easily get combinations that cancel each other out, so you get hash codes with a distribution heavily skewed towards zero.

Multiplying values by a prime number on the other hand gives a pretty good distribution. Back in the day it was actually used to produce random numbers, partly because of how it spreads the values reasonably even over the number range.

A method that would give an even better distribution would be to use an advanced hashing algorithm like MD5 or CRC32, but the drawback would be that they are a lot slower. You would lose much of the advantage of using a hash code in the first place.

All in all, multiplying by a prime number gives a very good distribution in relation to the processing time needed. It's a compromise well suited for the GetHashCode method.


Draw random 1's and 0's I was wondering if anyone would be so kind as to look at this code I've written for practice.  Tell me what am I doing wrong, where can I optimize, anything.

What this code does is (in terminal) draw a bunch of random 1's and 0's, but I added an effect kinda like a lattice.  The bulk of this I got help on here.

#include <stdio.h>
#include <stdlib.h>
#include <sys/ioctl.h>

int main (int argc, char *argv[]){      //check for whitch effect to print
    switch(*argv[1]){
        case '1': lattus();break;
        case '2': normal();break;
        default: printf("1 = lattus effect | 2 = static effect\n");break;
    }
}

char *randstring (char *buffer, int length){        //genertate a random number
    int i = length;
    while(--i >= 0) {
        buffer[i] = (rand() % 2) ? '1' : '0';
    }
    buffer[length] = 0;
    return buffer;
}

int normal(){           // normal drawing of 1's and 0's
    struct winsize w;
    ioctl(0, TIOCGWINSZ, &w);
    int width = w.ws_col;
    int height = w.ws_row;      //get terminal width and height
    char buffer[width*height + 1];  //create a buffer big enough to hold one draw to the screen
    int i = 25;
    while(i-- >= 0) {
        printf("%s\n", randstring(buffer, width*height));   //draw to screen
        usleep(90000);
    }
    system("clear");        //clear screen
}

int lattus (void){
    struct winsize w;
    ioctl(0, TIOCGWINSZ, &w);
    int width = w.ws_col;       //get the terminal width
    char buffer1[width + 1];    //create 3 buffers for each segment
    char buffer2[width + 1];    //each big enough to hold the width of the terminal
    char buffer3[width + 1];
    int first = 1;          //how many before the space
    int second = width - 8;     //how many in the middle of the space
    int third = 1;          //how many at the end of the space
    int i = 1000;           //draw 1000 lines
    int on = 0;         //switch for growing and shrinking
    while(i-- >= 0){
        usleep(9000);
        if(first == 1 && third == 1 && second == width - 8 | second == width - 9){  //is it at min?
            if(second % 2 == 0){        //is it an even number (had problems with buffer if it was odd)
                second = second - 2;
            }else{
                second = second - 3;
            }
            first ++;
            third ++;
            on = 0;     //keep growing
        }else if(first == (width - 8) / 2 && third == (width - 8) / 2 && second == 2){  //untill it gets to max
            if(second % 2 == 0){
                second = second + 2;
            }else{
                second = second + 1;
            }
            third --;
            first --;
            on = 1;     //start shrinking
        }else if(on == 0){  //else if suppost to grow, grow
            second = second - 2;
            third ++;
            first ++;
        }else if(on == 1){  //else if suppost to shrink shrink
            second = second + 2;
            third --;
            first --;
        }else{
            break;
        }
        printf("%s   %s   %s\n", randstring(buffer1, first), randstring(buffer2, second), randstring(buffer3, third));  //print it out
    }
    system("clear"); //clear screen
}

A few pointers:

First, variable length arrays are only allowed in C99:

char buffer1[width + 1];

If the code is to be strictly ANSI C, one would make an pointer to an array and use a function like malloc or calloc to allocate some memory for the array:

char* buffer = malloc(sizeof(char) * (width + 1));

(Then, if necessary, the buffer would be initialized, or calloc used instead to initialize and allocate at once.)

Second, code readability:

if(second % 2 == 0){
    second = second - 2;
}else{
    second = second - 3;
}

Could be better written as:

if (second % 2 == 0) {
    second = second - 2;
} else {
    second = second - 3;
}

It may seem like nitpicking, but it does make it easier for people to read your code and understand what is going on. Coding is a form of communication as well -- make it easier for others to read, and it others will be able to better understand what you're trying to achieve.

The switch statement could be similarly improved:

switch(*argv[1]){
    case '1': lattice();break;
    case '2': normal();break;
    default: printf("1 = lattice effect | 2 = static effect\n");break;
}

To:

switch(*argv[1]) {
    case '1':
        lattice();
        break;
    case '2':
        normal();
        break;
    default:
        printf("1 = lattice effect | 2 = static effect\n");
}

Also, adding line breaks between certain blocks of statement which perform one task may improve readability.

There seems to be copious use of while loops, but I don't see a single for loop -- for loops are probably more useful for performing "counter"-type activities, such as performing repetitions for a certain number of iterations:

int i;
for (i = 0; i < 100; i++) {
    /* Action to perform 100 times */
}

The basic structure of a for loop is as follows:

for (initial-condition; loop-condition; expression-on-each-iteration) {
    /* Action to perform on each iteration. */
}

(Note: The name for the conditions are not official names, I just made them up to describe what they're for.)

The initial-condition is what to do when the for loop first begins. Many times, this is used to initialize a counter variable to some value. For example, i is set to 0.

The loop-condition is an expression that is evaluated at the beginning of each iteration which determines whether the loop should continue. If the expression is true, then the loop is performed, but if false, then the loop terminates.

The expression-on-each-iteration is something that is performed at the beginning of each iteration of the loop, if that iteration is to take place. Commonly, this is part is used to increment the counter used to control the loop.


Simpler and faster code for reversing list? I wrote this code for reversing a doubly linked list containing words in each node, which works perfectly fine. My teacher says the algorithm is difficult to understand and the code as a whole could be made more efficient(reducing overhead and memory consumption).

What changes can i make to the code/the reversing algorithm?
Also is there a way I could input the sentence without having to ask the number of words in advance?

Here is the code:

#include<stdio.h>
#include<conio.h>
#include<string.h>
typedef struct NODE
{
    char *item;
    struct NODE *next;
    struct NODE *prev;
}NODE;
void Insert(char data[],NODE **List)
{
    NODE *temp,*last;
    last=(*List);
    temp=(NODE*)malloc(sizeof(NODE));
    temp->item=(char*)malloc(strlen(data));
    temp->item=data;
    temp->next=NULL;
    temp->prev=NULL;
    if((*List)->item==NULL)
        (*List)=temp;
    else
    {
        while(last->next!=NULL)
            last=last->next;
        temp->prev=last;
        last->next=temp;
        last=temp;
    }
}
void Reverse(NODE **List)
{
    int flag1=0;
    NODE *temp,*temp1,*last,*flag;
    temp1=(NODE*)malloc(sizeof(NODE));
    last=(*List);
    while(last->next!=NULL)
        last=last->next;
    temp=last;
    while(temp->prev!=NULL)
    {
        temp1->item=temp->item;
        temp1->next=temp->next;
        temp1->prev=temp->prev;
        temp->next=temp->prev;  //swap prev and next links
        temp->prev=temp1->next;
        temp=temp->next;
        if(flag1==0)  //record the last node which becomes the first node in new list
        {
            flag1++;
            flag=temp;
        }
    }
    temp1->item=temp->item;
    temp1->next=temp->next;
    temp1->prev=temp->prev;
    temp->next=NULL;
    temp->prev=temp1->next;
    (*List)=flag->prev;  //give control of the first node
    free(temp1);
};
void display(NODE *List)
{
    if(List->next==NULL)
    {
        printf("%s",List->item);
        return;
    }
    NODE *temp;
    temp=List;
    do
    {
        printf("%s<-->",temp->item);
        temp=temp->next;
    }while(temp->next!=NULL);
    printf("%s\n",temp->item);
}
int main()
{
    int i=0,n;
    char s[10][50];
    NODE *List;
    List=(NODE*)malloc(sizeof(NODE));
    List->item=NULL;
    List->next=NULL;
    List->prev=NULL;
    printf("Provide number of words(max 10): ");
    scanf("%d",&n);
    printf("Enter string of words for the list: ");
    while(i<n)
    {
        scanf("%s",s[i]);
        Insert(s[i],&List);
        i++;
    }
    printf("\nOriginal List is: ");
    display(List);
    Reverse(&List);
    printf("\nReversed List is: ");
    display(List);
    getch();
    return 0;
}

Your code has some issues from my perspective.  The first that jumps out is that you are passing a pointer to a pointer to a list.   This is one level of indirection too many. Just pass a pointer to the list.  To make this easier, your list pointer in main() should just be a pointer to the first entry in the list; it should not be the first entry. In other words don't malloc space for a NODE in main().

So when you construct the list it will look like this:

 first _____
            |
            v
            -------       -------       -------       -------
  NULL <-- |p      | <--- |p     | <--- |p     | <--- |p     |
           |      n| ---> |     n| ---> |     n| ---> |     n| ---> NULL
            -------       -------       -------       -------

To reverse this list, you could take the approach given by @iaimtomisbehave, from earlier.  But this has two notable points: It needs a last pointer to the end of the list (this might be a plus) as well as first (or start/end); if you just swap last and first then you haven't really reversed the list because to traverse the list you now have to follow the prev pointers instead of following the next pointers. But there is a simple solution: just swap the prev and next pointers of each node as well as swapping first and last.

Some other points:

Naming: upper case names are usually reserved for constants. So call NODE something else: eg. Node or node.
malloc should not be cast
you malloced the string and then didn't use it.(Insert() line 5/6))
you have code to find the end of the list twice. It is a simple loop that belongs in a separate function.
Add some space to make the code look less cramped.  Note that while, for, if etc are special and deserve to be followed by an extra space: eg while (condition) (this is just my preference; others may disagree).
One variable per line.

That's enough I think :-) Here is my version of your program, for what it is worth.

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

typedef struct Node
{
    char *item;
    struct Node *next;
    struct Node *prev;
} Node;

static Node *last_node(Node *list)
{
    while (list->next != NULL) {
        list = list->next;
    }
    return list;
}

static Node *insert(char *data, Node *list)
{
    Node *node = malloc(sizeof(Node));
    /* should handle malloc/strdup failure... */
    node->item = strdup(data);
    node->next = NULL;

    if (list == NULL) {
        list = node;
        node->prev = NULL;
    }
    else {
        Node *last = last_node(list);
        node->prev=last;
        last->next=node;
        last = node;
    }
    return list;
}

static Node *reverse(Node *list)
{
    Node *last = NULL;

    for (;list != NULL; list = list->prev) {
        Node *prev = list->prev;
        list->prev = list->next;
        list->next = prev;
        last = list;
    }
    return last;
}

static void display(const char *msg, Node *list)
{
    printf("%s\n", msg);

    for (;list != NULL; list = list->next) {
        if (list->prev != NULL) {
            printf("<-->");
        }
        printf("%s", list->item);
    }
    printf("\n");
}

int main(int argc, char ** argv)
{
    char s[50];
    Node *list = NULL;
    (void) argc;
    (void) argv;

    printf("Enter string of words for the list.  "
           "End the list with a full-stop on a blank line: ");
    while ((scanf("%49s", s) == 1) && strcmp(s, ".")) {
        list = insert(s, list);
    }

    display("Original List is: ", list);
    list = reverse(list);
    display("Reversed List is: ", list);
    return 0;
}


Movie info website (React, axios and Semantic UI) import React, { Component } from 'react';
import { Grid, Header, Divider, Image, Embed, Button, Icon } from 'semantic-ui-react'
import axios from 'axios';

class Movie extends Component {
  constructor(props) {
    super(props);
    this.state = {
      title: '',
      year: '',
      poster: '',
      plot: '',
      rating: '',
      genres: '',
      runtime: ''
    };
  }

  componentWillMount() {
    axios.get('http://www.omdbapi.com/?i=tt0096895')
      .then((response) => {
      console.log(response.data);
        this.setState(
          {
            title: response.data.Title,
            year: response.data.Year,
            poster: response.data.Poster,
            plot: response.data.Plot,
            rating: response.data.imdbRating,
            genres: response.data.Genre,
            runtime: response.data.Runtime
          })
      })
      .catch((err) => {
        console.log(err);
      });
  }

  render() {
    return (

        <Grid container>
          <Grid.Column width={5}>
            <Image
              src={this.state.poster}
              fluid
            />
            <Button attached='top'>
              <Icon name='plus'/>
              Add to watchlist
            </Button>

            <Divider section/>

RATING

            <img className='imdb-icon' src="https://lh3.googleusercontent.com/lOC6oCAWrB7TNqvPdgJkQzUyC2pLc63n0iLCBtIE9CF-FFUjbbiAyW9AUPiCfkz3nOUxNE0SVZtxGsKRwDPsHqpOuHYadm2lEhx6wfobYyoPp8CfV3tO65ZTjJ8-91vT9Q7lwrOvDQF_sIOUGJ32TeIYdgWXHLiM3RNYS5LUq1fL71l5nL_37He7XokgUO6lbuwPOm7nNEAyBuyQgYzPgB-hxkzSGeUg6o4Tp7dAz8O3X6Cuhhxew-N23-zgqr4ag3dZ9L4De21WLO-WppAmgh752F4uY4Zt-IrbtxldHnOkve8OX3ktlRCHkrtcdHVL9jCWruyBpn3KZhL2fuMZ9oeWSVU3v4Gpkxb5xlh3MTveT8N45ih3WAYoP6Hm-Jz8qAhMQpfnmSRGzuQaX22FITypo_XeKcvkoSLUQMC1V68nuWcOT2MphDFyMgTjunmLBca74UpL5s3k2_FhJfS0RX0k0Y6IC6bUOL5gWcEu0d4rJv8iJHnXKft4zohH4PryAuE-G2q7rs4Vnb_65grz89QQGmF6OLGUhZowBfnGD8_MRE6Ev9Q92x8hMSFWZJZsz2wNk0eUxnCVSJFNVO1jphG2bSHy6ojYNVe9b-JVVQ3NlJr9KjB6=w64-h32-no"/>
            {this.state.rating}

GENRES

{this.state.genres}

RUNTIME

{this.state.runtime}

            <Divider section/>
          </Grid.Column>
          <Grid.Column width={11}>

              {this.state.title}

                ({this.state.year})

            <Embed id='O6Xo21L0ybE' source='youtube'/>

PLOT

{this.state.plot}

          </Grid.Column>
        </Grid>

    );
  }
}

export default Movie;

I am very new to React. This is for a school project and I would love to if someone reviewed my code. What improvements can I do?

Some things I would change:

Make initial state look like this:

this.state = {
    movie: null
};

Instead of making the api request in componentWillMount, do it in componentDidMount because componentWillMount is considered legacy.
In componentDidMount be more declarative, which means to be more descriptive of what you are doing, so call a method that gets the movie like this:

componentDidMount() {
    this.getMovie();
}

getMovie() would look like this:

getMovie() {
    axios
      .get("http://www.omdbapi.com/?i=tt0096895")
      .then(response => {
        const movie = {
          title: response.data.Title,
          year: response.data.Year,
          poster: response.data.Poster,
          plot: response.data.Plot,
          rating: response.data.imdbRating,
          genres: response.data.Genre,
          runtime: response.data.Runtime
        };
        this.setMovie(movie);
      })
      .catch(err => {
        console.log(err);
      });
}

setMovie(movie):

setMovie(movie) {
    this.setState({ movie });
}

Lastly, in your render, I would check if there was a movie on state before rendering anything, if there is no movie, display some text saying no movie.

render() {
    if (!this.state.movie) {
      return
No movie found.
;
    }

    return (
        ...what you currently have
    );
}


Delete all elements from list whose sum with preceding element equals target value Let me start off by saying that the question specifically implies using std::list<int> and not anything else AND you are allowed to iterate over the list only once. Taking this into account, we assume the target sum value to be 10.

The task is:

  You are given a list of integers (std::list<'int'>). Iterating over the list only once, delete all elements that give a sum of 10 with the preceding element.

I was wondering how I can make my code more efficient and clear, because my application was rejected immediately.

#include <list>
#include <iostream>
#include <conio.h>

bool target_sum(const int i, const int j)
{
    return (i+j) == 10;
}

void print(const std::list<int> &nmbrs)
{
    for (const int p : nmbrs)
        std::cout << p << " ";
    std::cout << std::endl;
}

int main()
{
    // initialises a list that holds the numbers
    std::list<int> nmbrs = {5, 5, 3, 7, 7, 7, 3, 3, 1, 2, 3, 7, 4, 6};

if (nmbrs.size() > 1)                     //if there are at least 2 numbers
{
    std::list<int>::iterator i = nmbrs.begin();
    while (i != std::prev(nmbrs.end()))   //we compare number *i to the following number *(i+1),
    {                                     //while incrementing the iterator at the same time
        if (target_sum(*i, *(i++)))
        {
            i = nmbrs.erase(i);
            i--;                          //move back in case the numbers repeat
        }

        //print(nmbrs);
    }
} else
{
    //print(nmbrs);
}

print(nmbrs);

_getch();

return 0;
}

Breaking down the requirements:

  You are given a list of integers (std::list<'int'>).

The algorithm is required to at least work with std::list<int>, so we cannot rely on random-access/contiguity.

  Iterating over the list only once,

Requires a single-pass algorithm.  Single pass algorithms have special rules associated with them, like only accessing the current element once and only moving forward.  Your implementation exhibits multi-pass behavior.

std::prev(nmbrs.end())
--i;

  delete all elements that give a sum of 10 with the preceding element.

Applying this to a single pass approach, you will need to store the preceding element in a temporary.

A source file should be self-sufficient in providing the definition of every symbol used.  Some implementations may include the library for you (<list> including <iterator>, but that behavior is neither standard nor portable.  std::prev requires <iterator>.

bool target_sum(const int i, const int j)
{
    return (i+j) == 10;
}

Don't use magic numbers.  The target sum for this test is 10, but will it always be 10?

void print(const std::list<int> &nmbrs)
{
    for (const int p : nmbrs)
        std::cout << p << " ";
    std::cout << std::endl;
}

Every line that gets printed prints an extra delimiter at the end of the list.  An extra whitespace character might not do anything, but it certainly isn't intended.  Strive for correctness in your programs.

Avoid std::endl. Directly state your intent in code and be aware of what std::endl does. The following two statements are equivalent (ignoring localization).

std::cout << std::endl;
std::cout << '\n' << std::flush;

Again, be explicit with your intent.

std::cout << std::endl;           // Flush intended?
std::cout << '\n';                // Definitely don't flush.
std::cout << '\n' << std::flush;  // Definitely flush.

// initialises a list that holds the numbers
std::list<int> nmbrs = {5, 5, 3, 7, 7, 7, 3, 3, 1, 2, 3, 7, 4, 6};

if (nmbrs.size() > 1)                     //if there are at least 2 numbers

Don't say in comments what can be or is already stated in code.

Vowels do not cost $500.  Having a smaller variable name because your abbreviation strips vowels or truncates the end doesn't translate to better code or performance.  Abbreviations negatively impacts readability and maintainability, which are two very important aspects in programming.

Factor out common code into functions.  Functions promote reuse and limits the scope for errors (making testing easier).

std::list<int>::iterator i = nmbrs.begin();

Use auto to avoid repetition of type names.

target_sum(*i, *(i++))

C++ has special rules regarding evaluation order.  In this case, the , of the argument list is not a sequence point because the argument list itself is not an expression.  Since the arguments are indeterminately sequenced, modifying the variable and trying to access the variable is undefined behavior.

// print(nmbrs);

Remove dead code rather than commenting.

_getch();

You don't need _getch(); or conio.h.  If you need to pause on input, the standard library already provides such a facility (std::cin.get()).

Instead of cluttering your program with unnecessary debugging support code, learn about your terminal emulator and how to use it correctly.  Some IDE's provide an option to keep the window open after running.  Other IDE's offer a full fledged debugging system (MSVC) where you can use breakpoints to externally "pause" the program.


Return a List based on very similar data I am rewriting an old VB.NET app in C#. I will not subject you to the old VB.NET code that I am writing (the original method that I have already refactored was over 700 lines long).

The following is what we call On Demand Reports. There is a stored procedure that these objects fill in for us. I don't have a 100% understanding of why most of what I am doing is just static string text but it is.

What I want to do is to find a better way to represent the List<MyObject> (see the embedded comments).

 private IEnumerable<MyObject> GetOnDemandInputsBy(int key)
        {
            var inputs = new List<MyObject>();
            //This is actually called 13 times with different parameter variables (the parameter called parameter) and a different static dictionary
            inputs.Add(AssembleMyObject(key, "@BeginTime", MyObjectPropertyConstants.BeginTime()));
            inputs.Add(AssembleMyObject(key, "@EndTime", MyObjectPropertyConstants.EndTime()));

            return inputs;
        }

        //This method is fine with me. It is just here for context
        private static IEnumerable<MyObjectProperty> AssembleMyObjectProperty(Dictionary<string, string> dictionary)
        {

            return dictionary.Select(item => new MyObjectProperty {PropertyName = item.Key, PropertyValue = item.Value});
        }

        //I feel like I am passing to many things into this method and its doing to much
        private MyObject AssembleMyObject(int key, string parameter, Dictionary<string, string> dictionary)
        {
            //this is set to 0 in the constructor, I am not sure why the original developers chose 10 for every new object
            _displayOrder += 10;
            var MyObject = new MyObject
            {
                DisplayOrder = _displayOrder,
                ParameterName = parameter,
                QueryKey = key,
            };
            MyObject.MyObjectProperties.AddRange(AssembleMyObjectProperty(dictionary));
            return MyObject;
        }

    //This seems like a bad idea as well but all the data is static and I created a separate dictionary for each of the parameter types
    public class MyObjectPropertyConstants
    {
        public static Dictionary<string, string> BeginTime()
        {
            return new Dictionary<string, string>
            {
                {"stuff", "@BeginTime"},
                {"VISIBLE", "TRUE"},
                {"CAPTION", "*beginTime"},
                {"DISPLAYTYPE", "DATEPICKER"},
                {"DATATYPE", "DATE"},
            };
        }

        public static Dictionary<string, string> EndTime()
        {
            return new Dictionary<string, string>
            {
                {"Name", "@EndTime"},
                {"VISIBLE", "TRUE"},
                {"CAPTION", "*EndTime"},
                {"DISPLAYTYPE", "DATEPICKER"},
                {"DATATYPE", "DATE"},
            };
        }
   }

As previously states this is the refactored version so far. The original code to do the above chunk is 180 lines. The method that I am rewriting is over 700 lines long.

Any Suggestions?

var inputs = new List<MyObject>();
inputs.Add(AssembleMyObject(key, "@BeginTime", MyObjectPropertyConstants.BeginTime()));
inputs.Add(AssembleMyObject(key, "@EndTime", MyObjectPropertyConstants.EndTime()));

return inputs;

You could rewrite this to make it more DRY using LINQ into:

return new Func<Dictionary<string, string>>[]
{
    MyObjectPropertyConstants.BeginTime,
    MyObjectPropertyConstants.EndTime
}.Select(f => AssembleMyObject(key, "@" + f.Name, f()).ToList();

var MyObject

Local variables are usually named in camelCase, e.g. myObject.

public static Dictionary<string, string> BeginTime()

Method should be named using verbs, this looks more like a property. Thought making this into a property would make the above LINQ rewrite more difficult.

new Dictionary<string, string>
{
    {"stuff", "@BeginTime"},
    {"VISIBLE", "TRUE"},
    {"CAPTION", "*beginTime"},
    {"DISPLAYTYPE", "DATEPICKER"},
    {"DATATYPE", "DATE"},
};

Does order matter here? If it does, you shouldn't use Dictionary, since it doesn't guarantee ordering.


Simple binary search Here the task (in Indonesian).

I think my binary search algorithm is correct.

Ideone

 #include<bits/stdc++.h>
using namespace std;
vector<pair<string, string>> v;
int awal1 = 1;
string check(string s, int awal, int akhir){
    int tengah = (awal+akhir)/2;
    bool temukan = false;
    if(v[tengah].first == s){
        return v[tengah].second;
        temukan = true;
    }
    else if(v[tengah].first > s){
        return check(s, awal, tengah-1);
    }
    else if(v[tengah].first < s){
        return check(s, tengah+1, akhir);
    }
    if(temukan == false) return "NIHIL";
}

int main(){
    ios::sync_with_stdio(0);
    cin.tie(0);
    //freopen("input.txt", "r", stdin);
    //freopen("output.txt", "w", stdout);
     string s, s1, s2; vector<string> h;
    int n, p;
    cin >> n >> p;
    for(int i=1;i<=n;i++){
        cin >> s >> s1;
        v.push_back(make_pair(s, s1));
    }
    for(int j=1;j<=p;j++){
        cin >> s2;
        h.push_back(s2);
    }
    for(int i=1;i<=h.size();i++){
        cout << check(h[i], 1, v.size()) << "\n";
    }
    /*for(int i=0;i<v.size();i++){
        cout << v[i].first << " " << v[i].second << "\n";
    } */
}

This is a non standard header.

#include<bits/stdc++.h>

The "bits" directory is an implementation detail of your compiler. You should NEVER use it in your code.

Your headers should be:

#include <vector>
#include <iostream>

You should not do this:

using namespace std;

Read the details about the issues here: Why is “using namespace std” considered bad practice?

You should avoid using global variables.

vector<pair<string, string>> v;

Here you are locking your check() function to searching only one list. It should search any list for the key.

This global variable does not seem to be used anywhere:

int awal1 = 1;

Bounds checking:

string check(string s, int awal, int akhir){

If awal and akhir are equal there is nothing to check. You have gone beyond the range. So You need to do a test here. If these two are equal then you failed to find the data.

Note: That akhir is one beyond the end of the range you are checking. For the very right hand end of the vector this means beyond the end of the v which means that accessing it is undefined behavior and likely to cause a crash.

This value is always false.

    bool temukan = false;

Because this statement is never executed.

    if(v[tengah].first == s){
        return v[tengah].second;

        // The line above is a return.
        // So the next line is never reached.
        // So we never execute this.
        temukan = true;
    }

    // So in this line  `temukan` is always false.
    // So this `if` test is always true
    // So this would always return 'NIHIL'
    if(temukan == false) return "NIHIL";

BUT. In the same function you also have another set of if else statements. That always result in a return. So the code never falls through to the last 'NIHL' statement.

if(v[tengah].first == s){              // Equal
    return v[tengah].second;
}
else if(v[tengah].first > s){          // Greater
    return check(s, awal, tengah-1);
}
else if(v[tengah].first < s){          // Less
    return check(s, tengah+1, akhir);
}

One of those branches must be true (unless you were comparing floats). So the code will never go past these set of tests.

So there are some massive bugs in the function check(). You definitely need to fix these. I am not going to fix them because I think it would do you good to try and work out what the correct solution is. But as it stands this would fail any unit tests you should have written.

This is C code.

    //freopen("input.txt", "r", stdin);
    //freopen("output.txt", "w", stdout);

What you probably want is the C++ std::fstream class.

One variable per line please:

     string s, s1, s2; vector<string> h;
    int n, p;

As containers like arrays/vectors are 0 based. We usually loop from 0 to 1 less than the number.

    for(int i=1;i<=n;i++){

    // Most C/C++/Java/C# etc programmer would write that loop as:
    for(int i = 0; i < n; ++i) {

Yes this works.

        v.push_back(make_pair(s, s1));

But you can use the emplace_back() method which makes it easier to read and removes an unneeded copy.

        v.emplace_back(s, s1);

As I mentioned above containers are indexed from 0.

        cout << check(h[i], 1, v.size()) << "\n";

So passing a 1 here is wrong. As you will never find the first element. You should have noticed there was not check for "agung". You can't find this person if you search from 1 as they are in position 0. Unfortunately your check loop also started at 1 so your code missed it.


Determining the cardinality of sets defined by recursively following indexes Problem :

  A zero-indexed array A consisting of N different integers is given.
  The array contains all integers in the range [0..N−1]. Sets S[K] for 0
  ≤ K < N are defined as follows:

  S[K] = { A[K], A[A[K]], A[A[A[K]]], ... }.

  Sets S[K] are finite for each K.

  Write a function that, given an array A consisting of N integers,
  returns the size of the largest set S[K] for any array input.

This is my solution :

    public int solution(int[] A) {

         int arrayLength = A.length;
         int maxsize = 0;
         int count = 0;
         Set<Integer> resultSet = null;

         if(arrayLength <= 0)
         return 0;

         for(int i = 0; i < arrayLength; i++) {
           Set<Integer> tempSet = new LinkedHashSet<Integer>();
           int tempNum = A[i];
           count = 0;
           while(!tempSet.contains(tempNum)){
               tempSet.add(tempNum);
               tempNum = A[tempNum];
               count++;
           }
           if(count > maxsize) {
              maxsize = count;
              resultSet = tempSet;
           }
        }
         return maxsize;
     }

For example, given array A such that:

  A[0] = 5    A[1] = 4    A[2] = 0
  A[3] = 3    A[4] = 1    A[5] = 6
  A[6] = 2

the function should return 4, because set S[2] equals { 0, 5, 6, 2 } and has four elements. No other set S[K] has more than four elements.

My solution normally works but gives TIMEOUT ERROR for large sizes. How can I optimize the complexity of this code from O(N*R) to O(N)?

You are visiting every node to check their chain, but if they've already been visited by a previous chain they don't need to be checked again.

So you can have a single "visited" HashSet outside the main loop which can be used to check whether they have been visited in the outside loop and used to break the inside loop.

Then in each of the outside loop iterations you can continue if you have previously visited.

Note also that you don't need to return and store the actual cycle, just the length.

public int solution(Integer[] A) {

     int arrayLength = A.Length;
     int maxsize = 0;
     int count = 0;
     Set<Integer> visited = new HashSet<Integer>();

     if(arrayLength <= 0)
     return 0;

     for(int i = 0; i < arrayLength; i++) {
       if(visited.contains(i))
            continue;

       int tempNum = A[i];
       count = 0;
       while(!visited.contains(tempNum)){
           visited.add(tempNum);
           tempNum = A[tempNum];
           count++;
       }

       if(count > maxsize) {
          maxsize = count;
       }
    }
     return maxsize;
 }


Find the rarest in a map Write a method rarest that accepts a map whose keys are strings and
whose values are integers as a parameter and returns the integer value
that occurs the fewest times in the map. If there is a tie, return the
smaller integer value. If the map is empty, throw an exception.
For example, suppose the map contains mappings from students' names
(strings) to their ages (integers). Your method would return the least
frequently occurring age. Consider a map variable m containing the
following key/value pairs:
{Alyssa=22, Char=25, Dan=25, Jeff=20, Kasey=20, Kim=20, Mogran=25,
Ryan=25, Stef=22}
Three people are age 20 (Jeff, Kasey, and Kim), two
people are age 22 (Alyssa and Stef), and four people are age 25 (Char,
Dan, Mogran, and Ryan).
So a call of rarest(m) returns 22 because only
two people are that age. If there is a tie (two or more rarest ages
that occur the same number of times), return the youngest age among
them. For example, if we added another pair of Kelly=22 to the map
above, there would now be a tie of three people of age 20 (Jeff,
Kasey, Kim) and three people of age 22 (Alyssa, Kelly, Stef). So a
call of rarest(m) would now return 20 because 20 is the smaller of the
rarest values.

Here is the link to the question.
public int rarest(Map<String, Integer> m)throws Exception{
    if(m.size() == 0){
      throw new Exception();//throwing an exception as per the question
    }else{
    Integer count = null;
    //Creating a TreeMap to get the lowest age.
    TreeMap<Integer, Integer> t = new TreeMap<Integer, Integer>();
    for(Map.Entry<String, Integer> me : m.entrySet()){
        if(t.containsKey(me.getValue())) {
            count = t.get(me.getValue());
            t.put(me.getValue(), count+1);
        }else {
            count = 1;
            t.put(me.getValue(), count);
        }
      }

     //If there is tie I am comparing the frequencies
     int freq = t.get(t.firstKey());

     TreeSet<Integer> val = new TreeSet<Integer>();

     for(Integer i : t.keySet()){
        if(freq > t.get(i) ){
            freq = t.get(i);
            val.add(i);
        }
     }
        if(val.size() > 0){
           return val.first();
        }else{
           return t.firstKey();
        }
    }
}

It gives the correct results for the given test cases, but I feel there should be a better way to do this.

This is more efficient than the accepted answer.  min is O(1) memory and O(n) time while sorted is O(n) memory and O(n*log(n)) time:

import static java.util.Comparator.comparingLong;
import static java.util.Comparator.comparingInt;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.groupingBy;

Comparator<Map.Entry<Integer, Long>> byRarity = comparingLong(Map.Entry::getValue);
Comparator<Map.Entry<Integer, Long>> byAge = comparingInt(Map.Entry::getKey);

int rarest = data.values().stream()
        .collect(groupingBy(x->x, counting()))
        .entrySet().stream()
        .min(byRarity.thenComparing(byAge))
        .get()        // or throw if map is empty
        .getKey();


Program to tell which if any of three numbers is in the middle I wrote this program, and it does what it asks, but it's so big and ugly. What can I change?

Some examples:

a  b  c --->
5  1  12   a is between b and c
5  12 1    a is between c and b
5  5  2    there is no gap

This was in an example of a test that is supposed to be small, so that's why I'm asking if there is another way.

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

int main (void){
int a,b,c;

printf("Inserir 3 números: "); //its in portuguese, insert 3 numbers
scanf("%i %i %i",&a,&b,&c);

if(a==b||b==c||a==b){
    printf("não há intervalo\n"); //there is no gap
    exit(0);
}

int maior, meio, menor,flag,flag2,flag3;

maior=a; //maior means biggest
flag=1;
if(maior<b){
    maior=b;
    flag=2;
}if(maior<c){
    flag=3;
    maior=c;
}

if(maior!=a){
    meio=a;  //meio means middle
    flag2=1;
    if(meio<b&&b<maior){
        meio=b;
        flag2=2;
    }if(meio<c&&c<maior){
        flag2=3;
    }
}else{
    meio=b;
    flag2=2;
    if(meio<c&&c<maior){
        flag2=3;
    }
}

menor=a; //menor means smallest
flag3=1;
if(menor>b){
    menor=b;
    flag3=2;
}if(menor>c){
    flag3=3;
}

if(flag2==1)
    printf("o a entre [");
if(flag2==2)
    printf("o b entre [");
if(flag2==3)
    printf("o c entre [");

if(flag3==1)
    printf("a e ");
if(flag3==2)
    printf("b e ");
if(flag3==3)
    printf("c e ");

if(flag==1)
    printf("a]\n");
if(flag==2)
    printf("b]\n");
if(flag==3)
    printf("c]\n");

return 0;
}

Well, first of all, consider adopting one of the myriad common indentation-styles and use it consistently.
Doing so will make your code far easier to read and write.
You should test whether you really got three numbers.
Currently, you don't, which leads to UB.
If in doubt, sort.

That eliminates an astonishing number of problems.
You aren't using anything from <math.h>, so don't include it.
exit(0); and return 0; have nearly the same effect when done in main.
The only differences are when main is called recursively, and we won't do such an abomination outside the IOCCC. (Or if we change to a different language, like C++, but who would do that?)

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

typedef struct namednumber {int num; char name; } namednumber;

static int comp_namednumber(const void* pa, const void* pb) {
    int a = ((namednumber*)pa)->num,
        b = ((namednumber*)pb)->num;
    return (a > b) - (a < b);
}

int main() { // No need for void in the parameter-list, this is a definition
    int a, b, c;
    printf("Please enter three numbers:\n");
    if(3 != scanf("%i %i %i", &a, &b, &c)) {
        printf("You didn't enter three numbers. Aborting.\n");
        return EXIT_FAILURE;
    }

    if(a == b || b == c || c == a) {
        printf("There is no gap.");
        return 0;
    }

    namednumber data[] = {{a,'a'},{b,'b'},{c,'c'}};
    qsort(data, 3, sizeof *data, comp_namednumber);
    printf("%c is between %c and %c\n", data[1].name, data[0].name, data[2].name);

    return 0; // This line is implicit in C99+
}

Actually, getting out qsort is severe overkill, so here with a standard sorting-network:

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

static void sort2(int* a, char* na, int* b, char* nb) {
    if(*a < *b) return;
    int t = *a;
    *a = *b;
    *b = t;
    char nt = *na;
    *na = *nb;
    *nb = nt;
}

int main() {
    int a, b, c;
    printf("Please enter three numbers:\n");
    if(3 != scanf("%i %i %i", &a, &b, &c)) {
        printf("You didn't enter three numbers. Aborting.\n");
        return EXIT_FAILURE;
    }

    if(a == b || b == c || c == a) {
        printf("There is no gap.");
        return 0;
    }

    char na = 'a', nb = 'b', nc = 'c';
    sort2(&a, &na, &b, &nb);
    sort2(&a, &na, &c, &nc);
    sort2(&b, &nb, &c, &nc);
    printf("%c is between %c and %c\n", nb, na, nc);
}


File-serving Twisted resource I've just written this Twisted resource base class for serving files. It's part of a larger application, this is just for serving its images, JavaScript files, etc.

Is it okay? I wonder if checking for .. in the path is enough -- all the files that exist in self.directory are intended to be served, to anyone, so this should be all that's needed, I think. Should these it be a Unicode string? Should they all be Unicode strings?

Will return open(file).read() be enough? I would think the resource would be closed when the function goes out of scope, but I'm not completely sure.

"""
Base class for resources serving files to the browser.

This doesn't use the `Page` class because it doesn't need to set up templates
or anything, just serve the file.
"""
from twisted.web.resource import Resource
import os.path

class File(Resource):
    """
    Serves a certain type of file from the server.

    It could be images, javascript, etc. It serves files from a single
    directory which have a single extension.

    members:
    content_type -- the HTTP content type of the files to be served
    directory -- the local directory the files are stored in
    extension -- the extension which the files are saved with.
    """
    content_type = None
    directory = None
    extension = None

    def render_GET(self, request):
        """
        Serve the requested file.

        Makes sure the file exists and the request doesn't contain '..'
        to escape the file directory.
        """
        file = request.path.strip("/")
        file = file.split("/", 1)[1]
        file = "{directory}/{filepath}.{extension}".format(
            directory=self.directory, filepath=file, extension=self.extension)

        request.responseHeaders.setRawHeaders("content-type", [self.content_type])

        if ".." in file:
            request.setResponseCode(400)
            return ""

        if not os.path.isfile(file):
            request.setResponseCode(404)
            return ""

        return open(file).read()

Firstly, don't try to think of everything that could go wrong. Instead, check for what you know to be valid.

import re
directory, filename = re.match('/([A-Za-z0-9]+)/([A-Za-z0-9]+)/?', alpha).groups()

By using this method to extract the directory and filename, we can be sure that directory and filename contain nothing except letters and numbers which are perfectly safe.

Its also best to use os.path.join to produce folder paths rather then string formatting to put them together.

    return open(file).read()

This works, but not if you try and run on IronPython/Jython/PyPy. CPython depends on reference counting which will cause the file to be closed as soon as it returns. A better version is probably:

with open(file) as source:
     return source.read()

This is guaranteed to close the file even without reference counting.