Doubly linked list code in C Let me know if it seems right to you and if anything can be optimized.

/*
 * Doubly Linked List
 *
 * Each node of the list contain two references (or links) – one to the previous node and other to the next node.
 * The previous link of the first node and the next link of the last node points to NULL.
 * In comparison to singly-linked list, doubly-linked list requires handling of more pointers
 * but less information is required as one can use the previous links to observe the preceding element.
 */

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

typedef struct Node {
    int data;
    struct Node *next;
    struct Node *prev;
} Node;

void insert(Node *current, int data);
void delete(Node *current, int data);
void print(Node *current);
int find(Node *current, int data);

void insert(Node *current, int data) {

    // current is pointing to first element
    // we iterate until we find the end
    while(current->next != NULL) {
        current = current->next;
    }
    // create a new Node and insert the item
    current->next = (Node *)malloc(sizeof(Node));
    (current->next)->prev = current;
    current = current->next;
    current->data = data;
    current->next = NULL;
}

void delete(Node *current, int data){

    // Iterate until we find a pointer next to the one we need to delete
    while (current->next != NULL && (current->next)->data != data) {
        current = current->next;
    }

    // Item is not found
    if(current->next == NULL) {
        printf("\nElement %d is not present in the list\n", data);
        return;
    }

    // The element is found in the node next to the one that current points to
    // We removed the node which is next to the pointer (which is also temp)
    Node *tmp = current->next;
    // In special case that we are deleting last node
    if(tmp->next == NULL) {
        current->next = NULL;
    } else {
        current->next = tmp->next;
        (current->next)->prev = tmp->prev;
    }
    tmp->prev = current;

    // We got rid of the node, now time to dellocate the memory
    free(tmp);

    return;
}
void print(Node *current) {
    while(current != NULL) {
        printf("%d ", current->data);
        current = current->next;
    }
}

int find(Node *current, int data) {
    // First pointer is head aka dummy node with no data
    // so we go to next one
    current = current->next;

    // Iterate through the linked list
    while(current != NULL) {
        if(current->data == data) {
            return 1;
        }
        current = current->next;
    }
    return 0;
}

int main() {

    Node *head = (Node *)malloc(sizeof(Node));
    head->next = NULL;
    head->prev = NULL;

    int data = 0;
    int usr_input = 0;

    while(1){
        printf("0. Exit\n");
        printf("1. Insert\n");
        printf("2. Delete\n");
        printf("3. Print\n");
        printf("4. Find\n");

        scanf("%d", &usr_input);

        // can also use a switch instead
        if( usr_input == 0) {
            exit(0);

        } else if(usr_input == 1) {
            printf("\nEnter an element you want to insert: ");
            scanf("%d", &data);
            insert(head, data);

        } else if(usr_input == 2) {
            printf("\nEnter an element you want to delete: ");
            scanf("%d", &data);
            delete(head, data);

        } else if(usr_input == 3) {
            printf("The list is ");
            print(head->next);
            printf("\n\n");

        } else if(usr_input == 4) {
            printf("\nEnter an element you want to find: ");
            scanf("%d", &data);
            int is_found = find(head, data);
            if (is_found) {
                printf("\nElement is found\n\n");
            } else {
                printf("\nElement is NOT found\n\n");
            }
        }
    }
    return 0;
}

Some mostly stylistic improvements you could apply:

There is no need to cast the return value of malloc(), unless you plan on compiling this code as C++. In C, void* converts implicitly to any other pointer type. So the cast is just boilerplate in this case.
Prefer using sizeof(variable) instead of sizeof(Type). Example:

Node *head = malloc(sizeof(*head));

This makes maintenance easier, since now if you introduce a new Node type, you only have to change one place, with no chance of updating one side of the expression and forgetting the other. Note that in this example, the dereference of head is valid because sizeof is a compile-time operation, so there is no actual pointer access taking place here, just type inference.
Consider giving more robust names to your functions. find(), insert(), etc, are very common names, which can apply to many things, not just lists. In C, you cannot overload function names, so in any major project, these names would be very likely to cause collisions with other global identifiers. Name prefixes are a common approach in C to solve this problem. Consider prefixing the function names with list_ or any other prefix you might prefer. I.e.:

void list_insert(Node *current, int data);
void list_delete(Node *current, int data);

Instead of returning an integer 0|1 to represent a boolean value, you can use the bool type, declared in the <stdbool.h> header, which is more idiomatic.
delete() function is a void function, so it doesn't need an explicit return statement at the end.
Looks like you are using printf() to log errors:

if(current->next == NULL) {
    printf("\nElement %d is not present in the list\n", data);
    return;
}

For error output, it is more correct to use fprintf and write to the stderr standard stream.
The list printing function print() could be made more flexible by receiving the output stream as a parameter. So you don't limit the caller to always printing to stdout, it can also be used to dump the list to a file (notice the use of fprintf()).

void print(Node *current, FILE *stream) {
    while(current != NULL) {
        fprintf(stream, "%d ", current->data);
        current = current->next;
    }
}

When you want to allocate and immediately zero-initialize data, such as in here:

Node *head = (Node *)malloc(sizeof(Node));
head->next = NULL;
head->prev = NULL;

The standard function calloc() can be used to make code more concise:

Node *head = calloc(1, sizeof(*head));

You are changing the head pointer in all the function. You should copy that to a temp pointer and traverse it.


Python Power function I'm a freshman in Python world, I used to program in C.

I wrote a function/method that returns the power of a number:

def pow(x, y):

    powered = x

    if y == 0:
        return 1

    while y > 1:
        powered *= x

        y -= 1

    return powered

Mine is a logic question (correctly): if I set y = 0 then it returns \$1\$, but on the last line of the code I wrote return powered then = x.

Does return work like as break?

return means "Eureka! I have found the answer! It is …."  It hands that result back to the caller and stops executing any more code in this function.

Unlike x**y, your function gives incorrect results for negative or non-integer values of y.  You should note that caveat in a docstring.

The standard way to do counting loops in Python is to use range() (or, in Python 2, xrange()).

To reduce the complexity of your code, you should avoid unnecessary special cases.

def pow(x, y):
    """Raise x to the power y, where y must be a nonnegative integer."""
    result = 1
    for _ in range(y):   # using _ to indicate throwaway iteration variable
        result *= x
    return result

This particular looping pattern (starting with some initial value and repeatedly applying an operation to the result) could be reduced to a one-liner:

from functools import reduce  # Optional in Python 2, required in Python 3

def pow(x, y):
    """Raise x to the power y, where y must be a nonnegative integer."""
    return reduce(lambda result, _: result * x, range(y), 1)

This solution is admittedly foreign-looking to a C programmer.  The reduce() function uses 1 as the starting value (equivalent to result = 1 in the first solution).  For each _ in range(y), it does result = result * x.  lambda is just a way to define a simple function on the spot.

A more efficient algorithm would be repeated squaring.

I want to improve time taken by the recursive solution proposed as an answer by @NlightNFotis:

If a ==0, we know that 0 raised to the power of any number will be equal to zero before computing it.
For even numbers, we use the property \$a^{2n}\$=\$(a^{n})^2\$ to reduce by 2 the time taken by the algorithm when calculating the power for an even number.

