Object oriented design for a Tree I am implementing a Tree in Java. It must be easy to add a new type of tree e.g. Binary tree and Tertiary tree. Each node can have at least one value. It must be easy to make a new type of node and make a tree of that node.

interface Node<T> {

    T getVal();

    boolean equals(Node<T> node);

    String toString();
}

interface Tree<T extends Node> {
    boolean addNode(T node);

    boolean removeNode(T node);

    String toString();
}

public class BinaryTreeNode<T> implements Node<T> {

    T value;
    BinaryTreeNode<T> firstChild, secondChild;

    BinaryTreeNode(T value) {
        this.value = value;
        firstChild = null;
        secondChild = null;
    }

    public T getVal() {
        return value;
    }

    public boolean equals(Node<T> node) {
        return this.value.equals(node.getVal());
    }

    public String toString() {
        return "" + value;
    }

    boolean setFirstChild(BinaryTreeNode<T> node) {
        firstChild = node;
        return true;
    }

    BinaryTreeNode<T> getFirstChild() {
        return firstChild;
    }

    boolean setSecondChild(BinaryTreeNode<T> node) {
        secondChild = node;
        return true;
    }

    BinaryTreeNode<T> getSecondChild() {
        return secondChild;
    }
}

import java.util.*;

public class BinaryTree<T extends BinaryTreeNode> implements Tree<T> {

BinaryTreeNode root;

    BinaryTree() {
        root = null;
    }

    public boolean addNode(T node) {

        if(root == null) {
            root = node;
        } else {
            addToLast(root, node);
        }

        return true;
    }

    boolean addToLast(BinaryTreeNode root, T node) {

        Deque<BinaryTreeNode> queue = new LinkedList<>();

        queue.addLast(root);
        while(!queue.isEmpty()) {
            BinaryTreeNode cur = queue.pollFirst();

            if(cur.getFirstChild() == null) {
                cur.setFirstChild(node);
                break;
            } else if(cur.getSecondChild() == null) {
                cur.setSecondChild(node);
                break;
            } else {
                queue.addLast(cur.getFirstChild());
                queue.addLast(cur.getSecondChild());
            }
        }

        return true;
    }

    public boolean removeNode(BinaryTreeNode node) {

        throw new UnsupportedOperationException();
    }

    public String toString() {

        Deque<BinaryTreeNode> queue = new LinkedList<>();

        if(root == null) {
            System.out.println("root is null");
            return "";
        }

        StringBuffer ans = new StringBuffer("");
        queue.addLast(root);
        while(!queue.isEmpty()) {
            BinaryTreeNode cur = queue.pollFirst();
            ans.append(" " + cur);

            if(cur.getFirstChild() != null)
                queue.addLast(cur.getFirstChild());

            if(cur.getSecondChild() != null)
                queue.addLast(cur.getSecondChild());

        }

        return ans.toString();
    }

    public static void main(String args[]) {

        BinaryTree<BinaryTreeNode> btree = new BinaryTree<>();

        BinaryTreeNode<Integer> node1 = new BinaryTreeNode<>(1);
        btree.addNode(node1);

        System.out.println(btree);
    }
}

Following this design pattern, I think it is easier to implement different types of binary trees like binary search tree, heap, red-black tree, etc. Also, one can easily add more information to a node while implementing own implementation of tree.

Is this a good design?

Short answer: no.

But you'll probably need some explanation to believe me.

The whole Node thing you got there should be internal information to the Tree class, not part of the type. Let's look at the following example method to show why it's a bad idea:

private static void addARandomNumber(Tree<Node<Number>> tree) {
    int randomNumber = generateRandomNumber();
    Node<Number> = new ... ???
    tree.addNode(node);
}

What class should we use to create a new Node here?

Had the Node been completely internal to the Tree implementation then the interface would look like this:

interface Tree<T> {
    boolean add(T node);

    boolean remove(T node);

    String toString();
}

And now we can also write the add random number method easily:

private static void addARandomNumber(Tree<Number> tree) {
    int randomNumber = generateRandomNumber();
    tree.add(randomNumber);
}

I also got a feeling that forcing some ordering on T, as required in most search trees, will be rather hard in your current design. But that might just be because I didn't really try it myself yet.

Defining the type of the search tree correctly would probably work, but I expect some type erasure issues with the addNode and removeNode methods.


Redefine count-leaves as an accumulation Exercise 2.35.  Redefine count-leaves
  from section 2.2.2 as an accumulation:

(define (count-leaves t) (accumulate <??> <??> (map <??> <??>)))

I wrote the following:

(define (accumulate op initial sequence)
  (if (null? sequence)
      initial
      (op (car sequence)
          (accumulate op initial (cdr sequence)))))

(define (count-leaves t)
  (accumulate +
              0
              (map (lambda (subtree)
                     (if (pair? subtree)
                         (count-leaves subtree)
                         1))
                   t)))

(define a (list 1 2 (list 1) (list 1 2 (list 2 (list 1 2)))))

What do you think?

The code doesn't handle the case of only a single leaf, i.e.

 (count-leaves 'leaf)

but I guess that's a side-effect of the original problem formulation. A cleaner solution would be

(define (count-leaves t)
  (if (pair? t)
      (accumulate + 0 (map count-leaves t))
      1)))

also because it's shorter but this doesn't fit the pattern of the original exercise.

Incidentally, in this case the standard Scheme procedure apply would be enough because the Scheme + routine takes in an arbitrary number of arguments, and you could write just:

(define (count-leaves t)
  (if (pair? t)
      (apply + (map count-leaves t))
      1)))


Is this a reasonable binary search implementation? import java.util.Arrays;

public class BinarySearch {

  // this class should not be instantiated

  private BinarySearch() { }

  // searches for the integer key in the sorted array a[]
  // @param key the search key
  // @param a the array of integers, must be sorted in ascending order
  // @return index of key in array a[] if present; -1 if not present

  public static int rank(int key, int[] a) {
    int lo = 0;
    int hi = a.length - 1;
    while (lo <= hi) {
      // key is in a[lo..hi] or not present
      int mid = lo + (hi - lo) / 2;
      if (key < a[mid]) hi = mid - 1;
      else if (key > a[mid]) lo = mid + 1;
      else return mid;
    }
    return 1;

  }

  // reads in a sequence of integers from the whitelist file, specified as a command line argument. reads in integers from standard input and prints to standard output
  // those integers that do not appear in the file.
  public static void main(String[] args){

    // read the integers from a file
    In in = new In(args[0]);
    int[] whitelist = in.readAllInts();

    // sort the array
    Arrays.sort(whitelist);

    // read integer key from standard input; print if not in whitelist
    while (!StdIn.isEmpty()) {
      int key = StdIn.readInt();
      if (rank(key, whitelist) == -1)
        StdOut.println(key);
    }
  }

}

Your implentation can search integers, we expect to search in an array of any comparable objects.

I think you have a bug if the element is not found. It will return 1 instead of -1 /  throwing an exception. Add unit tests.

Also, Java is now open source, you can compare your implementation with the one from the JDK (Arrays.binarySearch).

Also, in code style:

it is recommended to use {} in if block, even if they are one-liner


AES256 Implementation I coded a little implementation to encrypt and decrypt bytes with AES256 (ECB) in C#. I wanted to know if I can optimize/cleanup my code a bit more, or if it i fine like this:

using System;
using System.Security.Cryptography;
using System.Text;

namespace AES256
{
    class Program
    {

        private static string getString(byte[] b)
        {
            return Encoding.UTF8.GetString(b);
        }

        static void Main(string[] args)
        {

            byte[] data = Encoding.UTF8.GetBytes("This is just a little message to test AES.");

            byte[] key = { 7, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8,
                           1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8 };

            byte[] enc = Encrypt(data, key);
            byte[] dec = Decrypt(enc, key);

            Console.WriteLine("Input : {0}", getString(data));
            Console.WriteLine("Key : {0}", getString(key));
            Console.WriteLine("KeySize : {0}", 256);
            Console.WriteLine("Encrypted : {0}", getString(enc));
            Console.WriteLine("Decrypted : {0}", getString(dec));

            Console.ReadKey();

        }

        public static byte[] Encrypt(byte[] data, byte[] key)
        {
            using (AesCryptoServiceProvider csp = new AesCryptoServiceProvider())
            {
                csp.KeySize = 256;
                csp.BlockSize = 128;
                csp.Key = key;
                csp.Padding = PaddingMode.PKCS7;
                csp.Mode = CipherMode.ECB;
                ICryptoTransform encrypter = csp.CreateEncryptor();
                return encrypter.TransformFinalBlock(data, 0, data.Length);
            }
        }

        private static byte[] Decrypt(byte[] data, byte[] key)
        {
            using (AesCryptoServiceProvider csp = new AesCryptoServiceProvider())
            {
                csp.KeySize = 256;
                csp.BlockSize = 128;
                csp.Key = key;
                csp.Padding = PaddingMode.PKCS7;
                csp.Mode = CipherMode.ECB;
                ICryptoTransform decrypter = csp.CreateDecryptor();
                return decrypter.TransformFinalBlock(data, 0, data.Length);
            }
        }

    }
}

The code is simple to understand and does what it needs to do. Just some remarks:

Whenever you use the ECB cipher mode, you must justify in written form why you use it. This is because this cipher mode can reveal patterns in your plain text.
Converting the encrypted bytes to an UTF-8 string doesn't make sense. You should better print them in hex or base64.
Your code contains duplicates. You could extract that code into a method:

private AesCryptoServiceProvider CreateProvider(byte[] key) {
    return new AesCryptoServiceProvider {
        KeySize = 256,
        BlockSize = 128,
        Key = key,
        Padding = PaddingMode.PKCS7,
        Mode = CipherMode.ECB
    };
}

You made the Encrypt method public, but Decode is private. I don't see a reason for this asymmetry.


Generating a QuestionID Using a Nested Loop for a Quiz Game So I started learning OOs and code refactoring and decided to do some project. It just some simple quiz game.

Here are some mechanics

There are 3 sets
Each set have 8 Brands (types)
Each brand have 10 questions

So what I did was this

Set Class

public class Set{
    public string setName;
    public Brand[] brands;
}

Brand Class

public class Brand{
    public string brandName;
    public Question[] questions;
}

Question Class

public class Question {
    public string question;
    public string[] choices = new string[3];  //choices for each question
    public string correctAnswer;  //will hold the correct answer for a question

    public string questionID(){
       // making a id for question
        string qID = "";

        qID += question + "|";

        foreach(string choice in choices){
            qID += choice + "|";
        }

        qID += correctAnswer + "|";

        qID += fontSize;

        return qID;
    }

}

So what I need is to create some kind of id for each question, what I did is this

public class QuestionManager : MonoBehaviour {
public void saveToDB(){
    int setID = 0;
      foreach(Set set in sets){

          int brandID = 0;
          foreach(Brand brand in set.brands){

              int questionID = 0;
             foreach(Question question in brand.questions){

                string questionKey = "s" + setID + "|" + "b" + brandID + "|" + "q" + questionID;
                string questionAnswer =  "value:"+ set.setName + "|" + brand.brandName + "|" + question.questionID() ;

                PlayerPrefs.SetString(
                key: questionKey,
                value: questionAnswer
                );  // save to the database

                questionID++;
          }
            brandID++;
          }
          setID++;
      }

}
}

