A reusable AJAX polling function I have written a small JavaScript function which performs an AJAX request every x seconds.

I was hoping some of you could give it a look over and give me any pointers on redundant code or bad practices.

The idea of the function is to:

Allow for polling to be initiated
Once initiated, poll once every x seconds and perform an AJAX request.
Stop polling once disablePolling() is executed.
Be able to be reused anywhere.

requester.js

function Request() {

this.poll = false;

this.activatePoll = function () {
    this.poll = true;
    this.runPoll();
};

this.disablePoll = function () {
    this.poll = false;
};

this.runPoll = function () {
    var self = this;
    var poll = setTimeout(function () {
        $.ajax({
            url: './request.php',
            success: function (response) {
                console.log(response);
            },
            dataType: "json",
            complete: function () {
                if (self.poll == false) {
                    clearTimeout(poll);
                } else {
                    self.runPoll();
                }
            }
        })
    }, 1000);
};
}

Usage:

  index.html

<!DOCTYPE html>

    <meta charset="UTF-8">
    <title>Requester</title>

Requester

    <input type="button" onclick="$request.test();" value="test">
    <input type="button" onclick="$request.activatePoll();" value="poll">
    <input type="button" onclick="$request.disablePoll();" value="turn off poll">

  request.php

<?php
echo json_encode($time());
?>

It should also be mentioned that theAajax request will need to also POST some parameters.

Your code is simply a GET request, so the shorthand getJSON can be used for this.
Instead of success and complete, use the more standard Promise method then.
Instead of a recursive setTimeout, use setInterval instead. This avoids your code from rescheduling a callback on each iteration. You can always clear interval timers with clearInterval.
When using setInterval, clearing the timer will suffice for stopping the polling. You don't need the boolean flag to tell you to stop.
When using setInterval, you don't need an extra function for a recursive call anymore.
You also seem to use OOP. It is normally advised to put methods on the prototype and not the instance so that methods are shared per instance and not created per instance.
It is usually not advised to use inline scripts. Use addEventListener to bind your click handlers.
Booleans are usually prefixed with an is, has or are depending on the grammar. This is to not mistake a property as some other non-boolean value.
Put the url and interval in a variable that acts as a constant. That way, you can easily configure as well as spot the values and not dig through code.
If ES6 is available for you, self = this can be taken care of by using arrow functions.

In the end, your code could be as simple as

function Request() {
  this.pollTimer = null;
  this.interval = 1000;
  this.url = './request.php';
}

Request.prototype.disablePoll = function () {
  clearInterval(this.pollTimer);
  this.pollTimer = null;
};

Request.prototype.activatePoll = function () {
  this.pollTimer = setInterval(() => {
    $.getJSON(this.url).then(response => console.log(response))
  }, this.interval);
};


Factory / Builder Design Pattern in Python Situation: I have implemented a factory design pattern in Python as below. There are different screens which have different configurations. One can register the screen in the factory and get the screen in case correct configuration is passed to respective factory class method.
Question: I am wondering if this is a clean solution as each screen class (Screen1, Screen2, ..) has a main-method which calls in a specific order the respective private class methods. Is there a better/cleaner way to handle the main methods in each class? Is it cleaner to implement a combination of factory and builder design pattern?
Highly appreciate any comments and improvements!!
Code:
import numpy as np
import pandas as pd

from abc import ABCMeta, abstractmethod

df = pd.DataFrame({"ident": ["A1", "A2", "B3", "B4"], "other_col": np.random.randint(1, 6, 4)})

class IScreens(metaclass=ABCMeta):

    @abstractmethod
    def main(self):
        pass

class Screen1(IScreens):

    def __init__(self, data, config):
        self.data = data
        self.batch = config["cfg1"]

    def __get_letter(self):
        self.data["campaign"] = self.data[self.batch].str[:1]

        return self.data

    def __get_number(self):
        self.data["num"] = self.data[self.batch].str[1:]

        return self.data

    def __some_other_stuff(self):
        self.data["other_stuff"] = self.data["num"].astype(int) * 100 / 2

        return self.data

    def main(self):
        self.data = self.__get_letter()
        self.data = self.__get_number()
        self.data = self.__some_other_stuff()
        # some more processing steps follow

        return self.data

class Screen2(IScreens):

    def __init__(self, data, config):
        self.data = data
        self.batch = config["cfg1"]

    def __some_cool_stuff(self):
        self.data["cool_other_stuff"] = self.data[self.batch] + "_COOOOL"

        return self.data

    def main(self):
        self.data = self.__some_cool_stuff()
        # some more processing steps follow

        return self.data

class ScreenFactory:

    def __init__(self):
        self._screens = {}

    def register_screen(self, screen_name, screen_class):
        self._screens[screen_name] = screen_class

    def get_screen(self, data, config):

        if "screen_indicator" not in config:
            raise AssertionError("Your config file does not include 'screen_indicator' key")

        screen = self._screens.get(config["screen_indicator"])
        if not screen:
            raise AssertionError("screen not implemented")
        return screen(data=data, config=config)

factory = ScreenFactory()

factory.register_screen(screen_name="s1", screen_class=Screen1)
config = {"screen_indicator": "s1", "cfg1": "ident"}
factory.get_screen(data=df, config=config).main()

