Kotlin Data Class Wildcard I'm writing a small custom DSL and interpreter as an exercise in understanding how the language stack (lexer/parser/(interpreter/compiler)) works. Because I want to be able to report the location of semantic errors, the AST needs to retain information about where the elements were in the source.

For this purpose I wrote a data class Point(val line: Int, val column: Int). All well and good, until I realized that manually specifying the position of EVERY node in the AST for unit tests on parsing would be prohibitively time consuming (and error prone). (Obviously, some tests need to do so to test the assigning of positions, but some is a lot fewer than all.)

To allow me to write more good unit tests quicker, I decided I wanted to default the position of all my nodes to a wildcard value that would match all valid Points. The parsed nodes would have the correct position, but the manually assembled AST to check against would not specify (and thus not test) the positions. This is the solution I came up with.

/**
 * DO NOT EXTEND THIS CLASS!!
 *
 * This is an custom open data class to allow a wildcard value.
 */
open class Point(val line: Int, val column: Int) {
    object Wildcard : Point(-1, -1) {
        override fun toString() = "Point.Wildcard"
        override fun copy() = this
    }

    override fun equals(other: Any?) = when {
        this === other         -> true
        other === Wildcard     -> true
        other !is Point        -> false
        this === Wildcard      -> true
        line != other.line     -> false
        column != other.column -> false
        else                   -> true
    }

    override fun hashCode(): Int { // generated
        var result = line
        result = 31 * result + column
        return result
    }

    override fun toString() = "Point(line=$line, column=$column)"
    operator fun component1() = line
    operator fun component2() = column
    open fun copy() = Point(line, column)
}

I wanted to keep all the behavior of a data class, but a data class cannot be open and this was the only way I could come up with of implementing the wildcard value. This solution still feels wrong and too much like a hack, however. It could easily be broken by outside code extending Point (as it must necessarily be available to be used) and so much of this code is just re-implementing what data class gives us for free. I feel like there must be a solution with a sealed class, but I haven't figured it out.

Recommended review points: idiomaticness (or not) of code, conventions, alternate solutions (and any other points)