I'm really curious if there are some good refactoring for this kind of loop. Also I should add that I'll be using this ids for getting the questions later.

You ask for a refactoring of the current method. It could be:

public void saveToDB()
{
  int setID = 0;
  foreach (Set set in sets)
  {
    int brandID = 0;
    foreach (Brand brand in set.brands)
    {
      int questionID = 0;
      foreach (Question question in brand.questions)
      {
        PlayerPrefs.SetString(
          key: $"s{setID}|b{brandID}|q{questionID}",
          value: $"value:{set.setName}|{brand.brandName}|{question.questionID()}"); // save to the database

        questionID++;
      }
      brandID++;
    }
    setID++;
  }
}

Here is used string interpolation $"s{setID}...." instead of string concatenation which is considered to be bad practice because it results in too many string instantiations and is hard to read.

It could be done by using LINQ, but that is maybe more confusing and will actually be slower:

public void saveToDB()
{
  var questionInfos = sets.SelectMany((set, setId) => set.brands.SelectMany((brand, brandId) => brand.questions.Select((question, questionId) => new { key = $"s{setId}|b{brandId}|q{questionId}", value = $"value:{set.setName}|{brand.brandName}|{question.questionID()}" })));

  foreach (var questionInfo in questionInfos)
  {
    PlayerPrefs.SetString(questionInfo.key, questionInfo.value);
  }
}

Just for the sport, it can be done in inline query syntax (with a little help from method syntax) as t3echb0t suggested:

public void saveToDB()
{
  var questionInfos = from setAndId in sets.Select((set, id) => (set, id))
                      from brandAndId in setAndId.set.brands.Select((brand, id) => (brand, id))
                      from questionAndId in brandAndId.brand.questions.Select((question, id) => (question, id))
                      select new
                      {
                        key = $"s{setAndId.id}|b{brandAndId.id}|q{questionAndId.id}",
                        value = $"value:{setAndId.set.setName}|{brandAndId.brand.brandName}|{questionAndId.question.questionID()}"
                      };

  foreach (var questionInfo in questionInfos)
  {
    PlayerPrefs.SetString(questionInfo.key, questionInfo.value);
  }
}

That said, I would consider an mandatory Id to be a property on each Set, Brand and Question instance:

public abstract class ObjectBase
{
  public ObjectBase(string name, int id)
  {
    Name = name;
    Id = id;
  }

  public string Name { get; }
  public int Id { get; }
}

public class Set : ObjectBase
{
  public Set(string name, int id) : base(name, id)
  {

  }

  public Brand[] Brands { get; set; }
}

public class Brand : ObjectBase
{
  public Brand(string name, int id) : base(name, id)
  {

  }

  public Question[] Questions { get; set; }
}

public class Question : ObjectBase
{
  public int FontSize;
  public string[] Choices = new string[3];  //choices for each question
  public readonly int CorrectAnswerId;  //will hold the correct answer for a question

  public Question(string question, int correctAnswerId, string[] choices, int id) : base(question, id)
  {
    CorrectAnswerId = correctAnswerId;
    Choices = choices;
  }

  public string TheQuestion => Name;

  public string QuestionID()
  {
    // making a id for question
    return $"{TheQuestion}|{string.Join("|", Choices)}|{CorrectAnswerId}|{FontSize}";
  }

}

public class QuestionManager //: MonoBehaviour
{
  Set[] sets = new Set[0];

  public void saveToDB()
  {
    foreach (Set set in sets)
    {
      foreach (Brand brand in set.Brands)
      {
        foreach (Question question in brand.Questions)
        {
          PlayerPrefs.SetString(
            key: $"s{set.Id}|b{brand.Id}|q{question.Id}",
            value: $"value:{set.Name}|{brand.Name}|{question.QuestionID()}"); // save to the database
        }
      }
    }
  }
}

Notice here that there is no point in calling the name of a Set "setName - Name is sufficient.
Further notice that it is common practice in C# to use PascalCase for public members.

The benefit of having the entities be born with an id, is that you can use that id to identify each instance uniquely throughout the session/lifetime of each object. And as shown the key/value-pair for PlayerPrefs.SetString is easier created.


Speeding up storage of about 220000 values in Core Data I use this code on the app's first run to save about 220000 values into core data. The file Navaids.txt has about 200000 and Waypoints.txt has about 20000 lines, where I get data from each line and put it in an entity in core data.

I am still kind of new to CD but would like to see how I can improve this code on app's first launch to make it as fast as possible. Right now it takes about 10 seconds to run on an iPad but would like to see how I can improve.

Let me know if I should provide samples of my .txt files or if you need to see any of my delegate class.