factory.register_screen(screen_name="s2", screen_class=Screen2)
config = {"screen_indicator": "s2", "cfg1": "ident"}
factory.get_screen(data=df, config=config).main()
```

IScreens is a callable
Actually, your IScreens class (interface) contains only one abstract method (the main method), so this class is a kind of callable.
Instead of creating a specific class, you can use collections.Callable:
import collections

IScreens = collections.Callable

To use that class, you can do:
class Screen1(IScreens):
    def __init__(self, data, config):
          ...

    def __call__(self, *args, **kwargs):  # instead of `main`
        return ...

s1 = Screen1(data=..., config=...)
result = s1()  # call the callable s1

IScreens can contain common data
I see that the data and config variables are common to Screen1 and Screen2.
It's a good practice to factorize this in the base class:
class IScreens(collections.Callable):
    def __init__(self, data, config):
        self.data = data
        self.batch = config["cfg1"]

NOTE: Here, I don't understand why you pass a config dictionary, but only get the batch (config["cfg1"]).

Avoid extra complexity
Private methods with double underscores, like __get_letter, are more difficult to debug,
because their names are disguised into _{className}__{attr_name}.
For instance:
class C:
    def __init__(self):
        self.__secret = "foo"

obj = C()
print(obj._C__secret)
# 'foo'

Very short functions like __get_letter should be avoided, unless reused several times.
Here, you add complexity (and extra computation time) with them.
Getters should not mutate the instance
A getter should not change the class instance state.
Here, in your __get_letter method, you are extracting a value (self.data[self.batch].str[:1])
and modifying the internal state (self.data["campaign"]). Is it intentional?
It is better to initialize everything in your constructor:
class Screen1(IScreens):
    def __init__(self, data, config):
        self.data = data
        self.batch = config["cfg1"]
        self.data["campaign"] = self.data[self.batch].str[:1]
        self.data["num"] = self.data[self.batch].str[1:]
        self.data["other_stuff"] = self.data["num"].astype(int) * 100 / 2

Factory registration
There are two different ways to implement factories:

The factory (or the main function) knows all its classes and is responsible for the class registration,
Each class knows the factory and it can auto-register itself, like a plugin.

It seems that you choose the first case. You may also need to add a method to unregister.
In the second case, registration is usually final: its more difficult to unregister…
Both are good, so.
To register a class in a factory, we used to use the class name (or a class instance) instead of an arbitrary name.
Example using the class name:
class ScreenFactory:
    def __init__(self):
        self._screens = {}

    def register_screen(self, screen_class):
        self._screens[screen_class.__name__] = screen_class

    def get_screen_class(self, class_name: str):
        return self._screens[class_name]

Example using the class attribute:
class IScreens:
    kind = "unknown"

class Screen1(IScreens):
    kind = "s1"

class ScreenFactory:
    def __init__(self):
        self._screens = {}

    def register_screen(self, screen_class):
        self._screens[screen_class.kind] = screen_class

    def get_screen_class(self, kind: str) -> IScreens:
        return self._screens[kind]

The second method is interesting if you want to select an implementation (a IScreens class)
according to a tag/label extracted from textual data…

NOTE: you may have noticed that I added a get_screen_class method.

Factory access

This is the difficult part

In your get_screen method, you are checking if a specific configuration is valid (existence of "screen_indicator")
and if there is a suitable class which satisfy a second condition (config["screen_indicator"] is a kind of IScreens).
I am OK with that, but it can be simplified. Here is a solution with error handling:
class ScreenNotImplementedError(Exception):
    fmt = "Screen not implemented for this configuration: {config}"

    def __init__(self, *, config):
        msg = self.fmt.format(config=config)
        super(ScreenNotImplementedError, self).__init__(msg)

class ScreenFactory:
    ...

    def get_screen(self, data, config):
        try:
            kind = config["screen_indicator"]  # may raise KeyError
            cls = self.get_screen_class(kind)  # may raise KeyError
        except KeyError:
            raise ScreenNotImplementedError(config=config)
        else:
            screen = cls(data=data, config=config)
            return screen


Cleaning up an utterance using multiple regex substitutions I need to use regex for some stuff (mentioned in code comments) and wrote the following code, which works perfectly fine for my need, but I wanted to know if there was a way to improve this code?

Am I using re.compile correctly in this case?

import re

def regex(utterance):

    utterance = utterance.lower()

    # Replacing non ASCII characters with space
    message_ascii = re.compile(r'[^\x00-\x7F]+')
    message_ascii = message_ascii.sub(r' ', utterance)

    # If comma after number, replace comma with space
    message_comma_no = re.compile(r'(?<=[0-9]),')
    message_comma_no = message_comma_no.sub(r' ',message_ascii)

    # If comma after words, add space before and after
    message_comma_word = re.compile(r'(?<=[a-z]),')
    message_comma_word = message_comma_word.sub(r' , ',message_comma_no)

    # If "Dot and space" after word or number put space before and after
    message_dot = re.compile(r'(?<=[a-z0-9])[.] ')
    message_dot = message_dot.sub(r' . ',message_comma_word)

    # If any other punctuation found after word or number put space before and after
    message_punct = re.compile(r"(?<=[a-zA-Z0-9])(?=[?;!()'\"])|(?<=[?;!()'\"])(?=[a-zA-Z0-9])")
    message_punct = message_punct.sub(r' ', message_dot)

    # Remove Excess whitespaces
    message = ' '.join(message_punct.split())

    return message

If you use a regular expression once, you don't get any performance improvement from compiling it. You could just use re.sub directly.
If a string doesn't contain any special characters, there's no point in using a raw literal.
r' ' could be just ' '.
Using the same variable to represent different things is a bad practice. It confuses the people who read your code. It's not a good idea to do things like:

message_ascii = re.compile(r'[^\x00-\x7F]+')
message_ascii = message_ascii.sub(r' ', utterance)

because the same variable holds a compiled regex in the first line and it's reassigned to a string later on.

If you call this function multiple times and want to benefit from pre-compiled regular expressions, you could create a new class that compiles the expressions in its constructor and reuses them:

class TextProcessor:

    def __init__(self):
        # Initializes regular expressions here
        self.ascii_regex = re.compile(...)
        # Other expressions go here

    def process_text(self, text):
        ascii_text = self.ascii_regex.sub(' ', text)
        # The rest of the substitions go here


Adding cells to a table I'm calling the below 2 function once per Cell in a table with over 135 columns and over 200 rows (that is 134*200 = 26800 times, I'm not running the functions on the first column).

I would like to optimize the functions as much as possible.  Do you have any suggestions on how to do that, if it even possible?

this.addBodyCellFunction = function(row, index, func, params) {
    var cell = $(document.getElementById("table_row_" + row +"_col_" +index));
    cell.click(function() {
        func.apply(this, params);
    });
    cell.css("cursor", "pointer");
};

this.addBodyCellTooltip = function(row, index, text) {
    var cell = $(document.getElementById("table_row_" + row +"_col_" +index));
    cell.attr("title", text);
};

There a few options here, for instance you don't need to use Jquery as much as you are using. This will optimize greatly all of your code.
You can simply use JavaScript commands related to table with document.getElementById("table").rows[].cell[]
 which is very simple and works very nicely and fast. By doing that there  is no need to assign ID's to everything on the table, as you probably are doing now.


Can I further secure this blog post loading script? The following code is developed around a mysql database where the posts table has 3 columns: post_ID, date and title.

config.php holds the values for $dsn, $username, $password, $options.

The content for each post is held in a directory format:

year/month/day/(database column)title/content.txt

By querying the database, I retrieve the date, which lets me order the posts (newest first).

$new_date_dir formats the date to serve as the directory for the include link $link.

$new_date_vis formats the date to serve as the date for the post.

The file is called using include within another page which uses exterior formatting.

 <?php include $_SERVER['DOCUMENT_ROOT']."/resources/config.php";

try {

    $dbh = new PDO($dsn, $username, $password, $options);

    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

} catch (PDOException $e) {

    echo 'Connection failed: ' . $e->getMessage();

}

function orderBlogPost($dbh) {
$statement = $dbh->prepare("
SELECT date, title
FROM post
ORDER BY UNIX_TIMESTAMP(date) DESC
LIMIT 0,5
");

$statement->execute();

    $statement->bindColumn('date', $date);

    $statement->bindColumn('title', $title);

    while ($row = $statement->fetch(PDO::FETCH_BOUND)) {

        $new_date_dir = date("Y-m-d",strtotime($date));

        $new_date_vis = date("d-m-Y",strtotime($date));

        $new_row = preg_replace("[-]", "/", $new_date_dir);

        $link = $_SERVER['DOCUMENT_ROOT']."/content/".$new_row."/".$title."/content.txt";

       echo "
".$title."".$new_date_vis."

";
       include "$link";
    }

}
orderBlogPost($dbh);

$dbh = null

?>

Whilst, this is outputting my desired result, I would like to know the following:

Is this procedural code basically secure/usable?
Can you suggest further reading on refactoring?
How would I benefit from using OOP in this scenario?

secure?

not entirely: you emit a debug message to the user when something goes wrong when connecting to the database which may leak information about the server (consider a dumb PDO implementation that emits the $dsn, $options and $username in the error message).

Replace the echo with a forward to a 502 page and log the error message internally.
you also include a file from a text file repository and assume it doesn't contain any php code that may do Bad Things™ (while the $dbh is still in scope) or any XSS shenanigans.

You can use readFile to avoid the php code but not the XSS.

usable?

Is is that just emits a fixed number of titles in a fixed way. to do anything else you need to write a new function most of it will be copy and paste.

Instead just return an array with date-title pairs and let the calling code figure out how it should be displayed.

also add a parameter to allow the calling code to pass in a parameter for the number of records requested and at what point it should start to allow paging?

Besides that you store a date as a string in the database, let dates be a dates.

In fact the function itself is not necessary with how the code is used.

Make OOP? well you could create a querryable object that will return the aforementioned array and keeps the object handle private.


Making a semi-random order of an array, based on a string I made code to shuffle an array in a semi-random way. That is, it has to be random based on a string, but for the same string, it should return the same order.

Is there a better way of doing it?

function djbHash(s) {
    let hash = 5381;

    for (let c of s) {
        hash = hash * 33 + c.charCodeAt(0);
    }
    return hash;
}

function mapToValues(s, values) {
    return values[djbHash(s) % values.length];
}

var randomArray=[];
var myArray = [0,1,2,3,4,5,6,7,8,9];
var string  = "bigcat";
var length = myArray.length;
for(var i=0;i<length;i++){
    console.log(myArray)
  var item = mapToValues(string, myArray);
    var removed = myArray.splice(myArray.indexOf(item),1);
  console.log(removed);
  randomArray.push(removed[0]);
}
console.log(randomArray);

jsFiddle

Random numbers, hashes and precision.

Good hasher.

The hash function you use is very poor with it only producing unique hash values for the first 8 characters of a string.

The following words all produce the same hash

"characters","charactesr","charactezz" = 8246171808294230000

The reason is that numbers when used in computers have a limited precision. Once you go over that precision you can no longer add numbers that are smaller.

For example adding 100 to the hash number does nothing the num+100 === num returns true.

var num = 8246171808294230000
console.log(num + 100 === num); // >> true

To generate a good hash you need to limit the size of the hash number. This can easily be done by just using integer math and bitwise manipulation.

An improvement (not perfect) hasher

function stringTo32BitHash(str){
     const a = 14827;  // Use a prime
     var v = 0;
     for(var i = 0; i < str.length; i += 1){
         v = ((v*a) + str.charCodeAt(i))%0xFFFFFFFF;
     }
     return v;
}

Pseudo random shuffle.

Next your shuffle is not working very well. You are not providing enough variance in the selection of which array items to reorder. With very different hashes producing the very same shuffle.

For hash 8.97895922819539e+21 the result is 4,1,0,2,8,9,3,6,5,7
For hash 8.978959228225448e+21 the result is 4,1,0,2,8,9,3,6,5,7

The best way to use a hash to scramble a list is to use the hash as a seed for a pseudo random number generator. The seed ensures that the sequence of random numbers will be the same for the same seed. Unfortunately javascript does not provide a seed value that we can access for it's Math.random() function so we have to create our own.

The most basic pseudo random number generator is seed = (A * seed + B) % M where A,B,M are large integers. Each time the function is called a new seed is generated and return as a random value. The seed is also used as the seed for the next value.

Once you have a good sequence of random values you can use them to create a better shuffle.

Putting it all together.

Thus a rewrite of your code with a random number generator to do the shuffling and an improved string to hash function.

// creates a random number generator function.
function createRandomGenerator(seed){
     const a = 5486230734;  // some big numbers
     const b = 6908969830;
     const m = 9853205067;
     var x = seed;
     // returns a random value 0 <= num < 1
     return function(seed = x){  // seed is optional. If supplied sets a new seed
         x = (seed  * a + b) % m;
         return x / m;
     }
}
// function creates a 32bit hash of a string
function stringTo32BitHash(str){
     var v = 0;
     for(var i = 0; i < str.length; i += 1){
        v += str.charCodeAt(i) << (i % 24);
     }
     return v % 0xFFFFFFFF;
}
// shuffle array using the str as a key.
function shuffleArray(str,arr){
    var rArr = [];
    var random = createRandomGenerator(stringTo32BitHash(str));
    while(arr.length > 1){
        rArr.push(arr.splice(Math.floor(random() * arr.length), 1)[0]);
    }
    rArr.push(arr[0]);
    return rArr;
 }

The need to test.

At this point I am 60% happy but by just looking at the code you can not tell how reliable it is. You need to test the function over a large set of values.

I have created two test.

Testing the random number generator.

This is a simple test, and rather than test it for standard deviation I do a visual test by graphing the distribution of the random numbers generated. I compare them to the inbuilt random function. It can give a good indication of the randomness. Why? because it's more fun...

var createImage=function(w,h){var i=document.createElement("canvas");i.width=w;i.height=h;i.ctx=i.getContext("2d");return i;}
var canvas = createImage(512,200);
document.body.appendChild(canvas);
var ctx = canvas.ctx;
// creates a random number generator function.
function createRandomGenerator(seed){
     const a = 54862307348;  // some big numbers
     const b = 69089698305;
     const m = 98532050673;
     var x = seed;
     // returns a random value 0 <= num < 1
     return function(seed = x){  // seed is optional. If supplied sets a new seed
         x = (seed  * a + b) % m;
         return x / m;
     }
}

ctx.font="18px arial"
var rand = createRandomGenerator(10394);
var values = new Array(canvas.width);
var valuesR = new Array(canvas.width);
for(var i = 0; i < canvas.width; i ++){
    values[i]=0;
    valuesR[i]=0;
}
function testRandom(){
    for(var i = 0; i < 1000; i ++){
        var r = Math.floor(rand()*canvas.width);
        var r1 = Math.floor(Math.random()*canvas.width);
        values[r] += 1;
        valuesR[r1] += 1;
    }
    var max = 0;
    for(var i = 0; i < canvas.width; i ++){
        max = Math.max(max,values[i],valuesR[i]);
    }

    ctx.clearRect(0,0,512,200)
    ctx.strokeStyle = "blue"
    ctx.beginPath();
    ctx.moveTo(0,canvas.height - (values[0]/max)*canvas.height);
    for(var i = 1; i < canvas.width; i ++){
        ctx.lineTo(i,canvas.height - (values[i]/max)*canvas.height);
    }
    ctx.stroke();
    ctx.strokeStyle = "red"
    ctx.beginPath();
    ctx.moveTo(0,canvas.height - (valuesR[0]/max)*canvas.height);
    for(var i = 1; i < canvas.width; i ++){
        ctx.lineTo(i,canvas.height - (valuesR[i]/max)*canvas.height);
    }
    ctx.stroke();
    ctx.fillStyle = "red";
    ctx.fillText("Red builtin Math.random()",10,canvas.height - 40)
    ctx.fillStyle = "Blue";
    ctx.fillText("Blue custom random function",10,canvas.height - 20)

    setTimeout(testRandom,100);

}
testRandom()

  note You would need to do a proper test if you were to use this function in an enterprise situation. It looks good does not cut it in the real world.

Test the shuffle and hash

Next I test the number of repeated shuffled array results for unique words, and also test the hash function to see how often it repeats a hash. There is no perfect solution but the aim it to keep the number of matches as low as possible.

Note that if the test was left to run indefinitely the number of matches as a fraction of unique words would start to approch one. This is normal. The test counts how many unique words are required to end up with 200 matched shuffled arrays then displays the score as number of words. The greater the value the better the shuffle. For a perfect result and 10 array items you would expect a score of about 38141.

// creates a random number generator function.
function createRandomGenerator(seed){
     const a = 5486230734;  // some big numbers
     const b = 6908969830;
     const m = 9853205067;
     var x = seed;
     // returns a random value 0 <= num < 1
     return function(seed = x){  // seed is optional. If supplied sets a new seed
         x = (seed  * a + b) % m;
         return x / m;
     }
}
// function creates a 32bit hash of a string
function stringTo32BitHash(str){
     var v = 0;
     for(var i = 0; i < str.length; i += 1){
        v += str.charCodeAt(i) << (i % 24);
     }
     return v % 0xFFFFFFFF;
}
// shuffle array using the str as a key.
function shuffleString(str,arr){
    var rArr = [];
    var random = createRandomGenerator(stringTo32BitHash(str));
    while(arr.length > 1){
        rArr.push(arr.splice(Math.floor(random() * arr.length), 1)[0]);
    }
    rArr.push(arr[0]);
    return rArr;
 }

var resultsEl = document.createElement("div");
document.body.appendChild(resultsEl);
const chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
const array = [0,1,2,3,4,5,6,7,8,9];
// for an array of ten unquie values
function randomWord(maxLen){
    var wLen = Math.floor(Math.random() * (maxLen-7)) + 7;
    var str = "";
    while(wLen-- >0){
        str += chars[Math.floor(Math.random()*chars.length)];
    }
    return str;
}
var count = 0;
const resultSet = new Set();
const wordSet = new Set();
const hashSet = new Set();
var matches = 0;
var hashMatches = 0;
function testASet(){
    var c =0;
    while(c++ < 1000){
        var word = randomWord(10);
        if(!wordSet.has(word)){
            wordSet.add(word);
            var hash = stringTo32BitHash(word)
            if(hashSet.has(hash)){
                hashMatches += 1;
            }else{
                hashSet.add(hash);
            }
            var r = shuffleString(word,[...array]).join("");
            if(resultSet.has(r)){
                matches += 1;
                if(matches === 200){
                    break;
                }
            }else{
                resultSet.add(r);
            }
            count++;
        }
    }

    resultsEl.innerHTML = "Matches : " + matches + "
Hash matches : " + hashMatches+"
 Unique words : " + count;
    if(matches === 200){
        resultsEl.innerHTML += "
Test complete. Score : "+ count + " higher is better ";
        resultsEl.innerHTML += "
"+((count / 38141)*100).toFixed(0) + "% of ideal score.";
    }else{
        setTimeout(testASet,350);
    }
}
testASet()

The results of the test indicate that the function shuffle string is only 9% of the ideal situation. But if you look at the numbers you will see that there is a high rate of hash clashes. Finding a better hash function will improve the score, also I used words from 7 -10 characters long, which puts the function at a slight disadvantage.

If you are wondering how I got the ideal score, I did it with the following test that simulated the ideal shuffle.

var resultsEl = document.createElement("div");
document.body.appendChild(resultsEl);
var v = 3628800; // 10! (10 factorial)
var topV = 0;
var match = 0;
var count = 0;
var total = 0;
function test(){
    for(var i = 0; i < 40000; i++){
        var r  = Math.floor(Math.random()*v);
        if(r < topV){
            match += 1;
            if(match === 200){
                break;
            }
        }
        topV += 1;
    }
    resultsEl.textContent = "Test # : " + (count+1) + " score " +topV;
    if(match !== 200){
        setTimeout(test,100)
    }else{
        total += topV;
        count += 1;
        if(count === 100){
            resultsEl.textContent = "Mean score is : " + (total/count).toFixed(0);
        }else{
            topV = 0;
            match = 0;
            setTimeout(test,100);
        }

    }
}

test();

Improvements.

In the end the weak point is the hash generator. Using a 64bit hash will improve the result. But that will not get much more. Using the characters of the word  as an integer will give a perfect hash, but then you will have numbers with bit counts as long as the string * 8, and the random number generator would have to be able to accommodate these large integers as seeds. It leaves the question, how many bits long does the number type have to be for  the fnction to get within 99% of ideal?


Extracting numbers before colon I need to extract all numbers before a colon in a string, e.g.:

abc1234:blah -> 1234
5n63:124 -> 63
stringwithnocolon -> ""
stringwith:nonumber -> ""

Limitations:

no external dependency (looking forward to keep it simple)
no << operator allowed
not C++11

Here is the code I came up with:

const std::string DNAStorage::get_block(const std::string& name)
{
    char* buffer = new char[name.size()];
    char* temp = new char[name.size()];
    size_t i, j;
    for (i = 0; i < name.size(); i++)
    {
        if (name[i] == ':')
            break;

        buffer[i] = name[i];
    }

    if (i == name.size())
        return ""; // : not found, cannot be done

    for (j = i - 1; i > 0; j--)
    {
        if (!isdigit(buffer[j]))
            break;

        temp[i - j - 1] = buffer[j];
    }

    temp[i - j - 1] = '\0';

    std::string result = std::string(temp);
    std::reverse(result.begin(), result.end());
    return result;
}

But I don't really like it. How could I make this code better?

Jamal's answer already covers some of this, but I wanted to provide slightly different reasons for some of his advice.

1. Return an std::string rather than a const std::string.

Returning an std::string makes your function more useful to a caller because it gives them more options.

They can do this:

std::string s1 = DNAStorage::get_block ("abc1234:blah") ;

Or this:

const std::string s1 = DNAStorage::get_block ("abc1234:blah") ;

Or even this:

const std::string &s1 = DNAStorage::get_block ("abc1234:blah") ;

Unless you are trying to force some kind of interface onto the caller, there isn't really any good reason (from a programming perspective) to return a const std::string.

2. Cut down on copying.

You copy a substring of name into buffer, then a substring of buffer into into temp, and then you copy temp into result. This is unnecessary. See section 4 of this post for an example that avoids all of this copying.

3. Avoid memory leaks and focus on exception safety.

You dynamically allocate memory for two character arrays. If you actually needed a buffer (which you don't), you could just do something like this:

std::string buffer (name.size (), ' ') ;

4. Here's an example using std::find() and iterators.

Edit: 200_success was kind enough to provide a more concise version:

std::string DNAStorage::get_block (const std::string &name)
{
    typedef std::string::const_iterator iter ;

    iter colon = std::find (name.begin (), name.end (), ':') ;

    if (colon == name.end ()) {
        return "";
    }

    iter start = colon ;

    while (start != name.begin () && std::isdigit (*(start - 1))) {
        --start ;
    }

    return std::string (start, colon) ;
}

Test cases:

int main ()
{
    const std::string s1 = DNAStorage::get_block ("abc1234:blah") ;
    const std::string s2 = DNAStorage::get_block ("5n63:124") ;
    const std::string s3 = DNAStorage::get_block ("nocolon") ;
    const std::string s4 = DNAStorage::get_block ("nonumber:ffff") ;
    const std::string s5 = DNAStorage::get_block ("5363:124") ;
    const std::string s6 = DNAStorage::get_block ("v32463:124") ;

    return 0 ;
}

5. Here's an example with std::reverse_iterator.

First we have to create a unary predicate for std::find_if.

bool is_not_digit (char c)
{
    return std::isdigit (c) == 0 ;
}

And here's how we can code the function:

std::string DNAStorage::get_block (const std::string &name)
{
    typedef std::string::const_iterator const_iterator ;
    const const_iterator colon_position = std::find (name.begin (), name.end (), ':') ;

    if (colon_position == name.end ()) {
        return "" ;
    }

    std::reverse_iterator <const_iterator> rstart (colon_position) ;
    std::reverse_iterator <const_iterator> rend = std::find_if (rstart, name.rend (), is_not_digit) ;

    std::string out (rstart, rend) ;
    std::reverse (out.begin (), out.end ()) ;
    return out ;
}


MySQL homeowner database This database must be able to track multiple home owners, who may own many properties, have many phone numbers, and their mailing address may depend on what month we're in. Each property may have more than one owner.

Please point out all bad coding practices.

create table home_owner
(
    owner_id        SMALLINT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    first_name      VARCHAR(10),
    last_name       VARCHAR(20)
);

create table email
(
    email_id        SMALLINT UNSIGNED PRIMARY KEY,
    email_address   VARCHAR(40)
);

create table owner_email
(
    owner_id        SMALLINT UNSIGNED,
    email_id        SMALLINT UNSIGNED,
    PRIMARY KEY emails_PK(owner_id, email_id),
    CONSTRAINT owner_email_FK FOREIGN KEY(owner_id) REFERENCES    home_owner(owner_id) ON DELETE CASCADE,
    CONSTRAINT email_owner_FK FOREIGN KEY(email_id) REFERENCES email(email_id) ON DELETE CASCADE
);

create table phone_number
(
    number_id       SMALLINT UNSIGNED PRIMARY KEY,
    phone_number    VARCHAR(10)
);

create table owner_number
(
    owner_id        SMALLINT UNSIGNED,
    number_id       SMALLINT UNSIGNED,
    PRIMARY KEY numbers_PK(owner_id, number_id),
    CONSTRAINT owner_number_FK FOREIGN KEY(owner_id) REFERENCES         home_owner(owner_id) ON DELETE CASCADE,
    CONSTRAINT number_owner_FK FOREIGN KEY(number_id) REFERENCES phone_number(number_id) ON DELETE CASCADE
);

create table seasonal_address
(
    seasonal_id     SMALLINT UNSIGNED PRIMARY KEY,
    address         VARCHAR(50),
    months          VARCHAR(27)
);

create table contact_info
(
    owner_id        SMALLINT UNSIGNED,
    seasonal_id     SMALLINT UNSIGNED,
    PRIMARY KEY contact_PK(owner_id, seasonal_id),
    CONSTRAINT owner_info_FK FOREIGN KEY(owner_id) REFERENCES home_owner(owner_id) ON DELETE CASCADE,
    CONSTRAINT seasonal_info_FK FOREIGN KEY(seasonal_id) REFERENCES seasonal_address(seasonal_id) ON DELETE CASCADE
);

create table property
(
    property_id     SMALLINT UNSIGNED PRIMARY KEY,
    address         VARCHAR(50),
    lot_number      SMALLINT UNSIGNED
);

create table owner_property
(
    owner_id        SMALLINT UNSIGNED,
    property_id     SMALLINT UNSIGNED,
    PRIMARY KEY properties_PK(owner_id, property_id),
    CONSTRAINT owner_property_FK FOREIGN KEY(owner_id) REFERENCES home_owner(owner_id) ON DELETE CASCADE,
    CONSTRAINT property_owner_FK FOREIGN KEY(property_id) REFERENCES property(property_id) ON DELETE CASCADE
);

insert into home_owner (first_name, last_name) values
("Angel", "Flop"),
("Bob", "Hoe"),
("Sue", "Hoe");

insert into email values
(1, "angel@hio.com"),
(2, "bob@ioj.com"),
(3, "sue@iojoiv.com");

insert into property values
(1, "123 Rainey", 123),
(2, "234 Bob", 1298),
(3, "697 Kolp", 782);

insert into seasonal_address values
(1, "7667 Noob", "1,2,3"),
(2, "2383 Fob", "4,5,6,7,8,9,10,11,12"),
(3, "7823 Flower", "1,2,3,4,5,6,7,8,9,10,11,12");

insert into phone_number values
(1, "5203601083"),
(2, "8039023093"),
(3, "2387784334"),
(4, "2377823782");

insert into owner_number values
(1, 1),
(1, 2),
(2, 3),
(3, 4);

insert into contact_info values
(1, 1),
(1, 2),
(2, 3),
(3, 3);

insert into owner_property values
(1, 1),
(1, 2),
(2, 3),
(3, 3);

insert into owner_email values
(1, 1),
(2, 2),
(3, 3);

select af.property_id
from property af
inner join owner_property op
on af.property_id = op.property_id
where op.owner_id = 1; //find all properties of owner_id = 1

select a.*
from home_owner a
inner join owner_property op
on a.owner_id = op.owner_id
where op.property_id = 3; //find all owners of property_id = 3

Table definitions here.

For the table definitions, just use an INT for the keys.  You probably aren't saving anything by using SMALLINTs.  Schema changes tend to be a pain in larger projects, so you might as well try to avoid any possibility of trouble.

CREATE UNIQUE INDEX index_PK ON owner_property(property_id, owner_id); is redundant, since the table already has PRIMARY KEY properties_PK(owner_id, property_id).

The foreign key constraints look well done.

Having separate tables just for email and phone_number is probably overkill.  The e-mail addresses and phone numbers could just be attributes on the owner_email and owner_number tables, respectively.  I would put NOT NULL constraints on both of those attributes.  The phone_number field should probably be longer than VARCHAR(10).  (Note that MySQL tends to silently discard the part of the string beyond the length limit.)

Similarly, VARCHAR(50) for the property address is probably too stingy.

For your first query…

select af.property_id
from property af
inner join owner_property op
on af.property_id = op.property_id
where op.owner_id = 1; //find all properties of owner_id = 1

… you don't need to join anything at all.

select property_id
    from owner_property op
    where owner_id = 1;

For your second query…

select a.*
from home_owner a
inner join owner_property op
on a.owner_id = op.owner_id
where op.property_id = 3; //find all owners of property_id = 3

Use better indentation.  SELECT has a FROM clause and a WHERE clause.  FROM has an INNER JOIN.  INNER JOIN has an ON.
Use a table alias that makes better sense (or don't use an alias at all).

select o.*
    from home_owner o
        inner join owner_property op
            on o.owner_id = op.owner_id
    where op.property_id = 3; //find all owners of property_id = 3


Parsing multiple line records using Java 8 Streams There was a suggestion in this StackOverflow question to move this question here. I added some more information and restructured the question a bit.

I'm trying to parse the following file which contains information in the following format:

  TABLE_NAME

  VARIABLE_LIST_OF_COLUMNS

  VARIABLE_NUMBER_OF_ROWS (Seperated by a tab seperator)

An example (using ',' as the seperator for the question; actual seperator is a tab):

  STUDENTS

  ID

  NAME

  1,Mike

  2,Kimberly

The idea is to build a list of insert sql statements (context for the code snippet). So the output would be (ignore numeric/null values for now):

  INSERT INTO STUDENTS (ID, NAME) VALUES ('1','Mike');

  INSERT INTO STUDENTS (ID, NAME) VALUES ('2','Kimberly');

What I want to know is whether this kind of multiline parsing is at all possible using java 8 streams API? This is what I have at the moment:

public final class StatementGeneratorMain {

    public static void main(final String[] args) throws Exception{
        List<String> fileNames = Arrays
            .asList("STUDENTS.txt");
        fileNames.stream()
            .forEach(fileName -> {
                String tableName;
                List<String> columnNames;
                List<String[]>  dataRows;
                try (BufferedReader br = getBufferedReader(fileName)) {
                    tableName = br.lines().findFirst().get();
                } catch (Exception e) {
                    throw new RuntimeException(e);
                }

                try (BufferedReader br = getBufferedReader(fileName)) {
                    //skip the first line because its been processed.
                    columnNames = br.lines().skip(1).filter(v -> v.split("\t").length == 1).collect(toList());
                } catch (Exception e) {
                    throw new RuntimeException(e);
                }

                try (BufferedReader br = getBufferedReader(fileName)) {
                    //skip the first line and the columns length to get the data
                    //columns are identified as being splittable on the delimiter
                    dataRows = br.lines().skip(1 + columnNames.size()).map(s -> s.split("\t"))
                        .collect(toList());
                } catch (Exception e) {
                    throw new RuntimeException(e);
                }

                String columns = columnNames.stream().collect(joining(",","(",")"));

                List<String> dataRow = dataRows.stream()
                    .map(arr -> Arrays.stream(arr).map(x -> "'" + x + "'").collect(joining(",", "(", ")")))
                    .map(row -> String.format("INSERT INTO %s %s VALUES %s;", tableName, columns, row))
                    .collect(toList());

                dataRow.forEach(l -> System.out.println(l));
            });
    }

    private static BufferedReader getBufferedReader(String fileName) {
        return new BufferedReader(new InputStreamReader(StatementGeneratorMain.class.getClassLoader().getResourceAsStream(
            fileName)));
    }
}

This piece of code does the job for me, but I don't really like it because I read the same file thrice (once for table name, again to deduce the columns, again to get the rows). I also don't think that it is proper functional style.

What I am looking for is a more elegant way to do this kind of multiline/multirecord parsing using the streams API.

What I thought about:

Using Files.lines() instead of BufferedReader.
Using partitioningBy() instead of filter().collect(toList()) when parsing the columns, then true values goes into the column list and false values goes into the dataRows list.

I'm not too particular about stuff like numeric column and null values at this point.

Simply put, you only need a Consumer<String>-implementing class that knows what lines to read as the table name, the columns and rows.

public class StatementGenerator implements Consumer<String> {

    private static String START = " (";
    private static String END = ") ";
    private static String ROW_DELIMITER = "\n";

    private String delimiter = "\t";
    private String parameter = "%";
    private ObjIntConsumer<List<String>> parameterSupplier =
            (list, i) -> { list.add(parameter); };
    private String table = null;
    private List<String> columns = new ArrayList<>();
    private List<String> rows = new ArrayList<>();

    public void setDelimiter(String delimiter) {
        this.delimiter = delimiter;
    }

    public void setParameter(String parameter) {
        this.parameter = parameter;
    }

    public void setParameterSupplier(ObjIntConsumer<List<String>> parameterSupplier) {
        this.parameterSupplier = parameterSupplier;
    }

    @Override
    public void accept(String t) {
        if (table == null) {
            table = t;
            return;
        }
        if (!t.contains(delimiter)) {
            columns.add(t);
            return;
        }
        rows.add(t);
    }

    public String getTableName() {
        return table;
    }

    public List<String> getColumns() {
        return columns;
    }

    public List<String> getRows() {
        return rows;
    }

    public String getParameterizedStatement() {
        StringBuilder result = new StringBuilder("INSERT INTO ");
        result.append(constructSegment(getTableName(), getColumns())).append(
                constructSegment("VALUES", IntStream.rangeClosed(1, getColumns().size())
                        .collect(ArrayList::new, parameterSupplier, List::addAll)));
        return result.append(";").toString();
    }

    public List<String> getRawStatements() {
        String placeholderStatement = getParameterizedStatement()
                .replaceAll(Pattern.quote(parameter), "%s");
        return getRows().stream().map(r -> String.format(placeholderStatement,
                    Pattern.compile(delimiter).splitAsStream(r)
                        .map(v -> "'" + v + "'").toArray()))
                .collect(Collectors.toList());
    }

    public String getFullStatement() {
        return getRawStatements().stream().collect(Collectors.joining(ROW_DELIMITER));
    }

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

    private static String constructSegment(String prefix, List<String> list) {
        return prefix + list.stream().collect(Collectors.joining(", ", START, END));
    }
}

In my sample implementation above, I've made the delimiter configurable, but there's one other configuration I want to highlight - parameter.

SQL injection

The sample implementation provides a getParameterizedStatement() as the starting point, because by right you should let your database driver handle the escaping of quotes (and possibly other magic values). We certainly do not need Little Bobby Tables to hang around here.

The parameter list is driven by an ObjIntConsumer, because if I'm not mistaken, there are certain drivers allowing for an index-based parameter substitution. That will mean you can potentially override parameterSupplier with something like (list, i) -> { list.add("::" + i); }; (or whatever the placeholder format is).

With that said, assuming you can absolutely trust your file-based input (i.e. sanitized inputs), we can then proceed with using getRawStatements(), which performs a simple substitution on the parameter with our String.format()'s "%s" placeholder. This is more aligned with what you are asking for. getFullStatement() simply concatenates all the rows into a single String, if that is what you actually require.

Here's what I used in my main() code:

public class StatementGeneratorMain {

    public static void main(String[] args) throws IOException, URISyntaxException {
        StatementGenerator generator = new StatementGenerator();
        generator.setDelimiter(",");
        try (Stream<String> lines = Files.lines(Paths.get(
                ClassLoader.getSystemResource("sqlRecords.txt").toURI()))) {
            lines.forEach(generator);
        }
        System.out.println(generator.getTableName());
        System.out.println(generator.getColumns());
        System.out.println(generator.getParameterizedStatement());
        generator.getRawStatements().forEach(System.out::println);
    }
}

And the sample output:

STUDENTS
[ID, NAME]
INSERT INTO STUDENTS (ID, NAME) VALUES (%, %) ;
INSERT INTO STUDENTS (ID, NAME) VALUES ('1', 'Mike') ;
INSERT INTO STUDENTS (ID, NAME) VALUES ('2', 'Kimberly') ;


Downloading images using CURL I've written this class that downloads an images, the URLs for which are read from a local .txt file. I'm keen to make sure my code is safe and efficient.

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include<cassert>
#include <algorithm>
#include <numeric>
#include <functional>
#include <curl/curl.h>

class DownloadTesting
{
  private:
    CURL *curl;
    FILE *file;
    CURLcode response;
    char outfilename[FILENAME_MAX];
    int count {0};
    std::vector<char*> URLS; //Stores the urls
    auto DownloadImage(const char *url) -> void;
    auto Convert(const std::string &s) -> char*;
    auto ReadDataStore() -> void;
    static auto WriteData(void *ptr, size_t size, size_t nmemb, FILE *stream) -> size_t;
  public:
    DownloadTesting();
    ~DownloadTesting();
    auto Interface() -> void;
};

DownloadTesting::DownloadTesting(){};

DownloadTesting::~DownloadTesting()
{
   if(!URLS.empty()) //Delete raw pointers
   {
     std::for_each(URLS.begin(),URLS.end(),[](char* url){delete url;});
   }
}

auto DownloadTesting::WriteData(void *ptr, size_t size, size_t nmemb, FILE *stream) -> size_t //Write data into file
{
   size_t written {fwrite(ptr, size, nmemb, stream)};
   return written;
}

auto DownloadTesting::Convert(const std::string &s) -> char* //Convert url from std::string to raw char pointer
{
  char *pc {new char[s.size()+1]};
  std::strcpy(pc,s.c_str());
  return pc;
}

auto DownloadTesting::ReadDataStore() -> void
{
  std::vector<std::string> storage_vector;
  std::string storage_string;
  std::ifstream storage_file {"./imagestore/urlstore.txt"};
  if(storage_file.is_open()) //Open local file
  {
     while(getline (storage_file,storage_string)) //Read each line
     {
       storage_vector.push_back(storage_string); //Storage each line in vector
     }
     storage_file.close();
  }
  std::transform(storage_vector.begin(), storage_vector.end(), std::back_inserter(URLS), [this](const std::string url){return this->Convert(url);}); //Perform conversion function on each url
}

auto DownloadTesting::DownloadImage(const char *url) -> void
{
  count++; //Use a counter to name each downloaded file
  std::string name {std::to_string(count)}; //Convert int to string
  strcpy(outfilename, name.c_str()); //Convert std::string to [char]

  curl = curl_easy_init(); //Initialise curl
  if(curl)
  {
    file = fopen(outfilename,"wb");
    curl_easy_setopt(curl, CURLOPT_URL, url);
    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &DownloadTesting::WriteData);
    curl_easy_setopt(curl, CURLOPT_WRITEDATA, file);
    response = curl_easy_perform(curl);
    curl_easy_cleanup(curl); //Clean up curl
    fclose(file); //Close file
  }
}

auto DownloadTesting::Interface() -> void
{
  this -> ReadDataStore(); //Read in the data
  assert(!URLS.empty()); //Ensure data is not empty
  std::for_each(URLS.begin(),URLS.end(), [this](const char* url){return this -> DownloadImage(url);}); //Perform download for each url
}

int main()
{
  DownloadTesting DTO;
  DTO.Interface();
}

Language Features

You are trying to use new C++ language features, but you appear to be using them inefficiently.

This:

std::for_each(URLS.begin(),URLS.end(), [this](const char* url){return this -> DownloadImage(url);});

can be replaced by a simple:

for (auto& url : URLS) DownloadImage(url)

Using auto for method declarations only to specify the return type at the end of the line is confusing.

Memory Management

Rule of 5: a class which needs a custom destructor for dealing with resource (de)allocations, most likely also needs a custom copy constructor, assignment operator and move constructor/assignment operator.

You are creating raw copies of strings only to store raw pointers to the characters. Simply use string.c_str() when you need to pass a raw pointer to the library and let the string class manage the memory.

CURL

Writing to the file via the callback and also asking curl to write the data to a file looks like duplicate work. Any reason for this?

OOP

The class isn't a good example of object-oriented design. The only available public method, Interface, doesn't indicate what it might do. The entire code might have been better off as a single function.


Bash pass multiple commands to SSH and make readable I have a script that sends multiple commands after logging in a remote machine via SSH.

Problem is, it's barely readable. I've tried quoting everything with "", but it's terrible (everything appears in pink), and here documents are just as bad (everything appears in gray). I'm using Gedit as an editor, but I tried Emacs as well.

Can I make the editor parse the remotely executed code as code?

#!/bin/bash

function remoteExec()
{
    echo "remote execution starting"
    ssh -n "$1" << 'EOF'
#the following code is _not_ highlighted by real editors
currIt=\"$2\" #I hope this parameter is passed correctly
while [ "$currIt" -lt 5 ]; do
    echo "hello"
    currIt=$(($currIt + 1))
done
EOF
}

I'd rather not create a separate script just for these lines, because then I'd have to copy it on the remote machine, or pipe it, and I have arguments too.

EDIT: as Glenn noticed, as far as highlighting goes, there isn't much to do. I'm open to solutions putting the code in a separate script. I need to transfer the code to the remote host then, and pass it the necessary arguments.

You're quoting the heredoc terminator, so $2 will certainly not be expanded

When you want to pass multiple commands to ssh, wrap them as a single script to an interperter, thus ssh sees one single command.

Lots of whitespace for readability.

And yes, not much opportunity for syntax highlighting.

Since you need variable expansion within the script, you need to worry about quoting variables you want to be expanded on the remote host. I changed the while loop to a for loop to minimize the number of $-signs to escape.

If the script you want to send is more complex, be careful about how you use single and double quotes.

function remoteExec()
{
    echo "remote execution starting"
    ssh -n "$1" << EOF
        bash -c '
            for (( currIt="$2"; currIt < 5; currIt++ )); do
                echo "hello"
            done
        '
EOF
}

Given an arbitrary number of args:

function remoteExec()
{
    local host=$1
    local startIt=$2
    local quotedArgs
    shift 2
    for arg in "$@"; do
        quotedArgs+="\"$arg\" "
    done

    echo "remote execution starting"
    ssh -n "$host" << EOF
        bash -c '
            for (( currIt="$startIt"; currIt < 5; currIt++ )); do
                echo "hello"
            done
            args=( $quotedArgs )
            printf "%s\n" "\${args[@]}"   # escape this sigil
        '
EOF
}

remoteExec host 3 arg1 arg2 "this is arg3"


Idiomatic way of handling optional values in Swift I'm new to iOS development and also Swift. I'm working on a project consisting of a menu that leads to my various experimentations with the iOS SDK.

I've written the menu titles and segueIdentifiers in a plist and I'm populating an array of NavigationItem in viewDidLoad().

Is the following code idiomatic, particularly in the way it handles optional values? What might be better?

override func viewDidLoad() {
        super.viewDidLoad()

        let path = NSBundle.mainBundle().pathForResource("NavigationMenu", ofType: "plist")

        if path == nil{
            NSLog("Unable to locate file : NavigationMenu.plist")
            return
        }

        let items = NSArray(contentsOfFile: path!)

        if items == nil {
            NSLog("Navigation menu items could not be read from plist.")
            return
        }

        for var i = 0 ; i < items!.count ; i++ {

            let item : NSDictionary = items!.objectAtIndex(i) as! NSDictionary
            navigationItems.append(NavigationItem(
                name:(item.objectForKey("name") as! String),
                andSegueIdentifier: (item.objectForKey("segueIdentifier") as! String)
                ))

        }

    }

A better way to handle optional values is "optional binding":

if let path = NSBundle.mainBundle().pathForResource("NavigationMenu", ofType: "plist") {
    // do something with `path`
} else {
    NSLog("Unable to locate file : NavigationMenu.plist")
}

which tests and unwraps the optional result as a single action.
Inside the if-block, path is the unwrapped String.

You can do the same with the items:

if let path = NSBundle.mainBundle().pathForResource("NavigationMenu", ofType: "plist") {
    if let items = NSArray(contentsOfFile: path) {
        // do something with `items` ...
    } else {
        NSLog("Navigation menu items could not be read from plist.")
    }
} else {
    NSLog("Unable to locate file : NavigationMenu.plist")
}

As of Swift 1.2, multiple optional bindings can be combined in
a single if-statement, which reduces the number of nested if-levels:

if
    let path = NSBundle.mainBundle().pathForResource("NavigationMenu", ofType: "plist"),
    let items = NSArray(contentsOfFile: path) {
        // do something with `items` ...
} else {
    NSLog("Unable to load NavigationMenu")
}

The downside is that you have only a common else-block for the
error condition and cannot distinguish which of the optional bindings
failed.

Instead of objectAtIndex() and objectForKey() you can use
subscripting for NSArray and NSDictionary:

for var i = 0 ; i < items!.count ; i++ {
    let item = items![i] as! NSDictionary
    let navItem = NavigationItem(name: item["name"] as! String,
        andSegueIdentifier: item["segueIdentifier"] as! String)
    navigationItems.append(navItem)
}

But a better way is to cast the NSArray to a Swift array of Swift dictionaries in the first step.
Then you can use array enumeration and don't need any casts later:

if
    let path = NSBundle.mainBundle().pathForResource("NavigationMenu", ofType: "plist"),
    let items = NSArray(contentsOfFile: path) as? [[String : String]] {

        for item in items {
            let navItem = NavigationItem(name: item["name"]!, andSegueIdentifier: item["segueIdentifier"]!)
            navigationItems.append(navItem)
        }
} else {
    NSLog("Unable to load NavigationMenu")
}

This assumes that values for the "name" and "segueIdentifier" are
present (and will crash at runtime otherwise). Alternatively, use
optional binding again:

for item in items {
    if let name = item["name"], ident = item["segueIdentifier"] {
        let navItem = NavigationItem(name: name, andSegueIdentifier: ident)
        navigationItems.append(navItem)
    } else {
        NSLog("Invalid navigation item")
    }
}

Finally, you could replace the inner loop by a map() operation:

if
    let path = NSBundle.mainBundle().pathForResource("NavigationMenu", ofType: "plist"),
    let items = NSArray(contentsOfFile: path) as? [[String : String]] {
        navitationItems = map(items) {
            NavigationItem(name: $0["name"]!, andSegueIdentifier: $0["segueIdentifier"]!)
        }
} else {
    NSLog("Unable to load NavigationMenu")
}


Effective way to find two string are anagrams My approach for checking anagrams is:

step 1: Remove all the space from both input strings.

step 2: Sort both of the strings.

step 3: Return false if the lengths differ.

step 4 : Return true if all char matches.

#include<iostream>
#include<set>
#include<string>
#include <algorithm>

bool anagrams(std::string usr1,std::string usr2)
{
        if(usr1.length()==usr2.length())
        {

                for(std::string::size_type pos = 0 ; pos<= usr1.length()-1 ; ++pos)
                {
                        if(pos==usr1.size()-1)
                        {
                                if(usr1[pos]==usr2[pos])
                                return true;
                        }

                        if(usr1[pos]==usr2[pos])
                        {
                                continue ;
                        }
                }

        }
        return false;
}

int main()
{
        std::string userInput1;
        std::string userInput2;

        std::getline(std::cin,userInput1);
        std::getline(std::cin,userInput2);

        std::string::iterator end_pos1 = std::remove(userInput1.begin(),userInput1.end(),' ');
        userInput2.erase(end_pos1,userInput1.end());

        std::string::iterator end_pos2 = std::remove(userInput2.begin(),userInput2.end(),' ');
        userInput1.erase(end_pos2,userInput2.end());

        std::sort(userInput1.begin(),userInput1.end());
        std::sort(userInput2.begin(),userInput2.end());

        if(userInput1.empty() || userInput2.empty())
                return 0;
        if(anagrams(userInput1,userInput2))
                std::cout<<"String is anagrams"<<"\n";
        else
                std::cout<<"String  not  anagrams"<<"\n";

        return 0;
}

anagram seems like a misnomer. A better name for such function would be is_anagram.
I don't see why <set> is included.
Fail early:

if (usr1.length() != usr2.length()) {
    return false;
}

spares you a level of indentation.
Test for pos==usr1.size()-1 is waste of time, because it fails at every iteration except the last one, and still tests for characters at pos.
Sorting inputs is an essential part of the algorithm, and hence shall be performed inside anagram. As written, anagram only tests two string for being identical.
On the other hand, removing spaces does not belong to the core algorithm, and you correctly let the caller do it. You may also want to  remove punctuation, and maybe convert the strings to the lower case. This logic indeed belongs to a caller.
Since you already #include <algorithm>, an std::equal(usr1.begin(), usr1.end(), usr2.begin()) looks more C++ish (if you are open to C++14, a variant std::equal(usr1.begin(), usr1.end(), usr2.begin(), usr2.end()) even spares the test for equal length).

All that said,

    bool is_anagram(std::string s1, std::string s2)
    {
        std::sort(s1.begin(), s1.end());
        std::sort(s2.begin(), s2.end());
        return std::equal(s1.begin(), s1.end(), s2.begin(), s2.end());
    }

As mentioned in comments, an alternative implementation using histograms is very appealing.


How can I reduce the number of nested if statements in this code? The application I'm building can accept two types of updates: Application Update and Parameter Updates. If there is an application update, then parameter updates can be ignored. This is the code that I've written, and it's messy.  How can I clean this up?

mContext = this.getApplicationContext();
//experimental();
//check for updates
UpdateMode updateInstallationMode = Parameters.load(mContext).getUpdateMode();
if(updateInstallationMode==UpdateMode.MANUAL)
{
    //User will manually install update - no further action needed to be performed by activity.
    //proceed to next screen.
    gotoNextActivity();
}else
{
    boolean shouldConfirm = (updateInstallationMode == UpdateMode.CONFIRM);

    lblStatus.setText(getString(R.string.checking_for_updates));
    Error outUpdateError = new Error();
    Map<String,String> availableUpdates = checkForUpdates(outUpdateError);

    boolean appUpdatesAvailable = availableUpdates.containsKey("application");
    boolean paramUpdatesAvailable = availableUpdates.containsKey("parameters");
    boolean appUpdatesAccepted = Parameters.load(mContext).getUpdateType()==UpdateType.BOTH
            || Parameters.load(mContext).getUpdateType()==UpdateType.APPLICATION;
    boolean paramUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
            || Parameters.load(mContext).getUpdateType() == UpdateType.PARAMETERS;

    if(appUpdatesAccepted && appUpdatesAvailable){
        boolean shouldInstall = shouldConfirm? confirmInstallUpdate(UpdateType.APPLICATION):true;
        if(shouldInstall){
            downloadUpdate(UpdateType.APPLICATION,availableUpdates.get("application"));
        }else{
            gotoNextActivity();
        }

    }else if(paramUpdatesAccepted && paramUpdatesAvailable){
        boolean shouldInstall = shouldConfirm? confirmInstallUpdate(UpdateType.PARAMETERS):true;
        if(shouldInstall){
            downloadUpdate(UpdateType.PARAMETERS,availableUpdates.get("parameters"));
        }else{
            gotoNextActivity();
        }
    }else{
        lblStatus.setText(getString(R.string.app_up_to_date));
        gotoNextActivity();
    }
}

I assume that the code in the question is a method. If not extract it out to a method.

boolean appUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
        || Parameters.load(mContext).getUpdateType() == UpdateType.APPLICATION;
boolean paramUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
        || Parameters.load(mContext).getUpdateType() == UpdateType.PARAMETERS;

I'd start with removing some duplication here.

UpdateParameters updateParameters = Parameters.load(mContext);

...

final UpdateType updateType = updateParameters.getUpdateType();
boolean appUpdatesAccepted = updateType == UpdateType.BOTH
        || updateType == UpdateType.APPLICATION;
boolean paramUpdatesAccepted = updateType == UpdateType.BOTH
        || updateType == UpdateType.PARAMETERS;

I'm not familiar with Android, so UpdateParameters and UpdateType types could be something else.
If you reorder the boolean variables you can find some more duplication:

final boolean appUpdatesAvailable = availableUpdates
    .containsKey("application");
final boolean appUpdatesAccepted = updateType == UpdateType.BOTH
    || updateType == UpdateType.APPLICATION;

final boolean paramUpdatesAvailable = availableUpdates
    .containsKey("parameters");
final boolean paramUpdatesAccepted = updateType == UpdateType.BOTH
    || updateType == UpdateType.PARAMETERS;

This could be extracted out to a method:

private boolean isUpdateAvailable(
        final Map<String, String> availableUpdates,
        final UpdateParameters updateParameters,
        final UpdateType updateType, final String updateKey) {
    final UpdateType availableUpdateType = updateParameters
        .getUpdateType();
    final boolean appUpdatesAvailable = availableUpdates
        .containsKey(updateKey);
    final boolean appUpdatesAccepted =
        availableUpdateType == UpdateType.BOTH
        || availableUpdateType == updateType;
    return appUpdatesAvailable && appUpdatesAccepted;
}

Usage:

final boolean tryApplicationUpdate =
    isUpdateAvailable(availableUpdates, updateParameters,
        UpdateType.APPLICATION, "application");
final boolean tryParametersUpdate =
    isUpdateAvailable(availableUpdates, updateParameters,
        UpdateType.PARAMETERS, "parameters");
if (tryApplicationUpdate) {
    ...
} else if (tryParametersUpdate) {
    ...
} else {
    ...
}

It's also possible to extract out the first two if blocks to a method, they are very similar to each other:

private void downloadUpdateIfConfirmed(
        final boolean shouldConfirm,
        final Map<String, String> availableUpdates,
        final UpdateType currentUpdateType, final String updateKey) {
    final boolean shouldInstall = shouldConfirm ?
        confirmInstallUpdate(currentUpdateType) : true;
    if (shouldInstall) {
        downloadUpdate(currentUpdateType, availableUpdates.get(updateKey));
    } else {
        gotoNextActivity();
    }
}

Usage:

if (tryApplicationUpdate) {
    downloadUpdateIfConfirmed(shouldConfirm, availableUpdates,
        UpdateType.APPLICATION, "application");
} else if (tryParametersUpdate) {
    downloadUpdateIfConfirmed(shouldConfirm, availableUpdates,
        UpdateType.PARAMETERS, "parameters");
} else {
    ...
}

I'd move the shouldConfirm flag into the method (to reduce its scope, see Effective Java, Second Edition, Item 45: Minimize the scope of local variables); use guard clauses; create another method which calls gotoNextActivity() to remove its duplication:

public void update() {
    doUpdate();
    gotoNextActivity();
}

private void doUpdate() {
    mContext = this.getApplicationContext();
    final UpdateParameters updateParameters = Parameters.load(mContext);
    final UpdateMode updateInstallationMode =
        updateParameters.getUpdateMode();
    if (updateInstallationMode == UpdateMode.MANUAL) {
        return;
    }
    lblStatus.setText(getString(RString.checking_for_updates));
    final Error outUpdateError = new Error();
    final Map<String, String> availableUpdates =
        checkForUpdates(outUpdateError);

    final boolean tryApplicationUpdate =
        isUpdateAvailable(availableUpdates, updateParameters,
            UpdateType.APPLICATION, "application");
    final boolean tryParametersUpdate =
        isUpdateAvailable(availableUpdates, updateParameters,
            UpdateType.PARAMETERS, "parameters");
    if (tryApplicationUpdate) {
        downloadUpdateIfConfirmed(updateInstallationMode,
            availableUpdates, UpdateType.APPLICATION, "application");
        return;
    }
    if (tryParametersUpdate) {
        downloadUpdateIfConfirmed(updateInstallationMode,
            availableUpdates, UpdateType.PARAMETERS, "parameters");
        return;
    }
    lblStatus.setText(getString(RString.app_up_to_date));
}

private void downloadUpdateIfConfirmed(
        final UpdateMode updateInstallationMode,
        final Map<String, String> availableUpdates,
           final UpdateType updateType, final String updateKey) {
    final boolean shouldConfirm =
        (updateInstallationMode == UpdateMode.CONFIRM);
    final boolean shouldInstall = shouldConfirm ?
        confirmInstallUpdate(updateType) : true;
    if (shouldInstall) {
        downloadUpdate(updateType, availableUpdates.get(updateKey));
    }
}


Mini bank application Today I went to an interview with the Cleartrip software company. For the first round programming, we need to build a mini bank application.

In the question paper they clearly mentioned that no data persistence is required.

Create an account
Deposit money
Withdraw money, honor daily withdrawal limit.
Check the balance

This is what I did on my own, and I was getting rejected.

import java.util.Scanner;

public class CreateAccount {
    int accountid;
    String accountantname;
    String IFSCcode;

   public CreateAccount(int accountid,String accountantname,String IFSCcode){
    this.accountid = accountid;
    this.accountantname = accountantname;
    this.IFSCcode = IFSCcode;
   }

    /*//adding deposit money with the balance
    public double despositMoney() throws MiniumAmountDeposit{

        double Currentbalance = 0.00;
        Scanner scn = new Scanner(System.in);
        System.out.println("please enter the deposit amount");
        double Depositamount = scn.nextDouble();
        Currentbalance += Depositamount ;
        System.out.println("your currentbalance="+Currentbalance);

        return Currentbalance;
    }*/

    //withdrawl money  and set daily withdrawl limit
    public void WithdrawMoney() throws InsufficientBalException, MiniumAmountDeposit{

        double Currentbalance = 0.00;
        Scanner deposit = new Scanner(System.in);
        System.out.println("please enter the deposit amount");
        double Depositamount = deposit.nextDouble();
        Currentbalance += Depositamount ;
        System.out.println("your currentbalance="+Currentbalance);

        /*double Currentbalanace = despositMoney();*/

        //setDaily Withdrawl limit
           final double setDailyLimit = 2500.00;

            Scanner withDraw = new Scanner(System.in);
            System.out.println("please enter the withdraw amount");
            double WithdrawMoney =withDraw.nextDouble();

        if(Currentbalance < WithdrawMoney)
            System.out.println("you have less amount : your current balance is="+Currentbalance);

        else if (WithdrawMoney > setDailyLimit)
            System.out.println("you have entered amount exceed than daily limit : your dailyLimit="+setDailyLimit);

        else
            Currentbalance -= WithdrawMoney;
            System.out.println("your current balance is="+Currentbalance);

    }

    /*public void setWithdrawlLimit()throws exceedLimit{
        Scanner scn = new Scanner(System.in);
        System.out.println("please enter the withdraw amount");
           double  enterAmount = scn.nextDouble();
        double DailysetLimit = 2500;

        if(enterAmount>DailysetLimit)
            System.out.println("you have exceed daily limit : your dailyLimit"+DailysetLimit);

    }*/

    public String toString(){

        return "accountid="+this.accountid + "accountantname="+this.accountantname + "IFSCcode="+this.IFSCcode;
    }

    public static void main(String[] args){
        CreateAccount account = new CreateAccount(1234455533,"samy","ICIC09");
        System.out.println("you have created account : " +account);
    /*
        try {
            account.despositMoney();
        } catch (MiniumAmountDeposit e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }*/

        try {
            account.WithdrawMoney();
        } catch (InsufficientBalException | MiniumAmountDeposit e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

        /*try {
            account.setWithdrawlLimit();
        } catch (exceedLimit e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

*/
    }

}

I'm sorry to hear you got rejected - that's never any fun. I'll go through line by line, with some general comments at the end.

Some of my notes are labeled as style suggestions; these usually mean generally accepted Java principles (naming conventions, etc.), so I'll try to point out where I'm trying to argue for a given style I personally recommend instead of giving a general rule.

import java.util.Scanner;

public class CreateAccount {

Style: classes should usually be nouns - e.g. Account or maybe AccountCreator, because you use them as Objects - so it makes more sense to have "an Account" than "a CreateAccount"

    int accountid;
    String accountantname;
    String IFSCcode;

Style: Java almost always uses Camel Case naming, so accountid should be accountID and accountantname accountantName, etc.

   public CreateAccount(int accountid,String accountantname,String IFSCcode){

Style: method names should start with a lower case letter EDIT: this is a constructor (my bad) and is therefore correctly named. Non-constructor methods should start with a lowercase letter.

    this.accountid = accountid;
    this.accountantname = accountantname;
    this.IFSCcode = IFSCcode;
   }

    /*//adding deposit money with the balance

What is this comment supposed to tell me? The function is called depositMoney, so I expect that it will deposit money - you don't need to tell me that. Instead, it might be helpful to explain that the deposit amount comes from the user's input, or what the return value means

    public double despositMoney() throws MiniumAmountDeposit{

        double Currentbalance = 0.00;
        Scanner scn = new Scanner(System.in);
        System.out.println("please enter the deposit amount");
        double Depositamount = scn.nextDouble();
        Currentbalance += Depositamount ;
        System.out.println("your currentbalance="+Currentbalance);

        return Currentbalance;
    }*/

This method is a little busy - it would be nice if the functionality to get input from the user was somewhere else (in a separate function), especially because you need to use it for multiple commands.

Also, it really seems like Currentbalance (currentBalance, if we use Camel Case ;p) should be an object variable.

  In the question paper they clearly mentioned that no data persistence is required.

BUT, I think what they meant is, no data persistence is necessary between program executions (you don't need to save account data to a file when the program shuts off, and read it back in when it starts up again). It would make a lot more sense to keep track of a user's balance while the program is running, and check, for example, that the user doesn't withdraw more money than they have in their account.

    //withdrawl money  and set daily withdrawl limit
    public void WithdrawMoney() throws InsufficientBalException, MiniumAmountDeposit{

        double Currentbalance = 0.00;
        Scanner deposit = new Scanner(System.in);
        System.out.println("please enter the deposit amount");
        double Depositamount = deposit.nextDouble();
        Currentbalance += Depositamount ;
        System.out.println("your currentbalance="+Currentbalance);

        /*double Currentbalanace = despositMoney();*/

See the above note - we probably don't want to force the user to deposit money before every withdrawal.

        //setDaily Withdrawl limit
           final double setDailyLimit = 2500.00;

This is a configuration constant - it should go at the top of the class, before methods. If all users should have the same withdrawal limit, it should be declared static, too.

            Scanner withDraw = new Scanner(System.in);
            System.out.println("please enter the withdraw amount");
            double WithdrawMoney =withDraw.nextDouble();

        if(Currentbalance < WithdrawMoney)
            System.out.println("you have less amount : your current balance is="+Currentbalance);

        else if (WithdrawMoney > setDailyLimit)
            System.out.println("you have entered amount exceed than daily limit : your dailyLimit="+setDailyLimit);

        else
            Currentbalance -= WithdrawMoney;
            System.out.println("your current balance is="+Currentbalance);

    }

These checks are good, nice job.

    /*public void setWithdrawlLimit()throws exceedLimit{
        Scanner scn = new Scanner(System.in);
        System.out.println("please enter the withdraw amount");
           double  enterAmount = scn.nextDouble();
        double DailysetLimit = 2500;

        if(enterAmount>DailysetLimit)
            System.out.println("you have exceed daily limit : your dailyLimit"+DailysetLimit);

    }*/

I don't understand what this function is for.

    public String toString(){

        return "accountid="+this.accountid + "accountantname="+this.accountantname + "IFSCcode="+this.IFSCcode;
    }

    public static void main(String[] args){
        CreateAccount account = new CreateAccount(1234455533,"samy","ICIC09");
        System.out.println("you have created account : " +account);
    /*
        try {
            account.despositMoney();
        } catch (MiniumAmountDeposit e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }*/

        try {
            account.WithdrawMoney();
        } catch (InsufficientBalException | MiniumAmountDeposit e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

        /*try {
            account.setWithdrawlLimit();
        } catch (exceedLimit e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

In general, you shouldn't leave auto-generated TODOs in submitted code. For an interview, I would be fine with leaving TODOs explaining functionality I wish I could implement but didn't have time for.

*/
    }
}

And that's the end, so, overall: It looks like you have a solid start on the problem. The biggest issues I see (in order of importance (my opinion) for code turned in as part of an application):

The code looks a little rushed. Just like in a face-to-face interview, appearances matter for code that will be read and reviewed. Big chunks of commented code with no explanation, as well as misspelling / inconsistent naming schemes, won't make a good impression.
It seems like you're a little unsure of the problem specification. It can help a lot to think carefully about the input / output to each function, and what data will need to be stored in object variables, as you're writing your code - these things can change as you go along and understand the problem more. It's very helpful to document (write in comments!) the assumptions you're making about how the program should run.
There are some spots where you deviate a lot from established Java rules. No company has the exact same code style (here are Google's for Java), but there are some things (camel case, for example) that are very central to a given language. Not using camel case correctly when writing Java is a signal that you don't know Java very well - depending on the position you're applying for, this may or may not be a problem.

My biggest suggestions for improvement are: always submit the cleanest code possible, try to follow broad, important, rules for how the language should look, so it looks clean to the reviewer, and work on writing short, helpful comments around tricky or interesting spots in the code.


Efficiency of sieve of Eratosthenes Can I make this more efficient?

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

void setvalue(int array_copy[], int i, int n)
{
    int j,p;

    for(j = i*i ; j <= n ; j +=(i*2) )
    {
        array_copy[j] = 1;
    }
}

int main()
{
    int n,i,l=0,sq;
    scanf("%i",&n);

    sq = sqrt(n);

    int prime_array[n];
    int array[n];
    for (i = 1; i <= n; ++i)
    {
        array[i] = 0;
    }
    for ( i = 4; i <= n; i+=2)
    {
        array[i] = 1;
    }

    for( i = 3; i <= sq; i+=2)
    {
        setvalue(array,i,n);
    }
    array[2] = 0;
    for(i = 2; i <= n; i++)
    {
        if(array[i] == 0)
        {
            prime_array[l] = i;
            l++;
        }
    }

    for ( i = 0; i <= (l-1) ; i++)
    {
        printf("%i\n",prime_array[i] );
    }
    return 0;
}

Disclaimer : most of my comments might be irrelevant as far as performance is concerned but should make your code better on different other aspects (readability for instance).

Listen to your compiler

Compiling your code with gcc -Wall your_file -lm gives me the following warning : sieve.c:7:11: warning: unused variable ‘p’ [-Wunused-variable]. I guess you know the first thing you can do :-).

Making things easier to follow

Splitting your code in small chunks is usually a good idea. Unfortunately, I am not quite convinced by setvalue because what it does is quite complicated and it makes it hard to give it a simple name, to test it out of the algorithm or to reuse it later on. Thus, from my point of view, it's easier to just remove this function.

A good habit : defining things in the smallest possible scope

It is quite common to try to define variables as late as possible and in the smallest possible scope. It prevents whoever reads the code to try to keep in mind the value of irrelevant things and to find what is used for what and it allows one to find easily when a variable becomes unused.

It makes even better if you use c99 as you can them define indices in for-loops.

Also, you can take this chance to define things that go together (such as prime_array and l) together.

Consistency

You should try to be consistent in how you write your code. You should define a way to put space in your code and try to stick to it as much as possible. Consistency is relevant also for braces (you did good), line breaks, incrementations (you used i++ and ++i).

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

int main()
{
    int n = 100; //scanf("%i",&n);

    int sq = sqrt(n);

    int array[n];
    for (int i = 1; i <= n; i++)
    {
        array[i] = 0;
    }
    for (int i = 4; i <= n; i+=2)
    {
        array[i] = 1;
    }
    for(int i = 3; i <= sq; i+=2)
    {
        for (int j = i*i; j <= n; j+=(i*2))
        {
            array[j] = 1;
        }
    }
    array[2] = 0;

    int prime_array[n];
    int l = 0;
    for (int i = 2; i <= n; i++)
    {
        if (array[i] == 0)
        {
            prime_array[l] = i;
            l++;
        }
    }

    for (int i = 0; i <= (l-1) ; i++)
    {
        printf("%i\n",prime_array[i] );
    }
    return 0;
}

Keeping things simple

If you just want to write prime number on the standard output, you don't really need a temporary container for the numbers.

Naming things

  There are only two hard problems in Computer Science: cache
  invalidation and naming things. -- Phil Karlton

Naming is really important : here array is a pretty bad name as it doesn't convey any information about how the array is used.

Because of the way you use it, naming it not_prime would probably be a good idea. An even better idea would be to name it prime (or something like that) and use it in such a way that a true-ish value corresponds to prime number and 0 to non-prime numbers. Then you'd get something like :

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

int main()
{
    int n = 100; //scanf("%i",&n);

    int sq = sqrt(n);

    int prime[n];
    for (int i = 1; i <= n; i++)
    {
        prime[i] = 1;
    }
    for (int i = 4; i <= n; i+=2)
    {
        prime[i] = 0;
    }
    for(int i = 3; i <= sq; i+=2)
    {
        for (int j = i*i; j <= n; j+=(i*2))
        {
            prime[j] = 0;
        }
    }
    prime[2] = 1;

    for (int i = 2; i <= n; i++)
    {
        if (prime[i])
        {
            printf("%i\n", i);
        }
    }

    return 0;
}

Algorithm

This is probably where it gets interesting : you have not properly implemented the sieve. At the moment, you apply the logic (from wikipedia) Starting from p, enumerate its multiples by counting to n in increments of p, and mark them in the list (these will be 2p, 3p, 4p, etc.; the p itself should not be marked). for p being the i in for(int i = 3; i <= sq; i+=2) but you should (or you can depending on the point of view) restrict yourself to prime values of i. This corresponds to adding if (prime[i]).  This should have a huge impact from a performance point of view.

A tiny problem

There is a variation of the quote above that goes like this :

  There are only two hard problems in Computer Science: cache
  invalidation, off-by-one errors and naming things.

It is relevant in your case because you have an array of n elements so the last index you should access is n - 1. The usual way to write the corresponding loop is with i < n (instead of the equivalent i <= n-1).

The i <= sq stays the same, I'll let you understand why on your own.

The code is now :

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

int main()
{
    int n = 122; //scanf("%i",&n);

    int sq = sqrt(n);

    int prime[n];
    for (int i = 1; i < n; i++)
    {
        prime[i] = 1;
    }
    prime[2] = 1;
    for (int i = 4; i < n; i+=2)
    {
        prime[i] = 0;
    }
    for(int i = 3; i <= sq; i+=2)
    {
        if (prime[i])
        {
            for (int j = i*i; j < n; j+=(i*2))
            {
                prime[j] = 0;
            }
        }
    }

    for (int i = 2; i < n; i++)
    {
        if (prime[i])
        {
            printf("%i\n", i);
        }
    }

    return 0;
}

Comments

Even though you have missed an key step in the algorithm, you have tried some fancy optimisation, it would probably be worth commenting on those to explain why and how it works.


Nicer way to work params if params[:package]
  city_ids = []

  city = params[:package].delete 'city'
  city_ids << city  if city.to_i > 0

  cities = params[:package].delete 'cities'
  city_ids += cities  if cities.class == Array && cities.length > 0

  params[:package][:cities] = city_ids.map{ |c| City.get(c) }.compact
end

This code looks pretty clean to me. Here are a few thoughts:

I'm not sure what your ORM is (or even that City is database backed), but most have a way to fetch more then one record at a time. Making a single call will save time and Database resources.

Another thought is that you are inducing a lot of local vars: city_ids, city, cities. What about trying to build up the array of ids in params[:package][:cities]? e.g.

#if cities is not an array, it is bad date get rid of it.
params[:package].delete 'cities' unless params[:package][:cities].kind_of? Array
params[:package][:cities] ||= []
params[:package][:cities] << (params[:package].delete 'city') if params[:package][:cities].to_i > 0
params[:package][:cities] = params[:package][:cities].map{ |c| City.get(c) }.compact

The code above doesn't seem very DRY because it has all this params[:package] stuff.  If you do things like this a lot (your question makes it sound like you do) factor the operation out into a separate method:

def extract_nested_reference(hash,values) # change name as appopriate
  if hash
    hash.delete values unless hash[values].kind_of? Array
    hash[values] ||=[]
    hash[values] << hash[values.singularize] if hash[values.singularize].to_i > 0

    hash[values] = hash[values].map do |x|
      Object.const_get(values.singularize.constantize).get(c)
    end.compact
  end
end

This method could then be called with extract_nested_reference(params[:package],'cities'). Refactoring like this will also encourage you to be consistent in how you build you params hash, and allow you to reuse code.

Finally, your question asked about better ways of dealing with hashes.  I don't think there is.  You will find code very similar to this living in most popular gems.  AS you say, it is kind of clunky, and therefore and excellent candidate for a code redactor.


Choosing a Color based on an int for a 2048 clone in Java Is it possible to make this cleaner / more efficient? I know that this is probably not the best way to do this and probably could be made better but I cannot figure out how I do that.

Basically, a number is given which is then compared to the numbers in Num[] and then returns a color based on that comparison.

private Color chooseColor(int i){
    Color Emerald = new Color(231, 223, 134); //2
    Color BelizeHole = new Color(151, 206, 104); // 4
    Color Carrot = new Color(240, 79, 3);// 8
    Color MidnightBlue = new Color(116, 116, 204); // 16
    Color SunFlower = new Color(89, 188, 251);// 32
    Color Alizarin = new Color(245, 213, 69);// 64
    Color Wisteria = new Color(46, 204, 113); // 128
    Color Silver = new Color(254, 198, 6); //256
    Color Concrete = new Color(151, 206, 104); //512
    Color Orange = new Color(136, 112, 255); //1024
    Color Amethyst = new Color(255, 215, 0); //2048
    Color[] RECT_COLORS = {Emerald, BelizeHole, Carrot, Amethyst, SunFlower, Alizarin, Wisteria, Silver, Concrete, Orange, Amethyst};
    int[] Num = {2,4,8,16,32,64,128,256,512,1024,2048};

    for(int j = 0 ; j < Num.length ; j++){
        if(i == Num[j]){
            return RECT_COLORS[j];
        } else if ( j == Num.length - 1 ){
            return RECT_COLORS[Num.length-1];
        }
    }

    return RECT_COLORS[Num.length-1];
}

This compiles and runs fine.

Outside of the method, it is called when painting a string on Canvas and returns a color based on the string value which is what color the string is then painted as.

Basically:

String is about to to be painted. What color should it be?
chooseColor(4);
chooseColor(4) = BelizeHole. Let's paint it with that color.

Something is wrong with your design. Your method is technically correct, but the problem is that it relies on a given magic int i to determine which color to return.

This can be seen by

int[] Num = {2,4,8,16,32,64,128,256,512,1024,2048};

This declares an array of fixed magic values. The issue is that, somewhere in your code, you're determining which i to give to this method and you're testing it against known fixed values only in the method.

This highly couples parts of your application. You should refactor this code so that the logic of "which number corresponds to which color" is centralized in a single place. One of comment says that you're doing a 2048 clone, as such, the centralized place could be a class called Block. You could even make it an enumeration of all the possible blocks, each with its respective value and color.

As such, don't use use Strings to represent blocks, use proper objects with attributes; this is what Object-Oriented Programming is about. An example could be the following (with perhaps a better name for the constants):

public enum Block {

    SQUARE_2(2, new Color(231, 223, 134)),
    SQUARE_4(4, new Color(151, 206, 104));

    private final int value;
    private final Color color;

    private Block(int value, Color color) {
        this.value = value;
        this.color = color;
    }

    // add getters

}

That said, you can improve a little bit the current code. You are currently doing:

for(int j = 0 ; j < Num.length ; j++){
    if(i == Num[j]){
        return RECT_COLORS[j];
    } else if ( j == Num.length - 1 ){
        return RECT_COLORS[Num.length-1];
    }
}
return RECT_COLORS[Num.length-1];

In the case where the Num array doesn't have the given value, you have a special else if to handle the last case. You actually don't need this: you can safely let your code exit the for loop normally and return the last value in this case:

for (int j = 0 ; j < Num.length ; j++) {
    if (i == Num[j]) {
        return RECT_COLORS[j];
    }
}
return RECT_COLORS[Num.length-1];

A second point concerns the variable names: you should try to respect Java naming conventions. The local variables should start with a lowercase letter (num instead of Num, which could arguably have a better name also; emerald instead of Emerald...).


Check if an XML file exists then change URLs in it This is my first VBScript I have ever written and I would like to know if there is any obvious blunders in the code. All the code should do is check if file exists then change the Yahoo URL to the Google URL in the XML file. Note: The code seems to function properly.

Dim objFSO
Set objFSO = CreateObject("Scripting.FileSystemObject")

If (objFSO.FileExists("C:\urlsinfo.xml")) Then
Const ForReading = 1
Const ForWriting = 2
Const ForAppending = 8

Dim textToReplace
Dim textReplacement
textToReplace = "http://www.yahoo.com"
textReplacement = "http://www.google.com"

Set objFile = objFSO.OpenTextFile("C:\urlsinfo.xml", ForReading)
strText = objFile.ReadAll
objFile.Close
strNewText = Replace(strText, textToReplace, textReplacement)

Set objFile = objFSO.OpenTextFile("C:\urlsinfo.xml", ForWriting)
objFile.WriteLine strNewText
objFile.Close
WScript.Quit()

Else
Dim errorFileSys, errorLogTxt

Set errorFileSys = CreateObject("Scripting.FileSystemObject")
Set errorLogTxt = errorFileSys.OpenTextFile("C:\urlsinfoError.log", ForAppending, True)
errorLogTxt.WriteLine(Now &"     urlsinfo.xml file did not exist in directory C:\. ")
errorLogTxt.Close

WScript.Quit()
End If

'Exit Script
WScript.Quit()

I reworked your VBScript a bit with comments, see below. In response to the comment from Konrad, I can agree with him for 90% of the cases. VBScript stays interesting for system management, logon scripts, etc., but I myself am replacing it with Ruby which isn't a big step up from VBScript, and on the other hand it has almost all the goodies from python. If you want to compare, take a look at e.g. http://yagni.com/rosetta-stone/#0 (what happened to https://rosettacode.org/ ?). You'll see that the Ruby code is most of the time the smallest and the most comprehensible.

The adapted VBScript version:

Option explicit 'always use this
'declarations first, be concise
Const ForReading = 1, ForWriting = 2, ForAppending = 8, CreateIfNeeded = true
Dim objFSO, objFile, textToReplace, textReplacement, strNewText, strText
Dim errorFileSys, errorLogTxt, urlsinfo
Set objFSO = CreateObject("Scripting.FileSystemObject")

If objFSO.FileExists(urlsinfo) Then 'indent
  textToReplace   = "http://www.yahoo.com"
  textReplacement = "http://www.google.com"
  urlsinfo = "C:\urlsinfo.xml"
  Set objFile = objFSO.OpenTextFile(urlsinfo, ForReading)
  strText = objFile.ReadAll
  objFile.Close
  strNewText = Replace(strText, textToReplace, textReplacement)
  Set objFile = objFSO.OpenTextFile(urlsinfo, ForWriting)
  objFile.WriteLine strNewText
  objFile.Close
Else
  Set errorFileSys = CreateObject("Scripting.FileSystemObject")
  Set errorLogTxt = errorFileSys.OpenTextFile("C:\urlsinfoError.log", ForWriting, CreateIfNeeded)
  errorLogTxt.WriteLine(Now &"     urlsinfo.xml file did not exist in directory C:\. ")
  errorLogTxt.Close
  Wscript.Quit 1 'you could use quit here to give an errorlevel back to the OS
End If

'Exit Script 'no need for obvious comments
'WScript.Quit() 'no need for this at the end of the script
'also quit is a sub, no need for the ()

And the same script in Ruby:

urlsinfo, urlsinfoError = "C:/urlsinfo.xml", "c:/urlsinfoError.log"
textToReplace, textReplacement = "http://www.yahoo.com", "http://www.google.com"

begin
  File.write(urlsinfo, File.read(urlsinfo).gsub(textToReplace, textReplacement))
rescue
  File.write(urlsinfoError, "#{Time.now}      urlsinfo.xml file did not exist in directory C:\. ")
end