Your equals method is not transitive (e.g. Point(2, 2) == Point.Wildcard and Point.Wildcard == Point(3, 3) but Point(2, 2) != Point(3, 3). See Any.equals - stdlib - Kotlin Programming Language for more details on the requirements for equals. You might also consider using EqualsTester from guava-testlib for testing your own implementations of equals.

If you define your own method instead of defining a custom, non-transitive equals implementation than your existing solution can become much simpler:

data class Point(val line: Int, val column: Int) {
    companion object {
        val wildcard = Point(-1, -1)
    }

    infix fun matches(other: Point): Boolean {
        return this == other || other === wildcard || this === wildcard
    }
}

Notes:

Point(2, 2) matches Point(-1, -1) will return false.
Point(2, 2) matches Point.wildcard will return true.
If you want both to return true then replace the === with == in matches.
Marking matches as infix is purely optional but it seems to me like a good fit.


Review of java interface for constructing brain model I have built a partial human brain model and the following is an example of how I use all the classes. I was wondering if anyone could critique my implementation strategies because I feel like the following setup is very clumsy. You can view all the code at:
https://github.com/quinnliu/WalnutiQ

public class HowToUseMARK_I extends junit.framework.TestCase {
    private NervousSystem nervousSystem;
    private MemoryClassifier memoryClassifier_Digits;

    private Gson gson;

    public void setUp() throws IOException {
        this.gson = new Gson();
        this.nervousSystem = this.constructConnectedNervousSystem();
        this.memoryClassifier_Digits = this.trainMemoryClassifierWithNervousSystem();
    }

    private NervousSystem constructConnectedNervousSystem() {
        // construct Neocortex with just V1
        Region rootRegionOfNeocortex = new Region("V1", 4, 4, 4, 50, 3);
        RegionToRegionConnect neocortexConnectType = new RegionToRegionRectangleConnect();
        Neocortex unconnectedNeocortex = new Neocortex(rootRegionOfNeocortex,
                neocortexConnectType);

        // construct LGN
        Region LGNRegion = new Region("LGN", 8, 8, 1, 50, 3);
        LateralGeniculateNucleus unconnectedLGN = new LateralGeniculateNucleus(
                LGNRegion);

        // construct Retina
        VisionCell[][] visionCells = new VisionCell[65][65];
        for (int x = 0; x < visionCells.length; x++) {
            for (int y = 0; y < visionCells[0].length; y++) {
                visionCells[x][y] = new VisionCell();
            }
        }
        Retina unconnectedRetina = new Retina(visionCells);

        // construct 1 object of NervousSystem to encapsulate all classes in
        // MARK II
        NervousSystem nervousSystem = new NervousSystem(unconnectedNeocortex,
                unconnectedLGN, unconnectedRetina);

        // connect Retina to LGN
        Retina retina = nervousSystem.getPNS().getSNS().getRetina();

        LateralGeniculateNucleus LGN = nervousSystem.getCNS().getBrain()
                .getThalamus().getLGN();

        SensorCellsToRegionConnect retinaToLGN = new SensorCellsToRegionRectangleConnect();
        retinaToLGN.connect(retina.getVisionCells(), LGN.getRegion(), 0, 0);

        // connect LGN to V1 Region of Neocortex
        Neocortex neocortex = nervousSystem.getCNS().getBrain().getCerebrum()
                .getCerebralCortex().getNeocortex();

        RegionToRegionConnect LGNToV1 = new RegionToRegionRectangleConnect();
        LGNToV1.connect(LGN.getRegion(), neocortex.getCurrentRegion(), 0, 0);

        return nervousSystem;
    }

I would consider making this code:

// construct Retina
VisionCell[][] visionCells = new VisionCell[65][65];
for (int x = 0; x < visionCells.length; x++) {
    for (int y = 0; y < visionCells[0].length; y++) {
        visionCells[x][y] = new VisionCell();
    }
}

a factory method on Retina (passing in the dimensions of the cell array) as it will probably be a common thing to create.
Call chains like these nervousSystem.getPNS().getSNS().getRetina() are a code smell. You could model your nervous system as some kind of repository where you can query the individual components like this:

nervousSystem.getRetina();
nervousSystem.getBrain();

or fully generic (additional bonus points ;))

nervousSystem.get<Retina>();
nervousSystem.get<Brain>();

More complex components like Brain can then be modeled in the same way:

nervousSystem.get<Brain>().get<Neocortex>();

This alleviates the caller from having to know the internal structure.
I find it quite weird that your ctor for the NervousSystem only takes some very specific components of individual subsystems. From the looks of it the NervousSystem is composed out of PNS and CNS with SNS being part of  the PNS and Brain being part of the CNS etc. so I'm wondering why the NervousSystem is not being built like that but gets some very specific subcomponents instead.

I would have thought the setup should look something like this (I haven't checked the code on github so the class names are purely guesswork):

Neocortex neocortex = new Neocortex(...);
CerebralCortex cerebralCortex = new CerebralCortex(neocortex, ...);
Cerebrum cerebrum = new Cerebrum(cerebralCortex, ...);

LateralGeniculateNucleus lgn = new LateralGeniculateNucleus(...);
Thalamus thalamus = new Thalamus(lgn, ...);

Brain brain = new Brain(cerebrum, thalamus, ...);
CNS cns = new CNS(brain, ...);

Retina retina = new Retina(...);
SNS sns = new SNS(retina, ...);
PNS pns = new PNS(sns, ...);

NervousSystem nervousSystem = new NervousSystem(cns, pns);

For wiring individual components up you could pass a WiringStrategy into the NervousSystem which decides how things are wired up (so you can simulate broken wiring for example).


Updates href values for a limited collection of links on a page As much as it hurts, I've been tasked with writing a script that prepends integration, QA, etc. environment segments to anchor href attributes that link to a series of sites within two specific domains.

Since we can't be certain of adherence to the requirement that content authors always use production URLs (without these segments) this script covers cases where the wrong segment is present in a particular downstream environment.

We do know the domains and finite list of environments.

/*
    Updates URL environment segments in page links (anchor hrefs)

    1. Determines the current environment by parsing location.hostname.
    2. Selects a list of links with absolute URIs or network-path references:
    (`a[href^="http"], a[href^="//"]`).
    3. Loops over the list of links and takes action only on items that contain
    _domainone.com_ or _domaintwo.com_ in the `href` attribute.
        a. If environment is production, removes the environment segment
        b. If downstream environment, adds or updates environmnet segment to
        match the current environment.

    Currently supports domainone.com and domaintwo.com domains. To
    add additional domains, update the `domainRegEx` variable. This script also
    assumes a finite list of environment names (authoring, integration,
    localhost, qa, and sys). To support additional environments, update the
    `envRegEx` variable.
*/
(function(window) {
    window.addEventListener('DOMContentLoaded', () => {
        const domainRegEx = /(domainone\.com)|(domaintwo\.com)/i;
        const envRegEx = /authoring\.|integration\.|localhost\.|qa\.|sys\./i;
        const environment = location.hostname.match(envRegEx);
        const links = document.querySelectorAll('a[href^="http"], a[href^="//"]');

        for (let i = links.length - 1; i > -1; i--) {
            const link = links[i];
            const href = link.getAttribute('href');
            if (domainRegEx.test(href)) {
                // If production, remove any environment segments ...
                if (environment === null) {
                    link.setAttribute('href', href.replace(envRegEx, ''));
                } else {
                    // ... else if downstream environment, ensure correct environment segment.
                    if (envRegEx.test(href)) {
                        link.setAttribute('href', href.replace(envRegEx, `${environment[0]}`));
                    } else {
                        link.setAttribute('href', href.replace(domainRegEx, `${environment[0]}$1`));
                    }
                }
            }
        }
    });
}(window));

There's no need to wrap this all in an IIFE. You aren't introducing any variables in the top scope, so no need to prevent them from leaking. Though it will become more useful when you follow my advice of introducing some helper functions. But even then there's no need to safe-guard the window global (as you aren't worried about the location and document globals).
Why are you running the loop backwards? Better use a looping construct that's meant for dealing with lists, like the for-of loop.
The environment variable contains a result of a regex match and is used like so:

// If production, remove any environment segments ...
if (environment === null) {

If you would translate the regex match into an actual environment name, the code would become much more self-explanatory:

if (environment === "production") {

For changing the link href value, I'd suggest extracting helper functions like stripEnvironment() and replaceEnvironment(), which could be used as:

link.setAttribute('href', stripEnvironment(href));
link.setAttribute('href', replaceEnvironment(href, environment));

If you want to have the domains and environments lists to be easily extensible, regex is probably not the best choice. Better to define them as arrays of strings which you can later transform into regexes using the RegExp constructor.
Your regexes look a bit too loose. The /sys\./ pattern would also match linksys.. You should at least anchor them at word boundary using \b.


Finding the longest Collatz sequence using Go, concurrently I am working through this problem on project euler.

Basic purpose of the code:

/*
if n is even n -> n/2
if n is odd n -> 3n + 1
which starting number under 1 million produces the longest sequence?
*/

The trouble with the code is that when I implement concurrency as I have, the program runs considerably more slowly than when I am just returning values directly from the get_seq_len function. Am I implementing concurrency incorrectly? Or is the program just not complex enough that there is speed to be gained from concurrency or something?

package main

import ("fmt";"math";"time")

This function takes a number x and then using the formula described in the above comment block creates a sequence of numbers from it until the number 1 is reached. Originally I had the function simply returning the length of the sequence, but now I have it sending the length to the specified channel.

func get_seq_len(x int, c chan int) {
    var count int = 1
    for {
        if x == 1 {
            break
        } else if x%2 == 0 {
            x = x / 2
        } else {
            x = 3 * x + 1
        }
        count++
    }
    // return count + 1
    c <- count + 1
}

Here the main function just step through every number from 1 to whatever the max is and then tests the length of the sequence for that number. I use channel c to pass the sequenced length back to main().

func main() {
    start := time.Now()
    var max int = int(math.Pow(10,6))
    // var max int = int(math.Pow(3,3))
    var max_length int = 0
    var max_num int = 0
    c := make(chan int, 50)

    for i:=1; i<max; i++ {
        // length := get_seq_len(i, c)
        go get_seq_len(i, c)
        length := <-c

        if length > max_length { max_length = length; max_num = i }
    }

    fmt.Println(max_num)
    t := time.Now()
    elapsed := t.Sub(start)
    fmt.Println(elapsed)

}

Longest Collatz sequence

  Problem 14

  The following iterative sequence is defined for the set of positive
  integers:

n → n/2 (n is even)
n → 3n + 1 (n is odd)

  Although it has not been proved yet (Collatz Problem), it is thought
  that all starting numbers finish at 1.

  Which starting number, under one million, produces the longest chain?

In Go, the testing package includes a benchmark facility. Therefore, if we are concerned about performance, as we often are, we start with a simple implementation and benchmark it. Starting number 837799 produces the longest chain of 525 elements. When I ran a Go benchmark, it took 231,949,902 nanoseconds, about one-quarter of a second.

BenchmarkEuler14-8   5   231949902 ns/op   0 B/op   0 allocs/op

euler14a_test.go:

package main

import "testing"

func lenChain(n int) int {
    c := 1
    for ; n > 1; c++ {
        if n&1 == 0 {
            n >>= 1
        } else {
            n += n<<1 + 1
        }
    }
    return c
}

func euler14() (maxn, maxc int) {
    maxs := 1000000 - 1
    for n := 1; n <= maxs; n++ {
        c := lenChain(n)
        if maxc < c {
            maxn = n
            maxc = c
        }
    }
    return maxn, maxc
}

func BenchmarkEuler14(b *testing.B) {
    for N := 0; N < b.N; N++ {
        euler14()
    }
}

If you you want it to be faster, perhaps we could remember some intermediate results. When I ran a Go benchmark, it took 16.979.849 nanoseconds, about than one-sixtieth of a second.

BenchmarkEuler14-8  100  16979849 ns/op  4005900 B/op  1 allocs/op

euler14b_test.go:

package main

import (
    "testing"
)

func lenChain(n int, a []int) int {
    if n < 1 {
        return 0
    }

    c := 1
    for m := n; m > 1; {
        if m < len(a) && a[m] > 0 {
            c += a[m] - 1
            break
        }

        if m&1 == 1 {
            // m is odd:
            // m = 3*m + 1
            m += m<<1 + 1
            c++
        }
        // m is even:
        // m = m/2
        m >>= 1
        c++
    }

    if n < len(a) && a[n] < c {
        a[n] = c
    }

    return c
}

func euler14() (maxn, maxc int) {
    maxs := 1000000 - 1
    a := make([]int, maxs/2)
    for n := 1; n <= maxs; n++ {
        c := lenChain(n, a)
        if maxc < c {
            maxn = n
            maxc = c
        }
    }
    return maxn, maxc
}

func BenchmarkEuler14(b *testing.B) {
    for N := 0; N < b.N; N++ {
        euler14()
    }
}

When I ran your "concurrent" program, using eight threads, it took 639,898,032 nanoseconds, about two-thirds of a second.

Some real-world code review questions: What is concurrency? What is parallelism? What do you expect to accomplish? What is your plan?

In your earlier Go performance question, Finding all prime numbers within a range, I pointed ot the importance of benchmarking and algorithms.

You only timed your program. You did not run Go benchmarks. You have no detail. You only used one simple algorithm. My benchmarks allowed me to peer into my code, often line by line. My code, using an optimized memoization algorithm, runs around nearly forty times faster than your concurrent code.

Efficient solutions to the problem exhibit little concurrency and little parallelism. Using concurrency tools adds overhead without any corresponding benefit.

  Comment:
  You compute a lot of intermediate values in the sequence, yet only
  cache the first (being m). I'd imagine your solution would be a lot
  faster if you made a second pass in lenChain that filled all the
  misses into a. If you bite the bullet with function call overhead for
  a recursive implementation, this change would even be trivial. –
  Dillon Davis

When I ran benchmarks, your idea, despite using more memory, was slower: 22,234,021 versus 16.979.849 nanoseconds.

euler14c_test.go:
BenchmarkEuler14-8   50  22234021 ns/op  8003587 B/op  1 allocs/op

euler14b_test.go:
BenchmarkEuler14-8  100  16979849 ns/op  4005900 B/op  1 allocs/op

euler14c_test.go:

package main

import (
    "testing"
)

func lenChain(n int, a []int) int {
    if n <= 1 {
        if n != 1 {
            return 0
        }

        c := 1
        if n < len(a) {
            if a[n] == 0 {
                a[n] = c
            }
        }
        return c
    }

    if n < len(a) {
        if a[n] > 0 {
            return a[n]
        }
    }

    m := n
    if m&1 == 0 {
        // m is even:
        // m = m/2
        m >>= 1
    } else {
        // m is odd:
        // m = 3*m + 1
        m += m<<1 + 1
    }
    c := 1 + lenChain(m, a)

    if n < len(a) {
        a[n] = c
    }

    return c
}

func euler14() (maxn, maxc int) {
    maxs := 1000000 - 1
    a := make([]int, maxs+1)

    for n := 1; n <= maxs; n++ {
        c := lenChain(n, a)
        if maxc < c {
            maxn = n
            maxc = c
        }
    }

    return maxn, maxc
}

func BenchmarkEuler14(b *testing.B) {
    for N := 0; N < b.N; N++ {
        euler14()
    }
}


Finding binary gap in Java I want to know the performance of my code for following problem description.

For example, number 9 has binary representation 1001 and contains a binary gap of length 2. The number 529 has binary representation 1000010001 and contains two binary gaps: one of length 4 and one of length 3. The number 20 has binary representation 10100 and contains one binary gap of length 1. The number 15 has binary representation 1111 and has no binary gaps.

public static int getBinaryGap(int N) {
    if (N < 5) {
        return 0;
    }
    String binaryRep = Integer.toBinaryString(N);
    int currentGap = 0;
    int finalGap = 0;
    for (int i = 0; i+1 < binaryRep.length(); i++) {
        if (currentGap == 0) {
            if (binaryRep.charAt(i) == '1' && binaryRep.charAt(i + 1) == '0') {
                currentGap++;
            }
        } else {
            if (binaryRep.charAt(i + 1) == '0') {
                currentGap++;
            }
            if (binaryRep.charAt(i + 1) == '1') {
                finalGap = finalGap<currentGap ? currentGap:finalGap;
                currentGap = 0;
            }
        }
    }
    return finalGap;
}

Java conventions

In Java variables are written in "lowerCamelCase", starting with a lowercase letter, so please use getBinaryGap(int n). See also Oracle  code conventions

String conversion

The performance can be increased because you use a conversion to String, while you can solve it using bit-fiddling only.

Complexity

You code has one loop, with all constant time operations. Integer.toBinaryString(int n) has cost that is linear with n. The total performance will be O(n), that is OK.

Bit flipping

Following your logic, but all as bit-test:

public static int getBinaryGapBitFiddling(int n) {
    if (n < 5) {
        return 0;
    }
    int currentGap = 0;
    int finalGap = 0;
    int currentBit;
    for (int i=0 ; (currentBit = (1<<i))< n; i++)
    {
        int nextBit = currentBit << 1;
        if (currentGap == 0) {
            if ( ((currentBit & n) > 0 )  &&  ((nextBit & n ) == 0 )) {
                currentGap++;
            }
        } else {
            if ((nextBit & n ) == 0) {
                currentGap++;
            }
            if ((nextBit & n ) > 0) {
                finalGap = finalGap<currentGap ? currentGap:finalGap;
                currentGap = 0;
            }
        }
    }
    return finalGap;
}

More optimizations

You could probably use bitmasks for faster scanning as well. For example 10 and 01 are the start and end of a run.


Matplotlib: Display y value as a marker I created a plot on which the y value is visible with the marker.
What's your opinion ?
I wonder if I could have done something simpler.

(code should run in a Jupyter notebook)

%matplotlib inline
fig = plt.figure()
axes = fig.add_axes((1,1,1,1))

X = np.array([3,3.2,3.5,3.8,3.9,5,5.5,5.8,5.9,5])
y = np.array([1,1,1,1,1,2,2,2,2,2])

markers = ['^', 'o']
for k, m in enumerate(markers):
   i = (y == k+1)
   flat = [1 for val in y[i]]
   axes.scatter(X[i], flat, marker=m)

axes.set_xlim([2,7])
axes.set_ylim([0,3])

The result:

First of all some style comments:

Please use spaces after commas and around operators – It is hard to read your sequence of values for the X and y lists when they are all compressed. And the same applies for the ([2,7]) when setting limits. It reads a lot better with [3, 3.2, 3.5, ..., 5.8, 5.9, 5] and ([2, 7])
Choose better names for variables – Why have you used uppercase X and then lowercase y? What does i, k, and m signify?  General recommendation is to use variable names meaning something, longer than one character long. For loop counters there is an exception for i, j, k, in that sequence.
Strange combination of i and k – When I read code having i and k, by convention I'm assuming there to be an j somewhere. This is missing. In addition the i is not a loop counter, but some predicate function for use with numpy's boolean arrays. Try finding a better name for i, possibly something like marker_range_predicate.

And then some considerations regarding your coding algorithm:

First of all I don't quite understand why you want to hide the y-value and replace it with a flat value of 1, and then use markers to denote the few distinct y-values. To me, intuitively, it would make more sense to add markers for given y-ranges, and interconnect markers of a given style with lines. This would show corresponding x-ranges and y-ranges of values

Limit the range of the plot – Your example plot has loads of empty space, which makes the entire plot look almost empty. If you use min and max, you can limit the plot better. If you want to avoid having the min and max values on the axes, you could cheat a little and subtract/add 0.5 to those values. (An even better version would be to calculate the range, and add/subtract a percentage of that range to be used as limits)

Separate x-ranges with different y-values for each marker – In a comment you state that you want to see if the values are linearly separable. To me that read as you might have overlapping x-ranges for different x-values, and you want to visualize this.
In your current code, if you switch the fourth element to y=2, the markers would overlap, so I suggest changing the 1 when you set flat to be the current marker index, i.e. k. This way you would easily see the line of a current marker, but can also see if they are overlapping.

Change size of markers – If adding a parameter, s, to your call to scatter, you can change the marker size, and make them a little more prominent. I found 200to a somewhat good size.

Adding more markers? – I don't know if it's an alternative for you to use even more markers, but I tried adding some more and extended your data array with two extra x-values with a y-value of 2.

Refactored code
import numpy as np
import matplotlib.pyplot as plt

%%matplotlib inline

fig = plt.figure()

axes = fig.add_axes((1, 1, 1, 1))

x = np.array([3, 3.2, 3.5, 3.7, 3.8, 3.9, 5, 5.5, 5.8, 5.9, 5, 5.8])
y = np.array([1, 1, 1, 3, 2, 1, 2, 2, 2, 1, 2, 3])

markers = ['^', 'o', 's', 'v', 'p']

for idx, marker in enumerate(markers):
    i = (y == idx + 1)
    flat = [idx + 1 for val in y[i]]
    axes.scatter(x[i], flat, marker=marker, s=200)

    axes.set_xlim([min(x) - 0.5, max(x) + 0.5])
    axes.set_ylim([min(y) - 0.5, max(y) + 0.5])

This resulted in the following figure:

As you can see there is a lot more empty space, bigger markers, and even easier to see that in my slightly changed base data the x-ranges are not totally separable.


One reader / one writer no-memory-allocation lock-free ring-buffer I have two threads - one thread queues items another thread dequeues. dequeue is fast enough and so we can assume that queue is never contains more than 65536 items.

C++ doesn't contain "ring-buffer."  Boost has one, but it doesn't allow "reuse" of elements. So I wrote my own ring-buffer, which I think is very fast and requires no memory allocation.  The code below has not been tested but it should show the general idea.

Do you find my code fine? Can you suggest how I can improve it or if there's something I can use instead of it?
    #include
    #include
    #include

#include <chrono>
#include <thread>

template<class T> class ArrayPool
{
public:
    ArrayPool() {
    };

    ~ArrayPool(void) {
    };

    // reader thread
    bool IsEmpty() {
        return curReadNum == curWriteNum;
    }

    // reader thread
    T* TryGet()
    {
        if (curReadNum == curWriteNum)
        {
            return NULL;
        }
        T* result = &storage[curReadNum & MASK];
        ++curReadNum;
        return result;
    }

    // writer thread
    T* Obtain() {
        return &storage[curWriteNum & MASK];
    }

    // writer thread
    void Commit()
    {
                    // Ensure storage is written before mask is incremented ?
                    // insert memory barrier ?
        ++curWriteNum;
        if (curWriteNum - curReadNum > length)
        {
            std::cout <<
                "ArrayPool curWriteNum - curReadNum > length! " <<
                curWriteNum << " - " << curReadNum << " > " << length << std::endl;
        }
    }

    // writer thread
    void ObtainAndCommit(T* val) {
        memcpy(&storage[curWriteNum & MASK], val, sizeof(T));
        // copy constructor will likely be slower cause every field is copied
        // storage[curWriteNum & MASK] = *val;
        Commit();
    }

private:
    static const uint32_t length = 65536;
    static const uint32_t MASK = length - 1;
    T storage[length];

    // curWriteNum must be volatile because access from both
    // reader and writer threads
    // curReadNum don't need to be volatile because accessed
    // only from one (reader) thread
    volatile uint32_t curWriteNum;
    uint32_t curReadNum;
};

struct myStruct {
    int value;
};

ArrayPool<myStruct> pool;

void ReadThread() {
    myStruct* entry;
    while(true) {
        while ((entry = pool.TryGet()) != NULL) {
            std::cout << entry->value << std::endl;
        }
    }
}

void WriteThread(int id) {
    std::chrono::milliseconds dura(1000 * id);
    std::this_thread::sleep_for(dura);

    myStruct* storage = pool.Obtain();
    storage->value = id;
    pool.Commit();
    std::cout << "Commited value! " << id << std::endl;
}

int main( void )
{
    boost::thread readThread = boost::thread(&ReadThread);
    boost::thread writeThread;
    for (int i = 0; i < 100; i++) {
        writeThread = boost::thread(&WriteThread, i);
    }
    writeThread.join();
    return 0;
}

there is absolutely no thread safety constructs, this means that you'll see a lot of interesting race conditions and memory inconsistencies
just assuming dequeues happen fast enough does not mean they will in production
you return T by value in Obtain(), instead of by reference so you can't actually write into it
Obtain is a bad name, it sounds like it gets the next value like TryGet does
memcpy is a bad idea to use to store the values if there is ever a custom copy constructor and a destructor
that said Obtain->store->Commit can be put into a single Put(T*) member function
eventually curReadNum and curWriteNum will overflow, it's not a real problem here because your length is a power of 2 and they are unsigned


Creating a byte array of a certain size I need to create a byte array with 100 bytes from a string. The length of the string is unknown but it is always less than 100 bytes. So if the string is 65 bytes then i need the array to be 35 zero bytes and then the String bytes to follow.

At the moment i have something like this, that does the job, but maybe something prettier and optimized is possible :)

    String string = "asdf";
    byte[] stringBytes = string.getBytes();
    ByteArrayOutputStream outputStream = outputStream = new ByteArrayOutputStream();
    outputStream.write(new byte[100 - stringBytes.length]);
    outputStream.write(stringBytes);
    byte[] result = outputStream.toByteArray();

You can try

String string = "asdf";
byte[] result = new byte[100];
System.arraycopy(string.getBytes(), 0, result, 100 - string.length(), string.length());


Largest palindrome product From the Project Euler challenge series:

  A palindromic number reads the same both ways. The largest palindrome made from the product of two 2-digit numbers is 9009 = 91 × 99.
  Find the largest palindrome made from the product of two 3-digit numbers.

The brute force method runs in 1.8s per loop on my machine:

def getGen(n):
    return range(n,-1,-1)

def bruteForce():
    gen = getGen(999)
    sc = maxpalin = 0
    for i in gen:
        for j in gen:
            sc += 1
            prod = i * j
            s = str(prod)
            if s==s[::-1] and prod > maxpalin:
                maxpalin = prod
    print(sc)
    return maxpalin

%timeit bruteForce()

When I initially approached the problem, I tried to use the properties of multiplying 2 different 3-digit numbers to find the solution. This approach, however, takes approximately 16.7s per loop, even though the sc for the bruteForce is 1 million and the sc for Palindrome.main() (see below) is considerably lower at 4536. Why does the Palindrome class run so much slower?

$$(a.b.c) * (x.y.z) = (l.m.n.n.m.l)$$
$$(100a + 10b + c) * (100x + 10y + z) = (100001l + 10010m + 110n)$$
$$(10000ax + 1000ay + 100az) + (1000bx + 100by + 10bz) + (100cx + 10cy + cz) = (100001l + 10010m + 110n)$$
$$10000ax + 1000(ay + bx) + 100(az + by + cx) + 10(bz + cy) + cz = (100001l + 10010m + 1100n)$$
$$10(1000ax + 100(ay + bx) + 10(az + by + cx) + (bz + cy)) + cz = 100001l + 10(1001m + 110n)$$

firstPass
$$10(1000ax + 100(ay + bx) + 10(az + by + cx) + (bz + cy) - 1001m - 110n) = 100001l - cz = 10p \implies 10 \mid 10p$$

secondPass
$$p = 10(100ax + 10(ay + bx) + (az + by + cx - 11n)) + (bz + cy) - 1001m \implies 10 \mid p + 1001m - (bz + cy) = 10q$$

thirdPass
$$q = 10(10ax + (ay + bx)) + (az + by + cx - 11n) \implies 10 \mid q + 11n - (az + by + cx) = 10r$$

main
$$r = 10ax + (ay + bx) \implies 10 \mid r - (ay + bx) = 10s$$

class Palindrome(object):
    def __init__(self):
        self.a = 0
        self.b = 0
        self.c = 0
        self.l = 0
        self.m = 0
        self.n = 0
        self.x = 0
        self.y = 0
        self.z = 0

    def firstPass(self):
        gen = getGen(9)
        for l in gen:
            for c in gen:
                for z in gen:
                    p = 100001 * l - (c * z)
                    if (p % 10):
                        continue
                    self.l = l
                    self.c = c
                    self.z = z
                    yield p / 10

    def secondPass(self, p):
        gen = getGen(9)
        for m in gen:
            for b in gen:
                for y in gen:
                    q = p + (1001 * m) - ((b * self.z) + (self.c * y))
                    if (q % 10):
                        continue
                    self.m = m
                    self.b = b
                    self.y = y
                    yield q / 10

    def thirdPass(self, q):
        gen = getGen(9)
        by = self.b * self.y
        for a in gen:
            for x in gen:
                for n in gen:
                    r = q + (11 * n) - (a * self.z + by + self.c * x)
                    if (r % 10):
                        continue
                    self.a = a
                    self.x = x
                    self.n = n
                    yield r / 10

    def main(self):
        sc = 0
        for p in self.firstPass():
            for q in self.secondPass(p):
                for r in self.thirdPass(q):
                    a = self.a
                    b = self.b
                    x = self.x
                    y = self.y

                    s = r - (a * y + b * x)
                    if s == 10 * a * x:
                        sc += 1
                        m = 100 * a + 10 * b + self.c
                        n = 100 * x + 10 * y + self.z

                        yield m * n
        print(sc)

i = Palindrome()
%timeit (max(i.main()))

A quick look at bruteForce.

The built-in max computes the maximum value of a collection.
Nested loops can often be combined into a single loop using product, combinations, or combinations_with_replacement, depending on how you need to handle repetitions.
The loops go downwards from 999, but no use is made of this (no early exit) so you might as well let the loops go upwards, which is simpler.

Revised code:

from itertools import combinations_with_replacement

def palindrome(n):
    """Return True if n is a palindrome in base 10."""
    s = str(n)
    return s == s[::-1]

def problem4():
    """Return the largest palindrome made from the product of two 3-digit
    numbers.

    """
    return max(i * j
               for i, j in combinations_with_replacement(range(1000), 2)
               if palindrome(i * j))


Downloading blobs asynchronously We have a working implementation that is doing I/O operations, returning data from blobs in an async/await manner from Azure Blob Storage.

//Method1 is not async, this is what is called from the controller
public IEnumerable<Data> Method1()
{
    //Running the async method and returning the result from task
    return Task.Run(() => GetDataAsync()).Result;
}

private async Task<IEnumerable<Data>> GetDataAsync()
{
    //There are multiple blob address where the data is held. The code creates, in parallel multiple tasks for each address.
    //It returns tasks that will be run in Async pattern
    var tasks = multipleBlobAddress.AsParallel.Select(blobAddress =>{
            Task<IEnumerable<Data>> task = GetDataFromBlobsAsync(blobAddress);
            return task;
    });

    //Awaits all tasks to complete
    var completedTasks = await Task.WhenAll<IEnumerable<Data>>(tasks);
    //Selects all the tasks and returns them. Each tasks has data.
    return completedTasks.SelectMany(t => t);
}

private Task<IEnumerable<Data>> GetDataFromBlobsAsync(string address)
{
    //Opens the blob and reads from it
    using (var blobStream = blobService.OpenRead(address))
    {
        //Deserialises the data read from the blob
        data = service.DeserialiseData(blobStream);
    }

    return Task.FromResult<IEnumerable<Data>>(data);
}

We have understood that the best way to read from blobs is to follow the async/await pattern and not using the AsParallel method (other suggestions are most welcomed). I have the following questions:

By creating tasks in parallel and then waiting for all of them to complete, do we lose any performance?
What are the obvious areas where we got it wrong or we can improve and potentially increase the performance or make the code better to maintain/read?
Are we correctly following the async / await pattern?

If you need any extra information about the code, I will happily provide it and hope to make it clearer.

There is never a need to do this:

return Task.Run(() => GetDataAsync()).Result;

Why start a new task and then synchronously wait with .Result? This is the sort of code that will bite you later when you find out that a synchronisation context can cause a deadlock. See Stephen Cleary's blog post on the subject of an async deadlock. Stephen's blog is an absolute gold mine of information on aysnc and await.

Either use the synchronous api all the way from your controller down into the Azure Storage SDK or use the asynchronous one (with async and await). Don't mix and match - that's a source of errors and also pointless. Aysnc is useful when there's something else the thread could be doing. E.g. in a web app that's serving requests. If the web app is blocked by .Result or Wait then there is no benefit and you should use the synchronous API.


LeetCode 862: Shortest Subarray with Sum at Least K I'm posting my code for a LeetCode problem copied here. If you would like to review, please do so. Thank you for your time!
Problem

Return the length of the shortest, non-empty, contiguous subarray of nums with sum at least k.
If there is no non-empty subarray with sum at least k, return -1.

Example 1:
Input: nums = [1], k = 1 Output: 1
Example 2:
Input: nums = [1,2], k = 4 Output: -1
Example 3:
Input: nums = [2,-1,2], k = 3 Output: 3
Note:

\$1 <= \text{nums.length} <= 50000\$
\$-10 ^ 5 <= \text{nums}[i] <= 10 ^ 5\$
\$1 <= k <= 10 ^ 9\$

Code
#include <vector>
#include <algorithm>
#include <deque>

class Solution {
public:
    static int shortestSubarray(std::vector<int> nums, const int k) {
        const int length = nums.size();
        int shortest = length + 1;

        std::deque<int> deque_indicies;

        for (int index = 0; index < length; index++) {
            if (index) {
                nums[index] += nums[index - 1];
            }

            if (nums[index] >= k) {
                shortest = std::min(shortest, index + 1);
            }

            while (!deque_indicies.empty() && nums[index] - nums[deque_indicies.front()] >= k) {
                shortest = std::min(shortest, index - deque_indicies.front());
                deque_indicies.pop_front();
            }

            while (!deque_indicies.empty() && nums[index] <= nums[deque_indicies.back()]) {
                deque_indicies.pop_back();
            }

            deque_indicies.emplace_back(index);
        }

        return shortest <= length ? shortest : -1;
    }
};

Problem

Solution

Discuss

Passing by reference or value
Currently,
std::vector<int> nums

forces callers to pass a copy of the entire vector by value. You could argue that this is actually useful, since your algorithm needs to mutate it (or a copy of it) in-place. My preference is usually to make this copy-step explicit instead:

Pass a const & reference, not a mutable copy
Make a local copy of the vector explicitly, invoking the copy constructor
Mutate your copy.

Spelling
deque_indicies -> deque_indices


Temperature converter program Can anyone suggest any improvements?  I already applied here the suggestions that was made for my first program (Guess the Number) that I posted.

import java.util.Scanner;

public class Temp { // convertion of fahrenheit to celcius and vise versa

    public static void main(String[] args) {
        double celcius, fahrenheit;
        Scanner in = new Scanner(System.in);

        System.out.print("Enter the value of temperature in degree celcius: ");
        celcius = in.nextDouble();
        fahrenheit = celcius * 9 / 5 + 32; // convertion formula for celcius to fahrenheit
        System.out.printf(celcius+" °C = %.1f °F\n", fahrenheit);

        System.out.print("Now enter the value of temperature in degree fahrenheit: ");
        fahrenheit = in.nextDouble();
        celcius = (fahrenheit - 32) * 5 / 9; // formula of fahrenheit to celcius
        System.out.printf(fahrenheit +" °F = %.1f °C\n", celcius);
    }
}

Here are some things I see that can improve your code.

Resource Management

To avoid resource leaks, it is usually good to close IO classes like java.util.Scanner if you won't be using them anymore. This is because the garbage collector only manages memory and not other resources.

in.close();

Modularity

Currently your program contains a main method that does all tasks. This is not a good idea if you choose to reuse some parts of it (E.G. doing celcius conversions multiple times). I think it is a good idea to put the two conversions into separate methods. That way the components of your program would be more reusable (specially if you choose to add more operations later) and would be more readable as a whole.

Error handling

At least write an error handling mechanism, even a general one. For example:

try (Scanner in = new Scanner(System.in)){
    // handle user inputs and conversions
} catch(IOException e) {
   // handle IO exception
}

Notice that I use Try with resources so the scanner resource rapped in the try block would automatically be closed after the block finishes executing. So no need to call in.close() anymore


Coherent usage of Hibernate's session and DAO public void something()
{
    Session session = HibernateUtil.newSession();
    AModelDAO amd = new AModelDAO(session);
    BModelDAO bmd = new BModelDAO(session);

    Transaction tx = session.beginTransaction();
    amd.savesomething(object);
    bmd.savesomething(object2);
    tx.commit();
    session.close();
}

I would like to know if my coding here is good enough or if there is a better method to produce the same result.

Instead of working with Hibernate sessions, I encourage you to use the JPA API. I think this is the most common way to work with Hibernate now. (http://www.theserverside.com/news/2240186700/The-JPA-20-EntityManager-vs-the-Hibernate-Session-Which-one-to-use)

For the transaction aspects and the initialization of your DAOs, I also think that a solution like Spring (http://projects.spring.io/spring-framework/) would help to simplify the code of your application.

Your code would look more like the following (with annotations stuff):

public class MyService {
  @Resource
  private AModelDAO aDao;

  @Transactional
  public void something(object) {
    // no need to manage transactions or sessions here
    aDao.save(object);
  }
}


How can I have jQuery code further optimized? I would like to see if my code below can still be optimized, resulting in lesser lines of code. My code retrieves the values from a multiselect field and assign them to hidden fields that will be passed to an integrated googlesheet. Thanks
 jQuery(".ff-btn-submit").click(function() {
        if(jQuery("input:radio[name=gift_choice]").is(":checked") ) {
            if (jQuery("input[name=gift_choice]:checked").val() == 'yes') {
                // Retrieve multi-select values
                var selectedValues = [];
                selectedValues = $('#ff_124_multi_select_gifts').val();

                // Set hidden input with the chosen gift.
                if (selectedValues.length > 0) {
                  jQuery("input[name=gift_1]").val(selectedValues[0]);
                  jQuery("input[name=gift_2]").val(selectedValues[1]);
                  jQuery("input[name=gift_3]").val(selectedValues[2]);
                  jQuery("input[name=gift_4]").val(selectedValues[3]);
                  jQuery("input[name=gift_5]").val(selectedValues[4]);
                  jQuery("input[name=gift_6]").val(selectedValues[5]);
                }
            }
        }
    });

I have few hints save jQuery in $ variable:
(function($) {
   // your code
})(jQuery);

$ is common for jQuery and it's easier to read because it shorter.
Second and obvious is using loop to run calls in:
    if (selectedValues.length > 0) {
       selectedValues.forEach(function(value, i) {
          var num = i + 1;
          $(`input[name=gift_${num}]`).val(value);
       });
    }

and further improvement is to move that into a function. So instead of comment:
// Set hidden input with the chosen gift.

you can have:
setHiddenInputs(selectedValues);

function setHiddenInputs(values) {
    values.forEach(function(value, i) {
        var num = i + 1;
        $(`input[name=gift_${num}]`).val(value);
    });
}

I would also merge those two lines:
var selectedValues = [];
selectedValues = $('#ff_124_multi_select_gifts').val();

you assign array for no reason and then replace it with output of val() call:
var selectedValues = $('#ff_124_multi_select_gifts').val();

also if selectedValues are gifts then you can use:
var selectedGifts = $('#ff_124_multi_select_gifts').val();
setHiddenInputs(selectedGifts);

which is exactly what your comment try to say


JavaScript/jQuery Image Slider I've just wrote this class for an image slider (it cross-fades rather than sliding, so a class rename may be beneficial). It's my first proper attempt at JavaScript OOP. Can anyone see any real issues with this?

// Wrapper is the container element, in this case $('#sliderWrap').
// images is an array of the images.

function ImageSlider(wrapper, images){

    // Elements.
    this.elem = wrapper;
    this.images = images;
    this.top = this.elem.find('#topImage');
    this.bottom = this.elem.find('#bottomImage');
    this.next = this.elem.find('#nextBtn');
    this.prev = this.elem.find('#prevBtn');

    this.lastImage = images.length - 1;
    this.currImage = 1; // This is 1 as the first iteration is rendered in the HTML.
    this.interval;
    this.fadeTime = 400;
    this.sliderDelay = 4000 + this.fadeTime;

    var self = this;

    this._init = function(){

        $(window).on('load', function(){
            self.elem.css('height', self.top.height());
            self.next.show();
            self.prev.show();
            self._animate();
            self._startInterval();
            self.next.click(function(){
                clearInterval(self.interval);
                self._animate(true);
                self._startInterval();
            });

            self.prev.click(function(){
                clearInterval(self.interval);
                self._animate(false);
                self._startInterval();
            });
        });
    }

    /**
     * @param next boolean. True next, false prev
     */

    this._animate = function(next){
        if(typeof next == 'undefined') {
            next = true;
        }

        if (next) {
            self.nextImage();
        } else {
            self.prevImage();
        }
    }

    this.nextImage = function(){
        self.currImage++;

        this.top.fadeOut(self.fadeTime, function(){
            self.bottom.addClass('active');
            self.top.attr('src', self.images[self.currImage - 1]).show();

            if (self.currImage > self.lastImage) {
                self.currImage = 0;
            }

            self.bottom.removeClass('active');

            self.bottom.attr('src', self.images[self.currImage]);
        });
    }

    this.prevImage = function(){

        var minusTwo = self.currImage - 2;

        switch (minusTwo){
            case -1:
                minusTwo = self.lastImage;
                break;
            case -2:
                minusTwo = self.lastImage - 1;
                break;
        }

        self.bottom.attr('src', self.images[minusTwo]);
        self.top.fadeOut(self.fadeTime, function(){
            self.bottom.addClass('active');

            var plusOne = minusTwo + 1;
            if (plusOne > self.lastImage){
                plusOne = 0;
            }

            self.top.attr('src', self.images[minusTwo]).show();
            self.bottom.attr('src', self.images[plusOne]);
            self.bottom.removeClass('active');
            self.currImage--;
            if(self.currImage < 0){
                self.currImage = self.lastImage;
            }
        });
    }

    this._startInterval = function(){
        self.interval = setInterval(function(){
            self._animate();
        }, self.sliderDelay);
    }

    this._init();
}

The HTML markup to go with this is.

I think the only really relevant CSS is the active class, which basically puts the bottomImage on top of the topImage (z-index wise).

Firstly, I don't see any "real" issues with this - you mostly have done an awesome job! I have a few recommendations though that might help this be a little more performant, readable, and refined:

Do you really need all of these to be instance/public variables? Later on when you instantiate an ImageSlider, are you going to need an external module to know about these properties? Most of them could probably be private with var
lastImage and currImage are improperly/confusingly named - really they should be lastIndex and currIndex, because they are storing the index of an image, not a reference to the image itself.
It is a common convention to store a reference to the current scope at the top of a constrctor function.
The prefix "_" (underscore) is usually used to mark a function as private (using var _animate, for example). The airbnb style guide shows a usage similar to yours here, but, it is a very bad idea and I can't imagine why they would spread it. If a variable is attached to this, i.e. this.anything/this['anything'], it is, in JavaScript, not private. Putting this _ prefix is incredibly unintuitive and many people have told them so. At first glance, I thought maybe you were using it to indicate that the property was a method (i.e., this.elem and this.images are properties, but this._animate and this._init are functions), but I see you have other methods that do not use this prefix. If you are going to use this prefix to denote that it is a function, definitely be consistent with it.
You can remove the typeof check by reversing your logic a bit.
Did you mean to use "this" here, or "self"? Be sure to be consistent.
Use jQuery chaining to reduce scope resolution - you were calling this.bottom twice but you only needed to resolve that variable once, since jQuery will return the element for chainability. You can break it onto two lines if you prefer as well.
Pass animate by reference to setInterval instead of creating an anonymous scope to wrap the item.
minusTwo and plusOne are, at least for me, difficult to understand. I have read the function a few times and still do not quite understand why these variables have this name.
Cache this.bottom and this.top in order to reduce scope resolution in this function, since you are using them both quite a bit.

Here is the code, slightly modified and rewritten with specific numeric callouts:

function ImageSlider(wrapper, images){

    // 3
    var self = this;

    // 1
    this.elem = wrapper;
    this.images = images;
    this.top = this.elem.find('#topImage');
    this.bottom = this.elem.find('#bottomImage');
    this.next = this.elem.find('#nextBtn');
    this.prev = this.elem.find('#prevBtn');

    // 2
    this.lastIndex = images.length - 1;
    this.currIndex = 1; // This is 1 as the first iteration is rendered in the HTML.
    this.interval;
    this.fadeTime = 400;
    this.sliderDelay = 4000 + this.fadeTime;

    // 4
    this._init = function(){

        $(window).on('load', function(){
            self.elem.css('height', self.top.height());
            self.next.show();
            self.prev.show();
            self._animate();
            self._startInterval();
            self.next.click(function(){
                clearInterval(self.interval);
                self._animate(true);
                self._startInterval();
            });

            self.prev.click(function(){
                clearInterval(self.interval);
                self._animate(false);
                self._startInterval();
            });
        });
    }

    /**
     * @param next boolean. True next, false prev
     */
    // 4, 5
    this._animate = function(next){
       if(!next){
         self.prevImage();
        } else {
          self.nextImage();
        }
    }

    this.nextImage = function(){
        self.currIndex++;

        // 6
        this.top.fadeOut(self.fadeTime, function(){
            self.bottom.addClass('active');
            self.top.attr('src', self.images[self.currIndex - 1]).show();

            if (self.currIndex > self.lastIndex) {
                self.currIndex = 0;
            }

            // 7
            self.bottom.removeClass('active').attr('src', self.images[self.currIndex]);
        });
    }

    this.prevImage = function(){

        // 9
        var minusTwo = self.currIndex - 2;

        switch (minusTwo){
            case -1:
                minusTwo = self.lastIndex;
                break;
            case -2:
                minusTwo = self.lastIndex - 1;
                break;
        }

        // 10
        var bottom = self.bottom,
            top    = self.top;

        bottom.attr('src', self.images[minusTwo]);
        top.fadeOut(self.fadeTime, function(){
            bottom.addClass('active');

            // 9
            var plusOne = minusTwo + 1;
            if (plusOne > self.lastIndex){
                plusOne = 0;
            }

            top.attr('src', self.images[minusTwo]).show();
            bottom.attr('src', self.images[plusOne]);
            bottom.removeClass('active');
            self.currIndex--;
            if(self.currIndex < 0){
                self.currIndex = self.lastIndex;
            }
        });
    }

    // 4
    this._startInterval = function(){
        // 8
        self.interval = setInterval(self._animate, self.sliderDelay);
    }

    // 4
    this._init();
}


Python abilities on strings the Encoding scheme is given below:

  Don't replace the characters in the even places. Replace the characters in the odd places by their place numbers . and if they exceed 'z', then again they will start from 'a'.

  example: for a message "hello" would be "ieolt" and for message "maya" would be "naba"

  how many lines of code [minimum] do we have to write in order to encode a lengthy string in python in the above format.?

mycode is given below:

def encode(msg):
    encoded_message = ""
    letters = "abcdefghijklmnopqrstuvwxyz"
    index_of_msg = 0
    for char in msg.lower():
        if char.isalpha():
            if index_of_msg%2 == 0 :
                encoded_message += letters[((letters.rfind(char) )+ (index_of_msg+1))%26]  # changed msg.rfind(char) to index_of_msg
            else:
                encoded_message += char
        else:
            encoded_message += char
        index_of_msg +=1
    return encoded_message

Here's a one liner for you (since the question asks for the minimum number of lines):

encode = lambda s: "".join([(c if (i+1) % 2 == 0 else chr(ord('a') + (ord(c) - ord('a') + i + 1) % 26)) for i, c in enumerate(s)])


Is this the right way to handle Exceptions in PHP? I'm using the function countLikes() to fetch URLs' Likes count from the Facebook API.

// Link.php

class Link
{

private $url;

public function __construct($url){
    $this->url = new Url($url);
}

public function getUrl(){
    return $this->url();
}

public function countLikes()
{
    global $config;

    $call = $config["facebook.api"]["baseurl"] .
            "/" .
            $config["facebook.api"]["version"] .
            "/?" .
            "id=" . $this->url->getEncodedUrl() .
            "&access_token=" . $config["facebook.api"]["key"];

    try {
        $response = Utils::curl($call);
    } catch (\Exception $e) {
        throw new LinkException("Unable to cURL Facebook API.", 0, $e);
    }

    if (is_array($response)) {
        if (array_key_exists("error", $response)) {
            throw new LinkException(
            "Facebook Graph API v2.2 error : "
            . $response["error"]["message"], $response["error"]["code"]
            );
        } elseif (isset($response["share"])) {
            $count = $response["share"]["share_count"];
        } else {
            $count = 0;
        }

        return $count;
    } else {
        throw new LinkException("Graph API Response is not an array");
    }
}
}

And this is how it's used :

$link = new Link("http://www.facebook.com");

try{
    $likes = $link->countLikes();
    echo "Link $link->getUrl()->getFullUrl() has $likes likes for the moment. <br>";
} catch(LinkException $e){
    do{
        $logger->add($e->getMessage());
    }while ($e = $e->getPrevious());
}

While this is working as it's supposed to do, I'm not happy with the way exceptions are treated and I'm not even sure this is the right way to do it. Is there a better way to manage exceptions and handle them?

You are using Exceptions correctly, however its possible to catch more than one type of exception with the same try block. Instead of specifically catching the LinkException in your logging routine, you should probably just catch any Exception, but prefix the error string with the Exception's class.

I also recommend putting the declaration of the $link object inside the try block in case the Url class throws an exception from inside the Link class's constructor.

try {
    // execute code
    $foo = new Foo();
} catch ( \Exception $e ) {
    // log exceptions
    $logger->add( get_class( $e ) . ': ' . $e->getMessage() );
}

Push your logging as high up in your application as possible so you are collecting all exceptions, not just your custom ones.


Function to test whether the source and destination squares represent a valid knight move The following is my solution to a problem. While it works - I want to learn how to optimize this in a much better way (I'm new to C++). My original idea was the use a map to map the r,c pairs and then test those but hoping someone can help optimize this for me.

  Write a function using a grid of string and two row/column pairs (r1, c1), (r2, c2) which returns true if there is a knight at chess board square (r1, c1) who can move to empty square (r2, c2). For your function to return true, there must be a knight at square (r1, c1), and the square at (r2, c2) must store an empty string, and both locations must be within the bounds of the grid.
  A knight makes an "L" shaped move, going 2 squares in one dimension and 1 square in the other. i.e. if the chess board (1, 2) stores "knight", then the call of knightMoves(board, 1, 2, 2, 4) returns true.

Here is my current solution

bool knightMoves(Grid<string>& board, int r1, int c1, int r2, int c2);

int main() {

    //create a fake board
    Grid<string> board(7,7);
    board[1][2] = "knight";
    board[3][1] = "rock";
    board[0][4] = "king";

    //fire the function
    bool result = knightCanMove(board, 1, 2, 2, 4);
    cout << result << endl;

    return 0;
}

bool knightMoves(Grid<string>& board, int r1, int c1, int r2, int c2) {

    bool canMove = false;
    string knight = "knight";

    //within grid bounds
    if(r1 > board.numRows() || c1 > board.numCols()) return false;
    if(r2 > board.numRows() || c2 > board.numCols()) return false;

    //test whether the co-ords match a knight string,
    //if yes, continue
    if (board[r1][c1] == knight) {
        if(board[r1 + 2][c1 + 1] == "" && r1+2 == r2 && c1+1 == c2) {
            canMove = true;
        }
        if(board[r1 + 2][c1 - 1] == "" && r1+2 == r2 && c1-1 == c2) {
            canMove = true;
        }
        if(r1 >= 2 && board[r1 - 2][c1 + 1] == "" && r1-2 == r2 && c1+1 == c2) {
            canMove = true;
        }
        if(r1 >= 2 && board[r1 - 2][c1 - 1] == "" && r1-2 == r2 && c1-1 == c2) {
            canMove = true;
        }
        if(board[r1 + 1][c1 + 2] == "" && r1+1 == r2 && c1+2 == c2) {
            canMove = true;
        }
        if(c1 >= 2 && board[r1 + 1][c1 - 2] == "" && r1+1 == r2 && c1-2 == c2) {
            canMove = true;
        }
        if(board[r1 - 1][c1 + 2] == "" && r1-1 == r2 && c1+2 == c2) {
            canMove = true;
        }
        if(c1 >= 2 && board[r1 - 1][c1 - 2] == "" && r1-1 == r2 && c1-2 == c2) {
            canMove = true;
        }
    }
    return canMove;
}

It looks like there may be an issue in range-checking.

You don't check for the rows and columns to be non-negative.
( < 0 ) The types aren't marked as unsigned, so they might be negative.
If board is zero-based, and it seems to be, checking against numXYZ should include the boundary.
( >= board.numXYZ() )

The range check becomes:

if ( r1 < 0 || r1 >= board.numRows() ||
     r2 < 0 || r2 >= board.numRows() ||
     c1 < 0 || c1 >= board.numCols() ||
     c2 < 0 || c2 >= board.numCols() ) {
  return false;
}

The jump check can be simplified. The absolute distance in one dimension must be one, and in the other must be two.

unsigned int rowdiff = abs(r1 - r2);
unsigned int coldiff = abs(c1 - c2);
if ( rowdiff == 1 && coldiff == 2 || rowdiff == 2 && coldiff == 1 )

Alternatively, we can note that the product of the distances must be ±2, leading to a more concise check, but that may obscure the underlying objective:

if ( abs((r1 - r2) * (c1 - c2)) != 2 ) return false;

At which point the final check is:

return board[r1][c1] == "knight" && board[r2][c2] == "";


Intersect two ranges in Swift Can you think of a reason the following extension should not be used in production, or a better way of implementing it:

public extension Range {

    @warn_unused_result
    public func intersect(other: Range) -> Range {

        let ds = startIndex.distanceTo(other.startIndex)
        let de = startIndex.distanceTo(other.endIndex)

        let s = ds <= 0 ? startIndex : startIndex.advancedBy(ds, limit: endIndex)
        let e = de <= 0 ? startIndex : startIndex.advancedBy(de, limit: endIndex)

        return s..<e
    }
}

The method should pass the following assertions:

(5...7).intersect(9...9) == (8..<8) // ....|||.---..
(5...7).intersect(1...3) == (5..<5) // ....---.|||..

(5...7).intersect(8...9) == (8..<8) // ....|||---...
(5...7).intersect(1...4) == (5..<5) // ....---|||...

(5...7).intersect(7...9) == (7..<8) // ....||+--....
(5...7).intersect(1...5) == (5..<6) // ....--+||....

(5...7).intersect(6...9) == (6..<8) // ....|++-.....
(5...7).intersect(1...6) == (5..<7) // ....-++|.....

(5...7).intersect(6...6) == (6..<7) // ....|+|......
(5...7).intersect(4...8) == (5..<8) // ....-+++-....

Note the first four assertions with an empty intersection: the method is currently returning startIndex..<startIndex if the other range is on the "left", or endIndex..<endIndex if it is on the "right". Is this a reasonable behaviour?

There is a general problem: The
distanceTo(_: Self) -> Self.Distance of the
ForwardIndexType protocol requires that

  start and end are part of the same sequence when conforming to RandomAccessSequenceType.
  end is reachable from self by incrementation otherwise.

(and RandomAccessSequenceType is probably a typo and should be
RandomAccessIndexType).

Here is a simple example where this conditions are not met, and
your code crashes:

let r1 = "foo".rangeOfString("")!
print(r1) // 1..<5
let r2 = "bar".rangeOfString("")!
print(r2) // 2..<6
let r = r1.intersect(r2) // fatal error: cannot increment endIndex

Incrementing the start index of r1 never hits the start index
of r2.

Here is another example:

struct MyIndex: ForwardIndexType {
    let value: Int

    func successor() -> MyIndex {
        return MyIndex(value: self.value + 1)
    }
}

func == (lhs: MyIndex, rhs: MyIndex) -> Bool {
    return lhs.value == rhs.value
}

is an index which can only be incremented. Then

let r1 = Range(MyIndex(value: 4) ... MyIndex(value: 6))
let r2 = Range(MyIndex(value: 3) ... MyIndex(value: 5))
let r = r1.intersect(r2)

will loop forever, because incrementing MyIndex(value: 4)
never reaches MyIndex(value: 3).

So intersecting ranges makes only sense if both ranges refer
to the same sequence. I don't know if that can be ensured at
compile time with a suitable restriction, I assume that it is
not possible. It should at least be documented (similar to the
above mentioned requirements of ForwardIndexType).

You could restrict the method to ranges of integer elements

public extension Range where Element : IntegerType { ... }

but of course that reduces its usability considerably.

The implementation itself looks good to me. The behavior in the case
of an empty intersection it a sensible choice. Is similar to that of the clamp()
method of ClosedInterval:

ClosedInterval(3, 4).clamp(ClosedInterval(5, 6)) // 4 ... 4
ClosedInterval(3, 4).clamp(ClosedInterval(1, 2)) // 3 ... 3

which returns the left or right bound, depending on wether the other
interval lies on the left or on the right.

If your ranges just represent intervals and do not refer to indices
of sequences, then ClosedInterval (and the existing clamp() method) might be an alternative.


Program to reverse a string using std::string Background: I'm trying to learn C++ and I'm doing that by writing some programs by myself. I'm decent with C, but I'm having trouble translating to OOP paradigm. I'm reading from learncpp.com and a book called Thinking in C++.

I'm noticing that reading doesn't really help much as much as writing programs and running into issues and solving them.

Here is a program I've written to reverse a string in C++ using std::string (SO tells me this should be used as opposed to C-style strings/char arrays):

example_one.h

#ifndef example_one_h
#define example_one_h

#include<iostream>
#include<string>
#include<cstdio>

using namespace std;

class stringOps{

    private:
        string arr;
        int length;

    public:
        // Constructor - Destructor
        stringOps(string arr);
        ~stringOps();
        // Member functions

        //1. get length of string
        int getLength();

        //2. print string
        const string& displayString(const stringOps &a);

        //3. Reverse the string and return obj of type stringOps.
        stringOps reverseStr(stringOps &a);

};

#endif

example_one.cc

#include "example_one.h"

/* Constructor and Destructor */
stringOps::stringOps(string a):arr(a), length(a.size()) {
}

stringOps::~stringOps() {
}

/* Member functions */
// 1.  get Length
int stringOps::getLength()
{
    return arr.size();
}

//2. display string
const string& stringOps::displayString(const stringOps &a)
{
    //cout << a.arr << endl;
    return a.arr;
}

//3. reverse array of the object.
stringOps stringOps::reverseStr(stringOps &a)
{
    reverse(a.arr.begin(), a.arr.end());
    return a;
}

/*  -------------------------------- Main  --------------------- */
int main()
{
    stringOps strA("Hello");

    string x = strA.displayString(strA);
    printf("Original string: %s \n", x.c_str());

    int len = strA.getLength();
    printf("String length:%d \n",(len));

    strA.reverseStr(strA);
    string y = strA.displayString(strA);
    printf("Reverse string: %s \n", y.c_str());

    return 0;
}

Questions:

What is good about this code (so that I can continue to do this for other programs)?
What is bad about this code? (Be brutal in your assessment.) Specifically, I have doubts about the best approach to take while writing functions 2 and 3.

Function 2:

const string& stringOps::displayString(const stringOps &a)
{
    //cout << a.arr << endl;
    return a.arr;
}

I tried doing this with return type as void and cout within the function, but apparently there's going to be an error for this with iostream. So is returning const string& a good way to do this? I passed by const reference because I do not change the object within the function.

Function 3:

stringOps stringOps::reverseStr(stringOps &a)
{
    reverse(a.arr.begin(), a.arr.end());
    return a;
}

This again takes the argument of type stringOps object by reference and directly manipulates the string. However, if I were to pass this argument as a const, how do I go about this?
Motivation for me to do so is that I want to return a manipulated string that is new, and not manipulate the object that is coming in directly.

The header would look something like this:

string stringOps::reverseStr(const stringOps &a)

Do I declare static string inside the function and return it? I tried using a local non-static string (which would be destroyed outside the function, but still for experiment purposes) and I got an error that said:

  read-only variable is not assignable

I'm assuming this is because it's a const reference.

The functionality is already present in the standard library. Your class has very limited usability and I would prefer simple std::reverse(str.begin(), str.end()). But lets put it aside and deal with the code you've written.

#include <iostream>
#include <cstdio>

Very bad idea to mix these two. I/O may become unordered.

using namespace std;

Why is “using namespace std” in C++ considered bad practice?.

class stringOps

Operations on class's data should be part of it, so naming it stringOps is not good idea. C++ programmers often use operator overloading to expand on some class.

You begin your class definition by private keyword, but everything in class is private by default, whereas in struct it is public. It is the only difference between the two.

    string arr;

What arr means? Nothing comes to mind. You should use something like str. It is good to give your variables good name. It greatly increases readability of the code.

   int length;

Declared, but never used.

    stringOps(string arr);

If you want to make a copy of the string type should be const std::string&. If you want to move from it, the type should be std::string&&.

    ~stringOps();

You declared destructor that does nothing. Compiler generates this by default if you don't provide any destructor. Remove it.

    int getLength();

From return type (int) I may suspect that the code can return negative size. It should be std::size_t, which is meant to be used in context of size. Further reading: When should I use std::size_t?

The functionality is present in std::string. On top of that, you use arr.length() as return value, not length.

    const string& displayString(const stringOps &a);

Why would you return const reference to it? I think users expect a copy of internal string. All operations on copying are noexcept, not including memory allocation, of course.

The next thing is arguments. It would be better to just omit argument, and return internal string.

stringOps reverseStr(stringOps &a);

Again, argument should be omitted. The internal string should be reversed and returned. Also, it would be better to return std::string.

Answer for your first question:

Don't do it in the future. It's not that your code is wrong. The purpose of it is very wrong.


Prepared statements from security viewpoint I've decided to go with OOP style and prepared statements, and so far I like it a lot more than the procedural style.  It's much more understandable in my opinion.

For this code review, I've just included my PHP code that interacts with my MySQL database. Everything is working how it should, but I'd like to know if you see anything that could be improved upon. Also, I'd like a security analysis/review of the code. Is my code safe from SQL injection since I'm using prepared statements? Is there anything else I should be doing?

I have this function in a folder/file above public_html, and just require it on the pages that need it.

function connectToDatabase() {
    $connect = new mysqli('localhost', 'myUserName123', 'myPassword123', 'myDatabaseName');
    if ($connect->connect_error) {
        exit;
    }
    return $connect;
}

This is the function I use to pull the question text, and answer integers from the database to display on the website.

function getQuestionsAndAnswers($connect) {
    $questionArray = array();
    $answersArray = array();
    $getStuff = $connect->prepare('SELECT `question`, `answer` FROM `Quizzes` WHERE `questionNumber`<? ORDER BY `questionNumber` ASC');
    $getStuff->bind_param('i', $questnum);
    $questnum = 21;
    $getStuff->execute();
    $getStuff->bind_result($singleQuestion, $singleAnswer);
    while ($getStuff->fetch()) {
       array_push($questionArray, $singleQuestion);
       array_push($answersArray, $singleAnswer);
    }
    $getStuff->close();
    $connect->close();
    return array($questionArray, $answersArray);
}

This is the code that gets run after the user takes the math quiz, submits his answers, and I validate that the answers are all integers, etc.

$connect = connectToDatabase(); // open database connection here

$stmt1 = $connect->prepare("SELECT MAX(`id`) FROM QuizAdministration");
$stmt1->execute();
$stmt1->bind_result($maxId);
$stmt1->fetch();
$stmt1->close();

$stmt2 = $connect->prepare("INSERT INTO PlayerAnswers (`QuizAdministrationId`, `questionNumber`, `playerAnswer`) VALUES (?, ?, ?)");
    for ($yty = 1; $yty < 21; $yty += 1) {
        $stmt2->bind_param('iii', $QuizAdministrationid, $questnum, $questansw);
        $QuizAdministrationid = $maxId + 1;
        $questnum = $yty;
        $questansw = $_POST['qora'.$yty];
        $stmt2->execute();
    }
    $stmt2->close();

$stmt3 = $connect->prepare("INSERT INTO QuizAdministration (`quizNumber`, `cookie`, `ip`, `score`) VALUES (?, ?, ?, ?)");
$stmt3->bind_param('issi', $quizNumber, $playerCookie, $playerIP, $playersPercent);
$quizNumber = 1;
$playerCookie = session_id();
$playerIP = getUserIp();
$playersPercent = 90;
$stmt3->execute();
$stmt3->close();

$connect->close(); // close database connection here
header('Location: http://www.myWebsite.com/quizResults.php');
exit;

This looks pretty good.  I'm glad to see the improvement.

You inexplicably call $connect->bind_params() using variables that have not yet been set.

The main issue I see now is $QuizAdministrationid = $maxId + 1,  which is prone to a race condition.  If two users submit the form simultaneously, it's possible that both pages could see the same result from SELECT MAX(id) … and generate the same next $QuizAdministrationid.

The solution is to ask the database to generate the next ID.  Every SQL database supports this in a slightly different way.  For MySQL, you should declare the id column of QuizAdministration to be AUTO_INCREMENT.  Then, leave id column unspecified when you do the INSERT.  MySQL will automatically fill in the id column with the next available number.  To find out what number MySQL picked for you, use $connect->insert_id.

To make this work for you, you'll need to insert a row into QuizAdministration first, then use the id of the new row to insert the PlayerAnswers.

$stmt2 = $connect->prepare("INSERT INTO QuizAdministration (`quizNumber`, `cookie`, `ip`, `score`) VALUES (?, ?, ?, ?)");
$quizNumber = 1;
$playerCookie = session_id();
$playerIP = getUserIp();
$playersPercent = 90;
$stmt2->bind_param('issi', $quizNumber, $playerCookie, $playerIP, $playersPercent);
$stmt2->execute();
$stmt2->close();

$quizAdministrationid = $connect->insert_id;

$stmt3 = $connect->prepare("INSERT INTO PlayerAnswers (`QuizAdministrationId`, `questionNumber`, `playerAnswer`) VALUES (?, ?, ?)");
for ($yty = 1; $yty <= 20; $yty += 1) {
    $questnum = $yty;
    $questansw = $_POST['qora'.$yty];
    $stmt3->bind_param('iii', $QuizAdministrationid, $questnum, $questansw);
    $stmt3->execute();
}
$stmt3->close();


MinHeap implementation Please comment on this.

class MinHeap<T> where T: IComparable
    {
        List<T> elements;

        public MinHeap()
        {
            elements = new List<T>();
        }

        public void Add(T item)
        {
            elements.Add(item);
            Heapify();
        }

        public void Delete(T item)
        {
            int i = elements.IndexOf(item);
            int last = elements.Count - 1;

            elements[i] = elements[last];
            elements.RemoveAt(last);
            Heapify();
        }

        public T PopMin()
        {
            if (elements.Count > 0)
            {
                T item = elements[0];
                Delete(item);
                return item;
            }
            //relook at this - should we just throw exception?
            return default(T);
        }

        public void Heapify()
        {
            for (int i = elements.Count - 1; i > 0; i--)
            {
                int parentPosition = (i+1) / 2 - 1;
                parentPosition = parentPosition >= 0 ? parentPosition : 0;

                if (elements[parentPosition].CompareTo(elements[i])>1)
                {
                    T tmp = elements[parentPosition];
                    elements[parentPosition] = elements[i];
                    elements[i] = tmp;
                }
            }
        }
    }

You should always try to program against an interface instead of against an implementation.

List<T> elements;

should be

IList<T> elements;

Restrict the scope of fields and methods to the smallest possible.
So public void Heapify() should be private instead.
The swapping of elements inside the Heapify() method can be extracted to a private method.

private void SwapElements(int firstIndex, int secondIndex)
{
    T tmp = elements[firstIndex];
    elements[firstIndex] = elements[secondIndex];
    elements[secondIndex] = tmp;
}

Former Heapify() method now

private void Heapify()
{
    for (int i = elements.Count - 1; i > 0; i--)
    {
        int parentPosition = (i+1) / 2 - 1;
        parentPosition = parentPosition >= 0 ? parentPosition : 0;

        if (elements[parentPosition].CompareTo(elements[i]) > 1)
        {
            SwapElements(parentPosition, i)
        }
    }
}

Potential bug
You are checking if the CompareTo() method returns a value > 1 but you miss ==1 as the documentation states

value < 0 => This instance precedes obj in the sort order.
value == 0 => This instance occurs in the same position in the sort order as obj.
value > 0 => This instance follows obj in the sort order.

//relook at this - should we just throw exception?
If you want to simulate a Stack then you should throw an InvalidOperationException but only if you also provide a property Count so that a user of your class can evaluate if he can call PopMin safely.


Random string generation in Java Can random string generation be faster in Java?

Here is my current code

import java.util.ArrayList;
import java.util.List;

public class FastestStringGeneration {

    private static final String ALPHA_NUMERIC_STRING = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    private static final char[] ch = ALPHA_NUMERIC_STRING.toCharArray();
    private static final java.util.concurrent.ThreadLocalRandom random = java.util.concurrent.ThreadLocalRandom.current();

    public static void main(String[] java) {
        // warmup code
        String[] warmUp = new String[10000];
        for (int j=0; j<10000; j++){
           warmUp[j] = getAlphaNumeric(1024);
        }
        // real code begins
        int numIterations = 1000000;
        String[] randomStrings = new String[numIterations];
        long start = System.currentTimeMillis();
        for(int i=0; i<numIterations; i++){
            randomStrings[i] = getAlphaNumeric(1024);
        }
        System.out.println(System.currentTimeMillis()-start);
        System.out.println(randomStrings.length);
    }

    public static String getAlphaNumeric(int len) {
        char[] c = new char[len];
        for (int i = 0; i < len; i++) {
            c[i] = ch[random.nextInt(ch.length)];
        }
        return new String(c);
    }
}

The total time it takes to generate 1 million random strings is about ~ 5.9 seconds. Can it be faster?

I was using java.util.Random but when I changed to java.util.concurrent.ThreadLocalRandom according to comments below I got the most performance improvement!

If the length is always identical you can save 1000000 calls to new().

Because new String(char[]) uses Arrays.copyOf(), you don't need a new char[] in every loop.

private static int ourlen = 1024;
private static char[] chars = new char[ourlen];

public static String getAlphaNumeric() {
    for (int i = 0; i < ourlen; i++) {
        chars[i] = ch[random.nextInt(ch.length)];
    }
    return new String(c);
}


Thrice nested "OS" in Python This Python main is what greets the user to a thrice nested super duper barebones OS.  The entire rest of my code isn't bug free but this section is, and it feels dirty anyway.  I'd like to know how to make it better, by adhering to conventions, or by removal of fatty code.

NOTE: I am not looking to use classes with any of this, because part of the challenge of this program is structuring it with procedural means.

from functions import *

innerLoopFlag = False
while True:
 if innerLoopFlag:
     break
 makeNew = raw_input("Login || Create New Account... ")
 if makeNew == "c-na":
     ui = createAccount()
     createFileSyst(ui)
     break
 if makeNew == "l-a":
     ui = [raw_input("Username... ").strip(),pw("Password... ").strip()]
     if not os.path.isdir(ui[0]):
         print "It appears that this username doesn't exist, please log in legitimately or create an account"
         continue
     if ui:
         while True:
             if takeCheck(ui):
                 print "Successful login..."
                 innerLoopFlag = True
                 break
             else:
                 print "Failure logging in, retrying..."

all()
print "Goodbye", ui[0]

I don't understand what you mean by "thrice nested", or why you call this an OS. I'm reviewing it as a login prompt.

  The entire rest of my code isn't bug free but this section is

Beware of believing something is bug-free. It never is. This is especially important for code which needs to be secure, as this presumably does.

Now for the review:

Many variables need names that say what they mean:

What does innerLoopFlag mean? Something like successful_login, right?
makeNew: command?
ui: credentials?
createFileSyst: (There's no need to abbreviate such a rarely used name.) Something like createUserDirectory? But it should be called from createAccount, not here.
takeCheck: What does this do? Is it isValidLogin?
all: Is it something like runUserSession?

Simplifications:

The conditional break at beginning of the loop can become the loop condition: while not innerLoopFlag.
The if ui test is always true.
The inner loop can be simplified by making the loop condition do the work:

while not takeCheck(ui):
    print "Failure logging in, retrying..."
print "Successful login..."
break

This also makes innerLoopFlag unnecessary. However, why are you retrying at all? Do you expect login to fail and then succeed?

Bugs:

Stripping the password is not safe, because passwords can contain whitespace.
Security hole: the isdir call lets anyone test for the existence of arbitrary directories. Imagine trying to log in with a username like '../../../some/private/directory'. Also, it depends on the current directory. Is there a user table you could look the user up in instead?
(Revealing whether a user exists is also sometimes considered a security hole, but that's debatable.)

Usability:

Are "c-na" and "l-a" user-supplied strings, or generated by raw_input, or what?
Output ending in "... " looks like a progress message ("Please wait..."), not a prompt. "Login: " and "Password: " are the standard prompts.


Counting the minimum number of blocks to remove to form a triangular temple PROBLEM STATEMENT :

  You want to build a temple for snakes. The temple will be built on a
  mountain range, which can be thought of as n blocks, where height of
  i-th block is given by hi. The temple will be made on a consecutive
  section of the blocks and its height should start from 1 and increase
  by exactly 1 each time till some height and then decrease by exactly 1
  each time to height 1, i.e. a consecutive section of 1, 2, 3, .. x-1,
  x, x-1, x-2, .., 1 can correspond to a temple. Also, heights of all
  the blocks other than of the temple should have zero height, so that
  the temple is visible to people who view it from the left side or
  right side.

  You want to construct a temple. For that, you can reduce the heights
  of some of the blocks. In a single operation, you can reduce the
  height of a block by 1 unit. Find out minimum number of operations
  required to build a temple.

  INPUT

  The first line of the input contains an integer T denoting the number
  of test cases. The description of T test cases follows.

  The first line of each test case contains an integer n.

  The next line contains n integers, where the i-th integer denotes hi

  OUTPUT

  For each test case, output a new line with an integer corresponding to
  the answer of that testcase.

  CONSTRAINTS

  1 ≤ T ≤ 10

  2 ≤ N ≤ 105

  1 ≤ Hi ≤ 10^9

  Example Input

3
3
1 2 1
4
1 1 2 1
5
1 2 6 2 1

  Output

0
1
3

MY SOLUTION

#include <stdio.h>

void quickSort(int arr[], int low, int high);
int partition (int arr[], int low, int high);
void swap(int* a, int* b);

int main(void) {
    // your code goes here
    int t=0, i=0, n=0, move=0, j=0;
    scanf("%d\n", &t);
    for(i=1;i<=t;++i)
    {
        scanf("%d\n", &n);
        int arr[n];
        int i5;
        for(i5=0;i5<n;++i5)
        scanf("%d ", &arr[i5]);

        quickSort(arr, 0, n-1);

        int max= (arr[n-1]!=arr[n-2])?arr[n-2]+1:arr[n-1];
        move = arr[n-1]-max;
        --max;
        int i3=0;
        for(i3=n-2;i3>0;--max)
        {
            move+=((arr[i3]-max)+(arr[i3-1]-max));
            i3-=2;
            if(max==1)
            {
                break;
            }
        }
        for(j=0;j<=i3;++j)
        {
            move+=arr[j];
        }
        printf("%d\n", move);
    }
    return 0;
}

// A utility function to swap two elements
void swap(int* a, int* b)
{
    int t = *a;
    *a = *b;
    *b = t;
}

/* This function takes last element as pivot, places
   the pivot element at its correct position in sorted
    array, and places all smaller (smaller than pivot)
   to left of pivot and all greater elements to right
   of pivot */
int partition (int arr[], int low, int high)
{
    int pivot = arr[high];    // pivot
    int i = (low - 1);  // Index of smaller element
    int j;

    for (j = low; j <= high- 1; j++)
    {
        // If current element is smaller than or
        // equal to pivot
        if (arr[j] <= pivot)
        {
            i++;    // increment index of smaller element
            swap(&arr[i], &arr[j]);
        }
    }
    swap(&arr[i + 1], &arr[high]);
    return (i + 1);
}

/* The main function that implements QuickSort
 arr[] --> Array to be sorted,
  low  --> Starting index,
  high  --> Ending index */
void quickSort(int arr[], int low, int high)
{
    if (low < high)
    {
        /* pi is partitioning index, arr[p] is now
           at right place */
        int pi = partition(arr, low, high);

        // Separately sort elements before
        // partition and after partition
        quickSort(arr, low, pi - 1);
        quickSort(arr, pi + 1, high);
    }
}

Your code is not working for some cases.
Consider for example input 1 1 2 2 4.
The answer should be 6, but your code returns 1.


"Day-of-the-week-finder" I'm new to Python/Programming and wrote this simple "Day of the week/Holiday finder" script. Ideally I'd use something so I can have a little widget on my desktop, but right now I'm looking for any and all suggestions to improve my code?

I'm importing this holiday module

import datetime
import holidays

today = datetime.date.today()
today = today.strftime("%m/%d/%Y")
today = datetime.datetime.strptime(today, "%m/%d/%Y")

def days_between(d1):
    d2 = today
    return (d2 - d1).days

def find_dt(input_dt):
    print(input_dt)
    dt1 = datetime.datetime.strptime(input_dt, "%m/%d/%Y")
    dt2 = dt1.strftime('%A')

    if days_between(dt1) == 0:
        print("Today IS {d}.".format(d=dt2))
    elif days_between(dt1) <= 1:
        print("It'll be a {d}.".format(d=dt2))
    else:
        print("It was a {d}.".format(d=dt2))

    us_holidays = holidays.UnitedStates()
    us_h = us_holidays.get(input_dt)

    if us_h is None and days_between(dt1) == 0:
        print("Today is not a holiday.")
    elif us_h is None and days_between(dt1) <= 1:
        print("It won't be a holiday.")
    elif us_h is None and days_between(dt1) >= 1:
        print("It wasn't a holiday.")
    elif days_between(dt1) == 0:
        print("Today is {h}.".format(h=us_h))
    elif days_between(dt1) <= 1:
        print("It'll be {h}.".format(h=us_h))
    else:
        print("It was {h}.".format(h=us_h))

def main():
    d = input('Type a date using MM/DD/YYYY: ')
    # d = '12/25/2015'
    find_dt(d)

main()

today

First of all, I'm super confused as to why you're doing

today = datetime.date.today()
today = today.strftime("%m/%d/%Y")
today = datetime.datetime.strptime(today, "%m/%d/%Y")

It seems like you'd be fine if you just did

today = datetime.datetime.today()

Additionally, constants (such as today) are traditionally named in ALL_CAPS.

days_between

It is very counter-intuitive to me that this only takes a single argument.  I think writing it this way

def days_between(d1, d2):
    return (d2 - d1).days

def days_from_today(date):
    return days_between(date, today)

makes much more sense.

find_dt

I don't like everything that is going on here.  First of all, I would argue that it shouldn't be the job of find_dt to convert an input string to a datetime object - the caller should be responsible for that.

Next, you're doing two distinct things.  One is displaying the day of the week for the given date, and the other is displaying information about that day being a US holiday.  These should probably be distinct.

def day_of_week(date):
    date_string = date.strftime("%A")

    from_today = days_from_today(date)
    if from_today == 0:
        info_string = "Today IS {}"
    elif from_today <= 1:
        info_string = "It will be a {}"
    else:
        info_string = "It was a {}"
    print(info_string.format(date_string))

A few notes about this.  I didn't call days_from-today every time - that's pretty wasteful.  Additionally, because each one does basically the same thing, instead of repeating the print I create the appropriate string and then just print into that at the end.

Next, holidays.

def holiday_status(date):
    us_holidays = holidays.UnitedStates()
    us_h = us_holidays.get(date)

    from_today = days_from_today(date)

    if us_h is None:
        if from_today == 0:
            print("Today is not a holiday")
        elif from_today <= 1:
            print("It won't be a holiday")
        else:
            print("It wasn't a holiday")
    else:
        if from_today == 0:
            info_string = "Today is {}"
        elif from_today <= 1:
            info_string = "It'll be {}"
        else:
            info_string = "It was {}"
        print(info_string.format(us_h))

Notice that I used some similar strategies as with day_of_week.  I also broke up your if statements a little more - while some people think nesting is bad, I think in this case it makes it much more clear what the conditions are.

__main__

You should have a block like this

if __name__ == '__main__':
    main()

at the bottom of your file.  Then it is import safe.

Locale

While right now you assume the US, it is conceivable that your users may be from other locales.  You probably want something like this (the specifics of this depend on your Python version, but this is a rough approximation)

LOCALE_ENUM = enum("US UK FR JP ...")
LOCALES = {
    LOCALE_ENUM.US: holidays.UnitedStates(),
    LOCALE_ENUM.UK: holidays.UnitedKingdom(),
    ...
}

and then to define both find_dt and holiday_status to take a LOCALE_ENUM instance and use it to determine what locale's holidays they should use.


Using 503 page when my site is in maintenance mode This is my code for 503 page:

header.php

if(getOption('maintence_mode')==1)
{
header("location:503.php");
exit;
}

503.php

<?php
$protocol = "HTTP/1.0";
if ( "HTTP/1.1" == $_SERVER["SERVER_PROTOCOL"] )
$protocol = "HTTP/1.1";
header( "$protocol 503 Service Unavailable", true, 503 );
header( "Retry-After: 3600" );
?>

    <!DOCTYPE html>

    <meta http-equiv="Content-type" value="text/html; charset=UTF-8" />
    <title>Page Not Found </title>

    <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">

    <meta http-equiv="refresh" content="6;url=index.php">

Site is currently in maintenance mode.

I want to know if my code is correct and how Google will handle SEO thing.  I don't have much knowledge about this, so it would be very helpful if someone could validate this code.

It looks good to me, but I do have one thing to say.

If I want to access http://yourdomain.com/example/page, and get a 503 error, I want to be able to refresh the page in a little while, to see if the 503 error is lifted.

What you do is redirect the user to the 503 page, which means that the address I'll get now if I refresh is http://yourdomain.com/503.php, this is not optimal.

Instead of redirecting, include the 503 error page.

Also some other things:

The error says 503, but your titles says "Page not Found"
Code indentation is important. Don't neglect it!
Never use an if block without curly brackets. Never.


Displaying the calenday day I have this (working) code:

class CalendarDay

  attr_accessor :from_month, :to_month, :year

  def self.line_of_day_numbers(month,year,start,offset)
    @line=''
    @offset=offset
    finish = (start+6)-@offset
    self.first_line_padding(start)
    start-= @offset
    self.day_numbers(month,year,start,finish)
  end

  private

  def self.first_line_padding(start)
    if start < 7
      @line=' ' * (@offset * 3)
      @offset=0
    end
  end
  ...

But the methods are tangled and there's a mix of instance and local variables.

Tests include:

it "shows the day numbers" do
  expect(CalendarDay.line_of_day_numbers(2,2000,1,2)).to eq '       1  2  3  4  5'
end
it "shows the day numbers" do
  expect(CalendarDay.line_of_day_numbers(10,2000,1,0)).to eq ' 1  2  3  4  5  6  7'
end
it "shows the day numbers" do
  expect(CalendarDay.line_of_day_numbers(1,2000,8,6)).to eq ' 2  3  4  5  6  7  8'
end

It looks like you want to create some helper functions as class methods.  If you do that, you probably want to use class variables (e.g. @@a) rather than instance variables.  I would like to suggest what I think may be a better way.

First, convert the instance variables you created with attr_accessor to class instance variables (not to be confused with class variables).  You can do that like this:

class CalendarDay
  class << self
    attr_accessor :from_month, :to_month, :year

    def line_of_day_numbers(month,year,start,offset)
      ...
      @sugar = 7
      x = 5
      @year = .....
      ...
    end
  end
end

Now you can use CalendarDay.from_month. CalendarDay.year = 2007, and so on.  Here class << self is opening CalendarDay's singleton class or metaclass.  (To be more precise, it sets self to that metaclass.)  You could add other class instance variables (@sugar), as well as local variables (x).  You can also add class instance methods, as I have done above.  You would invoke this method with:

CalendarDay.line_of_day_numbers(month,year,start,offset)

Notice that self. is not used in defining class instance methods.  That's because self is the class's metaclass when they are created.

Note that these are class instance variables and class instance methods, not class variables or instance variables or class methods or instance methods.  They are only available to the class object, CalendarDays.  If you were to create an instance of the class, cal_day = CalendarDay.new, you'd find cal_day would have no access to any of these class instance variables or class instance methods.

One of the nice things about doing it this way is that you can write your code between class << self and end in the same way you'd write code for a class instance.  All instance variables, local variables and instance methods are available to the class only.

For the record, there are a few (equivalent) alternatives to the class << self/end construct.  One is:

module CalendarDayModule
  attr_accessor :from_month, :to_month, :year

  def line_of_day_numbers(month,year,start,offset)
      ...
  end
end

class CalendarDay
  extend CalendarDayModule
end

I discussed that here.

Beyond this, I can make no suggestions for improving your code without seeing more of it, day_numbers() in particular.  BTW, within your line_of_day_numbers() class method, neither  first_line_padding() nor day_numbers() need be preceded by self..