Printing even and odd using two concurrent threads Please review the code below where thread Odd prints all odd numbers from 1 to 9 and thread Even prints all even numbers from 2 to 8. I tested it and it works but can we improve it from design or performance perspective?

To print odd

public class Odd implements Runnable{

private Monitor sharedObject;

public Odd(Monitor monitor){
    this.sharedObject = monitor;
}

@Override
public void run() {

    try {
        printOdd();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

private void printOdd() throws InterruptedException {
    synchronized(sharedObject){
        for(int i=1; i< 10; i++){
            if(i%2!=0){
                while(!sharedObject.isOdd()){
                    sharedObject.wait();
                }
                System.out.println("Odd: "+ i);
                sharedObject.setOdd(false);
                sharedObject.notifyAll();
            }
        }
    }

}

To print even

public class Even  implements Runnable{

private Monitor sharedObject;

public Even(Monitor monitor){
    this.sharedObject = monitor;
}

@Override
public void run() {

    try {
        printEven();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

private void printEven() throws InterruptedException {
    synchronized(sharedObject){
        for(int i=1; i< 10; i++){
            if(i%2==0){
                while(sharedObject.isOdd()){
                    sharedObject.wait();
                }
                System.out.println("Even: "+ i);
                sharedObject.setOdd(true);
                sharedObject.notifyAll();
            }
        }
    }

}

Monitor class

public class Monitor {

    private volatile boolean isOdd;

    public boolean isOdd() {
      return isOdd;
    }

    public void setOdd(boolean isOdd) {
      this.isOdd = isOdd;
    }

}

public class Odd implements Runnable{

private Monitor sharedObject;

public Odd(Monitor monitor){
    this.sharedObject = monitor;
}

You implemented the class as a Runnable - great first step.  You are injecting the shared Monitor in the constructor, which is good; the member variable should be final though -- you aren't intending to change it after initialization.  Also, it should be named "monitor".

@Override
public void run() {

    try {
        printOdd();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

If you are trying to learn about using multiple threads, one of the things you must come to understand is what InterupptedException means.  In particular, printStackTrace isn't likely to tell you anything interesting (yup, the exception was thrown by sharedObject.await()), and if you are going to catch the exception, you must reset the interrupt flag so that the function that called you can know that cancellation is in progress.

private void printOdd() throws InterruptedException {
    synchronized(sharedObject){
        for(int i=1; i< 10; i++){
            if(i%2!=0){
                while(!sharedObject.isOdd()){
                    sharedObject.wait();
                }
                System.out.println("Odd: "+ i);
                sharedObject.setOdd(false);
                sharedObject.notifyAll();
            }
        }
    }
}

You are testing your condition in a while() loop -- good job, that's really important.  You are also notifying all threads that are blocked on the sharedObject, which is another Good Thing.

There's a generalization to your code that might be appropriate to make explicit:

for(int i=1; i< 10; i++){
    if(i%2!=0){
        ...
    }
}

The for loop is... well, here's the thing.  The threads are supposed to be taking turns counting the same numbers, right?  That is more clearly expressed in the code by having the two threads share a data structure

for(int i : this.numbersToCount){

Where possible, you should use functions to clarify the intent of complex boolean expressions:

if( isOdd(i))

The big problem I see is that you then duplicate all of your code to create Even.  Something has gone very badly wrong.  Let's generalize your loop, by replacing the thread mechanics with functions that express your intent.

synchronized(sharedObject){
    while(this.notFinished()) {
        this.waitForMyTurn();
        this.takeMyTurn();
        this.endMyTurn();
    }
}

Notice what this exposes - the monitor isn't "even or odd", it's a entity that is responsible for coordinating the activity of the two threads.  You've modeled its internals as true / false, but that's wrong -- it's really two different states that flip back and forth.

public class Monitor {

    private volatile State currentState;
    private final EnumMap<State,State> transitions;

    public boolean isInState(State state) {
       return currentState.equals(state);
    }

    public void changeState() {
        currentState = transitions.get(currentState);
    }

    public enum State {
        Odd, Even
    }
}

Or better:

public class Monitor<State> {

    private volatile State currentState;
    private final Map<State,State> transitions;

    public boolean isInState(State state) {
       return currentState.equals(state);
    }

    public void changeState() {
        currentState = transitions.get(currentState);
    }
}

If you imagine something like

interface Task {
    boolean isFinished();
    void doNext();
}

Then it should be clear that your current Even and Odd are both instances of the same class, with a different State and a different Task, which might look like

class CountingTask implements Task {
    private final int [] numbersToCount;
    private int nowAt;

    CountingTask(int [] numbersToCount, int startAt) {
        ...
    }

    boolean isFinished () {
        return nowAt >= numbersToCount.length;
    }

    void doTask () {
        System.out.println(numbersToCount[nowAt]);
        nowAt += 2;
    }
}


Creating elements with special corners I've been given a task to create elements with some special corners, and those elements should be responsive.

This works, but am I wondering if something better could come up.
Less code to maintain.

I wish NOT to use any CSS Preprocessor.  I prefer the more readable pure and simple CSS.

jsFiddle

          Jump Session Outdoor Editon

Party

          London

CSS:

p{
  margin: 0;
  padding: 0;
}

.module-wrapper {
    width: 20%;
    margin: 40px auto;
}

.sub-module {
  margin: 0 -5px;
}

.sub-module p {
    padding: 10px;
    color: white;
    text-align: center;
}

.type {
    background-color: red;
}

.local {
  background: black;
  padding-bottom: 5px;
}

.local p {
    background-color: black;
}

.title {
  background: green;
  padding-top: 5px;
}

.title p {
    background-color: green;
}

Yes, you can reduce your markup!  How small?  This small:

Jump Session Outdoor Editon

Party

London

How?  With pseudo elements.  http://codepen.io/cimmanon/pen/sIgju

.module-wrapper {
  width: 20%;
  margin: 40px auto;
  padding: 10px 0;
  color: white;
  text-align: center;
}
.module-wrapper:before, .module-wrapper:after {
  content: '';
  display: block;
  height: 10px;
}
.module-wrapper > :first-child, .module-wrapper > :last-child {
  position: relative;
}
.module-wrapper > :first-child:before, .module-wrapper > :last-child:after {
  content: '';
  display: block;
  background-color: inherit;
  height: 10px;
  position: absolute;
  left: 10px;
  right: 10px;
}
.module-wrapper > :first-child:before {
  top: -10px;
}
.module-wrapper > :last-child:after {
  bottom: -10px;
}
.module-wrapper div {
  padding: 15px;
}
.module-wrapper .title {
  background: green;
}
.module-wrapper .type {
  background-color: red;
}
.module-wrapper .locale {
  background: black;
}

Now I know you're convinced that you don't need a CSS Preprocessor because you're certain that vanilla CSS is more readable.  But what if you wanted to change the size of your corner cut-outs?  How do you know which value you need to change?  Here's the Sass I used to generate the above CSS:

$corner-width: 10px;
$corner-height: 10px;

.module-wrapper {
  width: 20%;
  margin: 40px auto;
  padding: 10px 0;
  color: white;
  text-align: center;

  // the pseudo elements here prevent us from having
  // to do the math on the parent element's margin
  // to ensure there's no overlap from preceding/
  // following elements
  &:before, &:after {
    content: '';
    display: block;
    height: $corner-height;
  }

  > :first-child, > :last-child {
    position: relative;
  }

  // this is where the illusion of the "corner"
  // happens
  > :first-child:before, > :last-child:after {
    content: '';
    display: block;
    background-color: inherit;
    height: $corner-height;
    position: absolute;
    left: $corner-width;
    right: $corner-width;
  }

  > :first-child:before {
    top: -$corner-height;
  }

  > :last-child:after {
    bottom: -$corner-height;
  }

  // pretty up our child elements
  div {
    padding: 15px;
  }

  .title {
    background: green;
  }

  .type {
    background-color: red;
  }

  .locale {
    background: black;
  }
}

With the use of variables, I only need to make 2 adjustments and everything is recalculated for me.  Since the padding on the child elements and the margin on the parent element is not using the variables, I know that I am free to change them without it affecting the corners.

I am almost certain you mean "locale" for your class name, not "local".

If your CSS goes away, does your content still make sense?  If not, then you may need something more to give it some context.  A table may be appropriate here.

If you ever have a class called "title", that's almost a sure sign that you should be using an h1-h6 element instead.


Performance of List representation of MonadPlus versus Tree representation I am working on a code where I represent a MonadPlus using a Tree data structure and then explore it, summing over all of the leaves (whose type must be an instance of Monoid);  I need to use a Tree representation to do this (rather than using the List representation) because as I explore the tree I want to manually keep track of things like my position in the Tree for checkpointing and workload balancing purposes.

My problem is that using a Tree data structure is slower than using the List data structure by a factor of ~ 4 for small depths (~ 1-10) and ~ 2.5 for higher depths (~15), and I am having trouble understanding exactly why this is;  in particular I am wondering if there are tricks that I can employ to make my code faster.

The remainder of this question will be some code that I wrote to benchmark Tree versus List. I will present my code in sections.  First, the prelude:

{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}

import Control.DeepSeq
import Control.Monad
import Control.Monad.Operational

import Criterion.Main

import Data.Functor.Identity
import Data.Monoid

My Tree type is defined using functionality in the operational package by specifying the type of instructions and then use operational to obtain a Program monad from this.

data TreeTInstruction m α where
    Choice :: TreeT m α -> TreeT m α -> TreeTInstruction m α
    Null :: TreeTInstruction m α

newtype TreeT m α = TreeT { unwrapTreeT :: ProgramT (TreeTInstruction m) m α } deriving (Monad)

type Tree = TreeT Identity

instance Monad m => MonadPlus (TreeT m) where
    mzero = TreeT . singleton $ Null
    left `mplus` right = TreeT . singleton $ Choice left right

The following two functions respectively build a perfect binary tree with Sum Int at all the leaves (which can be any type that is an instance MonadPlus) and explore a given Tree.

makeTree :: MonadPlus m => Int -> m (Sum Int)
makeTree 0 = return (Sum 1)
makeTree d = makeTree (d-1) `mplus` makeTree (d-1)

exploreTree :: Tree (Sum Int) -> Sum Int
exploreTree v =
    case view (unwrapTreeT v) of
        Return x -> x
        Choice left right :>>= k ->
            let x = exploreTree $ left >>= TreeT . k
                y = exploreTree $ right >>= TreeT . k
                xy = mappend x y
            in xy

As a technicality we need to write an instance of NFData for Sum Int for the benchmarking code.

instance NFData (Sum Int) where
    rnf s@(Sum x) = x `seq` s `seq` ()

Finally, we have the benchmarking code.  First, it benchmarks using makeTree to construct a tree using the List monad and then using mconcat to sum over all the results, and second, it benchmarks using makeTree using the Tree monad and sums over all the leaves using the exploreTree function.

main = defaultMain
    [bench "list" $ nf (mconcat . makeTree) depth
    ,bench "tree" $ nf (exploreTree . makeTree) depth
    ]
  where
    depth = 1 -- this is a knob that controls the tree size

For benchmarks, the list and tree times were as follows on my machine:

       List    Tree  Tree/List
 1:   140ns   460ns  3.3x
 2:   330ns  1300ns  4.0x
 4:  1600ns  6300ns  3.9x
 8:    32us   130us  4.0x
10:   150us   600us  4.0x
12:   990us  2900us  2.9x
14:   4.6ms    12ms  2.6x
16:    24ms    55ms  2.3x
17:    50ms   110ms  2.2x

(17 is the largest depth because after that exploring the list causes a stack overflow.)

If anyone has advice on exactly what the cause is of this slow down and if anything can be done about it then I would greatly appreciate it.

A bit late, but perhaps it'll still help someone.

From the benchmark it's not clear if the time difference is caused by producing a tree or consuming it. I'm not very familiar with operational to make a guess.

It seems that what you're looking for is a free MonadPlus. The closest to this is probably FreeT f [] using FreeT, see also this question. I tried to benchmark it with your code with

exploreFreeTree :: FreeT Identity [] (Sum Int) -> Sum Int
exploreFreeTree = mconcat . iterT runIdentity

that flattens a tree into a list and then concatenates it, and it was almost as fast as plain [].

But there is another potential problem. For free monads the bind operation >>= needs to traverse the whole sequence of operations in the left argument, which can cause performance issues. It's similar to appending two lists. I don't know if operational has a similar problem or not. For free monads the solution is to use the Codensity monad, in particular it's enough to just wrap your computation with improve. For this it might be better to use Free [] instead of FreeT [] Identity (although I haven't checked the details), as Codensity works only on Free, not FreeT.

It might be relevant to read
Is there a Codensity MonadPlus that asymptotically optimizes a sequence of MonadPlus operations?, although it's somewhat complicated stuff.

An good alternative could be to use logict. You could express your computations using MonadLogic and then run your computation in LogicT to do whatever processing you need to do. The advantage of this is that LogicT is internally build with continuations, which alleviates the problem with >>= performance of Free, and is built exactly to deal with branching computations. If you'd be interested, I could try to work out an example.


Finding the total time elapsed in the union of time intervals Given a set of time intervals, I have to find the total time elapsed in the union of the intervals.

Test Cases:

int[][] test = {{10, 14}, {4, 18}, {19, 20}, {19, 20}, {13, 20}};
int[][] test2 = {{1, 3}, {3, 6}};
answer(test) == 16
answer(test2) == 5

Both of the tests pass, but I get a "time limit exceeded". How can I get this function to run faster?

Here's my answer function:

public static int answer(int[][] intervals) {
    ArrayList<Integer> t = new ArrayList<Integer>();
    for (int i = 0; i < intervals.length; i++) {
        for (int j = intervals[i][0]; j < intervals[i][1]; j++) {
            if (!t.contains(j))
                t.add(j);
        }
    }
    return t.size();
}

I also made an attempt without using the ArrayList and instead a regular array (int[]):

public static boolean wasWatched(int[] array, int time) {
    for (int i = 0; i < array.length; i++) {
        if (array[i] == time)
            return true;
    }
    return false;
}
public static int[] addTimeWatched(int[] array, int time) {
    int[] temp = new int[array.length + 1];
    for (int i = 0; i < array.length; i++) {
        temp[i] = array[i];
    }
    temp[array.length] = time;
    return temp;
}

The answer() function was the same except t was an int[] and it used wasWatched(t, j) instead of t.contains(j) as well as t = addTimeWatched(t, j) instead of t.add(j).

Here is the full challenge text:

Zombit monitoring

The first successfully created zombit specimen, Dolly the Zombit, needs constant monitoring, and Professor Boolean has tasked the minions with it. Any minion who monitors the zombit records the start and end times of their shifts. However, those minions, they are a bit disorganized: there may be times when multiple minions are monitoring the zombit, and times when there are none!

That's fine, Professor Boolean thinks, one can always hire more minions... Besides, Professor Boolean can at least figure out the total amount of time that Dolly the Zombit was monitored. He has entrusted you, another one of his trusty minions, to do just that. Are you up to the task?

Write a function answer(intervals) that takes a list of pairs [start, end] and returns the total amount of time that Dolly the Zombit was monitored by at least one minion. Each [start, end] pair represents the times when a minion started and finished monitoring the zombit. All values will be positive integers no greater than 2^30 - 1. You will always have end > start for each interval.

Test cases

Inputs:
    (int) intervals = [[1, 3], [3, 6]]
Output:
    (int) 5

Inputs:
    (int) intervals = [[10, 14], [4, 18], [19, 20], [19, 20], [13, 20]]
Output:
    (int) 16

Contraints

Your code will be compiled using standard Java 7. It must implement the answer() method in the solution stub.

Execution time is limited. Some classes are restricted (e.g. java.lang.ClassLoader). You will see a notice if you use a restricted class when you verify your solution.

Third-party libraries, input/output operations, spawning threads or processes and changes to the execution environment are not allowed.

You can make your current \$O(n^2)\$ algorithm run in \$O(n \log n)\$ time by first sorting the input intervals by interval start time, then do a linear scan of the sorted array:

public static int answer(int[][] intervals) {
    if (intervals.length() < 1) {
        return 0;
    }
    java.util.Arrays.sort(
        intervals,
        new java.util.Comparator<int[]>() {
            public int compare(int[] a, int[] b) {
                return Integer.compare(a[0], b[0]);
            }
        });
    int totalTime = 0;
    int currentIntervalEnd = intervals[0][0];
    for (int[] interval : intervals) {
        int intervalStart = interval[0];
        int intervalEnd = interval[1];
        if (intervalEnd > currentIntervalEnd) {
            totalTime +=
                intervalEnd - Math.max(intervalStart, currentIntervalEnd);
            currentIntervalEnd = intervalEnd;
        }
    }
    return totalTime;
}


Python API Query + Email Notification I wrote this to query the Trello API to find the cards that haven't been updated in 7 days and send an email notification to the card's members as a digest.

I'm a Node.JS & client-side JS dev, and this is my first Python script. I'd really appreciate any suggestions around best practices, how it could be more modular/reusable, etc.

import os
from os.path import join, dirname
from dotenv import load_dotenv
from urllib import urlencode
import json
import urllib2
from datetime import datetime
import dateutil.parser
import pytz
import mandrill
from jinja2 import Environment, PackageLoader

# Load environment variables
dotenv_path = join(dirname(__file__), '.env')
load_dotenv(dotenv_path)

MIN_DAYS = os.environ.get("MIN_DAYS")
TRELLO_KEY = os.environ.get("TRELLO_KEY")
TRELLO_TOKEN = os.environ.get("TRELLO_TOKEN")
TRELLO_BOARD = os.environ.get("TRELLO_BOARD")
MANDRILL_KEY = os.environ.get("MANDRILL_KEY")
MSG_FROM_EMAIL = os.environ.get("MSG_FROM_EMAIL")
MSG_FROM_NAME = os.environ.get("MSG_FROM_NAME")
MSG_SUBJECT = os.environ.get("MSG_SUBJECT")
MEMBER_EMAILS = os.environ.get("MEMBER_EMAILS")

mandrill_client = mandrill.Mandrill(MANDRILL_KEY)

# Prepare template
template_env = Environment(loader=PackageLoader(__name__, "templates"))
template = template_env.get_template("reminder_email.html")

# Split member emails into a dict
member_emails = {}
member_email_lines = MEMBER_EMAILS.split(",")
for member_email_line in member_email_lines:
    member_email_line_parts = member_email_line.split(":")
    member_emails[member_email_line_parts[0].strip()] = member_email_line_parts[1].strip()

# Querystring parameters
params = {
    "key": TRELLO_KEY,
    "token": TRELLO_TOKEN,
    "fields": "name,url,dateLastActivity",
    "members": "true"
}

# Construct trello URL and fetch
url = "https://api.trello.com/1/boards/" + TRELLO_BOARD + "/cards?" + urlencode(params)
cards = json.load(urllib2.urlopen(url))

today = datetime.now(pytz.utc)
members = {}

# For each card that's older than a week
for card in cards:
    dateLastActivity = dateutil.parser.parse(card[u'dateLastActivity'])
    if abs(today - dateLastActivity).days > int(MIN_DAYS):
        # Add card to each member's list
        for member in card[u'members']:
            username = str(member[u'username'])
            if username not in members:
                members[username] = []
            members[username].append(card)

# For each member
for username in members:
    if username in member_emails:
        # Build message data
        message = {
            "subject": MSG_SUBJECT,
            "from_email": MSG_FROM_EMAIL,
            "from_name": MSG_FROM_NAME or MSG_FROM_EMAIL,
            "to": [{
                "email": member_emails[username],
                "name": username,
                "type": "to"
            }],
            # Construct HTML from template
            "html": template.render(num_days=MIN_DAYS, cards=members[username])
        }

        # Send email
        try:
            result = mandrill_client.messages.send(message=message)
            print result
        except mandrill.Error, e:
            print 'A mandrill error occurred: %s - %s' % (e.__class__, e)
            raise

Looks good at a first glance.  For reusability the code should be
divided into functions instead of just being a single script file.

I'd load the multiple environment keys in a loop and into a separate
dictionary instead of globals.  That's nicer to refactor later and
it's easy to pass around, or even update from command line arguments
as well (e.g. using argparse).  You can also look at
dotmap or something similar to
get a bit nicer syntax.  E.g.:

options = {}
for name in ["MIN_DAYS",
            "TRELLO_KEY",
            "TRELLO_TOKEN",
            "TRELLO_BOARD",
            "MANDRILL_KEY",
            "MSG_FROM_EMAIL",
            "MSG_FROM_NAME",
            "MSG_SUBJECT",
            "MEMBER_EMAILS"]:
    options[name] = os.environ.get(name)

The loop at the end can be exited early with
if ... not in ...: continue to remove one level of indentation.
The print should be using function call syntax for forwards
compatibility with Python 3 and similarly the except handler should
also use the except ... as e: syntax instead.  I.e.:

...
        print(result)
    except mandrill.Error as e:
...

The iteration over members can use iteritems to get both name and
cards in the same step.
For the date comparison I'd recommend using a dedicated library, that
way both the code will be easier to read and one less wheel is
reinvented.  E.g. I had good experiences with
arrow.
For the HTTP requests you can also look at
requests for a nicer
interface.


Javascript / React data mapper My solution to implement a status (string) from combining two other ones.
I need to declare a function which takes two params (two strings) and needs to return another one based on the combination of those two strings.
For ex:
carStatus(status, secondaryStatus) => string

where secondaryStatus can have multiple options.
I'm using an if/else if statement which returns a third status which I need.
For ex when status is 'OPEN' and secondaryStatus is 'payment1' or 'payment2' or 'payment3', the function must return a new string (status) like 'CONFIRMED'.
So, my solution at this moment would be something like this:
carStatus = (status, secondaryStatus) => {
  if(status === 'OPEN' && (secondaryStatus === 'payment1' || 'payment2' || 'payment3')){
    return 'CONFIRMED';
  } else if(status === 'CANCELLED' && (secondaryStatus === 'payment4' || 'payment5' || 'payment6')){
    return 'REMOVED';
  } else if(status === 'REVIEW' && (secondaryStatus === 'payment2' || 'payment5' || 'payment5')){
    return 'CHECKED';
  }
}

carStatus('OPEN', 'payment1')

In div must be rendered 'CONFIRMED'.
In my implementation, I'll have to write I think other 5 else if statements.. so maybe there is a cleaner way to implement this.
Any help will be appreciated.

Condition bug Your current implementation has a bug. Your lines like these have the problem:
(secondaryStatus === 'payment1' || 'payment2' || 'payment3')

JavaScript only has unary operators, binary operators, and a single ternary operator. All the operators used above are binary operators; two expressions will be evaluated into a single expression until a single one is left. Since === has higher operator precedence than ||, your code is equivalent to:
(secondaryStatus === 'payment1' || 'payment2' || 'payment3')

(trueOrFalse || 'payment2' || 'payment3')

The payment2 and payment3 are not compared against secondaryStatus, and if secondaryStatus is not payment1, the whole expression will evaluate to 'payment2', a truthy value (because || will evaluate to the second value if the first is falsey):
// || operates left-to-right:

(trueOrFalse || 'payment2' || 'payment3')

((false || 'payment2') || 'payment3')

(('payment2') || 'payment3')

// payment2 is truthy, so the `||` evaluates to it:
('payment2')

To fix the logic, use an array instead, and check the status against each element in the array.
Typo? You have (secondaryStatus === 'payment2' || 'payment5' || 'payment5'), with payment5 repeated twice. Did you mean something else, like payment9?
Names carStatus does not contain the car's status, as it would sound to contain; it's a function that, when called, returns the car's status. It also takes a confusingly-similarly named status parameter. Call them something else, if at all possible: perhaps getCarStatus and whatever more specific thing the status parameter represents. Maybe call secondaryStatus: paymentType.
DRY You also want to make the code more DRY, which fits in well with the idea of using arrays instead. You might use an object, whose keys are the status that's required, and whose values are the possible secondaryStatus values as well as the return value if the secondaryStatus is found.
Also, since all the payment strings end in a number, and that number is the only thing that changes, use that number to set up the config object instead of the full payment strings:
const statusOptions = {
    OPEN:      { paymentNumbers: [1, 2, 3], status: 'CONFIRMED' },
    CANCELLED: { paymentNumbers: [4, 5, 6], status: 'REMOVED' },
    REVIEW:    { paymentNumbers: [2, 5], status: 'CHECKED' },
};
// ...
getCarStatus = (status, paymentType) => {
    const possibleOption = statusOptions[status];
    if (possibleOption) {
        const paymentNumber = Number(paymentType.match(/\d+$/)[0]);
        if (possibleOption.paymentNumbers.includes(paymentNumber)) {
            return possibleOption.status;
        }
    }
}

Live snippet:

const statusOptions = {
    OPEN:      { paymentNumbers: [1, 2, 3], status: 'CONFIRMED' },
    CANCELLED: { paymentNumbers: [4, 5, 6], status: 'REMOVED' },
    REVIEW:    { paymentNumbers: [2, 5], status: 'CHECKED' },
};
// ...
getCarStatus = (status, paymentType) => {
    const possibleOption = statusOptions[status];
    if (possibleOption) {
        const paymentNumber = Number(paymentType.match(/\d+$/)[0]);
        if (possibleOption.paymentNumbers.includes(paymentNumber)) {
            return possibleOption.status;
        }
    }
};

console.log(
  getCarStatus('OPEN', 'payment3'),
  getCarStatus('REVIEW', 'payment2')
);

Component type It looks like you're using a class component. React recommends that you try out using functional components in new code; they say they're a bit easier to work with and understand than class-based components in most circumstances, and I agree. Try using functional components instead, if you haven't already, you might like them.


URL router that loads dependent objects How can I write my code more efficient (not repeating same code)?

And is there a solution that simplifies the way of having to create a lot of dependency objects over and over again whenever I instantiate a new class (as seen in the load method)?

Any suggestions are welcome.

class Router
{
    private $request;
    private $cleanUri;
    # control attributes
    private $controller;
    private $action;
    private $params = [];

    public function __construct(Request $request)
    {
        $this->request  = $request;
        $this->cleanUri = preg_replace(['#/+#', '(\.\./)'], '/', trim($request->getUri(), '/'));

        $this->parseUri();
        $this->load();
    }

    private function parseUri()
    {
        $parts            = explode('/', $this->cleanUri);
        $this->controller = ($this->cleanUri !== '' ? ucfirst(array_shift($parts))  : 'Index') . 'Controller';
        $this->action     = array_shift($parts);
        $this->params     = !empty($parts) ? $parts : null;
    }

    private function load()
    {
        if (class_exists($this->controller)) {
            if (empty($this->action)) {
                $this->action = 'index';
            } elseif ($this->action !== 'index' && method_exists($this->controller, $this->action)) {
                //$action value stays the same
            } else {
                if (class_exists('_404Controller')) {
                    $controller = new _404Controller();

                    $controller->index();
                } else {
                    throw new Exception('404 controller not found. Page doesn\'t exist for: ' . $this->request->getUri());
                }
            }

            $controller = $this->controller;

            //create dependencies
            $db   = new Database();
            $user = new User($db, $this->request);
            $menu = new MenuHelper($this->request, $user);

            $controller = new $controller($this->request, $user, $menu);
            $action     = $this->action;

            $controller->$action();
        } else {
            if (class_exists('_404Controller')) {
                $controller = new _404Controller();

                $controller->index();
            } else {
                throw new Exception('404 controller not found. Page doesn\'t exist for: ' . $this->request->getUri());
            }
        }
    }
}

If this unit of code is a router, then it should remain a router. Have the controller handle its dependencies. You could have multiple controllers, each with its own dependencies. You wouldn't want a single router code become a monster just to keep up with the controllers.

class Router{

  private $request;
  private $cleanUri;

  private $controller;
  private $action;
  private $params = [];

  public function __construct(Request $request){
      $this->request  = $request;
      $this->cleanUri = $this->sanitizeUri($request->getUri())
      $this->parseUri();
      $this->load();
  }

  // Extract sanitize. You might want to add more than just this and the constructor
  // is a bad place to put it.
  private function sanitizeUri($uri){
    return preg_replace(['#/+#', '(\.\./)'], '/', trim($uri, '/'));
  }

  private function parseUri(){
      $parts            = explode('/', $this->cleanUri);
      $this->controller = ($this->cleanUri !== '' ? ucfirst(array_shift($parts))  : 'Index') . 'Controller';
      $this->action     = array_shift($parts);
      $this->params     = !empty($parts) ? $parts : null;
  }

  // Extract the 404
  private function send404(){
    if (class_exists('_404Controller')) {
      $controller = new _404Controller();
      $controller->index();
    } else {
      throw new Exception('404 controller not found. Page doesn\'t exist for: ' . $this->request->getUri());
    }
  }

  private function load(){

    // If action is empty, use index
    if (empty($this->action)) $this->action = 'index';

    // If either controller or method doesn't exist, 404
    if(!(class_exists($this->controller)  && method_exists($this->controller, $this->action)))
      $this->send404();

    // else execute
    $controller->$action($this->params);

  }
}

Then the controller could have:

class MyController extends Controller{

  public function __construct(){
    $this->load(array(
      'database',
      'cats',
      'unicorns'
    ));
  }

  public function index(){
    $this->database->getSomething();
  }
}


Implementation of generic Graph class and Dijkstras I participate in competitive programming contests, and I found myself implementing and re-implementing graphs constantly. So I decided to create a reusable implementation of a Graph class, and implement some common methods for it, including DFS, BFS, and Dijkstras.

Are there any edge cases that my code misses? Is there anything I could do to improve it?

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.PriorityQueue;
import java.util.Queue;
import java.util.Stack;
import java.util.function.BiConsumer;
import java.util.stream.Collectors;

public class Graph<T> {

    public class Node {
        public T value;
        public Map<Integer, Integer> edges;

        public Node(T value) {
            this.value = value;
            edges = new HashMap<>();
        }
    }

    public List<Node> nodes;

    public boolean directed;
    public int numNodes = 0;
    public int numEdges = 0;

    public Graph() {
        this(false);
    }

    public Graph(boolean directed) {
        nodes = new ArrayList<>();
        this.directed = directed;
    }

    public void addNode(T value) {
        nodes.add(new Node(value));
    }

    public void connect(int i, int j, int weight) {
        nodes.get(i).edges.put(j, weight);
        if (!directed)
            nodes.get(j).edges.put(i, weight);
    }

    public class DijkstrasNode extends Node {
        int dist = -1;
        boolean visited = false;
        DijkstrasNode previous;

        public DijkstrasNode(T value) {
            super(value);
        }

        public DijkstrasNode(Node node) {
            super(node.value);
            this.edges = node.edges;
        }
    }

    public void processBFS(int source, BiConsumer<Node, Integer> consumer) {
        Queue<Integer> q = new LinkedList<>();
        boolean[] visited = new boolean[nodes.size()];
        q.add(source);
        while (!q.isEmpty()) {
            int id = q.poll();
            if (visited[id])
                continue;
            visited[id] = true;
            Node n = nodes.get(id);
            consumer.accept(n, id);
            for (int c: n.edges.keySet())
                q.add(c);
        }
    }

    public void processDFS(int source, BiConsumer<Node, Integer> consumer) {
        Stack<Integer> q = new Stack<>();
        boolean[] visited = new boolean[nodes.size()];
        q.push(source);
        while (!q.isEmpty()) {
            int id = q.pop();
            if (visited[id])
                continue;
            visited[id] = true;
            Node n = nodes.get(id);
            consumer.accept(n, id);
            for (int c: n.edges.keySet())
                q.add(c);
        }
    }

    public List<DijkstrasNode> dijkstras(int source) {
        List<DijkstrasNode> djk = nodes.stream().map(DijkstrasNode::new).collect(Collectors.toList());
        djk.get(source).dist = 0;
        PriorityQueue<DijkstrasNode> q = new PriorityQueue<>((i, j) -> i.dist - j.dist);
        q.add(djk.get(source));
        int visitCount = 0;
        while (!q.isEmpty() && visitCount < djk.size()) {
            DijkstrasNode n = q.poll();
            if (n.visited)
                continue;
            n.visited = true;
            visitCount++;
            for (int child : n.edges.keySet()) {
                DijkstrasNode cn = djk.get(child);
                if (!cn.visited && (cn.dist == -1 || n.dist + n.edges.get(child) < cn.dist)) {
                    if (cn.dist != -1)
                        q.remove(cn);
                    cn.dist = n.dist + n.edges.get(child);
                    cn.previous = n;
                    q.add(cn);
                }
            }
        }
        return djk;
    }

}

Just a small remark design-wise:

I think you should not extend Node to DijkstraNode, but rather have a Node<Dijkstra>.  Or, if you intent to store info in the DijkstraNode, have a Node<Dijkstra<T>>.


How can this swift, firebase image fetching, function be made more concise? I am trying to fetch a group of images within a place in Firebase database ref. I then loop through each image adding it to an array which is then supposed to be added to a tableview cell. I was wondering if it could be more concise? am i doing anything unnecessary?

    func fetchAllUserFristImage() {
    Database.database().reference().child(“Posts”).observe(.childAdded, with: {(snapshot) in
        if snapshot.value as? [String: AnyObject] != nil {
            let user = snapshot.key
            self.databaseRef = Database.database().reference()
            let usersPostRef2 = self.databaseRef.child(“Posts”).child(user)
            usersPostRef2.observe(.value, with: {(postXSnapshots) in
                if let postDictionary2 = postXSnapshots.value as? [String:AnyObject] {
                    for (p) in postDictionary2 {
                        if let posts = p.value as? [String:AnyObject] {
                            //to get back to where i was delete the below for i
                            for (i) in posts {

                                if let imageUrlString = i.value as? [String:AnyObject], let postUrl = imageUrlString[“image1”] as? String {
                                    self.feedArray.append(Post(fetchedImageURL: postUrl))
                                    let imageUrl = URL(string: “\(postUrl)“)
                                    do {
                                        print(try Data(contentsOf: imageUrl!))
                                    } catch {
                                        print(error)
                                    }
                                    if let imageDataL = try? Data(contentsOf: imageUrl!) {
                                        let image = UIImage(data: imageDataL)
                                        self.tableData.append(UserImage(image: image!, postNum: p.key, userID: user))
                                        self.tableView.reloadData()
                                    } else {print(“users had no posts, was nil”)}
                                }
                            }
                        }
                    }
                }
            })
            //below shud stay same
            DispatchQueue.main.async {
                self.tableView.reloadData()
            }
        }
    })
}

Edit: Posibly I could do somthing wit this postsSnap.childSnapshot(forPath: "ImageUrl").childSnapshot(forPath: "image1") to make it more consise
Thanks!

I would start by investigating the cases where you are performing if lets and basic unwrapping. Cases like if snapshot.value as? [String: AnyObject] != nil can be turned into guard statements and provide routes for early termination. This reduces the pyramid by one level for each early termination.

After that, I see some redundant code in the try section on the innermost level that can be brought together.

Other minor changes I'd make would be regarding variable names, some of the names are not entirely that descriptive and make for some less than ideal reading. More descriptive names with less abbreviation would help understand the code better. Things like usersPostRef2 don't make a lot of sense since we don't see a usersPostRef.

Also lastly, make sure you update the UI on main threads, which you appear to be doing at the end, but not necessarily within the second snapshot closure.

This would give us the following:

func fetchAllUserFristImage() {
    Database.database().reference().child(“Posts”).observe(.childAdded, with: {(snapshot) in
        guard snapshot.value as? [String: AnyObject] != nil else {
            return
        }

        let user = snapshot.key
        self.databaseReference = Database.database().reference()
        let usersPostReference = self.databaseRef.child(“Posts”).child(user)
        usersPostReference.observe(.value, with: {(postXSnapshots) in
            guard let postDictionary = postXSnapshots.value as? [String:AnyObject] else {
                return
            }

            for anyPosts in postDictionary {
                guard let posts = anyPosts.value as? [String:AnyObject] else {
                    return
                }

                //to get back to where i was delete the below for i
                for anyImage in posts {
                    guard let imageUrlString = anyImage.value as? [String:AnyObject], let postUrl = imageUrlString[“image1”] as? String else {
                        return
                    }

                    self.feedArray.append(Post(fetchedImageURL: postUrl))
                    let imageUrl = URL(string: “\(postUrl)“)
                    do {
                        let imageData = try Data(contentsOf: imageUrl!))
                        let image = UIImage(data: imageData)
                        self.tableData.append(UserImage(image: image!, postNum: p.key, userID: user))
                        DispatchQueue.main.async {
                            self.tableView.reloadData()
                        }
                    } catch {
                        print(error)
                    }
                }
            }
        })
        //below shud stay same
        DispatchQueue.main.async {
            self.tableView.reloadData()
        }
    })
}


How to make this code fit into the base class I have lots of classes that inherit from abstract base class, which has abstract method

public class BaseClass<T>
{
    protected abstract void GetItems();
}

each subclass have almost the same implementation

protected override void GetItems()
{
    // some preparation code here which is the same for all sublcasses

    // calls async method that handles the result in anonymous method
    // method name is different for each of the subclass that implements it
    _model.GetXXXAsync(a =>
    {
        // same code here for all sublclasses
    });
}

So, basically, the difference is only in the GetXXXAsync method. This is the method signature:

void GetXXXAsync(Action<EntityResultArgs<T>> operationCompleted);

WHat would be the best way to refactor this code to place it in a base class, to avoud repeating 30 lines of code in each subclass, while changing only the method name and T type?

You can create an abstract method that will wrap the GetXXXAsync() method:

public class BaseClass<T>
{
    protected void Getitems()
    {
        // some preparation code here which is the same for all sublcasses

        GetItemsInternal(a =>
        {
            // same code here for all sublclasses
        });
    }

    // probably needs a better name
    protected abstract void GetItemsInternal(Action<EntityResultArgs<T>> operationComplete);
}

public class DerivedClass : BaseClass<SomeType>
{
    protected override void GetItemsInternal(Action<EntityResultArgs<SomeType>> operationComplete)
    {
        _model.GetXXXAsync(operationComplete);
    }
}


Loading articles dynamically on page change I just created a script on jQuery which loads my articles dynamically with an AJAX call if the user changes the page.

I added some cool effects like the articles move on the left, then the new ones appear in fade while a loading black screen cover the articles.

The thing is, I'm afraid my code is too heavy and not really well optimized. Could you give me your feedback about it? By the way, do you have any idea if Google is going to be able to read all my articles with that system?

$(".pages li a").click(function(e){ // IF PAGE NUMBER IS CLICKED
    e.preventDefault();
    $(".pages li a").removeClass("active");
    $(this).addClass("active");

    //VARIABLES
    var currentPage=currentPage;
    var nextPage=$(this).attr("data-page");
    var currentCategory="1";

    //SHOW DYNAMIC LOADING WINDOW
    $(".layerarticles").before("

");
    $(".layerloading").height($(".layerarticles").height());
    $(".layerloading img").css("margin-top",$(".layerarticles").height()/2-16);
    $(".layerloading").fadeIn("slow",function(){

    //DATA LOAD
    $.post("ajax/articles-request.php", { currentPage: currentPage, nextPage: nextPage },function(data){ //AJAX CALL

            //WE KICK ALL OLD ARTICLES
            $(".layerarticles").animate({"margin-left":"-=630px"},'slow','easeOutQuint',function(){
                $(".layerarticles").css("height",$(".layerarticles").height());
                $(".layerarticles article").remove();
                $(".layerarticles").css("margin-left","0px");

                //WE MAKE APPEAR THE OTHERS IN THE HTML
                $.each(data, function(){
                    $(".layerarticles").append("

"+this.title+"

"+this.chapeau+"

");
                });

                //WE SHOW THEM WITH FADE EFFECT
                $('.layerarticles article:hidden:first').delay(100).fadeIn("fast",function(){
                     if($('.layerarticles article:hidden').length!=0){
                        $(this).next().delay(100).fadeIn("fast",arguments.callee);
                     }else{

                        //ONCE ALL ARTICLES APPEARED WE REMOVE THE LOADING
                        $(".layerloading").fadeOut("slow",function(){
                            $(".layerloading").remove();
                        });

                     }
                });

            });
        }, "json");

    });
});

One big step to opimize your code is to cache your jQuery objects.

var $layer = $('#layer');

You can also chain your method calls

$layer.css({ height: '30px' }).fadeIn(300).append('hi');

Also, make use of context

var $layerLoading = $('.layerLoading');
$layerLoading.fadeOut("slow",function(){
  //$(".layerloading").remove();
    // instead do - this refers to the context which is '.layerloading' in this case
  $(this).remove();
});


Creating an AlertDialog with a list I am wanting to implement an AlertDialog with a list into my app and I created a sample to see if I can get the result that I am looking for. It seems I do have the result I wanted but before I implement it into my actual app, is there a way to make my code better?:

activity_main.xml:

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:id="@+id/activity_main"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:padding="16dp"
    tools:context=".MainActivity">

    <Button
        android:id="@+id/button"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:text="Press here" />

</RelativeLayout>

MainActivity.java:

public class MainActivity extends AppCompatActivity implements View.OnClickListener {

    Button button;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        button = (Button) findViewById(R.id.button);
        button.setOnClickListener(this);
    }

    @Override
    public void onClick(View view) {
        switch (view.getId()) {
            case R.id.button:
                showAlertDialogWithList();
        }
    }

    public void showAlertDialogWithList() {
        List<String> mAnimals = new ArrayList<>();
        mAnimals.add("Cat");
        mAnimals.add("Dog");
        mAnimals.add("Horse");
        mAnimals.add("Elephant");
        mAnimals.add("Rat");
        mAnimals.add("Lion");
        mAnimals.add("Tiger");
        mAnimals.add("Bird");
        mAnimals.add("Zebra");
        mAnimals.add("Cheetah");
        mAnimals.add("Hyena");

        final CharSequence[] animals = mAnimals.toArray(new String[mAnimals.size()]);
        AlertDialog.Builder dialogBuilder = new AlertDialog.Builder(this);
        dialogBuilder.setTitle("Animals");
        dialogBuilder.setItems(animals, new DialogInterface.OnClickListener() {
            @Override
            public void onClick(DialogInterface dialogInterface, int i) {
                String selectedText = animals[i].toString();
                Toast.makeText(getApplicationContext(), selectedText, Toast.LENGTH_SHORT).show();
            }
        });

        AlertDialog alertDialogObject = dialogBuilder.create();
        alertDialogObject.show();
    }

}

I don't know how many times you wish to use this on your project so I tested and I suggest you to first extract the list from the method context and receive it as argument.

public void showAlertDialogWithList(List<String> mAnimals) {

    final CharSequence[] animals = mAnimals.toArray(new String[mAnimals.size()]);
    AlertDialog.Builder dialogBuilder = new AlertDialog.Builder(this);
    dialogBuilder.setTitle("Animals");
    dialogBuilder.setItems(animals, new DialogInterface.OnClickListener() {
        @Override
        public void onClick(DialogInterface dialogInterface, int i) {
            String selectedText = animals[i].toString();
            Toast.makeText(getApplicationContext(), selectedText, Toast.LENGTH_SHORT).show();
        }
    });

    AlertDialog alertDialogObject = dialogBuilder.create();
    alertDialogObject.show();
}

Then you can extract a class and put it in some util package making it available to entire project like this one:

public class AlertDialogList {
    public static void showAlertDialogWithList(String title, List<String> mAnimals, final Activity activity) {

        final CharSequence[] animals = mAnimals.toArray(new String[mAnimals.size()]);
        AlertDialog.Builder dialogBuilder = new AlertDialog.Builder(activity);
        dialogBuilder.setTitle(title);
        dialogBuilder.setItems(animals, new DialogInterface.OnClickListener() {
            @Override
            public void onClick(DialogInterface dialogInterface, int i) {
                String selectedText = animals[i].toString();
                Toast.makeText(activity.getApplicationContext(), selectedText, Toast.LENGTH_SHORT).show();
            }
        });

        AlertDialog alertDialogObject = dialogBuilder.create();
        alertDialogObject.show();
    }
}

and use it like this

 AlertDialogList.showAlertDialogWithList("Animals Title", this.animals, this);


Optimize nested enumerate blocks? I have 3 nested NSEnumeration loops, which are used to get the textfields of a custom cell, in a custom table in a custom view in a controller.

How can I make this code more readable and more optimizated?

- (void) textfieldsOperations:(APOperation)op
{
  __block int Valid = 0;
  __block NSArray *sub = _Table.subviews;
  __block MyCell *cell = nil;
  __block UIView *view = nil;
  __block NSMutableArray *ret = [NSMutableArray arrayWithCapacity:5];

  [sub enumerateObjectsUsingBlock:^(id c, NSUInteger index, BOOL *stop)
  {
    if ( [c isKindOfClass:[MyCell class]] ) {
      cell = c;
      [cell.subviews enumerateObjectsUsingBlock:^(id v, NSUInteger index, BOOL *stop)
      {
        if ( [v isKindOfClass:[UIView class]] ) {
          view = v;
          [view.subviews enumerateObjectsUsingBlock:^(id t, NSUInteger index, BOOL *stop)
          {
            if ( [t isKindOfClass:[MyTextField class]] )
            {
              MyTextField *txt = t;
              switch ( op )
              {
                case APOperationClear:
                  // something with "txt"
                  break;
                case APOperationEnableSearch:
                  // something with "valid"
                  break;
                case APOperationGetText:
                  // something with "txt"
                  break;
                case APOperationPos: {
                  // something with "txt"
                  break;
                }
              }
            }
          }];
        }
      }];
    }
  }];
  //[...]
}

honestly: this code should be replaced completely.

you dont give any informations about how your MyCell and MyTextField look like, so I can just give you a general advice: Let MyTextField have a delegate it can call if a certain operation is triggered on it.

ie it could call  textField:(MyTextField *)textField didTriggerOperation:(APOperation)op and the implementation would be similar to:

-(void)textField:(MyTextField *)textField didTriggerOperation:(APOperation)op
{
    if(textField == self.descriptionTextField){
        if(op){
            switch ( op )
            {
              case APOperationClear:
                // something with "txt"
                break;
              case APOperationEnableSearch:
                // something with "valid"
                break;
              case APOperationGetText:
                // something with "txt"
                break;
              case APOperationPos: {
                // something with "txt"
                break;
              }
            }
        }

    }
}

your textField would call is when the operation was triggered like

[self.delegate textField:self didTriggerOperation:operation];


Circular FIFO Buffer I was bored the other day and got to wondering how I would implement a circular FIFO buffer in Rust. My goal was to create an implementation I could use on a bare metal microcontroller. This means no dynamic allocation and a fixed size known at compile time, so I used a primitive fixed size array instead of a vector as a backing store.

My next pass at this will be to make the type stored in the FIFO buffer generic and to find a way to specify the capacity, but I wanted to get some feedback before doing so. Am I missing any test cases? What can I improve here?

src/lib.rs

const FIFO_CAPACITY: usize = 32;

pub struct Fifo {
    size: usize,
    read_idx: usize,
    write_idx: usize,
    buffer: [u8; FIFO_CAPACITY]
}

impl Fifo {
    pub fn new() -> Fifo {
        Fifo {
            size: 0,
            read_idx: 0,
            write_idx: 0,
            buffer: [0; FIFO_CAPACITY]
        }
    }

    pub fn push(&mut self, item: u8) -> Result<(), &'static str> {
        if self.buffer_full() {
           Err("Buffer full.")
        } else {
            self.buffer[self.write_idx] = item;

            self.write_idx = Fifo::increment_index(self.write_idx);
            self.size = self.size + 1;

            Ok(())
        }
    }

    pub fn pop(&mut self) -> Option<u8> {
        if self.size == 0 {
            None
        } else {
            let result = self.buffer[self.read_idx];

            self.read_idx = Fifo::increment_index(self.read_idx);
            self.size = self.size - 1;

            Some(result)
        }
    }

    pub fn buffer_full(&self) -> bool {
        self.size == FIFO_CAPACITY
    }

    fn increment_index(idx: usize) -> usize {
        (idx + 1) % FIFO_CAPACITY
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn pop_item_that_was_pushed_to_buffer() {
        let mut buffer = Fifo::new();
        assert!(buffer.push(5).is_ok());
        let pop_result = buffer.pop();
        assert_eq!(Some(5), pop_result);
    }

    #[test]
    fn popping_empty_buffer_returns_none() {
        let mut buffer = Fifo::new();
        assert_eq!(None, buffer.pop());
    }

    #[test]
    fn popping_returns_first_pushed_first() {
        let mut buffer = Fifo::new();
        assert!(buffer.push(1).is_ok());
        assert!(buffer.push(2).is_ok());

        assert_eq!(Some(1), buffer.pop());
        assert_eq!(Some(2), buffer.pop());
    }

    #[test]
    fn pop_beyond_write_index_returns_none() {
        let mut buffer = Fifo::new();
        assert!(buffer.push(1).is_ok());
        assert_eq!(Some(1), buffer.pop());
        assert_eq!(None, buffer.pop());
    }

    #[test]
    fn pop_beyond_write_index_continuing_on_works() {
        let mut buffer = Fifo::new();
        assert!(buffer.push(1).is_ok());
        assert_eq!(Some(1), buffer.pop());
        assert_eq!(None, buffer.pop());

        assert!(buffer.push(2).is_ok());
        assert_eq!(Some(2), buffer.pop());
    }

    #[test]
    fn buffer_wraps_around() {
        let mut buffer = Fifo::new();
        let capacity = FIFO_CAPACITY as u8;
        for x in 0..=(capacity * 3) {
            assert!(buffer.push(x).is_ok());
            assert_eq!(Some(x), buffer.pop());
        }
    }

    #[test]
    fn push_fails_if_buffer_is_full() {
        let mut buffer = Fifo::new();
        let capacity = FIFO_CAPACITY as u8;

        for x in 0..(capacity) {
            assert!(buffer.push(x).is_ok());
        }

        assert!(buffer.push(capacity + 1).is_err())
    }
}

Your code looks pretty decent. My two cents:

Implement Default

impl Default for Fifo {
    fn default() -> Fifo {
        Fifo {
            size: 0,
            read_idx: 0,
            write_idx: 0,
            buffer: [0; FIFO_CAPACITY],
        }
    }
}

Then you could simplify your new to:

pub fn new() -> Fifo {
    Fifo::default()
}

Simplify expressions

L27: self.size = self.size + 1 will become self.size += 1
L40: self.size = self.size - 1 will become self.size -= 1
Replace str error with enums

pub enum FifoError {
    FifoFull,
}

pub fn push(&mut self, item: u8) -> Result<(), FifoError> {
    if self.buffer_full() {
       Err(FifoError::FifoFull)
    } else {
        ...
}

Add assert_eq!(None, buffer.pop()); to the end of every test where feasible, e,g.

pop_item_that_was_pushed_to_buffer
popping_returns_first_pushed_first
pop_beyond_write_index_continuing_on_works
buffer_wraps_around

For further exercices I would recommend:

Implement Iterator/IntoIterator/FromIterator
Next, implement Debug, which is fairly easy (Hint: Take a look at the implementation of Debug for slice)
Make it accept a generic type


Find two numbers that add up to target I am trying to solve this problem:

  Given an array of integers, return indices of the two numbers such that they add up to a specific target.

and this is my implementation:

public int[] twoSum(int[] numbers, int target) {
    Map<Integer, Integer> numbersMap = new HashMap<Integer, Integer>();
    int[] requiredNumbers = null;
    int index = 0;
    for (int number : numbers) {
        if (numbersMap.containsKey(target - number)) {
            requiredNumbers = new int[2];
            requiredNumbers[0] = numbersMap.get(target - number);
            requiredNumbers[1] = index;
            return requiredNumbers;
        } else {
            numbersMap.put(number, index);
            index++;
        }
    }
    return requiredNumbers;
}

How can I improve its execution time?

If the size of your input array can be large, you can get a speed-up by preallocating the capacity of your HashMap:

Map<Integer, Integer> numbersMap = new HashMap<Integer, Integer>(numbers.length * 2);

As the algorithm runs, data will be added to the HashMap.  When number of entries exceeds the capacity * load_factor, the hashmap's capacity is doubled, and the elements are re-binned for the larger capacity.  This capacity doubling and rebinning takes time.  It doesn't happen often, \$O(\log N)\$ times, but it can be eliminated by starting with a hashmap of sufficient capacity.

The load_factor defaults to 0.75, so an initial capacity larger than numbers.length * 4/3 is required.  numbers.length * 2 is a simple expression that satisfies that requirement.


Finding strings in a matrix Looking for feedback especially regarding design/structure/naming but also anything else that can be improved upon.

Task: Given a (hardcoded) matrix, find out if a given string can be found without visiting a cell twice.
Movement is restricted to horizontal/vertical and only adjacent cells can be visited.
Print "True" if the input string can be found "False" otherwise.

Matrix:

ABCE
SFCS
ADEE

Example:

Input: CSE

Result: True. (Start from the C in the 2nd row, go right, go up)

Source code:

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <tuple>
#include <set>

using std::get;
using std::make_tuple;

const std::vector<std::vector<char>> matrix =
{
    {'A', 'B', 'C', 'E'},
    {'S', 'F', 'C', 'S'},
    {'A', 'D', 'E', 'E'},
};

const std::vector<std::tuple<int, int>> directions =
{
    make_tuple(-1, 0),
    make_tuple(0, 1),
    make_tuple(1, 0),
    make_tuple(0, -1),
};

std::set<std::tuple<int, int>> used;
std::string chars;

bool find_next_match(size_t index, std::tuple<int, int> cur_pos)
{
    if (index == chars.size() - 1) return true;

    ++index;

    used.insert(cur_pos);

    for (const auto &dir : directions)
    {
        auto new_pos = make_tuple(get<0>(cur_pos) + get<0>(dir), get<1>(cur_pos) + get<1>(dir));

        if (get<0>(new_pos) < 0 || get<0>(new_pos) > 2) continue;
        if (get<1>(new_pos) < 0 || get<1>(new_pos) > 3) continue;

        if (used.count(new_pos)) continue;

        if (matrix[get<0>(new_pos)][get<1>(new_pos)] != chars[index]) continue;

        if (find_next_match(index, new_pos)) return true;
    }

    used.erase(cur_pos);

    return false;
}

int main (int argc, char **argv)
{
    std::ifstream infile(argv[1], std::ios::in | std::ifstream::binary);

    std::string line;
    while (getline(infile, line))
    {
        chars.clear();
        chars = line;

        bool found = false;
        for (int m = 0; m < 3; ++m)
        {
            for (int n = 0; n < 4; ++n)
            {
                if (matrix[m][n] == chars[0])
                {
                    used.clear();
                    if (find_next_match(0, make_tuple(m, n)))
                    {
                        found = true;
                        break;
                    }
                }
                if (found) break;
            }
        }

        if (found) std::cout << "True\n";
        else std::cout << "False\n";
    }
}

Edit 1:

I changed the find_next_match function to operate on the string directly instead of passing an addiontal argument to keep the amount of arguments under 3 (as per recommendation in Robert C. Martins Clean Code)

Revision 2 based on advice by @vnp

Edit 2:

Revision 3 based on advice by @miscco

Avoid magic numbers.

It is not immediately clear what 2 and 3 in find_next_match stand for (same for 3 and 4 in main). Even though the matrix is hardcoded, I recommend to infer its dimensions, rather then hardcode them also.
Avoid globals.

Passing std::string& line as an additional parameter to find_next_match is cleaner then reliance on the global, and obviates the (pretty strange)

    chars.clear();
    chars = line;

tuple vs custom class.

Using a tuple makes the code too verbose to my taste. I'd invest into a struct position with an operator+ overloaded to perform what

    make_tuple(get<0>(cur_pos) + get<0>(dir), get<1>(cur_pos) + get<1>(dir));

does.
Nitpicks.

You should test if infile is opened correctly.
Final print may use a ternary

    std::cout << (found? "True": "False") << '\n';

I don't want to start a holy war, but I'd return 0; from main.
Kudos for not using namespace std;


Is my routing good or bad? in PHP I just want to checking if this kind of routing is good or bad?

I implement the routing of my web appl to something like this.

All page will be pointing to the index.php

in index.php I have this code

$url = array_values(array_filter( explode( '/', strtolower($_SERVER["REQUEST_URI"]) ) ) );
switch( $url[0] ) {
    case "admin"    : {
        if( !$this->us->isAdmin ) {
            $this->GoToLoginPage();
        }
        array_shift( $uri );
        $department = new Admin( $uri );
    } break;
    case "article"  : {
        if( count( $uri ) == 2 ) {
            displayArticle( $uri[1] );
        }
        else {
            displayError();
        }
    } break;
    default : {
        displayIndex();
    } break;
}

I just want to know is there any pro and con of doing something like this.

Separate your logic and site specific functionality like this:

function Router{
    $url = array_values(array_filter(explode('/', strtolower($_SERVER["REQUEST_URI"]))));
    switch($url[0]){
        case "admin": $this->Admin($url);
            break;
        case "article": $this->Article($url);
            break;
        default: displayIndex();
    }
}

function Admin($uri){
    if( !$this->us->isAdmin ) {
        $this->GoToLoginPage();
    }
    array_shift( $uri );
    $department = new Admin( $uri );
}

function Article($uri){
    if( count( $uri ) == 2 ) {
        displayArticle( $uri[1] );
    } else {
        displayError();
    }
}

It's much cleaner, and easier to update.


Optimize Data Sending/Querying to Parse Swift iOS I have a swift method that I'm using to update Parse in the backend when the button is tapped. Votes are being updated: The method acts as a voting system, incrementing a label every time it's tapped. This action is happening in a Custom Cell:

    @IBOutlet weak var votesLabel: UILabel!

    var parseObject:PFObject?
   @IBAction func happyBtn(sender: AnyObject) {

    if(parseObject != nil) {

        if var votes: Int? = parseObject!.objectForKey("votes") as? Int {

            votes!++
            parseObject!.setObject(votes!, forKey: "votes")
            parseObject!.saveInBackground()

          votesLabel?.text = "+\(votes!)"
            // print(votes)
        }

    }
}

Was wondering how I can optimize this function to send the data to and from Parse in the most efficient manner?

All help is appreciated!

I have some feedback concerning the usage of Swift optionals  in
your code.

The main point is that there is far too much "forced unwrapping". The forced unwrap operator ! can and should be avoided
in most cases. Instead of comparing with nil and forced unwrapping

if (parseObject != nil) {
     parseObject!.doSomething()
     parseObject!.doSomethingElse()
}

use optional binding to test and get the unwrapped value:

if let theParseObject = parseObject {
     theParseObject.doSomething()
     theParseObject.doSomethingElse()
}

Or, if parseObject is expected to be non-nil when the method
is called (e.g. because it is set up in viewDidLoad), declare it
as an implicitly unwrapped optional

var parseObject : PFObject!
// ...
parseObject.doSomething()
parseObject.doSomethingElse()

There is no reason to declare var notes as an optional Int?.
With

if var votes = parseObject.objectForKey("votes") as? Int { ... }

you get rid of more forced unwrap operators.

Finally, the votesLabel textfield outlet is expected to be non-nil
(otherwise you did not connect it correctly in the interface builder).
That is the reason why it is declared as an implicitly unwrapped optional
UILabel!. There is no need for optional chaining in

 votesLabel?.text = "+\(votes!)"

Putting it all together, the method should look like this:

@IBAction func happyBtn(sender: AnyObject) {

    if let theParseObject = self.parseObject {
        if var votes = theParseObject.objectForKey("votes") as? Int {
            votes++
            theParseObject.setObject(votes, forKey: "votes")
            theParseObject.saveInBackground()
            votesLabel.text = "+\(votes)"
        }
    }
}

No ! anymore!


Find all subsets of an int array whose sums equal a given target I am trying to implement a function below:

  Given a target sum, populate all subsets, whose sum is equal to the target sum, from an int array.

For example:

Target sum is 15.

An int array is { 1, 3, 4, 5, 6, 15 }.

Then all satisfied subsets whose sum is 15 are as follows:

15 = 1+3+5+6
15 = 4+5+6
15 = 15

I am using java.util.Stack class to implement this function, along with recursion.

GetAllSubsetByStack class

import java.util.Stack;

public class GetAllSubsetByStack {

    /** Set a value for target sum */
    public static final int TARGET_SUM = 15;

    private Stack<Integer> stack = new Stack<Integer>();

    /** Store the sum of current elements stored in stack */
    private int sumInStack = 0;

    public void populateSubset(int[] data, int fromIndex, int endIndex) {

        /*
        * Check if sum of elements stored in Stack is equal to the expected
        * target sum.
        *
        * If so, call print method to print the candidate satisfied result.
        */
        if (sumInStack == TARGET_SUM) {
            print(stack);
        }

        for (int currentIndex = fromIndex; currentIndex < endIndex; currentIndex++) {

            if (sumInStack + data[currentIndex] <= TARGET_SUM) {
                stack.push(data[currentIndex]);
                sumInStack += data[currentIndex];

                /*
                * Make the currentIndex +1, and then use recursion to proceed
                * further.
                */
                populateSubset(data, currentIndex + 1, endIndex);
                sumInStack -= (Integer) stack.pop();
            }
        }
    }

    /**
    * Print satisfied result. i.e. 15 = 4+6+5
    */

    private void print(Stack<Integer> stack) {
        StringBuilder sb = new StringBuilder();
        sb.append(TARGET_SUM).append(" = ");
        for (Integer i : stack) {
            sb.append(i).append("+");
        }
        System.out.println(sb.deleteCharAt(sb.length() - 1).toString());
    }
}

Main class

public class Main {

    private static final int[] DATA = { 1, 3, 4, 5, 6, 2, 7, 8, 9, 10, 11, 13,
        14, 15 };

    public static void main(String[] args) {
        GetAllSubsetByStack get = new GetAllSubsetByStack();
        get.populateSubset(DATA, 0, DATA.length);
    }
}

Output in Console is as follows:

15 = 1+3+4+5+2
15 = 1+3+4+7
15 = 1+3+5+6
15 = 1+3+2+9
15 = 1+3+11
15 = 1+4+2+8
15 = 1+4+10
15 = 1+5+2+7
15 = 1+5+9
15 = 1+6+8
15 = 1+14
15 = 3+4+6+2
15 = 3+4+8
15 = 3+5+7
15 = 3+2+10
15 = 4+5+6
15 = 4+2+9
15 = 4+11
15 = 5+2+8
15 = 5+10
15 = 6+2+7
15 = 6+9
15 = 2+13
15 = 7+8
15 = 15

Please help me with the following 2 things:

How can I improve this code to reduce the times for recursion? Is sorting the int array (from high to low) before recursion a better way?
Is there a way to improve the code without using recursion?

There are three reasonable responses here:

yes, your recursion code can be improved for performance.
yes, part of that improvement can come from sorting the data.
yes, there's a way to refactor the code to not use recursion, and it may even be faster.

Bearing that in mind, this answer becomes 'complicated'.

Basic performance improvements for current code:

if (sumInStack == TARGET_SUM) {
    print(stack);
}

can easily be:

if (sumInStack >= TARGET_SUM) {
    if (sumInStack == TARGET_SUM) {
        print(stack);
    }
    // there is no need to continue when we have an answer
    // because nothing we add from here on in will make it
    // add to anything less than what we have...
    return;
}

I dislike any recursive function which rely on external (outside-the-method) values. In your case, the sumInStack is external. This makes the target hard to 'see'.

Additionally, if we do sort the data, there are some benefits we can have, and a way to restructure the recursion to make it do less work (since we can guarantee that all values after a point have certain properties...):

consider the method (assuming sorted data):

public void populateSubset(final int[] data, int fromIndex,
                           final int[] stack, final int stacklen,
                           final int target) {
    if (target == 0) {
        // exact match of our target. Success!
        printResult(Arrays.copyOf(stack, stacklen));
        return;
    }

    while (fromIndex < data.length && data[fromIndex] > target) {
        // take advantage of sorted data.
        // we can skip all values that are too large.
        fromIndex++;
    }

    while (fromIndex < data.length && data[fromIndex] <= target) {
        // stop looping when we run out of data, or when we overflow our target.
        stack[stacklen] = data[fromIndex];
        populateSubset(data, fromIndex + 1, stack, stacklen + 1, target - data[fromIndex]);
        fromIndex++;
    }
}

You would call this function with:

Arrays.sort(data);
populateSubSet(data, 0, new int[data.length], 0, 15);

So, that is 'can the code be improved?' and 'will sorting help'

As for the 'unrolled' (no recursion) version of the system, it can be done. It would require three int[] arrays:

int[] data = {....}
int[] sum = new int[data.length];
int[] indices = new int[data.length];
int depth = 0;
int lastindex = -1;

The sum gives and indices act like a stack, and the depth is how deep the stack is (again, assume sorted data):

Arrays.sort(data);
while (depth >= 0) {
    lastindex++;
    if (lastindex == data.length) {
        // we have run out of data.
        do {
            // walk up the stack till we find some data.
            depth--;
        while (depth >= 0 && (lastindex = indices[depth] + 1) < data.length);
    }
    if (depth >= 0) {
         .....
         you then add your code in here to check the target,
         keep it updated in your 'stack'.
         go down a level and move on....
    }

}


Solving a linear system with two variables Background:

Equation 1: 2x-y=6

Equation 2: 4x+3y=22

Because 1 is ax-by=c and 2 is dx-ey=f

y=(a * f - c * d) / (a * e - b * d) and x is simply (c - (b * y)) / a)

What I am trying to do:

Ultimately, generate linear equations that can be solved simultaneously for practice (which the code I wrote below can do). So I am creating variations of Equations 1 and 2 above that have whole number solutions for x and y. I do this by:

1.Creating random numbers.

2.Use the above calculation to find x and y from these numbers.

3.Ensure that x and y are whole numbers and if not, create new numbers for a-f until they are.

Working code:

The function NumberGeneration() is called with a button click.

double a;
double b;
double c;
double d;
double e;
double f;
double x;
double y;
System.Random rnd = new System.Random();
string question;
string answer;

public void NumberGeneration()//1.Generates the variables, calculates x and y.
{
    a = rnd.Next(1, 10);
    b = rnd.Next(1, 10);
    c = rnd.Next(1, 10);
    d = rnd.Next(1, 10);
    e = rnd.Next(1, 10);
    f = rnd.Next(1, 10);
    y = ((a * f - c * d) / (a * e - b * d));
    x = ((c - (b * y)) / a);
    CheckForZeros();
}

void CheckForZeros() //2.Checking for zeros
{
    if ((a * e - b * d) == 0) //To avoid dividing by 0, if this is 0 generate new numbers.
    {
        NumberGeneration();
    }
    else if((a * e - b * d) != 0) // otherwise proceed to make sure x and y are whole numbers
    {
        EnsureWhole();
    }

}

void EnsureWhole()
{
    decimal dy = Convert.ToDecimal(y);
    decimal dx = Convert.ToDecimal(x);
    if ((dy % 1) == 0 && (dx % 1) == 0) //3.If both x and y are whole numbers, setup the question with these numbers
    {
        question = string.Format($"{a}x+{b}y={c}, \n {d}x+{e}y={f}", a, b, c, d, e, f);
        answer = string.Format($"x={x} and y={y}");
    }
    else if((dy % 1) != 0 || (dx % 1) != 0)//Otherwise start again, generate a new set of numbers and attempt for a new answer where x and y are ints
    {
        NumberGeneration();
    };
}

Questions:

What is the proper way to do this?
How to optimize the code?
Is this the optimal way to generate random numbers?

Bonus follow-up:
Do you have any personal resource suggestions for learning C#? I get that instinctive feeling that my methods have holes in them, I know I am not good at coding, I just don't have the education to know better and I want to educate myself. Even better within the context of maths.

The good thing is, that you have made some meaningful methods with descriptive names.

But you have a quite peculiar workflow:

public void NumberGeneration()//1.Generates the variables, calculates x and y.
{
    a = rnd.Next(1, 10);
    b = rnd.Next(1, 10);
    c = rnd.Next(1, 10);
    d = rnd.Next(1, 10);
    e = rnd.Next(1, 10);
    f = rnd.Next(1, 10);
    y = ((a * f - c * d) / (a * e - b * d));
    x = ((c - (b * y)) / a);
    CheckForZeros();
}

Here you check for dividing by zero after you do the calculation that is supposed to be guarded by the check.

And calling NumberGeneration() recursively from CheckForZeros() and EnsureWhole()   if something goes wrong has the potential to end in a stack overflow, if all the generated sets of values fail in the two tests. This is not a good use of recursion, instead you should use an iterative approach and provide a value for max retries before the generator returns unsuccessfully.

Have you considered this condition properly:

  else if ((dy % 1) != 0 || (dx % 1) != 0)//Otherwise start again, generate a new set of numbers and attempt for a new answer where x and y are ints
  {
    NumberGeneration();
  };

Why must at least one of the result values in the current failed calculation be a decimal number in order to allow a new calculation with an entire new set of data? It makes no sense to me.

If everything go well you end the calculations by formatting a pair of private strings and then nothing happens:

    question = string.Format($"{a}x+{b}y={c}, \n {d}x+{e}y={f}", a, b, c, d, e, f);
    answer = string.Format($"x={x} and y={y}");

I imagine that you provide these results to the client in some way or else all the efforts seem useless :-)

In

  (dy % 1) != 0

  (a * e - b * d) == 0

The parenteses are unnecessary.

And so are the outmost parenteses in this expression:

  y = ((a * f - c * d) / (a * e - b * d))

I think you normally will solve the system with + between the parts:

  ax + by = c
  dx + ey = f

  <=>

  y = (cd - af) / (bd - ae)
  x = (c - by) / a

The randomly generated input values (a..f) are all integers, so you could do all calculations as integer calculations, because you only want solutions with integral x and y. That will make the calculations a lot easier and more reliable. Checking if two double values are equal is often not reliable.

There are a lot of ways to structure this kind of exercise. Below I have posted one version. I'm not claiming it to be the way to do it, but you may find some inspiration.

The approach is rather traditional, and I have focused on an easy-to-follow workflow, naming and testability - at least to a some level. It's only using integer values and calculations.

The solver itself controlling the workflow and doing the calculations:

///
/// Creates a random linear system of two equations with two variables of first degree and find a
/// possible solution with integral values for the variables x and y. This is the same as
/// finding the intersection between two lines in the plane.
///
public class LinearSystemSolver
{
  private readonly int maxRetries;
  private readonly ICoefficientProvider coefficientProvider;

  ///
  /// Constructor
  ///
  /// <param name="maxRetries">The number of times to try to find a valid solution before returning false from TrySovle().</param>
  /// <param name="coefficientProvider">An object that provides the coefficients in a linear system to solve.</param>
  public LinearSystemSolver(int maxRetries, ICoefficientProvider coefficientProvider)
  {
    this.maxRetries = maxRetries;
    this.coefficientProvider = coefficientProvider
                               ?? throw new ArgumentNullException(nameof(coefficientProvider));
  }

  public bool TrySolve(out IntegralLinearSystem result)
  {
    result = IntegralLinearSystem.Empty;

    for (int i = 0; i < maxRetries; i++)
    {
      Coefficients coefficients = GenerateCoefficients();
      if (HasSolution(coefficients) && GetIntegralSolution(coefficients, out int x, out int y))
      {
        result = new IntegralLinearSystem(coefficients, x, y);
        return true;
      }
    }

    return false;
  }

  private Coefficients GenerateCoefficients()
  {
    return coefficientProvider.GetCoefficients();
  }

  private bool HasSolution(Coefficients coefficients)
  {
    // This is a test of bd - ae != 0
    return coefficients.B * coefficients.D != coefficients.A * coefficients.E;
  }

  private bool GetIntegralSolution(Coefficients coefficients, out int x, out int y)
  {
    int numY = coefficients.C * coefficients.D - coefficients.A * coefficients.F;
    int denomY = coefficients.B * coefficients.D - coefficients.A * coefficients.E;
    y = numY / denomY;

    int numX;
    int denomX;
    if (coefficients.A != 0)
    {
      numX = coefficients.C - coefficients.B * y;
      denomX = coefficients.A;
    }
    else
    {
      // A and D can not both be 0 because the system then doesn't have a solution (the two lines are parallel with the x-axis)
      // A = D = 0 would have been caught by HasSolution() before ending here
      numX = coefficients.F - coefficients.E * y;
      denomX = coefficients.D;
    }
    x = numX / denomX;

    return numY % denomY == 0 && numX % denomX == 0;
  }
}

An object representing the integral solution - if found:

///
/// Object representing a linear system of first degree with two variables with integral solutions for x and y.
///
public struct IntegralLinearSystem
{
  public static readonly IntegralLinearSystem Empty = new IntegralLinearSystem(Coefficients.Empty, 0, 0);

  public IntegralLinearSystem(Coefficients coefficients, int x, int y)
  {
    Coefficients = coefficients;
    X = x;
    Y = y;

    if (!IsValid)
      throw new InvalidOperationException("Inconsistent integral linear system");
  }

  public readonly Coefficients Coefficients;
  public readonly int X;
  public readonly int Y;

  public bool IsValid
  {
    get
    {
      return
        Coefficients.A * X + Coefficients.B * Y == Coefficients.C &&
        Coefficients.D * X + Coefficients.E * Y == Coefficients.F;
    }
  }

  public override string ToString()
  {
    StringBuilder builder = new StringBuilder();
    builder.Append(Coefficients);
    builder.AppendLine();
    builder.AppendLine("<=>");
    builder.AppendFormat("x = {0}, y = {1}", X, Y);

    return builder.ToString();
  }
}

An object representing the constant values (coefficients) for the system

///
/// Object holding the coefficients to x and y and values on the right side
/// of the equal sign in a linear system with two variables of first degree
/// on the form ax + by = c and dx + ey = f
///
public struct Coefficients
{
  public static readonly Coefficients Empty = new Coefficients();

  public readonly int A;
  public readonly int B;
  public readonly int C;
  public readonly int D;
  public readonly int E;
  public readonly int F;

  public Coefficients(int a, int b, int c, int d, int e, int f)
  {
    A = a;
    B = b;
    C = c;
    D = d;
    E = e;
    F = f;
  }

  public override string ToString()
  {
    return $"{A}x + {B}y = {C}{Environment.NewLine}{D}x + {E}y = {F}";
  }
}

The contract between the solver and the generator of the constant values:

public interface ICoefficientProvider
{
  Coefficients GetCoefficients();
}

An object providing randomly generated coefficients to the system:

///
/// Creates randomly generated coefficients to a linear system of two variables.
///
public class RandomCoefficientProvider : ICoefficientProvider
{
  private readonly int min;
  private readonly int max;
  private readonly Random random;

  ///
  /// Constructor
  ///
  /// <param name="min">The inclusive lower boundary of the random numbers returned.</param>
  /// <param name="max">The exclusive upper boundary of the random numbers returned.</param>
  /// <param name="randomSeed"></param>
  public RandomCoefficientProvider(int min, int max, int? randomSeed = null)
  {
    this.min = Math.Min(min, max);
    this.max = Math.Max(min, max);
    random = randomSeed == null ? new Random() : new Random(randomSeed.Value);
  }

  public Coefficients GetCoefficients()
  {
    return new Coefficients
    (
      random.Next(min, max),
      random.Next(min, max),
      random.Next(min, max),
      random.Next(min, max),
      random.Next(min, max),
      random.Next(min, max)
    );
  }
}

Use Case:

  LinearSystemSolver solver = new LinearSystemSolver(10, new RandomCoefficientProvider(1, 10));
  if (solver.TrySolve(out IntegralLinearSystem result))
  {
    Console.WriteLine($"Result:{Environment.NewLine}{result}");
  }
  else
  {
    Console.WriteLine("No integral solution found");
  }

As seen, I have created the interface ICoefficientProvider in order to separate the (random) creation of the constant values of the system from the workflow and calculations.
It gives me the possibility to implement another coefficient provider like the below - for testing purposes - without changing anything in the solver:

///
/// A Coefficient Provider that can be used for testing purposes
///
public class ConstantCoefficientProvider : ICoefficientProvider
{
  private readonly Coefficients coefficients;

  public ConstantCoefficientProvider(int a, int b, int c, int d, int e, int f)
  {
    coefficients = new Coefficients(a, b, c, d, e, f);
  }

  public Coefficients GetCoefficients()
  {
    return coefficients;
  }
}

You could easily extend this pattern of dependency injection to handle other parts of the program, for instance the calculations.


Averaging outputs of each execution of a file The only way I could think of doing this is creating a text file and modifying it each time the program is executed and outputting the average but this has many flaws so I was wondering if it was possible to do this any other way.

int num1, num2, counter = 0;
num2 = rand();
do {
cin >> num1;
counter++;
} while(num1 != num2)

const char *path = "C:/Users/Edsheeran/AppData/Roaming/times.txt";
double x = 0;
double sum = 0;
vector<double> times;

ofstream write(path, ios::app);
if(write.is_open()) {
write << counter << endl;
};
write.close();

ifstream file(path, ios::app);
if(file.is_open())
{
    while (!file.eof()) {
        file >> x;
        times.push_back(x);
    }
}
else exit(EXIT_FAILURE);
file.close();

for (unsigned int i = 0 ; i < times.size(); i++) {
sum += times[i];
}

cout << "Your average tries to find the number is: " << sum / times.size()   << endl;

I think the short answer to your question is "what you're doing is fine, but there are other approaches depending on what you want to do." However, first:

When posting on CodeReview, try to post the most complete, compilable code that you can. This is often less work for you (because you can just paste straight from your source file with no editing), and less work for the reviewers (because they can just paste it straight into a source file and compile it). What you've posted here seems to be just the body of main.
Please indent your code consistently. This is good(ish):

if(file.is_open())
{
    while (!file.eof()) {
        file >> x;
        times.push_back(x);
    }
}

but this is bad:

if(write.is_open()) {
write << counter << endl;
};

You can use a tool like clang-format, astyle, or indent to format your code automatically until you acquire the muscle memory to do it yourself.

However, both of those snippets I just pasted have formatting "bugs" besides the indentation:  In the second one, you have a trailing ; that shouldn't be there.  In the first one, you mix brace styles:

if (x)
{

and

while(x) {

(Notice the mix of pre-parenthesis-spacing styles as well.)  Pick one style for a given program and stick with it.

Lastly, try to structure your program by breaking it down into subroutines. For this program, I'd probably have subroutines

// Generate a random number and return the number of tries for the user to guess it.
int guess_the_number();

// Append a number to the named file.
void append_to_file(const char *filename, int value);

// Retrieve the contents of the named file.
std::vector<int> load_ints_from_file(const char *filename);

And then I'd strongly consider refactoring the program along the lines suggested by symmetry: namely, would it make more sense to provide a function

// Retrieve the contents of the named file.
void save_ints_to_file(const char *filename, const std::vector<int>& values);

instead of the append_to_file function?  There are valid reasons you might not want to in production code (for example, the append-only version has better efficiency and atomicity, if you do it right — which is not to say that you have), but if your goal is just to make a readable and sensible toy guess-the-number program, personally I'd go with the load/save dichotomy in place of the load/append dichotomy.

As for your question about "how to keep a running average": if all you care about is the average number of guesses (i.e. total number of guesses divided by total number of games played), then you don't need to keep a record of how the total number of guesses happened to divide up into individual games-played. Just store those two numbers in your file:

std::vector<int> values = load_ints_from_file("times.txt");
if (values.empty()) values = std::vector<int>{0, 0};
values[0] += guess_the_number();
values[1] += 1;
std::cout << "Average is " << values[0] / values[1] << std::endl;
save_ints_in_file("times.txt", values);


Sorting Algorithms Optimisation Python def MinimumSwaps(Queue):
        MinSwaps = 0
        for i in range(len(Queue) - 1):
            if Queue[i] != i+1:
                for j in range(i+1,len(Queue)):
                    if Queue[j] == i+1:
                        Queue[i], Queue[j] = Queue[j], Queue[i]
                        MinSwaps += 1
                        break
            else:
                continue
        return MinSwaps

def main():
    Result = MinimumSwaps([7, 1, 3, 2, 4, 5, 6])
    print(Result)
if __name__ == "__main__":
    main()

The question: You are given an unordered array consisting of consecutive integers  [1, 2, 3, ..., n] without any duplicates. You are allowed to swap any two elements. You need to find the minimum number of swaps required to sort the array in ascending order.
The issue is that what I have provided is inefficient and fails on very large arrays, however Ive tried to optimise it as much as I can and im not aware of another technique to use. This question is likely related to a particular sorting algorithm but is there any way to modify the above code to make it faster?

Instead of searching for the value that belongs at Queue[i] to swap it there, just swap the value that is there to where that belongs:
def MinimumSwaps(Queue):
    MinSwaps = 0
    for i in range(len(Queue) - 1):
        while Queue[i] != i + 1:
            j = Queue[i] - 1
            Queue[i], Queue[j] = Queue[j], Queue[i]
            MinSwaps += 1
    return MinSwaps

This improves the performance to \$O(1)\$ for using a value instead of the \$O(n)\$ for searching. And thus total performance to \$O(n)\$ instead of your original \$O(n^2)\$. Still using only O(1) extra space.


Streaming hacker news posts Hacker News recently released an official API. Unfortunately, it cannot be used to get the newest posts submitted. I wrote a script to provide a faux streaming-api like method (like praw reddit's submission_stream) so that I can simply put it in a loop and do something like make a gtk-notification when title contains the string 'python'.

"""
Usage:

from hn import HNStream

h = HNStream()

for item in h.stream():
    print "Title: ", item.title
    print "By user: ", item.by
"""

from lxml import html as lh
import re
import requests
import threading
import time
import ujson as json

__all__ = ['HNStream']

class Item:

    def __init__(self, **kwargs):
        self.__dict__.update(kwargs)

    def __str__(self):
        return "{}: {}".format(self.type, self.title)

    __repr__ = __str__

class HackerNews:

    def __init__(self):
        self.base_url = 'https://hacker-news.firebaseio.com/v0/{api}'

    def get(self, uri):
        response = requests.get(uri)
        if response.status_code == requests.codes.ok:
            return json.loads(response.text)
        else:
            raise Exception('HTTP Error {}'.format(response.status_code))

    def item(self, item_id):
        uri = self.base_url.format(**{'api': 'item/{}.json'.format(item_id)})
        return Item(**self.get(uri))

class SimpleFIFO:

    def __init__(self, length):
        self.length = length
        self.values = length * [None]

    def contains(self, iid):
        return iid in self.values

    def append(self, value):
        assert isinstance(value, str), "Only append strings"
        self.values.append(value)
        while len(self.values) > self.length:
            self.values.pop(0)
        assert len(self.values) == self.length

    def __str__(self):
        return str(self.values)

class HNStream(threading.Thread):

    def __init__(self):
        threading.Thread.__init__(self)

    def stream(self):

        itembuffer = SimpleFIFO(length=30)
        while True:
            r = requests.get('https://news.ycombinator.com/newest')
            tree = lh.fromstring(r.text)
            # http://stackoverflow.com/a/2756994
            links = tree.xpath("//a[re:match(@href, 'item\?id=\d+')]/@href",
                               namespaces={"re": "http://exslt.org/regular-expressions"})
            links = set(links)
            for link in links:
                iid = re.match(r'item\?id=(\d+)', str(link)).groups()[0]
                if not itembuffer.contains(iid):
                    itembuffer.append(iid)
                    yield HackerNews().item(iid)
            time.sleep(30)

I have a number of suggestions:

Don't use self.dict

def __init__(self, **kwargs):
    self.__dict__.update(kwargs)

It is pretty unclear which attributes the class will have after calling the constructor, as it is basically up to the caller to decide this. This is not optimal, as you want to be in control of what data is set in an item. I'd suggest to use named arguments instead:

def __init__(self, id, type, title):
    self.type = type
    self.title = title
    self.id = id
    # Add other attributes that I don't know of?

While this may seem like blowing up the code, it makes it a lot easier to understand for an uninformed reader, and therefor should be preferred IMHO.

Append instead of format

Since you are basically just appending something to self.base_url, you might as well append using + instead of using format. I find it a lot easier to read, but that depends a lot on what you prefer. Consequently:

self.base_url = 'https://hacker-news.firebaseio.com/v0/'

Then

uri = self.base_url.format(**{'api': 'item/{}.json'.format(item_id)})

would look like this if using append:

uri = self.base_url + "item/{}.json".format(item_id)

HackerNews.get()

The get() method's name does not explain what the method does. I'd suggest something offering a bit more documentation, for example: download_item() or download_entry().

On a little side note: You can remove the else: statement, as the raise Exception(..) command will only be executed if the if statement evaluates to False. This depends a bit on personal taste though. I'd also change the ordering like this:

if response.status_code != requests.codes.ok:
    raise Exception('HTTP Error {}'.format(response.status_code))
return json.loads(response.text)

I'd prefer this ordering as it has the return statement at the end, which is where I'd look first when searching for it.

SimpleFIFO

Maybe you can use Python's Queue class? See here for documentation: https://docs.python.org/2/library/queue.html

It also provides a max size, the only thing missing is the check for adding only strings.

HNStream.init()

You can use the super keyword (see also: https://stackoverflow.com/questions/576169/understanding-python-super-and-init-methods) instead of explicitly calling the constructor like this:

def __init__(self):
    super(HNSTream, self).__init__()

Or in Python 3:

def __init__(self):
    super().__init__()

HNStream.stream()

Just a few naming suggestions:

r -> result
iid -> item_id

That was it, hope it helps!