if (self.firstRun == TRUE) {
        NSString* navaidPath = [[NSBundle mainBundle] pathForResource:@"Navaids" ofType:@"txt"];
        NSString* navContent = [NSString stringWithContentsOfFile:navaidPath encoding:NSUTF8StringEncoding error:NULL];
        NSArray *navContentLines = [navContent componentsSeparatedByString:@"\r\n"];

        NSString* wayptPath = [[NSBundle mainBundle] pathForResource:@"Waypoints" ofType:@"txt"];
        NSString* wayptContent = [NSString stringWithContentsOfFile:wayptPath encoding:NSUTF8StringEncoding     error:NULL];
        NSArray *wayptContentLines = [wayptContent componentsSeparatedByString:@"\r\n"];

        @autoreleasepool {
        for (int i = 0; i < [wayptContentLines count]; i++) {
            @autoreleasepool {

            NSArray *brokenFix = [[wayptContentLines objectAtIndex:i] componentsSeparatedByString:@"|"];

                newContact = [NSEntityDescription
                              insertNewObjectForEntityForName:@"Waypoints"
                              inManagedObjectContext:context];

                [newContact setValue: [brokenFix objectAtIndex:0] forKey:@"name"];
                [newContact setValue: [brokenFix objectAtIndex:1] forKey:@"latitude"];
                [newContact setValue: [brokenFix objectAtIndex:2] forKey:@"longitude"];

            }
        }
        }

        @autoreleasepool {
            for (int i = 0; i < [navContentLines count]; i++) {
                @autoreleasepool {

                    NSArray *brokenNav = [[navContentLines objectAtIndex:i] componentsSeparatedByString:@"|"];

                    newContact = [NSEntityDescription
                                  insertNewObjectForEntityForName:@"Navaids"
                                  inManagedObjectContext:context];

                    [newContact setValue: [brokenNav objectAtIndex:0] forKey:@"icao"];
                    [newContact setValue: [brokenNav objectAtIndex:1] forKey:@"name"];
                    [newContact setValue: [brokenNav objectAtIndex:6] forKey:@"latitude"];
                    [newContact setValue: [brokenNav objectAtIndex:7] forKey:@"longitude"];
                 }
            }
        }

         NSError *error;
        [context save:&error];

Ten seconds for reading/writing nearly half a million lines from/to disk is actually relatively decent.

The autorelease pool doesn't actually slow us down.  Autorelease pool is handled by the preprocessor.  So is ARC.  Instead of us manually writing our release/retains, ARC, or autorelease pool, will do this for us.  However, I actually would make a change to how you're handling the autorelease pools so that you're minimizing the amount of information you're keeping in RAM at any given time.  For example:

@autoreleasepool {
    // read waypoints from file into memory
    // loop through waypoints array putting them into core data
}

@autoreleasepool {
    // read navaids from file into memory
    // loop through navaids array putting them into core data
}

Make no reference to waypoints outside the first autorelease pool and no reference to navaids outside the second autorelease pool and you'll be using approximately half as much RAM throughout this method.

Though... a cleaner way might be to just put each in their own method and then the same is accomplished without autorelease pools (which again, will not slow down your code).

We can however speed up your code.  If we use a forin loop rather than a traditional for loop the code will run faster as we'll overall be passing significantly fewer messages.  For example:

for (NSString *waypoint in wayptContentLines) {
    NSArray *brokenFix = [waypoint componentsSeparatedByString:@"|"];

    newContact = [NSEntityDescription insertNewObjectForEntityForName:@"Waypoints"
                                               inManagedObjectContext:context];

    [newContact setValue: [brokenFix objectAtIndex:0] forKey:@"name"];
    [newContact setValue: [brokenFix objectAtIndex:1] forKey:@"latitude"];
    [newContact setValue: [brokenFix objectAtIndex:2] forKey:@"longitude"];
}

Now, instead of calling to wayptContentLines twice per iteration (nearly a million times) (once to check length, once to grab the object at index), we're instead calling to the array less than once per iteration (the exact amount varies).

A forin loop in Objective-C reads several objects into a buffer then deals with the objects in the buffer before reading the objects in.  I think on average it reads in about 8 objects, so you go from 2/i to 1/8i messages to wayptContentLines.  In English, a for loops probably sends about 16 times as many messages to the array as a forin loop would... and over 400,000+ iterations, thats a LOT of messages and it will take up a lot of time.


The problem “Fun game” from a competition I recently began to learn Python and encountered this problem.
The condition:
You and your friends are playing the next game. Friends write  natural numbers in a row on the board. Your task is to find as many consecutive numbers as possible that are divisible by the same number greater than 1. Since it is difficult to find the answer manually, you decided to write a program that will do the job for you.
Input:
The first line of the input contains the number  (1 ≤  ≤ 100000). The second line is separated by a space  integers 1... (1 ≤  ≤ 1000, 1 ≤  ≤ ). These are the numbers that your friends wrote. They are given in the same order as they are placed on the board.
Output:
Your program should output a single integer — the largest number of consecutive numbers in a given sequence that would be divisible by the same natural number greater than 1.
Here is my code:
Python

from math import gcd

def func(n,a):

    mx = 0
    for i, cur_g in enumerate(a):
        if n - i < mx:
            break
        p = 0
        for j in range(i, n):
            cur_g = gcd(cur_g, a[j])
            if cur_g == 1:
                break
            p += 1
            if mx < p:
                mx = p

    return mx

print(func(int(input()), [int(i) for i in input().split()]))

The problem is that I can't pass the time check: the program runs for longer than 0.5 seconds. And I can't think of any way to speed up the program. It may even be necessary to solve the problem itself in a different way. Please help me. Thanks in advance!

The way your loops are organized results in an unpleasant phenomenon. Namely, you process the same subsequence many times. Let's say, a long good range starts at the index k and has a length n. The inner loop finds it. Then the next iteration of the outer loop starts processing it again, this time from index k + 1. Then from index k+2, etc. It will take \$O(n^2)\$ of unnecessary computations. In fact, each good range contributes a term quadratic to its length to the total complexity.
The fix is, once the inner loop terminated, adjust i to continue from the number which broke the good range.


Best practice for lining up PHP syntax in HTML page I'm using Codeigniter but this goes for any project in PHP. Let's say I have the following code in my view. I've been struggling, trying to figure out how best to indent and display PHP code within standard HTML code.

...
...

    <?php
        if ($this->session->flashdata('message')) {
            ...
            ...
    ?>

            ...
            ...

    <?php
        }
    ?>

So notice how I nested HTML within the PHP code by escaping w/ the PHP syntax. This can get out of hand if you have lines and lines of code. I'm using Komodo and I've enabled "Indentation Guidelines" but even with that, most of my code doesn't line up. So my question is, how best to handle this?

I've actually resorted to keeping the PHP marker <?php ?> to the far left and not indenting them. Seems to work so far ...

You can use other kind of syntax:

...
...

    <?php if ($this->session->flashdata('message')): ?>

            ...
        ...

    <?php endif; ?>

So, <?php ?> can be in 1 line and I think it's beauty:)

Anyway, There are no any 'right' way to do it. You can(and should) choose your own one


Determining whether a string ends with one of a few given options I wrote the following function to determine whether a string ends with one of a few given options.

I'm sure it can be written more elegantly, probably avoiding the loop.

bool EndsWithOneOf(string value, IEnumerable<string> suffixes)
{
    foreach(var suffix in suffixes)
    {
        if value.EndsWith(suffix)
            return true;
    }
    return false;
}

You can LINQify it to improve readability:

bool endsWithOneOf = suffixes.Any(x => value.EndsWith(x));

Note that this doesn't "avoid the loop", since Any() will iterate through the suffixes (stopping when it hits a match.) But that's ok, since how else could you do it? You have an enumeration of suffixes, so to do anything with them, you must enumerate them.


A simple fully connected ANN module I've written a simple module that creates a fully connected neural network of any size. The arguments of the train function are list of tuples with a training example array first and an array containing its class second, list that contains a number of neurons in every layer including the input and the output layers, a learning rate and a number of epochs.
To run a trained network I've written the run function, its arguments are an input and weights from the trained network. Since I'm a beginner to programming and machine learning I'll be very happy getting advice regarding computational efficiency and optimization.

import numpy as np
def weights_init(inSize,outSize): #initialize the weights
    return 2*np.random.random((inSize,outSize))-1

def Sigmoid(input, weights): #create a sigmoid layer and return a layer along with its derivative
    out = 1/(1+np.exp(-np.dot(input,weights)))
    derivative = out*(1-out)
    return out,derivative

def backProp(layers, weights, deriv, size, rate = 1):
    derivative = deriv.pop()#get the cost function derivative
    #reverse all the lists because we need to go backwards
    deriv = deriv[::-1]
    layers = layers[::-1]
    weights = weights[::-1]
    new_weights=[]
    #backpopagate
    new_weights.append(weights[0]+(layers[1].T.dot(derivative*rate))) #this one does not fit well the algorithm inside for loop, so it's outside of it
    for i in range(len(size)-2):
        derivative = derivative.dot(weights[i].T)*deriv[i]
        new_weights.append(weights[i+1]+(layers[i+2].T.dot(derivative*rate)))
    return new_weights[::-1]

def train(input,size,rate=1,epochs=1): #train the network
    layers=[]
    weights=[]
    derivs=[]
    for i in xrange(len(size)-1): #weights initialization
        weights.append(weights_init(size[i],size[i+1]))
    for i in xrange(epochs): #the training process
        for example, target in input:  #online learning
            layers.append(example)
            for i in xrange(len(size)-1):
                layer, derivative = Sigmoid(layers[i],weights[i])#calculate the layer and itd derivative
                layers.append(layer)
                derivs.append(derivative)

            loss_deriv = target-layer[-1] #loss function

            derivs[-1] = loss_deriv*derivs[-1] #multiply the loss function by the final layer's derivative
            weights = backProp(layers,weights,derivs,size) #update the weights
            layers=[]
            derivs = []
    return weights

def run(input,weights): #run a trained neural network
    layers=[input]
    for i in xrange(len(weights)):
        layer,derivative = Sigmoid(layers[i],weights[i])
        layers.append(layer)
    return layers

An example:

X = [(np.array([[0,0,1]]),np.array([[0]])),(    np.array([[0,1,1]]),np.array([[1]])), (np.array([[1,0,1]]),np.array([[1]])), (np.array([[1,1,1]]),np.array([[0]]))]

weights = train(X,[3,4,1],epochs=60000)
run(X[0][0],weights)

Without commenting on the algorithm for a lack of knowledge regarding neural networks, this answer provides some style suggestions. Take the first method:

def weights_init(inSize,outSize): #initialize the weights
    return 2*np.random.random((inSize,outSize))-1

In python it is against the PEP style guide to use camelCase for arguments. That comment should be dropped down a line. Place some spaces in between your arguments and operators so it can breath better:

def weights_init(in_size, out_size):
    #initialize the weights
    return 2 * np.random.random((in_size, out_size)) - 1

Function names shouldn't be Uppercased.

def sigmoid(input, weights):
    #create a sigmoid layer and return a layer along with its derivative
    out = 1 / (1 + np.exp(-np.dot(input, weights)))
    derivative = out * (1 - out)
    return out, derivative

Default arguments in the param signature shouldn't have a space between the equals sign.

Also it would be nice to add double line breaks for ease or reading.

def backProp(layers, weights, deriv, size, rate=1):
    #get the cost function derivative
    derivative = deriv.pop()

    #reverse all the lists because we need to go backwards
    deriv = deriv[::-1]
    layers = layers[::-1]
    weights = weights[::-1]
    new_weights=[]

    #backpopagate
    new_weights.append(weights[0] + (layers[1].T.dot(derivative * rate)))

    #this one does not fit well the algorithm inside for loop, so it's outside of it
    for i in range(len(size) - 2):
        derivative = derivative.dot(weights[i].T) * deriv[i]
        new_weights.append(weights[i + 1] + (layers[i + 2].T.dot(derivative * rate)))
    return new_weights[::-1]

def train(input, size, rate=1, epochs=1):
    #train the network
    layers=[]
    weights=[]
    derivs=[]

    #weights initialization
    for i in xrange(len(size) - 1):
        weights.append(weights_init(size[i], size[i + 1]))

    #the training process
    for i in xrange(epochs):

        #online learning
        for example, target in input:
            layers.append(example)

            for i in xrange(len(size) - 1):
                #calculate the layer and itd derivative
                layer, derivative = sigmoid(layers[i], weights[i])

                layers.append(layer)
                derivs.append(derivative)

            #loss function
            loss_deriv = target-layer[-1]

            #multiply the loss function by the final layer's derivative
            derivs[-1] = loss_deriv * derivs[-1]

            #update the weights
            weights = backProp(layers, weights, derivs, size)
            layers=[]
            derivs = []

    return weights


Improving the load time of a commit list for a specific branch and tag from a SVN repo My app lists commits from a specific branch and tag on a SVN repo.
The commit list is afterwards transferred to the front end which is a web app.

The method for retrieving the commits takes a rather long time, around half a minute when there are around 100 commits associated with a tag.

I am using SVNKit to communicate with the SVN repository.

<dependency>
    <groupId>org.tmatesoft.svnkit</groupId>
    <artifactId>svnkit</artifactId>
    <version>1.2.3.5521</version>
</dependency>

Here are the 2 methods responsible for getting the commit list:

/**
 * get a list of all commits associated with a branch and a specific tag
 * @param branch
 * @param tag
 * @return list<Commit>
 * @throws SVNException
 */
public List<Commit> getBranchCommits(String branch, Tag tag) throws SVNException{

    List<Commit> commitList=new ArrayList<Commit>();
    @SuppressWarnings("unchecked")
    Collection<SVNLogEntry> logEntries = repository.log(new String[] {"/Assurance/branches/"+ branch }, null, -1, tag.getRevision(), true, true);

    for (Iterator<SVNLogEntry> entries = logEntries.iterator(); entries.hasNext();) {
        SVNLogEntry logEntry = (SVNLogEntry) entries.next();
        if(logEntry.getAuthor() == null || "SVN_LOGON".equals(logEntry.getAuthor().toUpperCase()))
            continue;
        Commit commit=constructCommit(logEntry);
        commitList.add(commit);
    }
    return commitList;
}

/**
 * construct the object Commit from a svn log entry
 *
 * @param logEntry
 *            a svn log entry
 * @return a constructed object Commit
 * @throws SVNException
 */
private Commit constructCommit(SVNLogEntry logEntry) throws SVNException {

    Commit commit = new Commit(logEntry.getRevision(), logEntry.getDate(), logEntry.getAuthor(), logEntry.getMessage());

    @SuppressWarnings("unchecked")
    Iterator<SVNLogEntryPath> changedPaths = logEntry.getChangedPaths().values().iterator();
    while (changedPaths.hasNext()) {
        SVNLogEntryPath entryPath = changedPaths.next();
        SVNNodeKind nodeKind=repository.checkPath(entryPath.getPath(), -1);
        if ( nodeKind != SVNNodeKind.DIR) {
            commit.getChangedPathList().add(new ChangedPath(entryPath.getPath(), entryPath.getType(), entryPath.getCopyPath()));
        }
    }
    commit.getChangedPathList().sort( new Comparator<ChangedPath>() {
        public int compare(ChangedPath o1, ChangedPath o2) {
            return o1.getPath().compareTo(o2.getPath());
        }
    });

    return commit;
}

Waiting for half a minute or even more is rather unpleasant.
Do you have any suggestions for improving this code to make it work faster?

First of all, please check whether it is possible to use a newer version of SVNKit. According to mvnrepository, the used version 1.2.3.5521 is dated of March 2009, which is quite dinosaurian in our days.

The obsolete version might explain the long delays you are experiencing (I underline might explain, because there might have been a lot of optimisations compared with the most recent version, but I don't have any precise idea about it).

Another explanation may be the network communication lags with the SVN server the app is connecting to. It hugely depends on your network conditions and it is possible that there is no remedy for that.

To prove that the problem is related with networks, roughly and manually, the calls of methods on  repository field can be surrounded with time measurements, as well as method entries and exit points, for example:

public List<Commit> getBranchCommits(String branch, Tag tag) throws SVNException {
  final long methodStart = System.nanoTime();
  // other code

  final long repoStart = System.nanoTime();
  Collection<SVNLogEntry> logEntries = repository.log(...);
  final long repoEnd = System.nanoTime();

  // other code

  final long methodEnd = System.nanoTime();

  System.out.println(String.format("Total time: %1$s ms.", (methodEnd - methodStart) / 1_000_000));
  System.out.println(String.format("Repo call time: %1$s ms.", (repoEnd - repoStart) / 1_000_000));
  return commitList;
}

If repository call times take most part of total execution times, the problem is definitely related with network communications.

As for the code you are trying to improve, I can say that there should not be problems with the code itself. Although the style is a bit oldish, it does not look problematic. Here are some suggestions, using Java 8 and functional style:

public List<Commit> getBranchCommits(String branch, Tag tag) throws SVNException {
  @SuppressWarnings("unchecked")
  Collection<SVNLogEntry> logEntries = repository.log(/* no changes in args */);
  return logEntries.stream()
      .filter(this::shouldSkipEntryByAuthor)
      .map(this::constructCommit)
      .collect(Collectors.toList());
}

private boolean shouldSkipEntryByAuthor(SVNLogEntry logEntry) {
  return !(logEntry.getAuthor() == null || "SVN_LOGON".equalsIgnoreCase(logEntry.getAuthor()));
}

private Commit constructCommit(SVNLogEntry logEntry) {

  final Commit commit = new Commit(logEntry.getRevision(), logEntry.getDate(), logEntry.getAuthor(), logEntry.getMessage());

  // the problem with this obsolete API is that getChangedPaths() returns an untyped Map, we need to cast it properly.
  final List<ChangedPath> changedPaths = ((Map<Object, SVNLogEntryPath>) logEntry.getChangedPaths()).values().stream()
                                                .map(entry -> extractChangedPath(entry))
                                                .filter(Optional::isPresent)
                                                .map(Optional::get)
                                                .sorted(Comparator.comparing(ChangedPath::getPath))
                                                .collect(Collectors.toList());
  commit.setChangedPathList(changedPaths);
  return commit;
}

private Optional<ChangedPath> extractChangedPath(SVNLogEntryPath entry) {
  // we need to wrap SVNException into an unchecked one in order to make it compile in lambdas
  try {
    SVNNodeKind nodeKind = repository.checkPath(entry.getPath(), -1);
    if (!SVNNodeKind.DIR.equals(nodeKind)) {
      return Optional.of(new ChangedPath(entry.getPath(), entry.getType(), entry.getCopyPath()));
    }
    return Optional.empty();
  } catch (SVNException ex) {
    throw new IllegalStateException(ex);
  }
}


Brainfuck interpreter in C This is my bare-bones Brainfuck interpreter in C using lots of unixisms. What improvements can I make (with respect to the clarity of code, or obvious features to add)?

#include <assert.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>

void error(char *msg) {
    fprintf(stderr, "Error: %s\n", msg);
}

enum { MEMSIZE = 30000 };

//unsigned char *mem;
unsigned char mem[MEMSIZE];
unsigned char *ptr;
unsigned char *prog;
size_t progsize;

int init(char *progname) {
    int f,r;
    struct stat fs;
    ptr = mem;// = calloc(MEMSIZE, 1);
    f = open(progname, O_RDONLY);
    assert(f != -1);
    r = fstat(f, &fs);
    assert(r == 0);
    prog = mmap(NULL, progsize = fs.st_size, PROT_READ, MAP_PRIVATE, f, 0);
    assert(prog != NULL);
    return 0;
}

/* As requested, a little more commentary on this, admittedly bizarre, function.
findmatch receives the current instruction-pointer and a bracket character
for which the "match" is desired. It returns a new instruction-pointer.
*/
int findmatch(int ip, char src){
    char *q, *p="[]";
    int i, defer, dir[]= { 1, -1 };
    i = strchr(p,src)-p;  // at this point, if(src=='[') i=0; else if(src==']') i=1;
    ip+=dir[i];           // ie. if(src=='[') ++ip else if(src==']') --ip
                          // so dir[i] tells us which direction we're going
    // next, we reuse the same idiom to check each character as we scan.
    // defer counts up or down as we encounter [ and ] and finally reaches 0
    // when the match is found.
    for (defer=dir[i]; defer!=0; ip+=dir[i]) {
        if (ip<0||ip>=progsize) error("mismatch");   // check exceeded bounds
        if (q = strchr(p,prog[ip])) {   // if the char is a bracket
            int j = q-p;                // j is 0 or 1
            defer+=dir[j];              // adjust the 'defer' level
        }
    }
    return ip - dir[i];  // return new ip value (one past the matched bracket)
}

int run() {
    int ip;
    for(ip = 0; ip>=0 && ip<progsize; ip++)
        switch(prog[ip]){
        case '>': ++ptr; break;
        case '<': --ptr; break;
        case '+': ++*ptr; break;
        case '-': --*ptr; break;
        case '.': putchar(*ptr); break;
        case ',': *ptr=getchar(); break;
        case '[': /*while(*ptr){*/
                  if (!*ptr)
                      ip=findmatch(ip,'[')-1; // modified because run does ip++
                  break;
        case ']': /*}*/
                  if (*ptr)
                      ip=findmatch(ip,']')-1;
                  break;
        }

    return 0;
}

int cleanup() {
    //free(mem);
    ptr = NULL;
    return 0;
}

int main(int argc, char *argv[]) {
    init(argc > 1? argv[1]: NULL);
    run();
    cleanup();
    return 0;
}

My biggest complaint on readability would be your findmatch() function. There are too many non-descriptive variables (i.e. i, j, p, etc). Changing the names to be more descriptive, or adding some comments to elaborate on what you're trying to do would help a lot.


Better way to write if/else conditions I've got the following code where:

$urlParams is the school year from 7 to 11
$Comms['Overall'] is the total amount of "commendations" a student has

This code iterates through the numbers based on the school year to determine which "award" a student has got.

Is there a better way to write this code?

if($urlParams['y'] == 7){
    if((300 <= $Comms['Overall']) && ($Comms['Overall'] <= 599)){ $Award = "Bronze"; }
    elseif((600 <= $Comms['Overall']) && ($Comms['Overall'] <= 899)){ $Award = "Silver"; }
    elseif((900 <= $Comms['Overall']) && ($Comms['Overall'] <= 1199)){ $Award = "Gold"; }
    elseif((1200 <= $Comms['Overall']) && ($Comms['Overall'] <= 1499)){ $Award = "Platinum"; }
    elseif($Comms['Overall'] >= 1500){$Award = "Diamond";}
}

elseif($urlParams['y'] == 8){
    if((250 <= $Comms['Overall']) && ($Comms['Overall'] <= 549)){ $Award = "Bronze"; }
    elseif((550 <= $Comms['Overall']) && ($Comms['Overall'] <= 849)){ $Award = "Silver"; }
    elseif((850 <= $Comms['Overall']) && ($Comms['Overall'] <= 1099)){ $Award = "Gold"; }
    elseif((1100 <= $Comms['Overall']) && ($Comms['Overall'] <= 1299)){ $Award = "Platinum"; }
    elseif($Comms['Overall'] >= 1300){ $Award = "Diamond"; }
}

elseif($urlParams['y'] == 9){
    if((200 <= $Comms['Overall']) && ($Comms['Overall'] <= 449)){ $Award = "Bronze"; }
    elseif((450 <= $Comms['Overall']) && ($Comms['Overall'] <= 699)){ $Award = "Silver"; }
    elseif((700 <= $Comms['Overall']) && ($Comms['Overall'] <= 899)){ $Award = "Gold"; }
    elseif((900 <= $Comms['Overall']) && ($Comms['Overall'] <= 1099)){ $Award = "Platinum"; }
    elseif($Comms['Overall'] >= 1100){ $Award = "Diamond"; }
}

elseif($urlParams['y'] == 10){
    if((150 <= $Comms['Overall']) && ($Comms['Overall'] <= 399)){ $Award = "Bronze"; }
    elseif((400 <= $Comms['Overall']) && ($Comms['Overall'] <= 599)){ $Award = "Silver"; }
    elseif((600 <= $Comms['Overall']) && ($Comms['Overall'] <= 799)){ $Award = "Gold"; }
    elseif((800 <= $Comms['Overall']) && ($Comms['Overall'] <= 999)){ $Award = "Platinum"; }
    elseif($Comms['Overall'] >= 1000){ $Award = "Diamond"; }
}

elseif($urlParams['y'] == 11){
    if((150 <= $Comms['Overall']) && ($Comms['Overall'] <= 299)){ $Award = "Bronze"; }
    elseif((300 <= $Comms['Overall']) && ($Comms['Overall'] <= 499)){ $Award = "Silver"; }
    elseif((500 <= $Comms['Overall']) && ($Comms['Overall'] <= 699)){ $Award = "Gold"; }
    elseif((700 <= $Comms['Overall']) && ($Comms['Overall'] <= 849)){ $Award = "Platinum"; }
    elseif($Comms['Overall'] >= 850){ $Award = "Diamond"; }
 }

You should separate these magic numbers into a new data structure. Then have some code run over it. This will be much nicer to read.

For example:

function getAward($year, $commends) {
    $commendationsNeeded = [
        7 => [
            "Bronze" => 300,
            "Silver" => 600,
            "Gold" => 900,
            "Platinum" => 1200,
            "Diamond" => 1500
        ],
        8 => [
            "Bronze" => 250,
            "Silver" => 550,
            "Gold" => 850,
            "Platinum" => 1100,
            "Diamond" => 1300
        ]
    ];
    $maxScore = 0;
    $maxAward = "None";
    foreach ($commendationsNeeded[$year] as $award => $needed) {
        if ($needed <= $commends && $maxScore < $needed) {
            $maxScore = $needed;
            $maxAward = $award;
        }
    }
    return $maxAward;
}

echo getAward(7, 700); // Prints "Silver"


Updating the total when any of a set of spinners changes its value I'm new person who working on jQuery and I need your help.

$('.spinner-input, #flight-class').change(function() {
    var ap = parseInt($('#adult-passenger').val());
    var sp = parseInt($('#student-passenger').val());
    var cp = parseInt($('#child-passenger').val());
    var bp = parseInt($('#baby-passenger').val());
    var fc = $('#flight-class option:selected').text();

    var totalCount = ap + sp + cp + bp;
    $('#kisi-sayisi').val(totalCount + ' - ' + fc);
});

You can see my code in here

And working demo is here

So, here is the situtation: There is nothing bad about this code, its working normally when you click "number of people" in form. You can see, if you click minus or plus sign, form is updating itself. But when I look this code, I'm feeling like repeating myself.

When I tried turn this code to DRY format, i do this:

('.spinner-input, #flight-class').change(function() {
    var passengerId = parseInt($(this).val());
});

Now I get value of input. I need to get sum of values but I can't because something is missing here.

How can I DRY this code?

You could use a jQuery object to query all four of the spinners.

function sum(values) {
    return values.reduce(function (prev, next) { return prev + next; }, 0);
}

function inputToInt(input) {
    return parseInt($(input).val());
}

$('.spinner-input, #flight-class').change(function() {
    var passengerCounts = $('.passenger-list .spinner-input');
    var totalCount = sum(passengerCounts.toArray().map(inputToInt));

    var fc = $('#flight-class option:selected').text();

    $('#kisi-sayisi').val(totalCount + ' - ' + fc);
});


Looping over a bidimensional array and extract data to a new one I have a bidimensional array like this:

const bArray =
[ [ 'Hello World',
    'Hi everybody',
    'How are you?'
  ],
  [ { text: 'Hola Mundo',
      from: [Object],
      raw: '' },
    { text: 'Hola a todos',
      from: [Object],
      raw: '' },
    { text: 'Cómo estás?',
      from: [Object],
      raw: '' },
  ]
]

And I need to get as a result, only one array that should look like this:

[
 { en: 'Hello World',
   es: 'Hola Mundo' },

 { en: 'Hi everybody',
   es: 'Hola a todos' },

 { en: 'How are you?',
   es: 'Cómo estás?' },
]

This is how I do it:

let val1 = bArray[0].map(tuple => tuple);
let val2 = bArray[1].map(tuple => tuple);

let result = val1.reduce((arr, v, i) => arr.concat({"en" : v, "es" : val2[i].text}), []);

And now in the result variable, I have only one array with the result showed before.

My question?

Is there any improved way in which I can get the same result but with fewer lines of code? I mean, something like a combination of map with reduce, filter or concat in only one or two lines of code.

You can compact a lot into a single line, but that is most times not the best thing to do in both source code quality and execution efficiency.

const bArray = [['Hello World', 'Hi everybody', 'How are you?'], [{ text: 'Hola Mundo', from: [Object],  raw: '' }, { text: 'Hola a todos',   from: [Object], raw: '' }, { text: 'Cómo estás?',  from: [Object],  raw: '' },]];

const resultArray = bArray[0].reduce((arr, item, i) => (arr.push({en : item, es : bArray[1][i].text}), arr), []);
console.log(resultArray);

However I would do it as follows.

const bArray = [['Hello World', 'Hi everybody', 'How are you?'], [{ text: 'Hola Mundo', from: [Object],  raw: '' }, { text: 'Hola a todos',   from: [Object], raw: '' }, { text: 'Cómo estás?',  from: [Object],  raw: '' },] ];

const result = [], aArr = bArray[0];
var i = 0;
for (const item of bArray[1]) { result.push({es : item.text, en : aArr[i++]}) }

console.log(result);

Which is easier to read and quicker when run.

Expanding it to idiomatic JS style.

const result = [];
let i = 0;
for (const en of bArray[0]) {
    result.push({
        en,
        es : bArray[1][i++].text
    });
}

// or
const result = [];
let i = 0;
for (const item of bArray[1]) {
    result.push({
        es : item.text,
        en : bArray[0][i++]
    });
}


SICP exercise 2.28 - counting leaves in a tree (recursive process) From SICP

  Exercise 2.28: Write a procedure fringe that takes as argument a tree
  (represented as a list) and returns a list whose elements are all the
  leaves of the tree arranged in left-to-right order. For example,

(define x (list (list 1 2) (list 3 4)))

(fringe x)
(1 2 3 4)

(fringe (list x x))
(1 2 3 4 1 2 3 4)

After writing this code, I found out that implementing this in an iterative way is easier and more straight forward. But I ended up implementing this in a more complicated way. I'd like to think this is a recursive process. The argument "remaining" is not an accumulator. It stores all remaining items that are not yet consed into the new list.

Please review my code.

(define (fringe tree)
    (define (aux lst remaining)
        (cond
            ((null? lst)
                (if (null? remaining)
                    '()
                    (aux remaining '())))
            ((pair? (car lst)) (aux (car lst) (aux (cdr lst) remaining)))
        (else (cons (car lst) (aux (cdr lst) remaining)))))
    (aux tree '()))

How does it compare to the iterative version I linked to above?
What is the order of growth of my function? Is it O(N)?
How can it be faster?
If I used append rather than cons, would it slow my function a lot?
How can I improve this code?

How does it compare to the iterative version I linked to above?

I actually don't see any iterative versions, all of the examples call themselves twice in the main recursive logic. (as far as I can tell)

What is the order of growth of my function? Is it O(N)?

The best case as for time in any complete tree traversal is O(N log(N)) as is yours.  The big problem is that it's memory overhead is O(N)

How can it be faster?

Memory overhead can be kept to O(log(N))

If I used append rather than cons, would it slow my function a lot?

Looking through the examples, append is used mainly to keep the double-recursive call in the same order as input. Notice with yours (aux remaining '()))) that you flip the remaining to lst. when you hit the bottom of the tree. Using append on average will cause a O(N log(N)) delay.

How can I improve this code?

Using an ADT would help readibility. empty-tree? instead of null? - left instead of car, right instead of cdr.

Don't recur twice at the same time call e.g. (append (recur left) (recur right))

(define (leaf? tree)
   (not (pair? tree)))
(define (left tree)
   (car tree))
(define (right tree)
   (cadr tree))

(define (fringe tree)
  (define (fringe-helper tree acc)
    (if (leaf? tree)
        (cons tree acc)
        (let ((right-fringes
                  (fringe-helper (right tree) acc)))
           (fringe-helper (left tree) right-fringes))))
 (fringe-helper tree '()))

This evaluates right-fringes first, That way at most you have at most log(n) number of fringe-helpers on the stack waiting for a return. It's the same as the second try in your link, with some abstract procedures instead of directly using null? pair? etc.


Track mail to clients Create an application class named LetterDemo that instantiates objects
  of two classes named Letter and CertifiedLetter and that demonstrates
  all their methods. The classes are used by a company to keep track of
  the letter they mail to clients. The letter class includes
  auto-implemented properties for the name of the recipient and date
  mailed. Also , include a ToString() method that overrides the Object
  class's ToString() method and returns a string that contains the name
  of the class (using GetType()) and the Letter's data field values.
  Create a child class named CertifiedLetter that includes an auto
  implemented property that holds the tracking number for the letter.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using static System.Console;

namespace ConsoleApp16
{
class Program
{
    static void Main(string[] args)
    {

        Letter letter1 = new Letter();
        letter1.name = "\nSender: Justin";
        letter1.date = ("\nDate: March 4,2017 ");

        CertifiedLetter track = new CertifiedLetter();

        Write("Your letter has been recieved!");
        Write("\n");

        Write(letter1);
        Write("\n");
        Write("Tracking number:"+ track.GetHashCode());
        Write("\n");

    }
}
}

class Letter
{
public string name { get; set; }
public string date { get; set; }

public override string ToString()
{
    return (GetType() + ":" + this.name + "" + this.date);

}

}

class CertifiedLetter : Letter
{
    public string tracking { get; set; }

}

Looking to see how I could tighten up the code or if there is a better way to do this.

You fail to address the first sentence of your homework: you do not have an application class named LetterDemo.

I'm a strong believer in always specifying the access modifier.  Therefore both your Letter and CertifiedLetter classes should be decorated with public.

The name of your properties should be Pascal-cased.  Specifically they should begin with a capital letter, as in Name and Date.  I would suggest that Date is too generic a name, and that you should rename it to SentDate.  Why?  One day you may modify CertifiedLetter to record a ReceivedDateand having clearly named variables adds clarity to your code.

The Name property should only record the actual name, and ditch the preface.  You should omit forcing the user of your class to include "\nSender: ".

Likewise SentDate should omit "\nDate: " and only record the actual date.  Furthermore, the type for this property should be Date and not String.


Java 8 spliterator for paged results Summary: I am using an API which returns paged results. I want to have these results as Java 8 Stream and implemented a Spliterator for this purpose.

I am using AWS S3 Java API to list objects in the S3 bucket. The API returns paged results: when I call client.listObjects(bucketName, rootKey) for the first time, I get an ObjectListing instance which may be complete or truncated i.e. return just one "page" of results.

If ObjectListing is truncated I have to request further "page" via client.listNextBatchOfObjects(objectListing) (providing current "page" as marker) and so on until I get an ObjectListing which is not truncated.

I want to use Java 8 Stream APIs to work with ObjectListings. Ideally, I want to hide querying pages of ObjectListings behind some facility which would just give me a Stream<ObjectListing>. For this I've implemented a Spliterator<ObjectListing>:

import java.util.Objects;
import java.util.Spliterator;
import java.util.function.Consumer;

import com.amazonaws.services.s3.AmazonS3;
import com.amazonaws.services.s3.model.ObjectListing;

public class ObjectListingSpliterator implements Spliterator<ObjectListing> {

    private final AmazonS3 client;
    private ObjectListing objectListing;
    private volatile boolean split = false;
    private volatile boolean currentObjectListingWasConsumed = false;

    public ObjectListingSpliterator(AmazonS3 client, ObjectListing objectListing) {
        Objects.requireNonNull(client, "client must not be null.");
        Objects.requireNonNull(objectListing, "objectListing must not be null.");
        this.client = client;
        this.objectListing = objectListing;
    }

    @Override
    public boolean tryAdvance(Consumer<? super ObjectListing> action) {
        if (!currentObjectListingWasConsumed) {
            action.accept(objectListing);
            currentObjectListingWasConsumed = true;
            if (!split && objectListing.isTruncated()) {
                objectListing = client.listNextBatchOfObjects(objectListing);
                currentObjectListingWasConsumed = false;
            }
            return true;
        } else {
            return false;
        }
    }

    @Override
    public Spliterator<ObjectListing> trySplit() {
        if (split) {
            // Already split, cannot be split once again
            return null;
        } else if (objectListing.isTruncated()) {
            split = true;
            final ObjectListing nextObjectListing = client
                    .listNextBatchOfObjects(objectListing);
            return new ObjectListingSpliterator(client, nextObjectListing);
        } else {
            return null;
        }
    }

    @Override
    public long estimateSize() {
        if (objectListing.isTruncated()) {
            return Long.MAX_VALUE;
        } else {
            return currentObjectListingWasConsumed ? 0 : 1;
        }
    }

    @Override
    public int characteristics() {
        return Spliterator.ORDERED | Spliterator.IMMUTABLE;
    }
}

Usage example - list all keys from the bucket and root key:

// Get the original objectListing
ObjectListing objectListing = client.listObjects(bucketName,
    rootKey);
// Create a stream of objectListings
Stream<ObjectListing> objectListings = StreamSupport.stream(
    new ObjectListingSpliterator(client, objectListing), false);

List<String> keys = objectListings
    .map(ObjectListing::getObjectSummaries)
    .flatMap(Collection::stream)
    .map(S3ObjectSummary::getKey);

What I'm interested in:

Is my spliterator OK?
I'm not sure if there might be a concurrency issue between tryAdvance and trySplit.
Not sure what characteristics should be.

The question is actually not specific to AWS API, it is generally about streaming paged result where:

there is some way to retrieve the first page;
with every page you know if it is complete or not;
you can get the next page using the previous page.

First of all, splitting the spliterator only makes sense, if both the remainder of the current spliterator and the returned spliterator still have work pending. In your case, this is (almost) not true, as you operate on the complete batches and the current spliterator at most returns its current batch after split. Thus, I'd replace the trySplit() with a simple return null. This also addresses any potential concurrency issues (which I have not looked into in depth.)

Characteristics basically tell the caller the - for lack of a better word - characteristics of your spliterator. :-)

I think for the batch approach you take, ORDERED, NONNULL, IMMUTABLE should be OK.

Apart from that, for more direct utility, I'd rather take the approach not to iterate over the batches, but over their contents, i.e. create a Spliterator which gets initialized with the first ObjectListing  batch and then transparently goes through the underlying collections element-wise and fetches the next batch as needed. This would eliminate the need to flatMap on the result stream and feel more natural for a streaming approach. (In fact, this sounds so useful that I'd like to have it :-))


Functional Re-Write in Scala: Rows of Strings with a Max Width I came across an issue the other day where I really just could not think of a functional solution. So I fell back on my imperative programming skills. Since a functional solution still eludes me, I figured I'd shout out for some help here.

Use Case

I have a List of Strings with arbitrary lengths. These Strings need to be combined into rows that have the same max length.

When a single String's length is greater than max length, that String just gets it's own row as each row eventually get's handled by a function that ensures all Strings are truncated to an appropriate length.

Example Usage

scala> val rows = combineStrings(List("Short", "This is a particularly long String...", "More", "Small", "Strings"), 35)

Current Output

scala> rows.foreach(println)
Short
This is a particularly long String...
More, Small, Strings

Desired Output

scala> rows.foreach(println)
Short, More, Small, Strings
This is a particularly long String...

Current Solution

def combineStrings(strs: List[String], maxLen: Int)
: List[String] = {
  import scala.collection.mutable.ListBuffer

  var scratch = strs.mkString("|")
  val rows = ListBuffer[String]()

  while (scratch.length > 1) {
    scratch.indexOf("|") match {
      case idx if (idx == -1 || scratch.length < maxLen) =>
        rows += scratch
        scratch = ""

      case idx if idx > maxLen =>
        rows += scratch.substring(0, idx)
        scratch = scratch.substring(idx+1)

      case idx =>
        val lIdx = scratch.lastIndexOf("|", maxLen)
        rows += scratch.substring(0, lIdx)
        scratch = scratch.substring(lIdx+1)
    }
  }

  rows.toList.map(_.replaceAll("\\|", ", "))
}

Notes

Pipes are used because the original Strings are more likely to have their own commas than their own pipes. Maybe other solutions won't need this hackery.
Strings are kept in the same order, but in the end I'd prefer to have the most "compact" rows possible.

Summary

Any ideas on how to make this more functional would be greatly appreciated. I found several functions in Scala's "String" that looked promising, but I'm not really sure how to integrate them with the length requirements.

This is much more "Functional" than my first attempt:

  def combineStrings(strs: List[String], maxLen: Int)
  : List[String] = {
    import scala.collection.mutable.StringBuilder

    strs.map(_.trim).filter(_.nonEmpty)
      .aggregate(List[StringBuilder]())({ (lbs, s) =>
        s.length match {
          case l if (l > maxLen) =>
            new StringBuilder(s) :: lbs
          case l =>
            lbs.find(_.length <= maxLen - (l+2))
              .map { sb => sb ++= s", $s"; lbs }
              .getOrElse(new StringBuilder(s) :: lbs)
        }
      }, _ ++ _
    ).map(_.toString).reverse
  }

Not sure why I didn't do this the first time around. This solution even provides the desired output, and accounts for the ", " separators in the length calculations.

scala> val rows = combineStrings(List("Short", "This is a particularly long String...", "More", "Small", "Strings"), 35)
rows: List[String] = List(Short, More, Small, Strings, This is a particularly long String...)

scala> rows.foreach(println)
Short, More, Small, Strings
This is a particularly long String...


Implementing a stack using a linked-list In my intro CS class we're reviewing data structures. I'm currently working on implementing a stack using a linked list (LIFO) in C. I'd appreciate a review of the implementation as well as of my understanding of how a stack should work.

// This program is implementation of stack data structure
// via linked list (LAST IN FIRST OUT STRUCTURE) LIFO

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

typedef struct node
{
   char string[20];
   struct node *next;
} node;

node *push(char *element, node *head);
node *pop(node *head);
void destroy_stack(node *p);

int main(void)
{

    // create new stack
    node *stack = NULL;

    // push 6 "functions" to the stack
    char *function[6] = {"first funct", "second funct", "third funct",
    "fourth funct", "fifth funct", "sixt funct"};
    for (int i = 0; i < 6; i++)
    {
        printf("function is : %s\n",function[i]);
        stack = push(function[i], stack);
        if (!stack)
        {
            fprintf(stderr,"Not enough memory space for new list");
            return 1;
        }
    }

    // display the stack
    for (node *temp = stack; temp != NULL; temp = temp -> next)
    {

        printf("Elements of the stack are: %s\n", temp -> string);
    }

    // pop the elements from the stack
    while (stack != NULL)
    {

        printf("Popped elemet is: %s\n", stack -> string);
        stack = pop(stack);

    }

    destroy_stack(stack);
    return 0;

}

node *push(char *element, node *head)
{
    // create space for new element on stack
    node *temp = malloc(sizeof(node));
    if (!temp)
    {
        return NULL;
    }

    // if stack is empty
    if (head == NULL)
    {
        strcpy(temp -> string, element);
        temp -> next = NULL;
        return temp;
    }

    strcpy(temp -> string, element);
    temp -> next = head;
    return temp;
}

node *pop(node * head)
{
    // create a new head
    node *newHead = head->next;

    // pop the element from stack
    free(head);

    return newHead;

}

void destroy_stack(node *p)
{
    if ( p == NULL )
    {
        return;
    }
    else
    {
        destroy_stack(p -> next);
    }
    free(p);
}

Redundant Code

In your push() function, you have these two cases which are redundant:

// if stack is empty
if (head == NULL)
{
    strcpy(temp -> string, element);
    temp -> next = NULL;
    return temp;
}

strcpy(temp -> string, element);
temp -> next = head;
return temp;

You can just remove the special case for head == NULL and be left with this, which handles both cases:

strcpy(temp -> string, element);
temp -> next = head;
return temp;

Better to iterate than recurse

Your destroy_stack() function is recursive, and that could be a problem if the stack has millions of elements because it could cause your program stack to overflow.  It is easy enough to write an iterative version of that function instead, like this:

void destroy_stack(node *p)
{
    while (p != NULL)
    {
        node *next = p->next;
        free(p);
        p = next;
    }
}

Minor issues

In push(), using strcpy() is unsafe because element might exceed 20 characters and you will overflow your buffer.
In push(), if malloc() returns NULL, you return NULL and thus destroy/leak the rest of the stack.  Perhaps you should just return head instead, to preserve the rest of the stack.
In pop(), you might want to check for head == NULL, otherwise you must depend on the caller to never pass NULL to your function.


Checking for null in delivery functions In my Repository class:

public class DeliveryRepository : IDeliveryRepository
{
    private readonly List<Delivery> deliveries = new List<Delivery>();

    public Delivery Add(Delivery item)
    {
        if (item == null)
        {
            throw new ArgumentNullException("item");
        }
        deliveries.Add(item);
        return item;
    }
}

...I could implement my Add() method this way:

public Delivery Add(Delivery item)
{
    Contract.Requires(item != null);
    deliveries.Add(item);
    return item;
}

...or this way instead:

public Delivery Add(Delivery item)
{
    Contract.Requires<ArgumentNullException>(item != null, "item");
    deliveries.Add(item);
    return item;
}

Which of the three is best? Does it matter? Just a matter of style/preference?

Are they 2 of one, a pair of the other, and a couple of the third?

If you are addressing Code Contracts specifically then you should adopt a programming style that reinforces exception handling policy in release build, i.e. whether you require argument validation at runtime and whether Code Contracts will be part of release build or not.

I suggest taking a look at this diagram (copied from Code Contracts user documentation) and consult it with your team.


Lowering the cyclomatic complexity of this method, suggestions? The PHP Mess Detector of my IDE warns about a Cyclomatic Complexity of 14 (threshold is 10). To me this code doesn't seem to hard to follow. Would you refactor it in some way to lower the metric?

Exit if $payload isn't an array or has a number of elements not equal to 1
If $payload is array or ArrayAccess build a proper checker/setter
If $payload is stdClass do the same as 2
Use control options and checker/setter to actually set the value inside $payload

The method:

public function onBeforeEncrypt(CryptDecryptEvent $event)
{
    // Get the actual parameters of the call
    $payload = $event->getPayload();

    // Avoid errors when __soapCall is invoked directly with custom arguments
    if (!is_array($payload) || 1 !== count($payload)) {
        return;
    }

    // Reference to the payload (by reference just in case it's an array)
    $payload = &$payload[0];

    // Shortcuts to control options
    $paramName      = $this->options['name'];
    $paramValue     = $this->options['value'];
    $allowOverride  = $this->options['allowOverride'];
    $nullLikeNotSet = $this->options['threatNullLikeNotSet'];

    // Dynamically build checker and setter based on the type
    $checker = $setter = null;

    // Payload is array or ArrayAccess implementation inside main array
    if (is_array($payload) || $payload instanceof \ArrayAccess) {
        $checker = function ($k) use ($payload, $nullLikeNotSet) {
            return isset($payload[$k]) || (null === $payload[$k] && $nullLikeNotSet);
        };

        $setter = function ($k, $v) use (&$payload) { $payload[$k] = $v; };
    }

    // Payload is stdClass inside the main array
    if ($payload instanceof \stdClass) {
        $checker = function ($p) use ($payload, $nullLikeNotSet) {
            return isset($payload->$p) || (null === $payload->$p && $nullLikeNotSet);
        };

        $setter = function ($p, $v) use ($payload) { $payload->$p = $v; };
    }

    // Set the value if we can override or parameter is abset
    if ($checker && $setter && ($allowOverride || !$checker($paramName)) {
        $setter($paramName, $paramValue);
    }
}

You could easily extract the 4 functions you have within if statements by having

arrayChecker()
arraySetter()
classChecker()
arraySetter()

And then have something like

//Exit immediately if override is not allowed ( why wait ? )
//Personally, that would be the first line for me
if(!$allowOverride)
  return;

// Payload is array or ArrayAccess implementation inside main array
if (is_array($payload) || $payload instanceof \ArrayAccess) {
  if(!$arrayChecker($paramName)) {
    $arraySetter($paramName, $paramValue);
  }
}

// Payload is stdClass inside the main array
if ($payload instanceof \stdClass) {
  if(!$classChecker($paramName)) {
    $classSetter($paramName, $paramValue);
  }
}

One more thing, you re-use the payload variable. For me, what you get from ->getPayload() is a payload of an array with something in the first position. ( crate plus the goods ). To then re-use the variable to point just to the goods seems wrong to me.


Eliminating global variables in GUI program I'm just getting started on GUI programming using swing in Java, and I would love some critiques on my code. I understand there is a danger in using global variables.  If someone can hint me on how to get rid of my globals (which will be later registered for event handling), I'd really appreciate it.

import java.awt.BorderLayout;
import java.awt.FlowLayout;
import java.awt.GridLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingConstants;
public final class Calculator extends JFrame
{
    //initialise various variables for use within the program
    //BUTTONS
    private final JButton additionButton = new JButton("+");
    private final JButton subtractionButton = new JButton("-");
    private final JButton divisionButton = new JButton("/");
    private final JButton multiplicationButton = new JButton("*");

    //PANELS
    private JPanel operatorPanel;
    private JPanel operandPanel;

    //LABELS
    private JLabel operationLabel;

    //constructor to initialise the frame and add components into it
    public Calculator()
    {
        super("Clancy's Calculator");
        setLayout(new BorderLayout(5, 10));
        setResizable(false);
        setSize(370, 200);
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setLocationRelativeTo(null);
        setVisible(true);

        //create a message label to display the operation that has just taken place
        operationLabel = new JLabel("YOU HAVE PERFORMED SOME OPERATION", SwingConstants.CENTER);

        add(getOperatorPanel(), BorderLayout.NORTH);
        add(getOperandPanel(), BorderLayout.CENTER);
        add(operationLabel, BorderLayout.SOUTH);

        pack();
    }

    //setter method for the operator panel
    public void setOperatorPanel()
    {
        operatorPanel = new JPanel();
        operatorPanel.setLayout(new FlowLayout());

        operatorPanel.add(additionButton);
        operatorPanel.add(subtractionButton);
        operatorPanel.add(multiplicationButton);
        operatorPanel.add(divisionButton);
    }
    //getter method for the operator panel
    public JPanel getOperatorPanel()
    {
        setOperatorPanel();
        return operatorPanel;
    }

    //setter method for operands panel
    public void setOperandPanel()
    {
        operandPanel = new JPanel();
        operandPanel.setLayout(new GridLayout(3, 2, 5, 5));

        //LABELS
        JLabel operandOneLabel = new JLabel("Enter the first Operand: ");
        JLabel operandTwoLabel = new JLabel("Enter the second Operand: ");
        JLabel answerLabel = new JLabel("ANSWER: ");

        //TEXT FIELDS
        JTextField operandOneText = new JTextField();   //retrieves one operand
        JTextField operandTwoText = new JTextField();   //retrieves another operand
        JTextField answerText = new JTextField();   //displays answer

        answerText.setEditable(false);  //ensure the answer field is not editable

        operandPanel.add(operandOneLabel);
        operandPanel.add(operandOneText);
        operandPanel.add(operandTwoLabel);
        operandPanel.add(operandTwoText);
        operandPanel.add(answerLabel);
        operandPanel.add(answerText);

    }
    //getter method for operand panel
    public JPanel getOperandPanel()
    {
        setOperandPanel();
        return operandPanel;
    }

    /** main method */
    public static void main(String[] args)
    {
        new Calculator();
    }
}

A thing I like to do is instead of using four different JButtons is use an array of four JButtons that is initialized in the same method that you build the JPanel this would remove the need to have them as global variables. Also instead of using two different methods to create your JPanels just use one that returns a JPanel as you can see you can remove the JButtons and the two JPanels this way:

public JPanel buildOperatorPanel()
{
    JPanel operatorPanel = new JPanel();
    operatorPanel.setLayout(new FlowLayout());

    JButton[] buttons = new JButton[4];//the array of JButtons
    String[] buttonNames = {"+", "-", "/", "*"};//the names to be displayed on the JButtons
    for(int i = 0; i < buttons.length; i++)
    {
        buttons[i] = new JButton(buttonNames[i]);
        //buttons[i].addActionListener(yourActionListener)//for whenever you add event handling
        operatorPanel.add(buttons[i]);
    }
    return operatorPanel;
}

Another thing I could suggest is instead of named reference variables for your JLabels use anonymous objects so for operationLabel (this is only assuming your not going to use the JLabels later):

add(new JLabel("YOU HAVE PERFORMED SOME OPERATION", SwingConstants.CENTER), BorderLayout.SOUTH);

However I am going to suggest that you move the JTextFields to global variables. Based on what I'm assuming you have to do with this calculator you are going to want the JTextFields to be accessible to everything.
Here is my revision of your code:

import java.awt.BorderLayout;
import java.awt.FlowLayout;
import java.awt.GridLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingConstants;

public final class Calculator extends JFrame
{
    private JTextField operandOneText;//these get initialized in buildOperandPanel() method which is called in the constructor
    private JTextField operandTwoText;
    private JTextField answerText;

    //constructor to initialise the frame and add components into it
    public Calculator()
    {
        super("Clancy's Calculator");
        setLayout(new BorderLayout(5, 10));
        setResizable(false);
        setSize(370, 200);
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setLocationRelativeTo(null);
        setVisible(true);

        add(buildOperatorPanel(), BorderLayout.NORTH);
        add(buildOperandPanel(), BorderLayout.CENTER);
        add(new JLabel("YOU HAVE PERFORMED SOME OPERATION", SwingConstants.CENTER), BorderLayout.SOUTH);

        pack();
    }

    //method builds and returns operatorPanel
    public JPanel buildOperatorPanel()
    {
        JPanel operatorPanel = new JPanel();//JPanels are FlowLayout by default

        JButton[] buttons = new JButton[4];
        String[] buttonNames = {"+", "-", "/", "*"};
        for(int i = 0; i < buttons.length; i++)
        {
            buttons[i] = new JButton(buttonNames[i]);//the array of JButtons
            //buttons[i].addActionListener(yourActionListener)//for whenever you add event handling
            operatorPanel.add(buttons[i]);
        }
        return operatorPanel;
    }

    //method builds and returns operandPanel
    public JPanel buildOperandPanel()
    {
        JPanel operandPanel = new JPanel();
        operandPanel.setLayout(new GridLayout(3, 2, 5, 5));

        //Initialize TEXT FIELDS
        operandOneText = new JTextField();  //retrieves one operand
        operandTwoText = new JTextField();  //retrieves another operand
        answerText = new JTextField();    //displays answer

        answerText.setEditable(false);  //ensure the answer field is not editable

        operandPanel.add(new JLabel("Enter the first Operand: "));
        operandPanel.add(operandOneText);
        operandPanel.add(new JLabel("Enter the second Operand: "));
        operandPanel.add(operandTwoText);
        operandPanel.add(new JLabel("ANSWER: "));
        operandPanel.add(answerText);

        return operandPanel;
    }

    /** main method */
    public static void main(String[] args)
    {
        new Calculator();
    }
}


Convert array of arrays to array without duplicates I have the following array, and I want to convert it to an array without duplicates.

const a = [
    {
        items: [
            { outerKey: { innerKey: 1 } },
            { outerKey: { innerKey: 2 } },
            { outerKey: { innerKey: 3 } },
        ],
    },
    {
        items: [
            { outerKey: { innerKey: 3 } },
            { outerKey: { innerKey: 5 } },
            { outerKey: { innerKey: 5 } },
        ],
    },
    {
        items: [
            { outerKey: { innerKey: 1 } },
            { outerKey: { innerKey: 8 } },
            { outerKey: { innerKey: 2 } },
        ],
    },
];

const finalArray = [...new Set(a.reduce((acc, curr) =>  { acc.push(curr.items.map(item => item.outerKey.innerKey)); return acc;} , []).flat())];

console.log(finalArray);

Is there a better approach than this? considering there could be > 1000 items.

Adding all the elements to a Set should be generally the most efficient way. However, before you add them to the Set you are creating lots of intermediate arrays. It will likely be more efficient to just iterate over the elements and add them to the set one by one:
let set = new Set();
a.forEach(o => {
    o.items.forEach(i => {
        set.add(i.outerKey.innerKey);
    });
});
const finalArray = [...set];
console.log(finalArray);

Not sure if forEach() is the fastest way to iterate over the array elements in whatever runtime you are using. See here for a discussion of how to iterate arrays.


Perl 6: Grammar issues (simple defines) My goal is to parse defines in Objective-C code:

#define wowFont [UIFont fontWithName:(DEV)?@"one":@"two" size:(DEV? 10: 12)]
#define wowColor [UIColor colorWithRed:(DEV?240/255.0f:255.0f) green:((DEV)? 230.0f / 255.0f) blue: 10.0f]

I try to write grammar for it:

grammar Defines::Grammar {
    token TOP {
        \s*<define_expression>\n
    }
    token define_expression {

        ^^ #define $<keyword>=\w+ \s+ $<value> = <general_expression>
    }

    token general_expression{
        # here, determine parenthes
        <simple_expression>|[(]<general_expression>[)]|<value_expression>
    }

    token simple_expression{
        # here, determine ?: behaviour
        \s*<general_expression>\s*[?]\s*<general_expression>\s*[:]\s*<general_expression>\s*
    }

    token value_expression {
        # stop here, need to determine ?: operator here
        <numeric_expression>|<string_expression>|<function_invocation_expression>|<bareword_expression>
    }

    token function_invocation_expression {
        [\[] (<class>|<bareword_expression>) \s+ <function_body>\s* [\]]
    }

    token function_body {
        <bareword_expression>|<bareword_expression> \s* [:] <general_expression>
    }

    token bareword_expression {
        [\w_]+
    }

    token string_expression {
        \@["][^"]["]
    }

    token numeric_expression {
        # we could +-*/ numeric expressions, also we can put lead sigil [-]. put parentheses and it could be a <number>
        <numeric_expression> [*/+-] <numeric_expression> | [-+]<numberic_expression> | [(] <numeric_expression> [)] | <number>
    }

    token number {
        # number expression here, ok
        # not needed legal sign here, numeric_expression already have it
        \d+ [\.] \d+
    }
}

Could anybody help to improve it?

P.S: As a part of answer (I hope that somebody will find this documents helpful) I place here a link to Perl 6 book (I found out that Grammars section is good enough for beginning). Perl 6 Book

I was about to submit this when I saw that you had visited #perl6. I'll leave this post as is because it is supposed to help others that might one day read this as well as you.

TL;DR I recommend you: make sure you're ready for an immature compiler, ecosystem, doc, etc; use Rakudo Star and the Rakudo debugger; start small and take small steps, making each step work before moving forward; rely very heavily on #perl6 -- they welcome any and all comers, including "newbies" and lots of basic questions.

First, let me make sure you are not wasting your time due to wrong expectations. I am pretty sure there are currently only a handful of folk using P6 in production and they are doing so for only a small range of use cases. Powerful parsing is one of them, so what you are attempting is reasonable, but it's slow and you need to be willing to patiently deal with the many other frustrations that can result from P6's immaturity. If this hasn't put you off, welcome aboard! :)

Second, let me make sure I am not wasting your time. Your code doesn't compile using the tools I'm using. I used a procedure you can repeat to test your code with the latest Rakudo Perl 6 compiler HEAD:

I joined #perl6, the freenode IRC channel. (A very friendly channel. All are welcome.)
I tested code by entering a line of the form "m: ...". (Replace the "..." with code, or a suitable "pastebin" URL as explained below, otherwise you'll get a "Stub code executed" exception. An evalbot spots such lines, runs the code using the HEAD of the Rakudo compiler using the MoarVM backend, and displays the result.)
I ran your code privately (recommended if you don't want to "spam" the channel with attempts that aren't interesting to others) so you won't be able to see this in the public IRC log. To run code privately enter the command /msg camelia r: say $*PERL, switch to the "camelia" window, and enter lines there.
So I entered "m: https://gist.github.com/raiph/11084741" which attempted to compile a copy of your code. It failed to compile.

Third, let's radically improve your debugging options. One option when debugging grammars is to use the Grammar::Tracer module. This is occasionally useful but for most purposes there's a much better option: Rakudo's good-trending-great debugger. Use the debugger all the time unless you have a compelling reason not to. The Rakudo debugger is not available using the on channel evalbots I just discussed above; it requires that you install something. I recommend the Rakudo Star package unless you have a compelling reason to use something else. Rakudo Star includes the debugger, modules, and doc including info on how to start the debugger.

Fourth, start simple and build up from there, only making another small step forward when what you currently have works. For example, a first step might be:

say so '#define foo [blah]' ~~ rule { '#define' <ident> .* }

prints:

True

meaning the string on the left of the ~~ was "accepted" by the thing on the right.

The thing on the right is a rule which is one of several convenient forms of regex introduced by P6. The P6 regex engine never backtracks once it's matched an atom in a rule and it replaces appropriate spaces in your rule with implicit calls to the whitespace matching rule called <ws>. The above line is equivalent to:

say so '#define foo [blah]' ~~ regex { :ratchet '#define' <.ws> <ident> <.ws> .* }

(The . in <.ws> tells P6 to call the <ws> rule without saving the text it matched in the match object being built up by the processing of the regex.)


Using OLEDB to connect to a MS Access 2007 file I am working on a program which use OLEDB to connect to a MS Access 2007 file. My program has a possibility to add and delete records from file by using SQL statements which select deleted item by ID.

Now, I was warned that my code might cause SQL injection which would delete all entries from the file. How can I prevent this? Where lies the problem? One person mentioned that I could avoid it by having good field verification, but what should I verify for?

This is how I delete items:

// SQL query which will delete entry by using entry ID.
string SQL = "DELETE FROM PersonalData WHERE DataID = " + txtEntryID.Text;

private void DeleteData(string SQL)
{
    // Creating an object allowing me connecting to the database.
    // Using parameters in command will avoid attempts of SQL injection.
    OleDbConnection objOleDbConnection = new OleDbConnection();
    // Creating command object.
    objOleDbConnection.ConnectionString =
        "Provider=Microsoft.ACE.OLEDB.12.0;" +
        "Data Source=" + filePath + ";" +
        "Persist Security Info=False;" +
        "Jet OLEDB:Database Password=" + pass + ";";
    OleDbCommand objOleDbCommand = new OleDbCommand();

    objOleDbCommand.CommandText = SQL;

    // Assigning a connection string to the command.
    objOleDbCommand.Connection = objOleDbConnection;

    try
    {
        // Open database connection.
        objOleDbConnection.Open();
        objOleDbCommand.ExecuteNonQuery();
    }
    catch (Exception ex)
    {
        // Displaying any errors that
        // might have occured.
        MessageBox.Show("Error: " + ex.Message);
    }
    finally
    {
        // Close the database connection.
        objOleDbConnection.Close();
    }

    // Refreshing state of main window.
    mainWindow.DisplayFileContent(filePath);

    lblMessage.Text = "Data was successfully deleted.";

    // Clearing text box field.
    txtEntryID.Clear();
}

Unless I misunderstand, you are using unfiltered user input from txtEntryID.
Never trust user input.

What if the user fills txtEntryID with 123 OR 1=1?
The query will delete everything:

DELETE FROM PersonalData WHERE DataID = 123 OR 1=1

What if, then, the user inputs 123; UPDATE Account SET credit = 1000 WHERE userId = me:

DELETE FROM PersonalData WHERE DataID = 123; UPDATE Account SET credit = 1000 WHERE userId = me


ES6 Javascript (React) code that maps and filters JSON data I have the following code in my React app that creates tags out of JSON data:

const files = Object.values(gist.files);
const tags = files.map((file) => {
    let tag = file.language ? file.language : 'Plain Text';
    return tag;
});
let unique = [...new Set(tags)];
const unique_tags = unique.map((tag) => {
    return <Tag value={tag}/>;
});

The code starts by creating an array out of the files' JSON object, next it maps through the files array and returns the value of the language key (Javascript, C, etc.) and if its not undefined, returns plaintext. After that, it filters so there are only unique tags. Finally, it maps through the unique tags and returns a React component.

As you can see, there are two maps and one filter. I feel like this could be done more efficiently.

The JSON is coming from the GitHub Gist API.
For example: https://api.github.com/gists/25c75fa76cf151568d12

You can use a for..of loop to avoid using .map() twice and Set

const files = Object.values(gist.files);
const unique_tags = [];

for (let file of files) {
  let tag = file.language ? file.language : 'Plain Text';
  if (unique_tags.indexOf(<Tag value={tag}/>) === -1) {
    unique_tags = [...unique_tags, <Tag value={tag}/>]
  }
}


PDO database class review I've taken this class and made it my own but I'm curious to know what other think of it? Are there things I'm missing? Anything you can recommend?

class Database {
    private $host = '';
    private $user = '';
    private $pass = '';
    private $dbname = '';

    private $dbh;
    private $error;
    private $stmt;

    public function __construct() {
        $dsn = 'mysql:dbname=' . $this -> dbname . ';host=' . $this -> host . '';
        // Set options
        $options = array(PDO::ATTR_PERSISTENT => true, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION);
        // Create a new PDO instanace
        try {
            $this -> dbh = new PDO($dsn, $this -> user, $this -> pass, $options);
        } catch(PDOException $e) {
            $this->error = $e -> getMessage();
        }
    }

    public function query($query) {
        $this -> stmt = $this -> dbh -> prepare($query);
    }

    public function bind($param, $value, $type = null) {
        if (is_null($type)) {
            switch (true) {
                case is_int($value) :
                    $type = PDO::PARAM_INT;
                    break;
                case is_bool($value) :
                    $type = PDO::PARAM_BOOL;
                    break;
                case is_null($value) :
                    $type = PDO::PARAM_NULL;
                    break;
                default :
                    $type = PDO::PARAM_STR;
            }
        }
        $this -> stmt -> bindValue($param, $value, $type);
    }

    public function execute() {
        return $this -> stmt -> execute();
    }

    public function fetchAll() {
        $this -> execute();
        return $this -> stmt -> fetchAll(PDO::FETCH_ASSOC);
    }

    public function fetch() {
        $this -> execute();
        return $this -> stmt -> fetch(PDO::FETCH_ASSOC);
    }

    public function rowCount() {
        return $this -> stmt -> rowCount();
    }

    public function countAll($arr) {
        return count($arr);
    }

    public function lastInsertId() {
        return $this -> dbh -> lastInsertId();
    }
}

I have set up something very similar with a couple changes. In __contruct() I assign the credentials to variables, then call an instance to start the PDO connection. I also use try-catch for each PDO statement to make sure that I am not getting any errors.

private $_pdo = null;
private $_host, $_dbname, $_uname, $_pass;
private $_sql, $_result;
private $_preparedStmt;

public function __construct($host, $dbname, $username, $password) {
    // You could run checks here to make sure that...
    // no values or specific values are not equal to ''
    $this->_host = $host;
    $this->_dbname = $dbname;
    $this->_uname = $username;
    $this->_pass = $password;
    $this->startPDO();
}

// ... Later on in code, under Private functions comment tag
private function startPDO() {
    try {
        $this->_pdo = new PDO('mysql:host='.$this->_host.';dbname='.$this->_dbname, $this->_uname, $this->_pass);
        $this->_pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    } catch (PDOException $e) {
        //$this->handleFatalErr($e->getMessage(), 'connection'); // 'die' error handling
        //$this->handleErr($e->getMessage(), 'connection'); // Non-'die' error handling
    }
}

In each function I check the connection, I would recommend something similar. You could either kill the script if there is no connection or you could re-connect using the current variables $this->_host, etc:

public function query($sql) {
    $this->checkConnection();
    $this->_sql = $sql; // This is used for logging and for a getter function
    try {
        $this->_result = $this->_pdo->query($this->_sql);
    } catch (PDOException $e) {
        //$this->handleFatalErr($e->getMessage(), 'connection');
    }
}

private function checkConnection() {
    ($this->getPDO() === null) ? die('There is no connection to the database, please re-establish connection.') : NULL;
        //$this->handleFatalErr(self::ERR_CONNECTION_LOST, 'CONNECTION_LOST');
}

private function getPDO() {
    return $this->_pdo;
}

public function close() {
    (isset($this->_pdo) && $this->_pdo != null) ? $this->_pdo = null : NULL;
}


Shorter way of writing my displayError() function? Is there a way I can write this function with less code?

jQuery

function displayError(display, string, xhr) {
    var args = arguments;
    var $error = $("div#error");
    var visible = $error.is(":visible");

    if(args.length == 1 && typeof display === "boolean" && display == false) {
        if(visible) {
            $error.slideUp();
        }
    } else if(args.length == 2 && typeof display === "boolean" && display == true && typeof string == "string") {
        if(visible) {
            $error.fadeOut(function () {
                $(this).html('(!) '+string).fadeIn();
            });
        } else {
            $error.html('(!) '+string).slideDown();
        }
    } else if(args.length == 3 && typeof display === "boolean" && display == true && typeof string == "string" && typeof xhr === "object") {
        if(visible) {
            $error.fadeOut(function () {
                $(this).html('('+xhr.status+") "+string).fadeIn();
            });
        } else {
            $error.html('('+xhr.status+") "+string).slideDown();
        }
    }
}

Based on Marco's answer I came up with this:

function displayError(display, string, xhr) {
    var $error = $("div#error");
    var errVisible = $error.is(":visible");
    var errStatus = typeof xhr === "object" ? xhr.status : "!";
    var errMsg = '(' + errStatus + ") " + string;

    if(display) {
        if(errVisible) {
            $error.fadeOut(function() {
                $(this).html(errMsg).fadeIn();
            });
        } else {
            $error.html(errMsg).slideDown();
        }
    } else {
        $error.slideUp();
    }
}

I don't use Javascript, but i could write it in this way:

function displayError(display, string, xhr)
{
    var args = arguments;
    var $error = $("div#error");
    var visible = $error.is(":visible");

    if (!display)
    {
        $error.slideUp();
    }
    else
    {

        if (visible)
        {
            switch (args.length)
            {
                case 1:
                case 2:
                    $error.fadeOut(function ()
                    {
                        $(this).html('(!) ' + string).fadeIn();
                    });
                    break;
                case 3:
                    $error.fadeOut(function ()
                    {
                        $(this).html('(' + xhr.status + ") " + string).fadeIn();
                    });
                    break;
            }
        }
        else
        {
            $error.html('(' + xhr.status + ") " + string).slideDown();
        }
    }
}

But why you use args.length == 1/2/3 if you don't do anything with it?
You could just write:

function displayError(display, string, xhr) {
    var $error = $("div#error");
    var visible = $error.is(":visible");
    var status = typeof xhr === "object" ? xhr.status : "!";

    if (display) {
        if (visible) {
            $error.fadeOut(function () {
                $(this).html('(' + status + ') ' + string).fadeIn();
            });

        } else {
            $error.html('(' + status + ") " + string).slideDown();
        }
    } else {
        $error.slideUp();
    }
}

I used var status = typeof xhr === "object" ? xhr.status : "!"; so you will get the status if xhr is an object, or ! if not.

I removed all typeof parts since i found it useless, you will always get a string so why you worry about it