This is just in case we want to use recursion, but it is not an efficient  solution in Python. Just to see the beauty of it, here is an optimization for @NlightNFotis's answer:

 def power(a,b):
      """this function will raise a to the power b but recursivelly"""
      #first of all we need to verify the input
      if isinstance(a,(int,float)) and isinstance(b,int):
        if a==0:
          #to gain time
          return 0
        if b==0:
            return 1
        if b >0:
          if (b%2==0):
          #this will reduce time by 2 when number are even and it just calculate the power of one part and then multiply
            if b==2:
              return a*a
            else:
              return power(power(a,b//2),2)
          else:
          #the main case when the number is odd
            return a * power(a, b- 1)
        elif not b >0:
          #this is for negatives exposents
          return 1./float(power(a,-b))
      else:
        raise TypeError('Argument must be integer or float')


Number to word converter (1 to 99 inclusive) Program

It takes a numerical input in range [1, 99] and outputs its word equivalent.

Concerns

Procedural approach
General code style

Code

#include <iostream>

void print1to20(int num)
    {
           switch(num)
           {
           case 1: std::cout << "one"; break;
           case 2: std::cout << "two"; break;
           case 3: std::cout << "three"; break;
           case 4: std::cout << "four"; break;
           case 5: std::cout << "five"; break;
           case 6: std::cout << "six"; break;
           case 7: std::cout << "seven"; break;
           case 8: std::cout << "eight"; break;
           case 9: std::cout << "nine"; break;
           case 10: std::cout << "ten"; break;
           case 11: std::cout << "eleven"; break;
           case 12: std::cout << "twelve"; break;
           case 13: std::cout << "thirteen"; break;
           case 14: std::cout << "fourteen"; break;
           case 15: std::cout << "fifteen"; break;
           case 16: std::cout << "sixteen"; break;
           case 17: std::cout << "seventeen"; break;
           case 18: std::cout << "eighteen"; break;
           case 19: std::cout << "nineteen"; break;
           case 20: std::cout << "twenty"; break;
           default: std::cout << "1 to 20 switch error";
           }
    }

void printTens(int num)
    {
           switch(num)
           {
           case 20: std::cout << "twenty"; break;
           case 30: std::cout << "thirty"; break;
           case 40: std::cout << "fourty"; break;
           case 50: std::cout << "fifty"; break;
           case 60: std::cout << "sixty"; break;
           case 70: std::cout << "seventy"; break;
           case 80: std::cout << "eighty"; break;
           case 90: std::cout << "ninety"; break;
           default: std::cout << "tens switch error";
           }
    }

int main()
{
    int num;
    do
    {
    std::cout << "Insert a number 1-99: ";
    std::cin >> num;
    } while(num < 1 || num > 99);

    int num_a, num_b; // num = (a)(b);
    num_a = num - num%10;
    num_b = num%10;

    std::cout << "Your number is ";

    if(num <= 20)
        print1to20(num);
    else
    {
        printTens(num_a);
        std::cout << ' ';
        print1to20(num_b);
    }

    return 0;
}

The code is broken. If a number is greater than 20 and ends with 0, print1to20(num_b) will not do what it should.
Don't mix the logic with the output. I'd change the return type of your functions to std::string. It'll make the code more testable and reusable.
Don't just print something to std::cout in a case of an error. Throw an exception instead.
Declare each variable as late as possible:

int num_a, num_b; // num = (a)(b);
num_a = num - num%10;
num_b = num%10;

should be

int num_a = ...;
int num_b = ...;

Use more descriptive names. I have no clue what num_a mean. What is a? Something like first_digit and second_digit would be better (well the first one is not exactly a digit, so you can come with even better names).
There's no need to return 0 from the main function explicitly (sure, you can if you want to).
Write proper automated tests for your code to reduce the number of bugs (your bug is very likely to get caught by tests). In fact, you can check all possible valid inputs. There're only 100 of them.
There's nothing wrong in having stand-alone functions. There's no entity here, so I don't see the point of modeling this problem in terms of objects. A function from an int to its string representation is just a function.

How I re-wrote my code:

#include <iostream>
#include <string>

std::string print1to20(int num)
    {
           switch(num)
           {
           case 1: return "one";
           case 2: return "two";
           case 3: return "three";
           case 4: return "four";
           case 5: return "five";
           case 6: return "six";
           case 7: return "seven";
           case 8: return "eight";
           case 9: return "nine";
           case 10: return "ten";
           case 11: return "eleven";
           case 12: return "twelve";
           case 13: return "thirteen";
           case 14: return "fourteen";
           case 15: return "fifteen";
           case 16: return "sixteen";
           case 17: return "seventeen";
           case 18: return "eighteen";
           case 19: return "nineteen";
           case 20: return "twenty";
           }
           return "Error!";
    }

std::string printTens(int num)
    {
           switch(num)
           {
           case 20: return "twenty";
           case 30: return "thirty";
           case 40: return "fourty";
           case 50: return "fifty";
           case 60: return "sixty";
           case 70: return "seventy";
           case 80: return "eighty";
           case 90: return "ninety";
           }
           return "Error!";
    }

int main()
{
    int num;
    do
    {
    std::cout << "Insert a number 1-99: ";
    std::cin >> num;
    } while(num < 1 || num > 99);

    int tens = num - num%10;
    int ones = num%10;

    std::cout << "Your number is ";

    if(num <= 20)
        std::cout << print1to20(num);
    else
    {
        std::cout << printTens(tens);
        if(num%10 != 0)
            std::cout << ' ' << print1to20(ones);
    }

    return 0;
}


Reading of named settings from the XML Usually it is a good style in Java to create a named constant for a string (and other) hardcoded values. Using the example below I want to know how pedantic I should be in it.

I use a Java class as a wrapper of XML settings. Each function is suppose to return a value from a related XML file.

I defined a lot of constants with keys names on the top and each function use one single constant. Is it a good idea to remove constants and use hardcoded strings directly in each function?

Here is my settings file:

package com.art.backend.configs;

import com.art.backend.configs.inherited.Config;

public class ConfigStorage extends Config {

    private static final String SETTINGS_FILENAME = "storage.cfg.xml";

    private static final String KEY_HOST = "host";
    private static final String KEY_PORT = "port";
    private static final String KEY_USERNAME = "username";
    private static final String KEY_PASSWORD = "password";
    private static final String KEY_CONNECTION_TYPE = "connection_type";
    private static final String KEY_SEPARATOR= "separator";
    private static final String KEY_REMOTE_ROOT = "remote_root";
    private static final String KEY_SUBDIR_UPLOADED = "subdir_uploaded";

    public ConfigStorage() {
        super(SETTINGS_FILENAME);
    }

    public String getHost(){ return getString(KEY_HOST); }

    public int getPort(){ return getInt(KEY_PORT); }

    public String getUsername(){ return getString(KEY_USERNAME); }

    public String getPassword(){ return getString(KEY_PASSWORD); }

    public String getSeparator(){ return getString(KEY_SEPARATOR); }

    public String getConnectionType(){ return getString(KEY_CONNECTION_TYPE); }

    public String getRemoteRoot(){ return getString(KEY_REMOTE_ROOT); }

    public String getSubdirUploaded(){ return getString(KEY_SUBDIR_UPLOADED); }
}

Also I will attach file "Config" as a base class for my settings and XML settings file just in case, but they are not really important.

package com.art.backend.configs.inherited;

import java.io.IOException;
import java.io.InputStream;
import java.util.Properties;

public abstract class Config {
    private static final String MSG_SETTING_NOT_FOUNT = "Settings file not found";

    private final String filename;
    private Properties properties = null;

    public Config(String filename){
        this.filename = filename;
        init();
    }

    private void init(){

        //Reading properties file in Java example
        try {
            properties = new Properties();
            InputStream fis = getClass().getClassLoader().getResourceAsStream("storage.cfg.xml");
            properties.loadFromXML(fis);
        } catch (IOException e) {
            e.printStackTrace();
            throw new IllegalArgumentException(MSG_SETTING_NOT_FOUNT);
        }
    }

    public String getFilename(){ return filename; }

    public String getString(String key){
        return properties.getProperty(key);
    }

    public int getInt(String key){
        return Integer.parseInt(properties.getProperty(key));
    }

    public float getFloat(String key){
        return Float.parseFloat(properties.getProperty(key));
    }

    public boolean getBoolean(String key){
        return Boolean.parseBoolean(properties.getProperty(key));
    }
}

XML settings

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
    <entry key="host">***</entry>
    <entry key="port">***</entry>
    <entry key="username">***</entry>
    <entry key="password">***</entry>
    <entry key="connection_type">sftp</entry>
    <entry key="separator">/</entry>
    <entry key="remote_root">/home/adzhus/art_storage/</entry>
    <entry key="subdir_uploaded">uploaded/</entry>
</properties>

In your use case, where the constants are private and only ever used in one place, they are pretty much boilerplate. One could argue that the constants group the keys into one condense place where they can be referred to easily when reading code.

No, its not good idea to directly hard code the value in the method itself.
Other way could be load all these values from resources properties file and use @value to load them inside your class.


Palindrome checker in JavaScript I've built a Palindrome checker in JavaScript. It's fairly simple.

I'm still learning JS and am looking to learn.

So I would love hear ideas on how to improve the JavaScript in this. Like more efficient ways to resolve the task of checking for Palindrome.

Source code is below. Link to CodePen is her: http://codepen.io/MarkBuskbjerg/pen/JWMWwN?editors=1010

HTML:

Palindrome checker

A nut for a jar of tuna

                <textarea class="col-md-12" id="inputPalindrome" rows="5">Borrow or rob?</textarea>

            <button id="checkPalindrome" class="btn btn-default btn-lg btn-block col-sm-6 col-sm-offset-4" type="submit">Check palindrome</button>

Palindrome has not been checked yet

CSS:

.cover {
  height: 100vh;
}

.row-margin {
  margin: 4vh auto;
}

body {
  background: #FF4E50;
  background: -webkit-linear-gradient(135deg, #FF4E50, #F9D423);
  background: linear-gradient(135deg, #FF4E50, #F9D423);
}

.header {
  color: #fff;
}

h1 {
  font-family: "Pacifico";
}

.btn-default {
  background-color: black;
  color: #fff;
  font-weight: bold;
  text-transform: uppercase;
 }

JavaScript:

var checkButton = document.getElementById("checkPalindrome");

function isPalindrome(str) {
    str = str.toLowerCase().replace(/[^a-z0123456789]+/g,"");
    var reversedStr = str.split("").reverse().join("");
    if (str == reversedStr) {
        return true
    }
    return false
}

checkButton.addEventListener("click", function() {
    var palindromeInput = document.getElementById("inputPalindrome").value;
    var palindromeReturn = isPalindrome(palindromeInput);
    if(palindromeReturn === true) {
        document.getElementById("notification").innerHTML = "Yay! You've got yourself a palindrome";
        document.getElementById("notification").className = "alert alert-success";
    } else {
        document.getElementById("notification").innerHTML = "Nay! Ain't no palindrome";
        document.getElementById("notification").className = "alert alert-danger";
    }
});

Link to CodePen with the Palindrome Checker
http://codepen.io/MarkBuskbjerg/pen/JWMWwN

RegEx

In character class, 0123456789 can be written as range 0-9. Although, this has no effect on working of RegEx, it saves some keystrokes and looks consistent with alphabets range a-z.

toLowerCase after replace

First remove the special characters and then convert the string to lower case. This, in my opinion will run fast than other way round as the number of characters to work on are reduced.(Not Tested)

When doing this, don't forget to add i flag on the RegEx.

str.replace(/[^a-z0-9]+/gi, "").toLowerCase();

return boolean;

if (str == reversedStr) {
    return true
}
return false

can be written as return str == reversedStr. It is also recommended to use strict equality operator.

return str === reversedStr;

See Which equals operator (== vs ===) should be used in JavaScript comparisons?

Caching DOM element reference

In the click handler of button, #notification is referenced four times. All times, it is read again from DOM. This can be slower. The element can be cached and used whenever required.

var notification = document.getElementById('notification');
...
...
notification.innerHTML = 'Hello World!';

Complete Code

Updated after suggestions from Ismael Miguel

// After DOM is completely loaded
document.addEventListener("DOMContentLoaded", function() {
    "use strict";

    // Cache
    var palindromeInput = document.getElementById("inputPalindrome");
    var notification = document.getElementById("notification");

    function isPalindrome(str) {
        str = str.replace(/[^a-z0-9]+/gi, "").toLowerCase();
        return str.split("").reverse().join("") === str;
    }

    document.getElementById("checkPalindrome")
        .addEventListener("click", function() {

            if (isPalindrome(palindromeInput.value)) {
                notification.innerHTML = "Yay! You've got yourself a palindrome";
                notification.className = "alert alert-success";
            } else {
                notification.innerHTML = "Nay! Ain't no palindrome";
                notification.className = "alert alert-danger";
            }
        });
});

You can solve it without using regular expressions or reversing the string by just comparing the first and last characters (ignoring any characters that do not meet your requirements - i.e. non-alphanumeric characters) and then iterating inwards from both ends until you reach the middle of the string:

function isValidPalindromeCharacter( char ){
  return ( 'A' <= char && char <= 'Z' )
      || ( 'a' <= char && char <= 'z' )
      || ( '0' <= char && char <= '9' );
}

function isCharactersEqual( a, b ){
  return a.toLowerCase() == b.toLowerCase();
  // return (a.charCodeAt(0)|32) == (b.charCodeAt(0)|32);
}

function isPalindrome(str){
  var lower = 0,
      upper = str.length - 1;
  while ( true )
  {
    while ( lower < upper && !isValidPalindromeCharacter( str[lower] ) )
    {
      lower++;
    }
    while ( lower < upper && !isValidPalindromeCharacter( str[upper] ) )
    {
      upper--;
    }
    if ( lower >= upper )
    {
      return true;
    }
    if ( !isCharactersEqual( str[lower], str[upper] ) )
    {
      return false;
    }
    lower++;
    upper--;
  }
}

var palindrome   = document.getElementById("inputPalindrome");
var notification = document.getElementById('notification');

document.getElementById("checkPalindrome")
  .addEventListener("click", function() {
    'use strict';

    // value is much clear
    var value = palindrome.value;

    if (isPalindrome(value)) {
        notification.innerHTML = 'Yay! You\'ve got yourself a palindrome';
        notification.className = "alert alert-success";
    } else {
        notification.innerHTML = "Nay! Ain't no palindrome";
        notification.className = "alert alert-danger";
    }
  });

  <textarea id="inputPalindrome" rows="5">Borrow or rob?</textarea>
  <button id="checkPalindrome" type="button">Check palindrome</button>

Palindrome has not been checked yet


Reversal of singly linked list in JavaScript I have already implemented reversal of link list in javascript. Can somebody tell optimized way of doing it?I want to do it by less code.

Here I am providing the code:

var tempItem;
var finalLastNode;

function reverseLL(ll) {
  var currentObj = ll,
    prevItem, lastItem, currentTempObj;
  currentTempObj = tempItem;
  if (!currentObj) {
    console.log("No change")
  } else {
    while (currentObj.next) {
      prevItem = currentObj;
      currentObj = currentObj.next;
      lastItem = currentObj;
    }
    if (lastItem) {
      if (!tempItem) {
        tempItem = lastItem;
      } else {
        while (currentTempObj.next) {
          currentTempObj = currentTempObj.next;
        }
        if (tempItem && !tempItem.next) {
          tempItem.next = lastItem;
          finalLastNode = ll;
        } else {
          currentTempObj.next = lastItem;
        }
      }
      prevItem && (prevItem.next = null)
      reverseLL(ll);
    } else {
      while (currentTempObj.next) {
        currentTempObj = currentTempObj.next;
      }
      currentTempObj.next = finalLastNode;
    }
  }
  console.log(tempItem);
}
reverseLL(ll1)

"I want to do it by less code."

It's relatively easy: you keep another variable that denotes the head of the new list. Then, you remove the head node of the input list and make it a new head node of the new list being constructed:

function reverse(head) {
    new_head = head;
    old_head = head.next;
    new_head.next = null; // Terminate cycle.
    while (old_head) {
        current = old_head;
        old_head = old_head.next;
        current.next = new_head;
        new_head = current;
    }
    return new_head;
}

Usage

function node(val) {
    this.val = val;
    this.next = null;
}

a = new node(1);
b = new node(2);
c = new node(3);

a.next = b; b.next = c;
a = reverse(a); // Don't forget to store the new head node.

Hope that helps.

Though much the same as the first answer this is a little more minimalistic.

Note that there is no need to specify undefined (or null). The terminator is added by default in the function entry Also there is only one top variable thus no need to set the new top after the reverse function, as the top is used to move the items up the list and ends up at the top automatically.

Also you can collapse two lines to one as you already have top you just need its next thus top = oldTop.next; oldTop.next = top.next; can be shortened to  oldTop.next = (top = oldTop.next).next;

"use strict";
function linkedList() {
  var top, next;
  const entry = (item, next) => ({item, next});
  return {
    add(item) {
      if (!top) { next = top = entry(item) }
      else { next = next.next = entry(item) }
      return this;
    },
    show() {
      next = top;
      while (next) {
        console.log(next.item);
        next = next.next;
      }
      return this;
    },
    reverse() {
      var current, oldTop;
      current = oldTop = top;
      while (oldTop.next) {
        oldTop.next = (top = oldTop.next).next;
        top.next = current;
        current = top;
      }
      return this;
    },
  }
}

linkedList()
  .add("A").add("B").add("C").add("D").add("E").add("F").add("G").add("Last")
  .show()
  .reverse()
  .show()


Piccoche - a coordination game Click the divs to remove them. Remove all div's in the shortest time and minimal clicks (to do). Code needs streamlining.

<!DOCTYPE html>

I have a To Do list:

To Do

Add timer,
Add 'missed click counter',
Colours,
Size,
Positions,
Number,
Streamline code into DOM format,
Score,
Play again

Options

Flashers,
Multi-size,
Movers,
Bonuses (double points, static, combos, chains),
Anti-bonuses (move, faster, half points, bombs)

This is cleaner.
added <title> for no HTML Errors W3C Markup Validator.
changed CSS units px to em, for 100% W3C Mobile OK
changed width to 20em (320px) for Mobile Viewport
added mobile meta viewport for Google PageSpeed Insights 100% (less when target divs overlap)
Moved JS below HTML
Made count global
added
 to get misses
added
 to replace alert
replaced CSS div { with .target{
changed div ids to d+index
created target array and init code instead of calling getElementById
added [CDATA[ to JS
added miss counter
added elapsed time
added "Again" button and function
removed function rInt(), was not worth the stack push and pop of calling another function.

While some might say the update() should be added by EventListener. When only one event is required onclick is easy, reliable, and more universally compatible.

W3C HTML Markup Validation Service HTML5: NO ERRORS
W3C HTML Markup Validation Service XHTML Basic 1.1: NO ERRORS
W3C CSS Validation Service: NO ERRORS
W3C mobileOK Checker: Perfect Score: 100%

Google's "Usability Assesment" PageSpeed Insights

Mobile User Experience: Perfect Score: 100%
Desktop User Experience: Perfect Score: 100%
Mobile Speed: Perfect Score: Perfect Score: 100%
Desktop Speed: Perfect Score: 100%

Yahoo's YSlow Grade Perfect Score: 100% (gtmetrix.com)
Page Speed Grade: 99% (gtmetrix.com) 1% loss due to bug in PageSpeed scoring
Page Load time 0.250 seconds (gtmetrix.com)

HTML CSS

<!DOCTYPE html>
<title>Piccoche</title><meta name="viewport" content="width=device-width, initial-scale=1.0" />

All Gone!

<button onclick="playAgain()">Again</button>

JS

New Play Again function

function playAgain(){
  for (var div=0; div<10; div++){
    target[div].style.top = Math.floor(Math.random()*290) + 'px';
    target[div].style.left= Math.floor(Math.random()*290) + 'px';
    target[div].style.display = 'block';
  }
  count = 10;
  misses = -1;
  elapsed = new Date();
  document.getElementById('msg').style.display='none';
}

<!DOCTYPE html>

<title>Piccoche</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0" />

All Gone!

<button onclick="playAgain()">Again</button>

Tidied it up a bit, added dynamic timer (added current miss count should be a breeze). Gonna fix the div's up next. Made everything 'em' - changed CSS units px to em, for 100% W3C Mobile OK missed your +'px' in the code.


3 layer design and multiple dataset I'm new to C# and OO programing.
I have an aspx page with 3 lists of checkboxes and I would like to generate them from the DB.
how should I structure my code?

Here is an example of what I did so far:

ASPX page

<asp:CheckBoxList ID="CheckBoxList1" runat="server"></asp:CheckBoxList>
<asp:CheckBoxList ID="CheckBoxList2" runat="server"></asp:CheckBoxList>
<asp:CheckBoxList ID="CheckBoxList3" runat="server"></asp:CheckBoxList>

ASPX Code behind:

protected void Page_Load(object sender, EventArgs e)
{
    if (!IsPostBack)
    {
        GenerateCheckBoxList1();
        GenerateCheckBoxList2();
    }
}

private void GenerateCheckBoxList1()
{
    CBLists cbl = new CBLists();
    CheckBoxList1.DataSource = cbl.GetCheckBoxList1();
    CheckBoxList1.DataTextField = "name";
    CheckBoxList1.DataValueField = "id";
    CheckBoxList1.DataBind();
}

private void GenerateCheckBoxList2()
{
    CBLists cbl = new CBLists();
    CheckBoxList2.DataSource = cbl.GetCheckBoxList2();
    CheckBoxList2.DataTextField = "country";
    CheckBoxList2.DataValueField = "country_id";
    CheckBoxList2.DataBind();
}

a class file to handle the connection to the db and return the DataSet:

public class CBLists
{
    private SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["CONN"].ConnectionString);
public DataSet GetCheckBoxList1()
{
    try
    {
        DataSet ds = new DataSet();
        string cmdstr = "select * from table1 where id<>999 order by id";
        SqlDataAdapter adp = new SqlDataAdapter(cmdstr, conn);
        adp.Fill(ds);
        return ds;
    }
    catch (Exception ex)
    {
        return null;
    }
    finally
    {
        conn.Close();
    }

}

public DataSet GetCheckBoxList2()
{
    try
    {
        DataSet ds = new DataSet();
        string cmdstr = "select * from table2";
        SqlDataAdapter adp = new SqlDataAdapter(cmdstr, conn);
        adp.Fill(ds);
        return ds;
    }
    catch (Exception ex)
    {
        return null;
    }
    finally
    {
        conn.Close();
    }
}

Is this the "correct" way to do it? multiple methods in a separate file?

In 3-Layer design, we have below three layers

Presentation Layer: In this case your aspx and code behind files.

Business Layer: It seems this is missing in your project. You may include this for various purposes e.g., filtering items based on different scenarios, logged in user, etc.

Data Layer: This layer interacts with data store, external services, etc. CBLists class in this case.

Also, in OO programming class names are singular so CBLists should be changed to CBList.

In general for OO programming you separate out code into objects and each object focuses on doing one thing and doing it well.

Each object should then generally have it's own source file in your code tree.


I want to take away WebService call from UIViewController I am using Moya to make network calls in Swift.

In my ViewController I am calling the WebService and creating a model from response.

import UIKit
import Moya

class LoginViewController: UIViewController {
  let provider = MoyaProvider<Api>()

  override func viewDidLoad() {
    super.viewDidLoad()
    generateAccessToken()
  }

  private func generateAccessToken() {
    provider.request(.generateAccessToken(
        mobile: phoneTextField.text!,
        countryCode: "91",
        name: "Saurabh",
        otp: "9612")
    ) { result in
        switch result {
        case .success(let response):
            let data = response.data
            let user = try? JSONDecoder().decode(User.self, from: data)
            if let authToken = user?.authToken {
                AuthManager.setToken(token: authToken)
            }
        case .failure(let error):
            print(error.response?.statusCode as Any)
        }
    }
  }

I want to move the WebService call to other file, say NetworkManager.swift

import Foundation
import Moya

struct NetworkManager {
  public static let shared = NetworkManager()
  private let provider = MoyaProvider<Api>()

  func generateAccessToken(with mobile: String, countryCode: String, name: String, otp: String)  {
    // Get result and return to calling class
  }
}

But it is strongly recommended not to use Singleton Class.

How should I call my WebService and return the response to the LoginViewController?

I would recommend looking into MVVM architecture. You can see a simple example in this repo.
One problem with it is that its author declares and assigned view models directly in a ViewController which can be a bit problematic when writing tests. A better approach would be to inject view model so it can be easily replaced with mock. To improve that approach you can take a look at coordinators patterns, a bit more advanced example can be found here
In most cases the WebService should go in the ViewModel in the MVVM pattern. Services can be injected to ViewModels, which are responsible for managing data provided by these services. Then when they perform all logical transformations (filtering, ordering, multiplying etc.) they provide this information to a view (ViewController in our case) which just consumes it. It allows us to easily write mocks and tests. There are of course more approaches like state machine, but ViewModel as content provider and meeting point for services is most common and most universal one in my opinion.

Could you make the following changes:

1) Create a file named LoginViewModel.swift

func getAccessToken(with mobile: String, code: String, otp: String, name: String, completion: @escaping(Bool, String)->Void) {
      networkManager.getAccessTokenFrom(mobile: mobile, code: String, otp: code, name: name) { (isSuccess, error) in
        if isSuccess {
            // Do neccessary logic
            // Set access token logic here
            completion(true, "")
        }
        else {
            completion(false, error.description)
        }
    }
}

2) Create a similar function in NetworkManager class to interact with Moya framework

let provider = MoyaProvider<Api>()

func getAccessTokenFrom(mobile: String, code: String, otp: String, name: String, completion: @escaping(error, String)->Void) {
      provider.request(.generateAccessToken(
        mobile: mobile,
        countryCode: code,
        name: name,
        otp: otp)
    ) { result in
        switch result {
    case .success(let response):
            let data = response.data
            let user = try? JSONDecoder().decode(User.self, from: data)
            if let authToken = user?.authToken {
                completion(true, authToken)
            }
        case .failure(let error):
        completion(false, error.description)

        }
}}
}

Inside LoginViewController.swift:

let loginViewModel = LoginViewModel()

func viewDidload() {
    // start activity loader
    loginViewModel.getAccessToken(with mobile: mobileTField.text, code: codeTField.text, otp: otpTField.text, name: nameTField.text) { (isSuccess, error) in
        if isSuccess {
            // Hide activity loader
        }
        else {
            // Show alert based on the "error" variable value
        }
    }
}


Check if a given string can be a palindrome by altering some character without using a library function I was asked in an interview to write code to check if a given string is a palindrome or can be a palindrome by altering some character without using library function.

My logic is to:

Compare 1st char with remaining chars. If it exists, remove both from array using removeElement().
If it does not exist in the char array, then use removeFirstElement() to remove the first element 1 time only.
chackPotentialPalindrom() returns true if a String can be a palindrome, otherwise false.

package package1;

import java.util.Scanner;

public class Palindrom {
    static int  temp=0;
    static char[] cArr;
        static boolean chackPotentialPalindrom(char[] cAr){
            cArr=cAr;
            if(cArr!=null){
                char current=cArr[0];
                for(int i=1;i<cArr.length;i++){
                    if(current==cArr[i]){
                        cArr=removeElement(i);
                        chackPotentialPalindrom(cArr);
                        break;
                    }
                    }
                if(cAr.length==2){
                if(cAr[0]==cAr[1]){
                    cArr=null;
                }}
                if(temp==0 && cArr!=null){
                    temp=1;
                    cArr=removeFirstElement(cArr);
                    chackPotentialPalindrom(cArr);
                    }
                }
            if(cArr==null){
                return true;
            }else{
                return false;
            }
        }
        static char[] removeFirstElement(char[] cAr){
            cArr=cAr;
            if(cArr!=null){
            if(cArr.length >1){
            char[] cArrnew=new char[cArr.length-1];
            for(int j=1,k=0;j<cArr.length;j++,k++){
                cArrnew[k]=cArr[j];
            }
            return cArrnew;
            } else {
                return null;
            }
                } else {
                    return null;
                }
        }
        static char[] removeElement(int i){
            if(cArr.length>2){
            char[] cArrnew=new char[cArr.length-2];
            for(int j=1,k=0;j<cArr.length;j++,k++){
                if(i!=j){
                    cArrnew[k]=cArr[j];
                }else{
                    k-=1;
                }
            }
            return cArrnew;}
            else{
                return null;
            }
        }
        public static void main(String[] args) {
            Scanner scn=new Scanner(System.in);
            while(true){
                temp=0;
            String s=scn.next();
            char[] arr=s.toCharArray();
            System.out.println(chackPotentialPalindrom(arr));
            }
        }
    }

Any tips to optimize this code?
I could not write this in an interview as they have given a pen and paper to code.
It took 3 hrs for me to write this. Can I be a developer?

The above looks to be more work than is necessary. Assuming that we are working with all lower case chars, the following should work.

We match the chars at each end, moving inwards on each cycle. Normally, if any do not match, we consider it a fail and break out. If we are allowed a one pair mismatch (i.e. change one character), then we need to keep track of the count of mismatches and if we have 0 or 1 (less than 2) we consider it to be a palindrome.

private static boolean isAlmostPalindrome(String str){
    int diffCount = 0;
    int left = 0;
    int right = str.length() -1;

    while(right>left){
        if (str.charAt(right--)!= str.charAt(left++)){
            diffCount++;
        }
    }
    return diffCount <2;
}

It's late and I'm not really awake, but should be able to do it by processing array elements together.  For odd length strings, the middle element is obviously equals and I believe this skips it.

boolean success = true;
char[] array = stringToCheck.toCharArray();
int length = stringToCheck.length();
int halfLength = (int) length / 2;

for (int i = 0; sucess && i < halfLength; i++) {
   success = (array[i] == array[length - 1 - i]);
}

System.out.println ("Palindrome=" + success);

So much simpler than you were making it out to be.


Persistent, dictionary-like object ensurance This is a part of a project I'm working on. The codebase is Python 2.7 and I don't have an option to switch to Python 3.

Code:

import shelve
def get_document_info():
    token_shelf = False

    try:
        token_shelf = shelve.open(os.path.join(current_dir, 'document_info.shelve'))
        token_mapping = token_shelf['token_mapping']

        if token_mapping:
            return token_mapping
        else:
            return False

    except Exception, e:
        pass

    finally:
        if token_shelf:
            token_shelf.close()

Shelve is a persistent, dictionary-like object. More info can be found at https://docs.python.org/2/library/shelve.html

I need to ensure that token_shelf is closed at the end. In the beginning of the function, what do I define token_shelf as? If I don't define something there, PyCharm will throw an error saying that the token_shelf in finally block does not exist.

What's the ideal way to write this piece of code? How can it be made more elegant and Pythonic?

PS: This code is working as expected right now.

Ideally you would want to use Python's with ... as syntax, which creates a context that calls the exit method of whatever object you create. For this, the shelve.Shelf classes would need to implement the dunder methods __enter__ and __exit__ (which they don't).

At this point you basically have three options:

Inherit from them and implement it yourself:

class Shelf(shelve.Shelf):

    def __new__(cls, file_name):
        return shelve.open(file_name)

    def __enter__(self):
        return self

    def __exit__(self, *args):
        self.close()

Or just write a simple wrapper class like this:

class Shelf(object):

    def __init__(self, file_name):
        self.file_name = file_name

    def __enter__(self):
        self.obj = shelve.open(self.file_name)
        return self.obj

    def __exit__(self, *args):
        self.obj.close()

Both can be used like this:

import os
import shelve

def get_document_info(current_dir):
    shelf_path = os.path.join(current_dir, 'document_info.shelve')
    with Shelf(shelf_path) as token_shelf:
        return token_shelf.get('token_mapping')

When you use try..except you should always guard against as specific exceptions as possible (so that unexpected exceptions can still rise to the top). In this case this would probably be a KeyError. Here I used the get method instead, which will return None if the key is not found.

I also removed the return False, because you should check for falsiness in the calling code. In other words, if you are doing if token_mapping is False or  if token_mapping == False in the calling code, you are doing it wrong. Just have the if not token_mapping/if token_mapping there.

Use Python's contextlib.closing, which already implements something like option 2:

from contextlib import closing
import os
import shelve

def get_document_info(current_dir):
    shelf_path = os.path.join(current_dir, 'document_info.shelve')
    with closing(shelve.open(shelf_path)) as token_shelf:
        return token_shelf.get('token_mapping')

def get_document_info():

global token_mapping

try:
    return token_mapping if (token_mapping = shelve.open(os.path.join(current_dir, 'document_info.shelve'))['token_mapping']) else None

except Exception, e:
    token_shelf.close()
    pass

Forcibly changed your return of False as that is bad practice as you can't substitute the shelve object everywhere unless you pre-wrap it in conditionals -- always. Honestly been a decade since I wrote anything in Python. So this is what I have pieced together from quick look-ups on tutorials online. Perhaps Python has more elegantly looking Ternary Operators (Perl is nice because it has default-ly named variables).

But, even then I never forgot that Python is a Dynamically-Typed language and that every variable is just a Reference to an instance and every instance is Strongly-typed.


Handling arrow keys for a console menu I have a relatively long method, and I'm wondering:

How readable is it?
Is a method this long okay, or will it be bad later on?

Note: This method enables users to select from a console menu - UpArrow and DownArrow change option, and LeftArrow and RightArrow change amount.

Here is the method (from here):

    public static List<Item> Spend(string[] text,List<Item> items,int currency,out int currencyLeft,string singular=null,string textZero=null){
        Initiate(items.Count);
        var maxNameLength=items.Max(n=>n.Name.Length);
        var sellIsEnabled=items.All(v=>v.Value>=0);
        var maxCurrencyLength=Math.Max(items.Aggregate(currency,(i,j)=>j.Value).Length()+text.Take(3).Aggregate(0,(i,j)=>i+j.Length)+
            Math.Max(0,(singular=singular??text[1].RemoveLast()).Length-text[1].Length),textZero?.Length??0);
        var line=Top+1;
        var totalItems=items.Select(i=>new Item(i)).ToList();
        while(true){
            switch(Input){
                case UpArrow:
                    Option--;
                    Index--;
                    break;
                case DownArrow:
                    Option++;
                    Index++;
                    break;
                case LeftArrow:
                    if(totalItems[Option].Amount-items[Option].Amount>0){
                        totalItems[Option].Amount--;
                        currency+=items[Option].Cost;
                    } else if(sellIsEnabled&&totalItems[Option].Amount>0){
                        totalItems[Option].Amount--;
                        currency+=items[Option].Value;
                    } else{
                        Input=ReadKey(true).Key;
                        continue;
                    }
                    break;
                case RightArrow:
                    if(currency-items[Option].Cost>=0){
                        totalItems[Option].Amount++;
                        currency-=items[Option].Cost;
                    } else if(sellIsEnabled&&currency-items[Option].Value>=0&&totalItems[Option].Amount-items[Option].Amount<0){
                        totalItems[Option].Amount++;
                        currency-=items[Option].Value;
                    } else{
                        Input=ReadKey(true).Key;
                        continue;
                    }
                    break;
                case Enter:
                    currencyLeft=currency;
                    return totalItems;
                default:
                    Input=ReadKey(true).Key;
                    continue;
            }
            if(Input==LeftArrow||Input==RightArrow){
                totalItems[Option].Amount.WriteAt(maxNameLength+3,line,maxLength:totalItems[Option].Amount.Length()+2);
                (currency!=0||textZero==null?text[0]+(currency==0?"no":currency.ToString())+(currency==1?singular:text[1])+text[2]:textZero).WriteAt(0,Top+MaxLines+2,maxCurrencyLength);
            }
            Maths.RMod(ref Option,TotalOptions);
            Select.TurnPage();
            if(ThePage.Was(Turned)){
                var maxAmountLength=totalItems.Max(t=>t.Amount.Length());
                var top=Top;
                foreach(var i in Indices){
                    items[i].Name.WriteAt(2,++top,maxNameLength);
                    totalItems[i].Amount.WriteAt(maxNameLength+3,top,maxAmountLength);
                }
                for(var i=Indices.Count;i<MaxLines;i++) "".WriteAt(2,++top,1+maxNameLength+maxAmountLength);
            }
            if(Input==UpArrow||Input==DownArrow){
                ' '.WriteAt(Left,line);
                '>'.WriteAt(Left,line=Index+Top+1);
            }
            Input=ReadKey(true).Key;
            PagePlus=0;
        }
    }

There's a lot about this method that makes it very difficult to follow... Let's start with the signature:

public List<Item> Spend(/* ... */)

So, based on the name and return type I'm going to "Spend" something(?) and get a list of items. To me, I find it had to believe this method is going to have a single responsibility straight away. The parameters only confirm the suspicion:

string[] text, List<Item> items, int currency, out int currencyLeft, string singular = null, string textZero = null

Wow. So I'm passing in some "text", a list of items, an amount of currency and optionally a "singular" string (no idea what that is) and a textZero string (again no idea) AND I'm getting an output of how much currency I have left. This is ringing alarm bells - why am I passing in text to a function that lets me spend money... Why is it an array? Why am I passing in items? At the very least I know this method is: showing info to the user, changing how much currency I have and probably changing what items I own. That's 3 separate responsibilities already and we haven't even got into the method yet.

Let's start on that:

var maxCurrencyLength=Math.Max(items.Aggregate(currency,(i,j)=>j.Value).Length()+text.Take(3).Aggregate(0,(i,j)=>i+j.Length)+
        Math.Max(0,(singular=singular??text[1].RemoveLast()).Length-text[1].Length),textZero?.Length??0);

That should be a method by itself! It's basically the most unreadable variable declaration I've ever seen (sorry if that sounds harsh).

Then there's this:

var totalItems=items.Select(i=>new Item(i)).ToList();

Okay, you're deep cloning the items (I would guess) - no indication of why and you've made me try to figure out what you're doing.

while (true) {
    switch (Input)

Most people writing C# prefer braces on new lines. What is Input? A property that gets input from the user?

I could go on but that's not very constructive. What you need to do, is go back to the drawing board and come up with some classes to encapsulate all the different things going on here.

public class Shop
{
    public IList<Item> GetAvailableItems()
    {
        // ..
    }

    public int GetSellItemPrice(Item item)
    {
        // ..
    }
}

You'll need other classes too. Like a Player with an ItemBag (for storing items) and a Wallet (for storing money). Potentially, Transaction objects to pass into Shops/Wallets to change the amount of money etc.

So to answer your question:  the method length is the least of your worries, you need to break the problem up into smaller pieces and make each piece responsible for a single thing.

The method size is probably ok, however it is hard to read and understand mainly because of:

lack of comments

what is the purpose of this methods?
what does it do?
what are the parameters and how they are related to each other?

too many chained operations and long lines


Search for a multiword string in a list of strings Unlike Perl, you can't to my knowledge match a regular expression inside an if statement in Python and assign the result to a varaible at the same moment. This leads to typical constructs like this:

match = re.search(REGEX, STRING)
if match:
    # do something

So far, so Python. But what if I want to iterate through a file / array of lines, check each line for a few regexes, and fire a catch-all when none has matched? I can't think my way around a rather unwieldy and deeply nested if-else-if-else...-construction:

import re
strings = ["abc zzz", "y", "#comment"]
for s in strings:
    match = re.search("(\S+) (\S+)", s)
    if match:
        print "Multiword: %s+%s" % (match.group(1), match.group(2))
    else:
        match = re.match("y$", s)
        if match:
            print "Positive"
        else:
            match = re.match("n$", s)
            if match:
                print "Negative"
            else:
                # a few more matches possible in real life script,
                # and then the last catch-all:
                print "No match found, line skipped"

Isn't there any way to put this in a much nicer looking elif-construction or something? The following doesn't work in Python, because if-clauses take only expressions, not statements. However, something along these lines would strike me as pythonic, or am I blind to something obvious here?

if match = re.search(" ", s):
    print "Multiword: %s+%s" % (match.group(1), match.group(2))
elif match = re.match("y$", s):
    print "Positive"
else:
    print "No match found, line skipped"

Just use the continue keyword to force the advancement to evaluate the next string.
Code after the if statement will only execute if the previous statement was false.

import re
strings = ["abc zzz", "y", "#comment"]
for s in strings:
    match = re.search("(\S+) (\S+)", s)
    if match:
        print "Multiword: %s+%s" % (match.group(1), match.group(2))
        continue
    match = re.match("y$", s)
    if match:
        print "Positive"
        continue
    match = re.match("n$", s)
    if match:
        print "Negative"
        continue

    # a few more matches possible in real life script,
    # and then the last catch-all:
    print "No match found, line skipped"

You can make flow of your application more linear by either return-ing from the method when failing or do it Python way using Exceptions, e.g.:

try:
    if not do_action_1:
        raise Exception('Action 1 failed')
    if not do_action_2:
        raise Exception('Action 2 failed')
    if not do_action_3:
        raise Exception('Action 3 failed')
    if not do_action_4:
        raise Exception('Action 4 failed')
    if not do_action_5:
        raise Exception('Action 5 failed')
except Exception as e:
    print 'Whoops: %s' % e


Recursive Regular Expressions I want to first search for a specific regular expression.
If it is not found then I would like to search for another regular expression
If that one is also not found then I would like to search for a third
(And so on...)

Whenever a matching regular expression is found, I want to return a String array containing:

The text before the match
The match itself
The text after the match

I've only come up with this horribly if-else nesting way of doing it. There's gotta be a better (more extensible and more clean) way but I can't come up with how...

Here is the code, it is currently only containing two regular expressions to search for and it's getting nasty already:

private static String[] findNumbers(String string) {
    Matcher match = Pattern.compile("\\d+[\\:\\.\\,]\\d+").matcher(string);
    if (match.find()) {
        String found = string.substring(match.start(), match.end());
        return new String[]{ string.substring(0, match.start()),
            found, string.substring(match.end()) };
    }
    else {
        match = Pattern.compile("\\d{3,}").matcher(string);
        if (match.find()) {
            String found = string.substring(match.start(), match.end());
            return new String[]{ string.substring(0, match.start()),
                found, string.substring(match.end()) };
        }
        else {
            // Another if-else could be added here
            return new String[]{ string, "", "" };
        }
    }
}

@Test
public void test() {
    assertArrayEquals(new String[]{ "ABC ", "2000", " DEF" }, findNumbers("ABC 2000 DEF"));
    assertArrayEquals(new String[]{ "42 ABC ", "2000", " DEF" }, findNumbers("42 ABC 2000 DEF"));
    assertArrayEquals(new String[]{ "42 ABC ", "18:47", " DEF" }, findNumbers("42 ABC 18:47 DEF"));
}

I want to put priority to one regular expression before trying the next, that's why I have written this code.

Nesting is not necessary, you can iterate instead...

But first:

naming the input String string can be confusing. Of course it's a string, what is it used for though?
there is no need to escape the values inside the [...] construct. the pattern: "\\d+[:.,]\\d+" is just fine, and more readable.
you should be pre-compiling and then reusing the patterns, instead of re-compiling them every time.

Consider the following:

private static final Pattern[] patternorder = {
        Pattern.compile("\\d+[:.,]\\d+"),
        Pattern.compile("\\d{3,}")
};

private static String[] findNumbers(String input) {
    for (Pattern pat : patternorder) {
        Matcher match = pat.matcher(input);
        if (match.find()) {
            return new String[] {
                input.substring(0, match.start()),
                input.substring(match.start(), match.end()),
                input.substring(match.end()),
            };
        }
    }
    return new String[]{input, "", ""};
}

You could use the (expression1|expression2|expression3) syntax. It makes for a long string, but you won't have to nest.


Populating fields from stored procedure I was just wondering if my code below is a good way of populating data from database. I have had no issues with this really but I am being taught this and just wondering if it is the best practice.

 Public Sub LoadProperty()
    '**Finds the Current Property Data

    'Error Checking
    ' On Error GoTo Err_LoadProperty
    Try

        Dim uRecSnap As ADODB.Recordset

        ' Check For Open Connection
        If uDBase Is Nothing Then
            OpenConnection()
            bConnection = True
        End If

        ' Run Stored Procedure - Load Property Record
        uCommand = New ADODB.Command
        With uCommand
            .ActiveConnection = uDBase
            .CommandType = ADODB.CommandTypeEnum.adCmdStoredProc
            .CommandTimeout = 0
                 uCommand.Parameters.Append(uCommand.CreateParameter("@PropertyID", ADODB.DataTypeEnum.adInteger, ADODB.ParameterDirectionEnum.adParamInput, , Val(lblPropertyIDValue.Text)))
            .CommandText = "PropertyMaster_LoadRecord"

            uRecSnap = .Execute
        End With

        ' Store Data Values
        Do Until uRecSnap.EOF
            lblPropertyIDValue.Text = If(IsDBNull(uRecSnap("PropertyID").Value), "", uRecSnap("PropertyID").Value)
            lblSPMReferenceValue.Text = If(IsDBNull(uRecSnap("SPMReference").Value), "", uRecSnap("SPMReference").Value)
            cmbPropertyManager.Text = If(IsDBNull(uRecSnap("PropertyManager").Value), "", uRecSnap("PropertyManager").Value)
            txtAddress1.Text = If(IsDBNull(uRecSnap("AddressLine1").Value), "", uRecSnap("AddressLine1").Value)
            txtAddress2.Text = If(IsDBNull(uRecSnap("AddressLine2").Value), "", uRecSnap("AddressLine2").Value)
            txtAddress3.Text = If(IsDBNull(uRecSnap("AddressLine3").Value), "", uRecSnap("AddressLine3").Value)
            txtTown.Text = If(IsDBNull(uRecSnap("Town").Value), "", uRecSnap("Town").Value)
            txtPostCode.Text = If(IsDBNull(uRecSnap("PostCode").Value), "", uRecSnap("PostCode").Value)
            chkAvailable.Checked = If(IsDBNull(uRecSnap("Availabilty").Value), "", uRecSnap("Availabilty").Value)
            dtpAvailable.Value = If(IsDBNull(uRecSnap("Available").Value), "1/1/1900", uRecSnap("Available").Value)
            chkFactored.Checked = If(IsDBNull(uRecSnap("Factored").Value), 0, uRecSnap("Factored").Value)
            txtFactorName.Text = If(IsDBNull(uRecSnap("FactorsName").Value), "", uRecSnap("FactorsName").Value)
            txtFactorsEmail.Text = If(IsDBNull(uRecSnap("FactorsEmail").Value), "", uRecSnap("FactorsEmail").Value)
            dtpPropertyBuilt.Value = If(IsDBNull(uRecSnap("PropertyBuilt").Value), "1/1/1900", uRecSnap("PropertyBuilt").Value)
            txtPropertyValue.Text = If(IsDBNull(uRecSnap("PropertyValue").Value), "", uRecSnap("PropertyValue").Value)
            txtMinimumFee.Text = If(IsDBNull(uRecSnap("MimimumFee").Value), "", uRecSnap("MimimumFee").Value)
            txtCostAuthorisationAmount.Text = If(IsDBNull(uRecSnap("CostSuthorisationAmount").Value), "", uRecSnap("CostSuthorisationAmount").Value)
            txtCommision.Text = If(IsDBNull(uRecSnap("Commission").Value), "", uRecSnap("Commission").Value)
            chkVacant.Checked = If(IsDBNull(uRecSnap("Vacant").Value), 0, uRecSnap("Vacant").Value)
            dtpVacant.Value = If(IsDBNull(uRecSnap("VacantDate").Value), "1/1/1900", uRecSnap("VacantDate").Value)
            txtStartingRent.Text = If(IsDBNull(uRecSnap("StartingRent").Value), "", uRecSnap("StartingRent").Value)

            uRecSnap.MoveNext()
        Loop

        ' Close Connection
        uRecSnap.Close()
        If bConnection Then CloseConnection()
        uRecSnap = Nothing

    Catch ex As Exception
        MsgBox(vbCrLf & ex.Message & vbCrLf & "Source---" & vbCrLf & ex.Source & vbCrLf & "Problem---" & vbCrLf & ex.StackTrace, MsgBoxStyle.Exclamation + MsgBoxStyle.OkOnly, "SouthSide- Error Reporting")

    End Try

End Sub

It might have been an ok-ish way of doing things... 15 years ago. This code is astonishingly similar to what one would be writing in vba - minus the Try..End Try block.

If you want to learn the .NET way of doing things, I strongly recommend you remove this from every VB.NET module you're writing:

Imports Microsoft.VisualBasic

Things in that namespace exist to make an easier transition between vb6/vba and vb.net - pretty much everything in it has an actual .net-idiomatic equivalent elsewhere in the framework.

ADODB / ActiveX Data Objects is replaced with ado.net, itself rendered obsolete with linq-to-sql, itself a predecessor of entity-framework. But one step at a time - start with ado.net.
MsgBox is replaced with System.Windows.Forms.MessageBox
vbCrLf should have been vbNewLine, and both are replaced with Environment.NewLine

You're not including the module-scope fields, and yet the code you posted seems to use plenty... but I don't understand why. You're opening a module-scoped connection, fetching data, and then closing it. Why would that object ever need to exist beyond that? You don't need that Boolean bConnection.

ADODB being COM / unmanaged, it doens't implement the IDisposable interface. See this Stack Overflow answer for information about proper cleanup of COM objects... which should motivate you to use managed code ;-)

In short:

  Use Marshal.ReleaseComObject to tidy up COM objects. ADODB is a COM library being used through COM Interop.

Now, what you have here is essentially a Smart UI pattern: the UI runs the show, and controls everything. That's good for learning how things work, fiddling around and prototyping things, but it's also one of the best ways to create a spaghetti mess out of production code.

Your form should only be responsible for one thing: being a user interface. Accessing a database is not the job of a UI, it's that of some data service.

You should create a class to act as your Model, that would expose a property for each value you're fetching from a database record.

Then the View (/form) wouldn't need to know about a database or connections or anything - it would simply display what the Model has in store for it. And then when the user makes changes, you can take the modified Model and pass it to the same data service that provided you with it, to write the modified values into the database.

Read up on the Model-View-Presenter pattern for more details.

Consider adding a finally block to safely close the connection.


Hanoi towers time exeeded I am having problem with a simple Hanoi tower program.  I need to do something to reduce its time.

#include <stdio.h>
#include <fstream>

using namespace std;

int hanojus(int n, char a, char b, char c) {

    ofstream fr("hanoj.out",ios::app);
    if (n == 1)
        fr<<a<< " -> " <<c<<endl;
    else {
         hanojus(n-1, a, c, b);
         fr<<a<< " -> " <<c<<endl;
         hanojus(n-1, b, a, c);
        }
}
int main()
{
    ifstream fd("hanoj.in");
    int sk;
    fd>>sk;
    hanojus(sk, 'A', 'B', 'C');
}

Try moving the ofstream outside of the method so that you're not creating it every time you make a recursive call.

Like this:

#include <stdio.h>
#include <fstream>

using namespace std;

int hanojus(int n, char a, char b, char c, ofstream& fr)
{
    if (n == 1)
        fr<<a<< " -> " <<c<<endl;
    else
    {
        hanojus(n-1, a, c, b, fr);
        fr<<a<< " -> " <<c<<endl;
        hanojus(n-1, b, a, c, fr);
    }
}

int main()
{
    ifstream fd("hanoj.in");
    ofstream fr("hanoj.out",ios::app);
    int sk;
    fd>>sk;
    hanojus(sk, 'A', 'B', 'C', fr);
}

The most efficient is to use non-recursive algorithm.
You can read about it in wikipedia.

Time:  Same as recursive algorithm

Memory:  Unlike the recursive version, the configuration of the
rings must be explicitly represented; it suffices to use 3 stacks, each
holding a maximum of n rings.

Note: to store stack it is better to use std::vector or static array instead of std::stack.
You can use C-style io. It often works faster. Do not create output stream each time (if you use recursive algorithm).
If you compile your code, use -O2 or -O3 to create more optimized program.


Connecting to RabbitMQ I would like a review for this connection to RabbitMQ. I just developed it and seems to be working well, but I would like another set of eyes on it before putting this on the sever.

package models

import com.rabbitmq.client.Connection
import com.rabbitmq.client.ConnectionFactory
import com.rabbitmq.client.ConnectionFactory
import com.rabbitmq.client.MessageProperties

import anorm.SQL
import anorm.sqlToSimple
import anorm.toParameterValue
import play.api.Play.current

object RabbitMQConnection {
  private var connection: Connection = null
  def getConnection(ss:Connection): Connection = {
    println(ss+"  connection <<<<<<<<<<<<<<<<<<<<<<<<")
    connection=ss
    connection match {
      case null => {
        val factory = new ConnectionFactory()
        println("waiting for new connection")
        factory.setHost("172.22.22.222")
        println("host setted")
        connection = factory.newConnection()
        println("connection created")
        connection
      }
      case _ =>{
        println("connection is not null")
        connection
      }
    }
  }
}

object RMQ {
  var connection = RabbitMQConnection.getConnection(null)

  def setQ(qName: String, message: String) = {
    println("ping received")
    try {
      println(connection)
      if (connection != null) {
        if (connection.isOpen()) {
          println("connection is open")
        } else {
          connection = RabbitMQConnection.getConnection(null)
          println("connection is new "+connection)
        }
        println("connetion is ready to use")
        val channel = connection.createChannel()
        channel.queueDeclare(qName, true, false, false, null) //suggestion
        channel.basicPublish("", qName, MessageProperties.PERSISTENT_TEXT_PLAIN, message.getBytes())
        println("status" + channel.close())
        println("setQ complete executed for " + qName)
        Map("result" -> "success")

      } else {
        println("connection can't established to rabbit mq for =>" + qName)
        LogFile.QLogs(qName, message)
        Map("result" -> "error")
      }
    } catch {
      case e: Exception =>
        println(e.printStackTrace())
        println("Rabbit Mq Server is Down for =>" + qName)
        LogFile.QLogs(qName, message)
        Map("result" ->"error")
    }
  }

}

Well, the first thing I would recommend is replacing all of those println statements with some kind of logging. In any kind of production environment, logging is something that needs more thought and care so that it is useful, without consuming resources unnecessarily. Or, if they're just for the original developer during development, maybe delete them.

I find the structure of this code to be rather odd. For all the trappings of OO programming, what you essentially have is two global public functions and one global public variable.

The function name setQ doesn't seem to say what you're actually doing - which appears to be creating a channel and publishing a message to it. publishMessage would make more sense, I think.

All of the code that checks the usability of the connection should be extracted into a single function, something along the lines of haveUsableConnection - the two if/else blocks are making it hard to see the code that is actually doing the work.

I question the hard coded IP address. Magic numbers and magic strings are frequently a bad idea.

To connect with RabbitMQ using C# code kindly follow below steps.

You should have valid Host Name that you may get it from RabbitMQ URL
Virtual Domain name
User name and Password if you are connecting with remote RabbitMQ server

Please see below code,

using RabbitMQ.Client;
using RabbitMQ.Client.Events;
using System;
using System.Collections.Generic;
using System.Linq;
 using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json;
using System.IO;
using System.Reflection;
namespace RMQConnect
{
public class RabbitMQQueueManager
   {
       private const string ORIGINAL_QUEUENAME_KEY = "originalQueueName";
      private const ushort PREFETCH_SIZE = 50;

    public IConnection GetRabbitMqConnection()
    {
        ConnectionFactory factory = new ConnectionFactory
        {
            HostName = "d1vmrmqtrv01.oldev.arol.shell.com",
            VirtualHost = "TRV_ANDD",
            UserName = "ol_sqlmaint_trv",
            Password = "P@ssw0rd",

        };
        return factory.CreateConnection();
    }

    public List<Message> GetMessagesFromQueueNoAck(string queueName, int
    messageCount = -1)
    {
        QueueingBasicConsumer consumer = null;
        var responseMessages = new List<Message>();
        BasicDeliverEventArgs result = null;
        using (var rmqConnection = GetRabbitMqConnection())
        {
            using (var channel = rmqConnection.CreateModel())
            {
                try
                {
                    var queueMessageCount = (int)channel.MessageCount(queueName);
                    var count = messageCount > -1 ? messageCount <=
                    queueMessageCount ? messageCount : queueMessageCount :
                    queueMessageCount;
                var pfCount = count >= PREFETCH_SIZE ? PREFETCH_SIZE : count;
                channel.BasicQos(0, (ushort)pfCount, false);
                consumer = new QueueingBasicConsumer(channel);
                channel.BasicConsume(queueName, false, consumer);
                for (int i = 0; i < pfCount; i++)
                {
                    if (!channel.IsOpen)
                    {
                        throw new ApplicationException("Channel is closed");
                    }
                    result = consumer.Queue.Dequeue();
                    try
                    {
                        string messageData =
                        System.Text.Encoding.UTF8.GetString(result.Body);
                        var rMessage = new Message(messageData);
                        rMessage.Header = new MessageHeader();

    RmqHeaderHandler.ReadRmqMessageProperties(result.BasicProperties, rMessage);
                        channel.BasicNack(result.DeliveryTag, false, true);
                        responseMessages.Add(rMessage);
                    }
                    catch (Exception ex)
                    {
                        channel.BasicNack(result.DeliveryTag, false, true);
                    }
                }
            }
            catch (Exception)
            {
                ////Nack the message back to queue in case of exception
                if (result != null)
                {
                    channel.BasicNack(result.DeliveryTag, false, true);
                }
                throw;
            }
        }
    }
    return responseMessages;
    }
   }
    public static class RmqHeaderHandler
    {
    private const byte NonPersistentDeliveryMode = 1;
  private const byte PersistentDeliveryMode = 2;
   private const string SecurityTokenKey = "SecurityToken";
  private const string Properties = "properties";
  private const string MessageNameKey = "MessageName";
  private const string SystemPropertiesKey = "SystemProperties";
  private const string ApplicationPropertiesKey = "ApplicationProperties";
  #region Public Methods
   public static void ReadDynamicMessageProperties(dynamic messageProperties,
   Message message)
   {
      try
      {
          message.Header.AppId = messageProperties.appId;
          message.Header.MessageId = messageProperties.messageId;
          message.Header.GeneratedAtUtc = messageProperties.generatedAtUtc;
          message.Header.ExpirationInMilliseconds =
          messageProperties.expirationInMilliseconds;
          message.Header.IsPersistent = messageProperties.isPersistent;
          message.Header.Delete = Convert.ToBoolean(messageProperties.delete);
          message.Header.Move = Convert.ToBoolean(messageProperties.move);
          if (messageProperties.ContainsKey("messageName"))
        {
            message.Header.MessageName = messageProperties.messageName;
        }
        if (messageProperties.ContainsKey("properties"))
        {
            var customProperties =
        Newtonsoft.Json.JsonConvert.DeserializeObject<Dictionary<string, string>>
       (Convert.ToString(messageProperties.properties));
            foreach (var propPair in customProperties)
            {
                message.Header.Properties.Add(propPair.Key, propPair.Value);
            }
        }
    }
    catch (Exception)
    {
        throw;
    }
}
public static void ReadRmqMessageProperties(IBasicProperties messageProperties,
Message message)
{
    //message.Header.AppId = messageProperties.AppId;
    message.Header.MessageId = messageProperties.MessageId;
    message.Header.GeneratedAtUtc = new
    DateTime(messageProperties.Timestamp.UnixTime);
    message.Header.ExpirationInMilliseconds = messageProperties.Expiration;
    message.Header.IsPersistent = messageProperties.DeliveryMode ==
    PersistentDeliveryMode;
    if (messageProperties.Headers!=null &&
     messageProperties.Headers.ContainsKey(SystemPropertiesKey))
    {
        var systemProperties =
        DeserializeMessageProperties((byte[])
      messageProperties.Headers[SystemPropertiesKey]);
        if (systemProperties.ContainsKey(MessageNameKey))
        {
            message.Header.MessageName = systemProperties[MessageNameKey];
        }
    }
    if (messageProperties.Headers!=null &&
     messageProperties.Headers.ContainsKey(ApplicationPropertiesKey))
    {
        var applicationProperties =
       DeserializeMessageProperties((byte[])
       messageProperties.Headers[ApplicationPropertiesKey]);
        foreach (var propPair in applicationProperties)
        {
            message.Header.Properties.Add(propPair.Key, propPair.Value);
        }
    }
}
#endregion
#region private methods
private static Dictionary<string, string> DeserializeMessageProperties(byte[]
  properties)
{
    //var serializer = new .JsonMessageSerializer();
    var serializedText = JsonConvert.SerializeObject(properties);
    return JsonConvert.DeserializeObject<Dictionary<string, string>>
   (serializedText);
 }
#endregion
}
public class QueueInfoModel
{
    public string Environment { get; set; }
  public string ApplicationGroup { get; set; }
  public string ApplicationName { get; set; }
public string ErrorQueueName { get; set; }
public string OriginalQueueName { get; set; }
public int MessageCount { get; set; }
}
  [Serializable]
  public class MessageHeader
 {
     public MessageHeader()
     {
        this.MessageId = Guid.NewGuid().ToString();
        this.Properties = new Dictionary<string, string>();
        this.IsPersistent = true;
    }
    public string AppId { get; set; }
    public string MessageId { get; set; }
    public string MessageName { get; set; }
    public DateTime GeneratedAtUtc { get; set; }
    public string ExpirationInMilliseconds { get; set; }
    public bool IsPersistent { get; set; }
    public bool Delete { get; set; }
    public bool Move { get; set; }
    public IDictionary<string, string> Properties { get; private set; }
 }
[Serializable]
 public class Message
{
    private readonly string serializableBody;
    public Message(string serializableBody)
    {
        this.serializableBody = serializableBody;
    }
public MessageHeader Header
{
    get;
    set;
}
public string MessageBody
{
    get
    {
        return this.serializableBody;
    }
}
public byte[] GetBody()
{
    return (byte[])((object)Encoding.ASCII.GetBytes(this.MessageBody));
   }
 }

 }


Simple rwlock implementation in c++11 For answering this question of stackoverflow I have written the code
https://stackoverflow.com/questions/12033188/how-would-you-implement-your-own-reader-writer-lock-in-c11

Can someone review it - so that I can understand the possible problems in the code.

#include <condition_variable>
#include <iostream>
#include <shared_mutex>
#include <thread>
#include <unistd.h>
#define NR_THREADS 10
#include <mutex>

class MySharedLock {
 public:
  void read_lock() {
    std::unique_lock<std::mutex> lk(rw_mutex);
    std::cout << "\nReader Lock Writers are  " << writers << std::flush;
    if (writers != 0) {
      rw_cv.wait(lk, [this]() { return (this->writers == 0); });
    }
    readers++;
    lk.unlock();
  }

  void write_lock() {
    std::unique_lock<std::mutex> lk(rw_mutex);

    std::cout << "\nWriter Lock Writers are  " << writers << " Readers are "
              << readers << std::flush;
    if (readers == 0 && writers == 0) {
      std::cout << "\nWriter Lock Writers are  " << writers << std::flush;
    } else {
      rw_cv.wait(
          lk, [this]() { return (this->writers == 0 && this->readers == 0); });
    }
    writers++;
    lk.unlock();

  }

  void write_unlock() {
    std::lock_guard<std::mutex> lk(rw_mutex);
    writers--;
    rw_cv.notify_all();
  }

  void read_unlock() {
    std::lock_guard<std::mutex> lk(rw_mutex);
    if (readers == 1) {  // I am the last one.
      rw_cv.notify_all();
    }
    readers--;
  }

  explicit MySharedLock() {}

 private:
  std::mutex rw_mutex;
  std::condition_variable rw_cv;
  uintmax_t readers = {0}, writers = {0};
};

First, I suppose it would be cheating to just use a std::shared_mutex, right? :)

Second, it seems a little weird to declare uintmax_t writers for a variable that can only ever hold the value 0 or 1. But, on the other hand, the symmetry between readers and writers is kind of nice.

uintmax_t readers = {0}, writers = {0};

Strongly prefer to declare one variable per line. Also, I know some people like to write int i{0};, and of course the natural thing to write is int i = 0;, but your belt-and-suspenders approach is relatively unusual. Prefer

uintmax_t readers = 0;
uintmax_t writers = 0;  // or, bool has_writer = false;

explicit MySharedLock() {}

I am a big fan of this explicit zero-argument constructor! However, you should probably be in the habit of =defaulting things instead of {}.

if (writers != 0) {
  rw_cv.wait(lk, [this]() { return (this->writers == 0); });
}

Nit: A lambda that doesn't escape should generally just capture [&]. And you needn't write out this-> if you don't want to. So:

if (writers != 0) {
  rw_cv.wait(lk, [&]() { return (writers == 0); });
}

However, this could be more cleanly expressed as simply

while (writers != 0) {
  rw_cv.wait(lk);
}

lk.unlock();

unique_lock is an RAII type; it unlocks the controlled mutex automatically in its destructor. You don't need to (and therefore shouldn't) write .unlock explicitly in your code.

This implementation has the potential problem that it delegates all of the "queueing" logic to the mutex/cv themselves. It is possible that if customers show up as "reader, writer, reader, writer, reader, writer...", then the readers could starve out the writers, or even vice versa. The only way to fix this, AFAIK, is to implement some kind of a "queue" of waiters, so that each waiter knows exactly when it's at the head of the queue. (Imagine the ticketing system in a deli: "Now serving number 47!")

Inside reader_unlock, you have rw_cv.notify_all() — but I think that this could safely be changed to rw_cv.notify_one(). We know that nobody is waiting to read; therefore all waiters are writers; therefore at most one of them will actually be able to do any work right now; therefore it is sufficient to wake up just one of them.

(And of course by the time that guy wakes up, somebody new may have come in and stolen the lock. So even that guy might not get to do any work.)

However, what notify_all does do is transfer over all the waiters from waiting on the rw_cv to waiting on the rw_mutex. If std::mutex implements some kind of fair queueing system, this ensures that no writer will get starved for longer than one epoch-of-readers, because as soon as the epoch ends, all the writers will get woken up and transferred over to the mutex, so that their respective epochs-of-writers will happen bam bam bam one after the other.

Yeah, I think I've convinced myself that notify_all is actually the most effective thing to do here. But it probably deserves a code comment explaining why you think it's the right thing to do!

Finally, it's worth mentioning that the standard names for your methods (according to the standard SharedMutex concept) are:

read_lock     ->  lock_shared
read_unlock   ->  unlock_shared
write_lock    ->  lock
write_unlock  ->  unlock

There's no standard C++ name for the promotion of a held reader lock to a writer lock (which is deceptively tricky), nor for the downgrading of a held writer lock to a reader lock (which is trivial). You might think about whether you want to provide these operations, and if so, what the interface to them should be.

here is my implementation
class RWLock {
public:
    virtual void rlock() = 0;
    virtual void runlock() = 0;
    virtual void wlock() = 0;
    virtual void wunlock() = 0;
};

/**
 * a reader preferred lock
 */
class RWLock1: public RWLock{
public:
    void rlock() override {
        unique_lock<mutex> guard(m_reader_lock);
        ++m_count;
        if (m_count == 1) {
            m_res_lock.lock();
        }
    }

    void runlock() override {
        unique_lock<mutex> guard(m_reader_lock);
        --m_count;
        if (m_count == 0) {
            m_res_lock.unlock();
        }
    }

    void wlock() override {
        m_res_lock.lock();
    }

    void wunlock() override {
        m_res_lock.unlock();
    }

private:
    mutex m_res_lock; // resource lock
    mutex m_reader_lock; // reader count lock
    int m_count = 0; // reader count
};

// a relatively fair rwlock
class RWLock2: public RWLock {
public:
    void rlock() override {
        lock_guard<mutex> ser_guard(m_ser_lock);
        lock_guard<mutex> read_guard(m_reader_lock);
        ++m_read_count;
        if (m_read_count == 1) {
            m_res_lock.lock();
        }
    }

    void runlock() override {
        lock_guard<mutex> read_guard(m_reader_lock);
        --m_read_count;
        if (m_read_count == 0) {
            m_res_lock.unlock();
        }
    }

    void wlock() override {
        lock_guard<mutex> ser_guard(m_ser_lock);
        m_res_lock.lock();
    }

    void wunlock() override {
        m_res_lock.unlock();
    }
private:
    mutex m_res_lock; // resource lock
    mutex m_ser_lock; // 服务锁，只有拿到服务锁才能被服务, 只有开始服务才释放锁
    mutex m_reader_lock; // reader count lock
    int m_read_count = 0; // 读计数
};

void run_rwlock_test(int r_count, int rw_ratio, RWLock &lock) {
    ASSERT_TRUE(rw_ratio > 0);
    vector<int> data;
    vector<thread> ths;
    for (int i = 0; i < r_count; ++i) {
        // add reader
        ths.emplace_back(thread([&, i=i]{
            cout << "reader [" << i << "] started" << endl;
            lock.rlock();
            cout << "reader [" << i << "] reading: " << data.size() << endl;
            this_thread::sleep_for(chrono::milliseconds(1));
            lock.runlock();
            cout << "reader [" << i << "] done" << endl;
        }));

        // add writer
        if (i % rw_ratio == 0) {
            ths.emplace_back(thread([&, i=i/rw_ratio]{
                cout << "writer [" << i << "] started" << endl;
                lock.wlock();
                data.push_back(i);
                cout << "writer [" << i << "] writing: " << data.size() << endl;
                this_thread::sleep_for(chrono::milliseconds(1));
                lock.wunlock();
                cout << "writer [" << i << "] done" << endl;
            }));
        }
    }

    for (auto &th: ths) {
        th.join();
    }

    ASSERT_EQ(data.size(), r_count / rw_ratio);
}
```


High Performance Calculation of Sums and Returns in Go I am writing a finance library with the intention to have the best of Golang's performance.

The function Re calculates the returns of a float list, and function Sum calculates the sum of float list.

//Calculate returns
func Re(data []float64) []float64 {

    re_ := make([]float64, len(data))
    for i := 1; i < len(data); i++ {
        re_[i] = ( (data[i]/data[i-1])-1)
    }

    return re_
}

// sum of float64 list
func Sum(floatList []float64) float64 {

    var totalx float64
    for _, valuex := range floatList {
        totalx += valuex
    }

    return totalx
}

is there any further possible optimisation on these two functions or another more performant way to write them?

In Go, when you want to measure function performance, use the Go testing package to run a benchmark.

For example,

returns.go:

package finance

// Calculate returns
func Re(data []float64) []float64 {
    var re []float64
    if len(data) > 0 {
        re = make([]float64, len(data))
    }
    for i := 1; i < len(data); i++ {
        if data[i-1] != 0 {
            re[i] = (data[i] / data[i-1]) - 1
        }
    }
    return re
}

// Calculate sum
func Sum(data []float64) float64 {
    var sum float64
    for _, x := range data {
        sum += x
    }
    return sum
}

returns_test.go:

package finance

import "testing"

func BenchmarkRe(b *testing.B) {
    data := make([]float64, 5*12)
    for i := range data {
        data[i] = float64(i)
    }
    b.ResetTimer()
    for N := 0; N < b.N; N++ {
        Re(data)
    }
}

func BenchmarkSum(b *testing.B) {
    data := make([]float64, 5*12)
    for i := range data {
        data[i] = float64(i)
    }
    b.ResetTimer()
    for N := 0; N < b.N; N++ {
        Sum(data)
    }
}

Output:

$ go test returns.go returns_test.go -bench=. -benchmem
BenchmarkRe-4     7545566   156 ns/op     480 B/op    1 allocs/op
BenchmarkSum-4   28272992    41.5 ns/op     0 B/op    0 allocs/op
$

I looked at some other alternatives, but that's the fastest.

Note: I fixed your divide-by-zero bug.


Improving search through XML file I'm working on a function that extracts information from an XML document and compares to what the user has typed in. Can you suggest any ways in which I can improve it? The variables in the XML file are compared against txtSearch.

function checkXML() {
    var s = "";
    var albumPages = [];

    for (var i = 0; i < xmlalbums.length; i++) {
        var xmlalbum = xmlalbums[i];
        var album_title = dataFromTag(xmlalbum, 'title');
        var artist = dataFromTag(xmlalbum, 'artist');
        var xmltracks = xmlalbum.getElementsByTagName('track');
        for (var t = 0; t < xmltracks.length; t++) {
            var track = xmltracks[t];
            song_title = dataFromTag(track, 'title')
            albumPages[albumPages.length] = artist.toUpperCase() + ':' + album_title.toUpperCase() + ':' + song_title.toUpperCase();
        }
    }

    var resultList = '';
    var resultCount = [];

    for (var i = 0; i < albumPages.length; i++) {
        var searchArtist = albumPages[i].substring(0, albumPages[i].indexOf(":"));
        var searchAlbum = albumPages[i].substring(albumPages[i].indexOf(":") + 1, albumPages[i].lastIndexOf(":"));
        var searchSong = albumPages[i].substring(albumPages[i].lastIndexOf(":") + 1, albumPages[i].length);
        if (searchArtist.indexOf(txtSearch) != -1 || searchAlbum.indexOf(txtSearch) != -1 || searchSong.indexOf(txtSearch) != -1) {
            resultList += "<option>" + searchSong + "</option>"
            resultCount[resultCount.length] = i;
        }
    }

    if (resultList.length == 0) {
        s += "No result found!"
    }
    else if (resultList.length > 0) {
        s += '<select size="' + (resultCount.length + 1) + '" onclick="parent.col3(value);">';
        s += resultList;
        s += '</select>';
    }
    else {
        s += "Awaiting search query..."
    }

    with(document.getElementById('col2').contentDocument) {
        open();
        write(s);
        close();
    }
}

A few general things from a quick scan:

declare all your variables at the top in one var statement rather than using it throughout the script
use somearray.push(x) rather than somearray[somearray.length]=x, as its shorter and means you don't have to look up the .length property every time in your loop
in general, avoid with; there are many reasons why e.g. http://www.yuiblog.com/blog/2006/04/11/with-statement-considered-harmful/
string concatenation is slow; instead of many += operators, create an array, use .push() as above to add strings to the array, then use .join('') to merge them together.

And yes, XPath would be a better approach to extract data from XML.

I have not benchmarked the different processes, but it is my belief that using XPath to traverse/parse your XML would be a much better option than using string methods.

It's a big subject, but you might want to start here