Decoding assembly instructions in a Game Boy disassembler I am coding a game boy disassembler in C++. The program (simplified here) converts a vector of byte into an assembly instruction and print it.

To do so, it iterates through the vector of bytes (char) with an iterator. I noticed that with the it!=char_vect.end() condition, there is the risk that the iterator goes out from the vector. So I have to check that I do not get out of the vector at each iteration and if it occurs, I throw an error.

#include <iostream>
#include <vector>

int disassemble(std::vector<char>::iterator& it){

    int opcode = *it;
    int opbytes = 1; //number of bytes used by the operator

    switch(opcode){
        case 0x76 : std::cout<<"HALT "<<std::endl; opbytes = 1; break;
        case 0x10 : std::cout<<"STOP $"<<(int)*(it+1)<<std::endl; opbytes =2; break;
        //... About 250 different cases
        default : std::cout<<"Instruction not implemented yet"<<std::endl;
    }

    return opbytes;
}

int main(){

    std::vector<char> char_vect = {0x76, 0x10, 0x20}; // Fill vector of char
    std::vector<char>::iterator it = char_vect.begin();

    // First I only did that
    //while (it!=char_vect.end()){
    //  it+=disassemble(it);
    //}

    // But I have to check that I do not go out of the char_vect

    while (it!=char_vect.end()){
        try{
            it+=disassemble(it);
            //check that it do not overpass end
            if (size_t(it - char_vect.begin())>char_vect.size()){
                throw "Try to disassemble instruction outside of memory bounds";
            }
        }
        catch(const char * c){
            std::cerr << "Fatal error: " << c << std::endl;
            return 1;
        }
    }
    return 1;
}

This code works fine. Now I wonder, I have a possible exception case which is the iterator going further than vect_char.end(). Is a try / catch an appropriate way to handle this case knowing that:

The reason for the iterator to go out of bounds could be either:

A non valid input byte sequence, for example {0x76, 0x20, 0x10} since the instruction 0x10 expects an argument. This should not append.
A mistake in the disassemble function. For exemple with the valid input {0x76, 0x10, 0x20}, if I code by mistake that 0x10 uses 3 opbytes instead of 2, the iterator will go out of bounds. This should not append either.

If I iterated through the vect_char with an index, I would not have this issue and the code would be more compact

I never really used try/catch before and I do not know if it is intented for such situations, so I would like to know, how would you have handled this unexpected case?

I have some ideas about how you might be able to improve your program.

Avoid problems

Rather than trying to deal with the problem for every instruction, one approach is avoiding it entirely.  One way to do that is to simply append a number of bytes to the end of the vector.  If the maximum bytes for an instruction is \$n\$, then append \$n-1\$ bytes to the end and stop when you've advanced into the padded area.

Check before advancing

One could also pass the number of remaining bytes to the disassemble function.  However, the mechanism I'd suggest would be to pass a range, e.g.:

int diassembleOne(std::vector<char>::iterator& it, std::vector<char>::iterator& end) {
    //  figure out number of bytes for this opcode
    if (std::distance(it, end) > opbytes) {
        it = end;
        // throw or return 0
    }
    // disassemble the instruction thoroughly
    std::advance(it, opbytes);
    return opbytes;
}

Use const iterators

If all the code is doing is disassembling, then it shouldn't alter the underlying vector.  For that reason, I'd recommend passing a std::vector<char>::const_iterator &.

Use classes

I'd suggest using an Opcode class like this:

class Opcode {
    char code;
    short int bytes;
    std::string_view name;
    bool operator==(char opcode) const { return code == opcode; }
    int decode(std::vector<char>::const_iterator& it, std::ostream& out=std::cout) const {
        out << name;
        ++it;
        for (int i{bytes-1}; i; --i) {
            out << static_cast<unsigned>(*it++);
        }
        out << '\n';
        return bytes;
    }
};

constexpr std::array<Opcode,2> instructions {
    { 0x10, 2, "STOP $" },
    { 0x76, 2, "HALT " },
};

Pass a pair of iterators to the dissemble function

As mentioned before, you can pass a pair of iterators to the disassemble function.  Using that plus the class above:

int disassembleOne(std::vector<char>::const_iterator& it, std::vector<char>::const_iterator& end){
    auto op{std::find(instructions.begin(), instructions.end(), *it)};
    if (op == instructions.end()) {
        std::cout << "Instruction not found\n";
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    if (std::distance(it, end) < op->bytes) {
        std::cout << "Not enough bytes left to decode " << op->name << '\n';
        it = end;
        return 0; // instruction not found or off the end of the passed array
    }
    return op->decode(it);
}

Now main becomes very simple:

int main(){
    const std::vector<char> char_vect = {0x76, 0x10, 0x20, 0x10}; // Fill vector of char
    auto end{char_vect.cend()};

    for (auto it{char_vect.cbegin()}; it != end; disassembleOne(it, end)) {
    }
}

Another way to do this would be to put more of the processing in the Opcode itself -- it would make sense that each opcode would know how to decode itself.

Be clear about caller expectations

This code, as with the original code, expects that the it being passed is a valid iterator that is not the end.  It is good to document that in comments in the code.


Reduce as many adjacent chars as possible in string This code is meant to reduce a given string as much as possible by deleting adjacent characters. Here are some examples:

reduce('aa') = 'Empty String'
reduce('ab') = 'ab'
reduce('abba') = 'Empty String' # remove 'bb' then 'aa'
reduce('bbb') = 'b'  # can only delete 2 chars at a time

This is based off this HackerRank question.

I'm doing this to improve my style and to improve my knowledge of fundamental algorithms/data structures for an upcoming coding interview.

def reduce(s):
    cur_chars = list(s)
    while cur_chars:
        new_chars = do_deletions(cur_chars)
        if new_chars == cur_chars:  # if nothing deleted then return
            new_s = ''.join(cur_chars)
            return new_s
        else:
            cur_chars = new_chars  # do another round of deletions
    return 'Empty String'

def do_deletions(org_chars):
    if len(org_chars) == 1:
        return org_chars

    new_chars = []
    i = 0
    while i < len(org_chars):
        if i == len(org_chars) - 1:
            new_chars.append(org_chars[i])
            i += 1
        elif org_chars[i] == org_chars[i+1]:
            i += 2  # don't include char i and i+1
        elif org_chars[i] != org_chars[i+1]:
            new_chars.append(org_chars[i])
            i += 1
    return new_chars

I was originally going to do deletions using 2 consecutive del commands (O(n)) but then I thought this performance would be too bad for an interview. Instead I opted to not do any deletions and just use appends to create gradually reduced strings. If my original approach would have been fine for an interview then let me know.

I think this algorithm is O(n^2) time and O(n) space. Any suggestions about how I can improve the complexities are welcome.

This is related to the task of checking for matching parentheses. You can solve it in a single pass if you compare the current org_char to the latest new_char, and if they are the same, remove the latest new_char and skip the current org_char.

The do_deletions function already defines the org_chars and the new_chars. Instead of comparing only the org_chars, you should check whether len(new_chars) > 0 and org_chars[i] == new_chars[-1].


Template function which determines how to move-assign a type In some code that I am writing, I am manipulating some heterogeneous containers of user-defined types.

I desperately need to be able to move-assign these types without generating exceptions.

However, this can cripple the usability of my code because on many older compilers, important standard library types do not have noexcept markings on their move-assignment operators. For instance std::string appears not to be noexcept move-assignable with gcc 4.9, it is only noexcept move constructible in C++11.

To work around this, I have developed a template function which tries various strategies for move-assigning things until it finds one which is noexcept, if it cannot then it gives up.

namespace traits {

template <bool b, typename V = void>
using enable_if_t = typename std::enable_if<b, V>::type;

} // end namespace traits

namespace detail {

template <typename T>
auto move_assign_noexcept(T & dest, T && src) noexcept -> traits::enable_if_t<std::is_nothrow_move_assignable<T>::value> {
  dest = std::move(src);
}

template <typename T, typename ENABLE = void>
struct is_nothrow_swappable : std::false_type {};

template <typename T>
struct is_nothrow_swappable<T, traits::enable_if_t<noexcept(std::swap(*static_cast<T*>(nullptr), *static_cast<T*>(nullptr)))>> : std::true_type {};

template <typename T>
auto move_assign_noexcept(T & dest, T && src) noexcept -> traits::enable_if_t<!std::is_nothrow_move_assignable<T>::value
                                                                   && is_nothrow_swappable<T>::value> {
  std::swap(dest, src);
}

template <typename T>
auto move_assign_noexcept(T & dest, T && src) noexcept -> traits::enable_if_t<!std::is_nothrow_move_assignable<T>::value
                                                                   && !is_nothrow_swappable<T>::value
                                                                   && std::is_nothrow_move_constructible<T>::value> {
  dest.~T();
  new (&dest) T(std::move(src));
}

} // end namespace detail

This code is supposed to work at C++11 standard.

I recall that there is a standard function std::move_if_noexcept which is useful for standard containers which are trying to avoid exception problems.

Is there an existing standard idiom for doing what I am trying to do?

Are there other strategies that I should try? Am I actually using standard traits correctly to ensure that each implementation actually won't throw?

I think you're putting the cart before the horse here.

  I am manipulating some heterogeneous containers of user-defined types.
  I desperately need to be able to move-assign these types without generating exceptions.

Let's say your container is a Cart<T>. And you need to define a move-assignment operator

Cart<T>& Cart<T>::operator=(Cart<T>&& rhs) noexcept;

Well, if Cart behaves like std::vector, then you could implement this as something like

Cart<T>& Cart<T>::operator=(Cart<T>&& rhs) noexcept
{
    delete [] this->pointer_to_Ts;
    this->pointer_to_Ts = rhs->pointer_to_Ts;
    rhs->pointer_to_Ts = nullptr;
}

Notice that this does not require T's own move-assignment operator at all; T might not even be movable! The only user-defined function we're executing here is T's destructor (and if that throws, we're in trouble anyway).

But okay, let's suppose that you're implementing Cart's move-assignment operator like this instead:

Cart<T>& Cart<T>::operator=(Cart<T>&& rhs) noexcept
{
    for (int i=0; i < n; ++i) {
        this->array_of_Ts[i] = std::move(rhs->array_of_Ts[i]);
    }
}

So now you're calling a ton of user-defined functions (well, just one function, but you're calling it n times). This is already violating the spirit of move-semantics, in that moving is supposed to be a fast operation, but we'll let that slide.

What should you do if T's move-assignment operator is defined as noexcept? Well, you're golden. Everything will work fine.

What should you do if T's move-assignment operator is not defined as noexcept, but doesn't throw in practice? Well, you're golden. Everything will work fine in practice. If it ever accidentally does throw an exception, then your program will call std::terminate and (presumably) exit. This seems like a good idea in most cases. If you really can't afford to call std::terminate when something as terrible as "move-assignment throws an exception" happens, then I bet you're coding to some strict government standards that will preclude the existence of non-noexcept move-assignment operators in the first place.

What should you do if T's move-assignment operator is explicitly defined as noexcept(false)? Well, you should probably go find the person who wrote that code and shoot them in the face. (Politely, of course.)

Now the unfortunate bit: It's actually impossible to tell in C++14 (and C++17 as well, AFAIK) whether a given function has been declared noexcept(false) or merely non-noexcept. So your code won't ever be able to distinguish case 2 above ("you're golden, in practice") from case 3 above ("you have to shoot somebody in the face") — it's useless to try.

So, should you go gray worrying about case 3? IMHO you should not. Just assume that every instance of case-2-or-3 in your codebase is really a case-2; and then write your code accordingly. It'll be simple and straightforward and efficient, and if you ever run into a throwing move-assignment operator, well, by definition it's the user who wrote the bad code, not you.

This is not the philosophy that led to the current pessimal behavior of std::vector::resize. I'll let someone else take a stab at explaining that philosophy, because I don't understand it at all.


Templated Stack of 5 This is my first time using C++ Templates to make a data structure. I have written a program to implement a stack of up to 5 items.

#include<iostream>
#include<iomanip>

using namespace std;

template<class T>
class Queue{
private:
    T arrays[5];
    int i; //empty stack
public:
    Queue()
    {
        arrays[5] = {0}; //Empty stack
        i = -1; //Setting stack to be empty
    }
    T PUSH(T item)
    {
        if(i == 4)
        {
            cout << "Stack is full" << endl;
        }
        else
        {
        i++;
        arrays[i] = item;
        }
    }
    T POP()
    {
        if(i == -1)
        {
            cout << "Stack is empty" << endl;
        }
        else
        {
        cout << arrays[i] << " has been popped" << endl;
        i--;

        }
    }
    T TOP()
    {
        if(i == -1)
        {
            cout << "Stack is empty" << endl;
        }
        else
        {
        cout << "Element at top of stack is " << arrays[i] << endl;
        }
    }

};

int main()
{
    Queue <int> Q;
    int option,push;
    do{
    cout << "Select any of the following option" << endl;
    cout << "1.Push\n2.Pop\n3.Top\nPress any other button to exit" << endl;
    cin >> option;

    switch(option)
    {
        case 1:
            cout << "Which element to push" << endl;
            cin >> push;
            Q.PUSH(push);
            break;
        case 2:
           Q.POP();
            break;
        case 3:
           Q.TOP();
            break;
        default:
            cout << "Error!" << endl;
            break;

    }
    }while(option < 4);
return 0

}

Though I have got the program working. I would like to ask if there is any way I can improve on the code.

Queue != Stack

Queue behaviour is FIFO.  Stack behaviour is LIFO.  Currently your datastructure is called Queue, but it implements Stack, has stack error messages etc.  This is very confusing.

Magic Numbers

There are three main numbers you're using for manipulating your stack 5, 4 and -1.  Out of context, it's not immediately obvious what they mean.  Replacing them with constants would make the code easier to read.

Undefined behaviour

As has been hinted at by @Quuxplusone, this:

_myarr[5]={0};

Isn't doing what you're expecting it to.  Rather than initializing all 5 of the elements in the array, which is what I suspect you expected, it is attempting to initialize the element with index 5.  Since arrays are index from 0 and it only contains 5 elements, you're overflowing the array.  This may or may not work, depending on how lucky you get.

Return Values

You've declared PUSH,POP,TOP all returning type T, however you're never returning anything.  Depending on your compiler + switches this may or may not actually compile.  PUSH probably doesn't need to return the value it's added anyway...

Console Output

Console output is great for testing your code to see if it's doing what you're expecting, however writing it into your classes gives the console access to private data members. Try to avoid putting console interaction in your data structures.  If you had written the console interactions outside of the class, you would have noticed that POP etc didn't return values a lot earlier in the cycle.

Error Conditions

If I pop from an empty stack, you write to the console, however the caller has no way to know that they haven't retrieved a value. Worse, if I push onto a full stack you simply throw the value away.  You should notify the user that there has been a failure, possibly by throwing an exception.

Exit

"Any other button" isn't a great hint that you need to press 4 or above to get out of the application.  Help your users, be explicit.

using namespace::std

Is generally frowned on, be explicit.

Indentation

Indentation matters, be consistent, it's a lot easier to read.

Putting it all together

I'm sure there's still room for improvement, however combining all of the above, the Stack would look a bit more like this:

#include<iostream>
#include<iomanip>
#include <exception>

template<class T>
class Stack {
private:
    static const int stack_size = 5;
    static const int stack_full = stack_size - 1;
    static const int stack_empty = -1;

    T arrays[stack_size] = {};
    int i = stack_empty;
public:
    void PUSH(T item)
    {
        if (i == stack_full)
        {
            throw std::runtime_error{ "Stack is full" };
        }

        i++;
        arrays[i] = item;
    }
    T POP()
    {
        if (i == stack_empty)
        {
            throw std::runtime_error{ "Stack is empty" };
        }

        T &removed = arrays[i];
        i--;
        return removed;
    }
    T TOP()
    {
        if (i == stack_empty)
        {
            throw std::runtime_error{ "Stack is empty" };
        }

        return arrays[i];
    }

};

int main()
{
    Stack <int> Q;
    int option, push;
    do {
        std::cout << "Select any of the following option" << std::endl;
        std::cout << "1.Push\n2.Pop\n3.Top\n4.Exit" << std::endl;
        std::cin >> option;

        try {
            switch (option)
            {
            case 1:
                std::cout << "Which element to push" << std::endl;
                std::cin >> push;
                Q.PUSH(push);
                break;
            case 2:
            {
                int removed = Q.POP();
                std::cout << removed << " has been popped" << std::endl;
                break;
            }
            case 3:
            {
                int top = Q.TOP();
                std::cout << "Element at top of stack is " << top << std::endl;
                break;
            }
            case 4:
                break;
            default:
                std::cout << "Error!" << std::endl;
                break;
            }
        }
        catch (std::runtime_error error) {
            std::cout << error.what() << std::endl;
        }
    } while (option < 4);
}


Apply capitalization of one string to another I want to take the capitalization of one string and apply it to another.

For example: take Apple and orange and turn that into Orange.

This is the solution I implemented. Is there a more efficient way of doing this?

public static String applyCapitalization(String to, String from) {
    int[] capArray = toCapitalizationArray(to);
    char[] charCap = from.toCharArray();
    for (int i = 0; i < capArray.length; i++) {
        if (capArray[i] == 1) {
            charCap[i] = Character.toUpperCase(charCap[i]);
        } else {
            charCap[i] = Character.toLowerCase(charCap[i]);
        }
    }
    return new String(charCap);
}
private static int[] toCapitalizationArray(String to) {
    int[] arr = new int[to.length()];
    for (int i = 0; i < to.length(); i++) {
        char c = to.toCharArray()[i];
        if (Character.isUpperCase(c)) {
            arr[i] = 1;
        } else {
            arr[i] = 0;
        }
    }
    return arr;
}

We can do this in single traversal rather then two

for (int i = 0; i < min(apple.length(),orange.length()); i++) {
    if (Character.isUpperCase(apple.toCharArray()[i]) &&
        Character.isLowercase(oranger.toCharArray()[i])){
           orange[i] = Character.toUppercase(oranger.toCharArray()[i]);
    }elseif(Character.isLowerCase(apple.toCharArray()[i]) &&
            Character.isUppercase(oranger.toCharArray()[i])){
           orange[i] = Character.toLowercase(oranger.toCharArray()[i]);
    }
}


Stateless Maximum Beauty of String implementation Inspired by The Beauty and the Strings question, I tried to implement the "Maximum Beauty of String" in a stateless fashion using ES6. Unfortunately, I ran into two places where I had to assign variables.

The questions I have:

How can I remove the state in the reducer for mapLettersByCount?
How can one use generators without assigning it to a variable? (see points variable in the maxBeauty function) Is this even considered state in this situation?

Code:

const EMPTY_STRING = '';
const LETTERS_ONLY = /[a-z]/i;

function mapLettersByCount(sentence) {
  return sentence.split(EMPTY_STRING)
    .filter(letter => LETTERS_ONLY.test(letter))
    .map(letter => [letter.toUpperCase(), 1])
    .reduce((letterCount, [key, count]) => {
      letterCount[key] = (letterCount[key] || 0) + count;
      return letterCount;
    }, {});
}

function * downCounter(maxValue) {
  yield maxValue;
  yield * downCounter(maxValue > 0 ? maxValue - 1 : 0);
}

export function maxBeauty(sentence = "") {
  const points = downCounter(26);
  return Object.values(mapLettersByCount(sentence))
    .sort().reverse()
    .reduce((score, count) => score + count * points.next().value, 0);
}

Test Cases:

import {maxBeauty} from './beauty';

describe('The maximum beauty', () => {
  it('should calculate for a word', () => {
    expect(maxBeauty('AbBCcC')).toBe(152);
  });

  it('should calculate ignoring spaces', () => {
    expect(maxBeauty('This is from Facebook Hacker Cup 2013.'))
      .toBe(551);
  });

  it('should calculate ignoring punctuation', () => {
    expect(maxBeauty('Ignore punctuation, please :)'))
      .toBe(491);
  });
});

How can I remove the state in the reducer for mapLettersByCount?

Return a new object on each iteration. Not optimal, but we're talking functional anyways. You can use object destructuring to do a shallow copy of the object. A more verbose version would be Object.assign. Then use computed keys to assign a dynamic key.

function mapLettersByCount(sentence) {
  return sentence.split(EMPTY_STRING)
                 .filter(letter => LETTERS_ONLY.test(letter))
                 .map(letter => [letter.toUpperCase(), 1])
                 .reduce((letterCount, [key, count]) => ({
                   ...letterCount,
                   [key]: (letterCount[key] || 0) + count
                 }), {});
}

  How can one use generators without assigning it to a variable? (see points variable in the maxBeauty function) Is this even considered state in this situation?

I believe that it is stateful since one call to points.next() does not return the same result as the next call. But I may be wrong.

One way you could do this is compute the points by subtracting the reducer's index from a constant 26. I believe it won't violate the rules of functional programming because

this 26 is a constant value
it's technically a closure scenario, similar to your other constants and
index is provided as an argument to the callback.

And it would look like this:

const POINTS = 26;

export function maxBeauty(sentence = "") {
  return Object.values(mapLettersByCount(sentence))
               .sort()
               .reverse()
               .reduce((score, count, index) => score + count * (POINTS - index), 0);
}


Checking user input for boxing sim engine I am creating a simple Boxing sim engine and have got things working fairly well. A few months ago I was instructed to avoid copy and pasting code and to try and "conserve my logic".

Anyways, I feel I have done a fairly good job overall but there is one area (posted below) that I feel definitely has room for improvement:

print ("1)Joe Smith\n2)Mike Jones\n3)Steve Roberts\n")
boxer1 = input("Choose a fighter: ")
boxer2 = input ("Choose his opponent: ")
if boxer1 == '1':
    B1 = JS
elif boxer1 == '2':
    B1 = MJ
elif boxer1 == '3':
    B1 = SR
if boxer2 == '1':
    B2 = JS
elif boxer2 == '2':
    B2 = MJ
elif boxer2 == '3':
    B2 = SR

MJ, JS, and SR are all variables for objects in my Boxer class. My concern is that I will have to continue adding four lines of code for each boxer I add to the program. While I don't mind typing each line out, I realize there may be a much more efficient way to approach this that I'm not seeing. I realize this isn't a major issue but, as I mentioned, this program is mainly for practice and therefore I want to make sure I'm programming as efficiently as possible.

Assuming that each Boxer object has a .name attribute…

BOXERS = [None, JS, MJ, SR]
for i in range(1, len(BOXERS)):
    print("%d) %s" % (i, BOXERS[i].name))
boxer1 = BOXERS[int(input("Choose a fighter: "))]
boxer2 = BOXERS[int(input("Choose his opponent: "))]

You want to generate the menu using the names already in the boxer objects, and retrieve the result with an array lookup.

Note that with this solution, you'll get an IndexError if the user picks an invalid entry.  You should decide how such errors should be treated, since your original code also lacked error handling.


Immutable objects in Java I just finished studying immutable objects and their advantages so I thought I'd create one of my own. Here is my employee class that extends a person class. It is not mutable since I have getters to my mutable date objects.

package objects.objects;

import java.util.Date;

public class Person {

    private final String name;
    private final Date dateOfBirth;

    Person(String name,Date dateOfBirth){
        this.name=name;
        this.dateOfBirth=dateOfBirth;

    }
    public String getName() {
        return name;
    }
    public Date getDateOfBirth() {
        return dateOfBirth;
    }
}

Employee Class:

package objects.objects;

import java.util.Date;

public final class Employee extends Person{

    private final String empolyeeID;
    private final Designation designation;
    private final double salary;
    private final Date dateOfJoining;

    public Employee(String Name,Date dateOfBirth,String employeeID,Designation designation,double salary,Date dateOfJoining)
    {
        super(Name,dateOfBirth);
        this.empolyeeID=employeeID;
        this.designation=designation;
        this.salary=salary;
        this.dateOfJoining=dateOfJoining;

    }

    public String getEmpolyeeID() {
        return empolyeeID;
    }

    public Designation getDesignation() {
        return designation;
    }

    public double getSalary() {
        return salary;
    }

    public Date getDateOfJoining() {
        return dateOfJoining;
    }
}

The designation enum:

package objects.objects;

public enum Designation {
    ASSOCIATE,SENIOR_ASSOCIATE,MANAGER,SENIOR_MANAGER,DIRECTOR
}

The Oracle Java documentation says:

  Don't share references to the mutable objects. Never store references
  to external, mutable objects passed to the constructor; if necessary,
  create copies, and store references to the copies. Similarly, create
  copies of your internal mutable objects when necessary to avoid
  returning the originals in your methods

Furthermore, the Oracle doc says:

  Don't allow subclasses to override methods. The simplest way to do
  this is to declare the class as final. A more sophisticated approach
  is to make the constructor private and construct instances in factory
  methods.

What if I want my class to be subclassed? What harm is there if I don't make my class final or have a private constructor?

Does it mean I shouldn't use getters or maybe pass a copy (a new Date object in the getter?) or is there some other approach that I am missing?

Also, could you point out apart from the date being mutable object references? Is there any other flaw in my code that violates the mutable policy or any other improvements as such?

All your fields are private and final, which is the first step to being immutable. This is good.

The Date class is not immutable, so, for example, you have a problem with the method:

public Date getDateOfBirth() {
    return dateOfBirth;
}

because, someone could do:

employee.getDateOfBirth().setYear(1900);

and suddenly your employee has aged... a lot.

What the oracle documentation is saying, is that, if you have mutable content, you should return a copy of that data. This is often called a 'defensive copy':

public Date getDateOfBirth() {
    return new Date(dateOfBirth.getTime());
}

Update: additionally, your method should be final so that no subclasses can change the behaviour of getDateOfBirth() (same is true for getName()). If the method is not final, then a subclass could possibly override the method, and do it in a way which make the name/dateOfBirth mutable.

Note, that the constructor for Date, in this instance, takes a long value. This leads on to a suggestion that the immutable class should store a long instead of a Date. The long is immutable anyway, if final.

public class Person {

    private final String name;
    private final long dateOfBirth;

    .....

    public Date getDateOfBirth() {
        return new Date(dateOfBirth);
    }

}

Apart from that, the immutability looks fine. Good, even.

The package name you have chosen though... is lacking: objects.objects. You could not name your package something better?


Convert NSInteger into NSString This is my sample code to convert a integer into string.

Is it very costly to do the multiple loops of [NSMutableString stringWithFormat:..]?

I.e should I just use appendString and get the NSString in reverse order and then do one for loop to reverse this "reverse string".

Is there any other faster/more elegant way?

NSString * intToString(NSInteger numberToConvert)
{
    BOOL isNegative = (numberToConvert < 0);
    if (isNegative)
    {
        numberToConvert = -1*numberToConvert;
    }

    NSMutableString *convertedString = [[NSMutableString alloc] init];

    while(numberToConvert != 0)
    {
        convertedString = [NSMutableString stringWithFormat:@"%ld%@",numberToConvert%10,convertedString];
        numberToConvert = numberToConvert/10;
    }

    if (isNegative)
    {
        convertedString = [NSMutableString stringWithFormat:@"-%@",convertedString];
    }

    return convertedString;
}

Umm… this is a complicated solution that makes no sense.  Why not keep it simple?

NSString *intToString(NSInteger numberToConvert)
{
    return [NSString stringWithFormat:@"%ld", numberToConvert];
}

If, on the other hand, the exercise is to implement your own solution from scratch, then I would consider it cheating to use NSMutableString stringWithFormat:.  Either way, I would consider your solution to be a failed response in an interview situation.


Auction function for a game This is an auction function for a game where you can bid on houses on the website.

You set a limit, and if posts limit below current bet or below your limit, it will raise the bid from current plus 1, until the limit is met. If your bid is over a player's limit, it updates it with your limit.

Let's say the current limit for house #55 is 500 and was submitted by player CykaBlyat and then Andrej comes and submit a limit of 800 it will update latest limit plus one (so bid will be 501) and keep raising with 1 until someone submit a limit higher than 800. And so on.

Is it safe? Is there something I have missed? Something I can do better? A cleaner way?

$id = isset($_GET['id']) ? intval($_GET['id']) : 0;

// Fetch some info about the house
$house_query = "SELECT h.id AS houseID, h.name AS houseName, h.rent, h.size, h.numofdoors, h.beds, h.owner, h.paid, p.name
            FROM houses h
            LEFT JOIN players p ON p.id = h.owner
            WHERE h.id = ?";

$stmt = $db->prepare($house_query);
$stmt->execute(array($id));
$row = $stmt->fetch();

// Is house on auction
$auctions = "SELECT *
            FROM house_auctions ha
            LEFT JOIN players p ON p.id = ha.player_id
            WHERE ha.house_id = $id";

$stmt = $db->prepare($auctions);
$stmt->execute(array($id));
$auction = $stmt->fetch();

if (count($_POST) > 0) {

$limit = trim($_POST['limit']);
$char = trim($_POST['character']);

// Get balance
$balance = $db->prepare("SELECT balance FROM players WHERE id = ?");
$balance->execute(array($char));

if ($limit <= $auction['bid'])
    $errors[] = "Your limit for the bid is lower than the current bid.";

else if ($limit == $auction['bid'])
    $errors[] = "Limit is same as current bid.";

// Check their balance
else if ($limit + $row['rent'] > $balance->fetchColumn())
    $errors[] = "You do not have enough gold.";

else if ($limit < $auction['limit']) {
    $bid = $limit + 1;
    $limit2 = $auction['limit'];
    $pid = $auction['player_id'];
}
else if ($limit > $auction['limit']) {
    $bid = $auction['limit'] + 1;
    $limit2 = $limit;
    $pid = $char;
} else {
   $bid = $auction['limit'] + $limit + 1;
   $limit2 = $limit;
   $pid = $char;
}
// Let's update!
if (empty($errors)) {
    echo "NO ERRORS =)";

    $stmt = $db->prepare("UPDATE house_auctions SET house_id = :house_id, player_id = :player_id, bid = :bid, `limit` = :limit, endtime = :endtime");
    $stmt->execute(array(':house_id'=>$id, ':player_id'=>$pid, ':bid'=>$bid, ':limit'=>$limit2,':endtime'=>time()));

}
}

// If there were any errors, show them
if (!empty($errors)) {
foreach ($errors as $cur_error)
echo '
'.$cur_error.'
';
}

Use placeholder there: WHERE ha.house_id = $id"; Placeholder is better due to security approach.

if (count($_POST) > 0) {

$limit = trim($_POST['limit']);
$char = trim($_POST['character']);

You should separate presentation from your logic. It will be better: read all $_POST outside the function and pass in arguments. Maybe you wanna later to play your game via AJAX, or something else, and you won't need $_POST anymore, but you still need the logic.

Due the same approach, returning status and errors from function is a better way, then just prints them out. You can return 2 values from function -- status and errors array.

Then do something like this:

list($status, $errors) = your_function(...);
if (!$status) {
    //printing errors
}

Or you can just return an array, and check length of it. If array contain some errors, print them out.

Rename your variables.

char to character

pid to playerId

limit2 to newLimit? Or what it means?

In your code it isn't clear enough. Especially limit2.


Solving a multi variable polynomial For this equation, I need to count possible solutions:

$$ a + b^2 + c^3 + d^4 \le S  $$

$$0\le a,b,c,d \le S$$

I tried the following approach:

       Integer S=Integer.parseInt(br.readLine());
      // get maximum possible value of a,b,c,d which satisfy equation
         int a=S;
        int b=(int) Math.sqrt(a);
        int d=(int) Math.sqrt(b);
        int c=(int) Math.cbrt(a);
        int count=0;
        for (int i = 0; i <= a; i++) {
            for (int j = 0; j <= b; j++) {
                for (int k = 0; k <= c; k++) {
                    for (int l = 0; l <= d; l++) {
                       int total=i+j*j+(int)Math.pow(k, 3)+
  (int)Math.pow(l,4);
                        if(total<=a) {
                            count++;
                        }
                    }
                }
            }
        }
          System.out.println(count);

Time complexity: \$O(n \times \sqrt{n} \times \sqrt[3]{n} \times \sqrt[4]{n})\$ (Not sure)

For large value, this takes time. Can someone please help me with better solution with time complexity?

a + b^2 + c^3 + d^4 <= S
  For above equation ,I need to count possible solutions.

  I tried following approach.

Integer inp=Integer.parseInt(br.readLine());
      // get maximum possible value of b,c,d which satisfy equation
      int b=(int) Math.sqrt(inp);
        int d=(int) Math.sqrt(b);
        int c=(int) Math.cbrt(inp);
        int count=0;

  ...

If I were the interviewer, I would already have marked you down heavily for your choice of variable names. The problem statement defines a, b, c, d, S. The natural choice of variable names for those values are a, b, c, d, S. Using b, c, d for something else almost looks as though you were deliberately trying to make the code unmaintainable.

                        if(total<=inp) {
                            count++;
                        }

Enumerating items one by one is nearly always the wrong way to count something.

If I asked you to count solutions to \$a \le S\$ would you write a loop? I hope not, because you can do it without any loop or any mathematical operations.

Now, how about \$a + b^2 \le S\$? This can be done with one mathematical expression. Hint: start from $$\sum_{b=0}^{\sqrt{S}} \textrm{countA}(S - b^2)$$

Cubes already grow quite fast, so

for (int c = 0; c <= maxC; c++) {
    for (int d = 0; d <= maxD; d++) {
        count += countAB(S - c*c*c - d*d*d*d)
    }
}

would be reasonably efficient. If you really want to microoptimise you can eliminate the multiplications in favour of some accumulators and addition, but that's probably not necessary for an interview question. They might ask about it in a follow-up.


Monitoring (i.e. loading updates) to log files I wrote a simple little script, designed to load some log file. Clearing lines and loading updates is done by double clicking on the log line to clear.

    <meta charset="utf-8">
    <title></title>

Looking at your code, it appears to be a django log. I'm not sure why you are reading logs with a webpage when you can always read them using the terminal. cat, grep, tailf and a few other commands would be your friends in this case.

As @Flambino mentioned, direct access to the log is a problem. If you are going to pursue this method, you should at least have the server side validate your access. You could use a login system, and if the user isn't logged in, you can simply throw a 403 (Forbidden) as the result of the AJAX. Otherwise, return the log contents.

Now back to the code. First off, you could wrap everything in a closure. That way, nothing is modifiable from the global scope. That prevents naming collisions and all that. At least your code is protected from other code that may reside in your page.

;(function(){

  // all your code here

}());

Next, you can use an AJAX library to abstract out your XHR code. That way, you don't have to deal with all the intricacies of XHR. A good minimum that I use, which is also forward-compatible, is using a Promise and fetch polyfill. Promise is needed for fetch to work. It's just under 20kb when minified. Rather big but keeps you sane.

fetch(url).then(function(response){
  // This is a required intermediate parsing step for fetch
  return response.text();
}).then(function(responseText){
  // Do what needs to be done with the log
}).catch(function(error){
  // Do what needs to be done during error
});

Another suggestion is to use a <textarea> instead of a <div> as your log output. That way, you don't have to deal with formatting, collapsing of tabs, escaping of characters. You can always set it to readonly to prevent editing, and modify scrollTop on update if ever you always want the bottom to be visible.

Sever logs can be huge and will have impact on your fetching operation. This happens especially if your team is using the logger extensively for easy debugging. I suggest you have some server-side logic to paginate your results and not just dump everything into the page.

I noticed your funny little characters for the title bar. While they are fun, not all computers might have those glyphs built in. On my rig, this character () doesn't appear, just a box with a question mark (or just a box on others). You might want to consider being creative in the ascii range of characters and not totally rely on these non-ascii ones. For example, this sequence -\|/- for a rotating bar.

On a side note, it appears as though you're trying to achieve something similar to what Scalyr does - a web-based log monitoring and search.


Prime number verifier I solved this prime number verifier problem:

import scala.io.StdIn

object Main extends App {
  val testCases = StdIn.readInt

  for (i <- 0 until testCases) {
    val limits = StdIn.readLine.split(" ")
    val m: Int = limits(0).toInt
    val n: Int = limits(1).toInt
    for (j <- m to n) {
      var prime = false
      if (isPrime(j))
        println(j)
    }
    println()
  }

  def isPrime(number: Int) : Boolean = {
    if (number == 1)
      false
    else {
      for (h <- 2 to Math.sqrt(number).asInstanceOf[Int]) {
        if (number % h == 0 || number == 1) {
          return false
        }
      }
      true
    }
  }
}

I wrote the code this way because Scala doesn't have break statements, and return statements aren't very functional either. How do I go about writing this code in a more functional and idiomatic manner?

How about this?

def isPrime(n:Int):Boolean = n match {
  case 1 => false
  case 2 => true
  case 3 => true
  case 5 => true
  case 7 => true
  case n if (n%2==0 || n%3==0 || n%5==0 || n%7==0) => false
  case _ => (Stream.iterate(3, Math.sqrt(n).toInt+1){_ + 2} dropWhile((x:Int) => n%x != 0)).isEmpty
}

The code is straightforward. We know that the prime numbers below 10 are 2, 3, 5, 7. If a given number is the multiple of 2, 3, 5, or 7, is't not a prime. In other case (the last case branch) it generates stream from 3 to ceil(sqrt(n)) with step 2. We know that even numbers can't be a prime. And check that the argument is a multiple of any number in the stream. Basically the same logic with the original code. The reason I use Stream is to use lazy evaluation. With lazy evaluation, we can avoid unnecessary computation.


PHP login page using prepared statements I am new to the concept of prepared statement and i am trying to adjust my "user login" function to be a prepared statement. The part i am really struggling with is the fact i have multiple SQL queries in the one function and the biggest hurdle right now is fetching from a prepared statement.

Here is the code i have written for the function so far, i do apologise its a mess and i understand there is parts that shouldn't be in there, i just left everything in.

function login_member() {

    $connection = mysqli_connect("localhost", "root", "", "ecom_db");
    if(isset($_POST['login_button'])) {

        $email = filter_var($_POST['log_email'], FILTER_SANITIZE_EMAIL);

        $_SESSION['log_email'] = $email;
        $password = $_POST['log_password'];

        $stmt = $connection->prepare("SELECT email AND password FROM users WHERE email=? AND password=? AND com_code IS NULL");
        $stmt->bind_param('ss', $email, $password);
        $stmt->execute();
        $stmt->bind_result($email, $password);
        $stmt->store_result();
        if($stmt->execute()) {

        $result = $stmt->get_result();

        while ($row = $result->fetch_assoc()) {

            //result is in row

            }

        }
        if($stmt->num_rows == 1) {
            if($stmt->fetch()) {
                while (($row = $res->fetch_assoc()))
                   $username = $row['username'];

            $closed = $connection->prepare("SELECT email AND password FROM users WHERE email=? AND user_closed=?");
            $closed->bind_param('ss', $email, "yes");
            $closed->execute();
            $closed->bind_result($email, "yes");
            $closed->store_result();
            $closed_res = $closed->get_result();

            if($closed->num_rows == 1) {
                $open = $connection->prepare("UPDATE users SET user_closed=? WHERE email=?");
                $open->bind_param('ss', "no", $email);
                $open->execute();
                $open->bind_result("no", $email);
                $open->store_result();
            }

            $_SESSION['username'] = $username;
            header("Location: ../public/member/index.php");
            exit();
        }
        else {
            set_message("Email or password was incorrect");
        }
    }
}

I understand that it looks like a complete mess, i have left all my working outs in there. My main issue here is after my initial statement, i want to be able to fetch the results from the returned row and set the "username" a column in that row to the variable $username.

My original, working code (without prepared statements) is as follows.

My original - without prepared satatements is as follows.

function login_new_member() {

    if(isset($_POST['login_button'])) {

        $email = filter_var($_POST['log_email'], FILTER_SANITIZE_EMAIL); //sanitize email

        $_SESSION['log_email'] = $email; //Store email into session variable
        $password = md5($_POST['log_password']); //Get password md5

        $check_database_query = query("SELECT * FROM users WHERE email='$email' AND password='$password' AND com_code IS NULL");
        confirm($check_database_query);
        $check_login_query = mysqli_num_rows($check_database_query);

        if($check_login_query == 1) {
            $row = mysqli_fetch_array($check_database_query);
            $username = $row['username'];

            $user_closed_query = query("SELECT * FROM users WHERE email='$email' AND user_closed='yes'");
            confirm($user_closed_query);
            if(mysqli_num_rows($user_closed_query) == 1) {
                $reopen_account = query("UPDATE users SET user_closed='no' WHERE email='$email'");
                confirm($reopen_account);
            }

            $_SESSION['username'] = $username;
            header("Location: ../public/member/indexhold.php?register_new");
            exit();
        }
        else {
            echo "Email or password was incorrect";
        }

    }

}

There are several problems with your code.

An extra query to get exactly the same row you just requested
Unreliable password hashing
Inconsistent query handling with strange query()/confirm() pair.
A lot of things intermixed here - POST, database internals, everything.

To make it better you have to store passwords after hashing them with password_hash() function and also utilize an improved version of query() function.
So the final code would be like this (it is using my simple DB wrapper which is based on PDO, but could be implemented using mysqli as well). So the idea is that you do all the prepared statements' related stuff inside the database class, and never touch it inside a user function.

function login_new_member($email, $password)
{
    $user = DB::run("SELECT * FROM users WHERE email=? AND com_code IS NULL", [$email]);

    // you really should switch to password_hash/password_verify
    // and make the following line as
    // if ($user && password_verify($password, $user['pass']))
    // but for now
    if ($user && md5($password) === $user['password']))
    {
        if ($user['user_closed'] == 'yes')
        {
            DB::run("UPDATE users SET user_closed='no' WHERE email=?", [$email]);
        }
        $_SESSION['log_email'] = $user['email'];
        $_SESSION['username'] = $user['username'];
        header("Location: ../public/member/indexhold.php?register_new");
        exit();
    } else {
        echo "Email or password was incorrect";
    }
}

so this function should be called like this

if(isset($_POST['login_button'])) {
    login_new_member($_POST['log_email'], $_POST['log_password'])
}


Regex to match strings safely The following regex is meant to improve stringmatching by sanitizing the user -defined commandPrefix and checking for whitespace and line-endings after the specified text. command will generally be a single word, hence not escaped.

var prefix = commandPrefix.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&');
return text.test("/^" + prefix + command + "(?=\s|$)/");

Can this be improved for code readability? And am I missing any regex special characters in the sanitation?

If you want code readability, check the escape-string-regexp solution from @CoolAJ86

The Long Answer

If you're going to use the function above at least link to this stack overflow post in your code's documentation so that it doesn't look like crazy hard-to-test voodoo.

var escapeRegExp;

(function () {
  // Referring to the table here:
  // https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/regexp
  // these characters should be escaped
  // \ ^ $ * + ? . ( ) | { } [ ]
  // These characters only have special meaning inside of brackets
  // they do not need to be escaped, but they MAY be escaped
  // without any adverse effects (to the best of my knowledge and casual testing)
  // : ! , =
  // my test "~!@#$%^&*(){}[]`/=?+\|-_;:'\",<.>".match(/[\#]/g)

  var specials = [
        // order matters for these
          "-"
        , "["
        , "]"
        // order doesn't matter for any of these
        , "/"
        , "{"
        , "}"
        , "("
        , ")"
        , "*"
        , "+"
        , "?"
        , "."
        , "\\"
        , "^"
        , "$"
        , "|"
      ]

      // I choose to escape every character with '\'
      // even though only some strictly require it when inside of []
    , regex = RegExp('[' + specials.join('\\') + ']', 'g')
    ;

  escapeRegExp = function (str) {
    return str.replace(regex, "\\$&");
  };

  // test escapeRegExp("/path/to/res?search=this.that")
}());


Python Bulls and Cows Bulls and Cows is a simple code-breaking game sometimes better known as "Mastermind." The rules are explained here: https://en.wikipedia.org/wiki/Bulls_and_Cows

I would appreciate some feedback on the following implementation in terms of overall code quality, layout and logic. It's meant to be at a fairly basic level so I don't want to over complicate things, but want to avoid any obviously bad choices. Suggestions for improvements very welcome.

import random

SECRET_NUMBER_SIZE = 2
MAX_GUESSES = 5

# Generate secret number with SECRET_NUMBER_SIZE digits and no duplicates
secret_number = ""
while len(secret_number) < SECRET_NUMBER_SIZE:
    new_digit = str(random.randint(0, 9))
    if new_digit not in secret_number:
        secret_number += new_digit

# print(f"For testing. Secret number is: {secret_number}")

print(f"Guess my number. It contains {SECRET_NUMBER_SIZE}\
unique digits from 0-9")

remaining_turns = MAX_GUESSES

while remaining_turns <= MAX_GUESSES:

    # Get user guess and validate length
    player_guess = input("Please enter your guess: ")
    if len(player_guess) != SECRET_NUMBER_SIZE:
        print(f"Your guess must be {SECRET_NUMBER_SIZE} digits long.")
        continue

    # Main game logic
    if player_guess == secret_number:
        print("Yay, you guessed it!")
        break
    else:
        bulls = 0
        cows = 0
        for i in range(SECRET_NUMBER_SIZE):
            if player_guess[i] == secret_number[i]:
                bulls += 1
        for j in range(SECRET_NUMBER_SIZE):
            if player_guess[j] in secret_number and \
              player_guess[j] != secret_number[j]:
                cows += 1
        print(f"Bulls: {bulls}")
        print(f"Cows: {cows}")
        remaining_turns -= 1
        if remaining_turns < 1:
            print("You lost the game.")
            break

@Josay's and @Reinderien's answers both give useful advice, with @Josay's being particularly comprehensive. However, there is one point that I've noticed neither of them have covered

Generation of the secret number

# Generate secret number with SECRET_NUMBER_SIZE digits and no duplicates
secret_number = ""
while len(secret_number) < SECRET_NUMBER_SIZE:
    new_digit = str(random.randint(0, 9))
    if new_digit not in secret_number:
        secret_number += new_digit

This is an inelegant way of generating the secret number for two reasons:

You concatenate to a string, an immutable type. This is much more expensive than necessary, which does not a notable impact on a program of this size, but could become problematic as you scale up; it's better to practice good techniques, even at small scales. Additionally, you should never need to iteratively concatenate to a string in situations where the string's contents are generated all at once, because there are two successively more efficient ways:

Since Python uses dynamic lists, you can create a list with the string's contents, and then use "".join(str(i) for i in src_lst)
To improve the efficiency even further, you can utilize the fact that you know the list's size already, and "pre-allocate" the list using lst = [None] * SECRET_NUMBER_SIZE and then iteratively fill in its contents using a numerical index variable. This is a bit unpythonic, as @Josay goes over in their answer, and is probably a bit of premature optimization, but it's a good trick to have up your sleave if you want to eek out a bit more performance, and may be extremely useful depending on the situation.
But there's an even better way, which we'll get to in a second...

Every number is checked to ensure it hasn't already occurred in the result list. The reason this is not ideal is because at larger scales, one could end up generating many numbers before they stumble upon one that hasn't already been chosen. To mitigate this, one could lower the generation range each time, and offset the numbers accordingly, but it's probably not worth the effort because this isn't much of a practical concern, even at larger scales; generating random numbers using the Mersenne Twister is very cheap. But this concern helps lead to an elegant solution...

random.sample() is your friend here. It allows you to generate multiple distinct random numbers quite easily; all you have to provide is a population sequence and the size of the sample. For this situation, a population sequence is quite simple: range(10) supplies all the numbers between 0 and 9 inclusive. You can now simplify your 5 lines above into ~1 line:

secret_number = "".join((str(i) for i in
    random.sample(range(10), SECRET_NUMBER_SIZE)))


PHP function (in WordPress) echoing HTML I'm really new to PHP, and a little less so in WordPress. I've created a working function that spits out some static HTML into a page template in WordPress. I'd like to know if there is a best practice for this, perhaps someone can help.

function my_html_function() {
    echo '
Title

A list item

A list item

A list item

A list item

';
    }

And within my WP page template I use:

<?php echo my_html_function(); ?>

This HTML is static and won't be changed.

There are several issues I have with your function. Some are about your function breaking the basics about what a function is/should do, others are perhaps a tad more personal (as in: a matter of personal preference).

Functions do stuff, without it impacting the rest of the code

It seems almost silly to explicitly state this, but let me explain what I mean. A function is a unit of code, that performs a single task, that might be required to be performed several times. Functions should not have "side-effects". That's a fancy way of saying that calling a function shouldn't disrupt the rest of the code (with the exception of a function throwing an Exception, of course).

Your function, however, generates output without the caller having any means of controlling what, how and to where that output is being sent. The function assumes the caller knows that it'll send output, but that's a rather dangerous assumption to make. A simple example: cookies can't be set after any output has been sent already. Therefore:

my_html_function();
set_cookie();//<-- FAILS!

The function contains an echo, and so output will be sent once a call is made. It might seem like an obvious thing to fix, but in larger projects, things like this can really trip you up, and you'll find yourself wasting hours trying to work out why the cookie isn't being set properly.

Bottom line: Functions return, they don't echo, so change the echo in your function with return, and echo in the template.

PHP stands for PHP: Hypertext Preprocessor

The important bit in this case is Preprocessor. You can use PHP to return/echo strings of markup, but why would you do that? You can embed PHP in HTML just as easily, so why not use that? If needs must, you can always use PHP to include bits and pieces of markup.

Markup isn't static

Sure, the markup your function echoes/returns is static in your case, but markup languages are, well, languages. They're not always static. It's not a big stretch to imagine a situation where you want your function to churn out a block of markup where the title value is different, or the number of list items is variable.
In those cases, you'll want to pass arguments to your function, and you'll find yourself generating markup dynamically. It's not a hard thing to do, and it's done all the time.

To this end, you have a number of options available to you. The easiest (and in this case reasonable) approach would be to string together the markup:

/**
 * ADD THIS, DOCUMENT YOUR CODE:
 * Pass the title string and an associative array of list items
 * @param string $title
 * @param array $listItems
 * @return string
 */
function getTitleList($title, array $listItems)
{
    $title = sprintf('
%s
', htmlentities($title));//<-- sanitize $title, it shouldn't contain markup
    $liNodes = [];//array of li nodes
    //again, we need to sanitize all this
    foreach ($listItems as $id => $value) {
        $liNodes[] = sprintf(
            '
%s
',
            str_replace('"', '\'', $id),//we're using "as id delimiter, so id can't use them
            htmlentities($value)//html entities must be removed
        );
    }
    $markup = $title . '
' . implode('', $liNodes) . '
';
    return $markup;
}

The alternative here would be to use an actual DOM parser/builder, like the DOMDocument class, although in this case, that would be overkill IMO. As @KIKOSoftware said: sprintf is marginally slower than regular concatenation, and it could be said that it's a bit harder to use at first. If you want, you can use regular concatenation just as easily:

$title = '
' . htmlentities($title) . '
';//same as sprintf above

Another thing that was pointed out to me in the comments was that, if the contents of any of your tags contains markup, then obviously htmlentities is not what you want to use. If you require your function to be flexible (and allow the insertion of nested tags), you should really look into using an actual DOMDocument-like tool. Something that enables you to build the DOM as you go (creating new nodes, setting attributes and inserting them throughout the actual DOM tree).


Checking a HTML element's class $('pre').each(function(index) {
    var c = $(this).attr("class");
    if (!c) {
        return true;
    }
    // Match only Javascript code snippets
    if (!c.match(/brush: js; class-name: 'jsbox'/)) {
        return true;
    }
    var code = $(this).text();
    $(this).after($("<button>Run example</button>").click(function() {
        run(code);
    }));

I noticed if I didn't have if (!c) { then the loop could abrubtly halt if it found a <pre> element without a class.

I wonder how to write this more succinctly. I'm also wondering about my matching options, since I've noticed different ways people match / compare strings in Javascript and I wondered what was "de rigueur".

Change

var code = $(this).text();

to

var code = $(this).val();


Optimize regex for maximum speed According to https://stackoverflow.com/questions/19608546/optimize-regex-for-maximum-speed
and comments to ask my question here .
Please help me to optimize following regex to best performance . I have read some articles but this problem should solve quickly to decrease cpu usage and delay time so i don't have enough time for try and false .

First one should match for example

http://microsoft.com/test/temp.iso

http://download.microsoft.com/TEMP.iso

http://www.download.microsoft.com/test.aspx?hdwjdhcjdgcjhdc=TEMP.iso

note:

All url should start with http://  so i don't know it is better to put ^http:// at first or not ?
first line and last line have specific rules but lines between them may combined.
These regexp are used in squid configuration.

Any help appreciated .

refresh_pattern -i (.+\.||)(microsoft|windowsupdate).com/.*\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|iso|psf)
refresh_pattern -i (.+\.||)eset.com/.*\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ver|nup)
refresh_pattern -i (.+\.||)avg.com/.*\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ctf|bin|gz)
refresh_pattern -i (.+\.||)grisoft.(com|cz)/.*\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ctf|bin|gz)
refresh_pattern -i (.+\.||)avast.com/.*\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|vpx|vpu|vpa|vpaa|def|stamp)
refresh_pattern -i (.+\.||)(kaspersky-labs|kaspersky).com/.*\.(cab|zip|exe|msi|msp|bz2|avc|kdc|klz|dif|dat|kdz|kdl|kfb)
refresh_pattern -i (.+\.||)nai.com/.*\.(gem|zip|mcs|tar|exe|)
refresh_pattern -i (.+\.||)adobe.com/.*\.(cab|aup|exe|msi|upd|msp)
refresh_pattern -i (.+\.||)symantecliveupdate.com/.*\.(zip|exe|msi)
refresh_pattern -i (.+\.||)(192\.168\.10\.34|mywebsite.com)/.*

After help of Stackoverflow guys i have changed it to this . Any more optimization ?

refresh_pattern -i (.+\.)?(microsoft|windowsupdate)\.com/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|iso|psf)$
refresh_pattern -i (.+\.)?eset\.com/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ver|nup)$
refresh_pattern -i (.+\.)?avg\.com/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ctf|bin|gz)$
refresh_pattern -i (.+\.)?grisoft\.(com|cz)/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ctf|bin|gz)$
refresh_pattern -i (.+\.)?avast\.com/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|vpx|vpu|vpa|vpaa|def|stamp)$
refresh_pattern -i (.+\.)?(kaspersky-labs|kaspersky)\.com/.*?\.(cab|zip|exe|msi|msp|bz2|avc|kdc|klz|dif|dat|kdz|kdl|kfb)$
refresh_pattern -i (.+\.)?nai\.com/.*?\.(gem|zip|mcs|tar|exe|)$
refresh_pattern -i (.+\.)?adobe\.com/.*?\.(cab|aup|exe|msi|upd|msp)$
refresh_pattern -i (.+\.)?symantecliveupdate\.com/.*?\.(zip|exe|msi)$
refresh_pattern -i (.+\.)?(192\.168\.10\.34|mywebsite\.com)/.*?$

Edit: Squid does not accept (?:content) so i removed it . any replacement ?

Since all your patterns have similar structure, we only need to focus on one, e.g.

refresh_pattern -i (.+\.)?avg\.com/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ctf|bin|gz)$

This pattern starts with (.+\.)?. This pattern says 'match any characters that end in a '.', but, the ? means that if nothing matches, then that's OK..... so, what the (.+\.)? really means is "Match something with a dot at the end, or match nothing".

This is apparently sensible, but, because the entire URL is not anchored to the start of the line (there is no ^) it really means "match anything".

For example, all of these URL's will successfully match:

http://www.avg.com/file.gz // we would expect this
http://avg.com/file.gz // we would maybe expect this
http://cravg.com/file.gz // we would not expect this
https://downloads.avg.com/file.gz // we would expect this
mailto:me@www.avg.com/file.gz // we would expect this
ftp://ftp.avg.com/file.gz // we would expect this

This is all beside the point, though, because whether we use (.+\.)? or not it makes no difference whatsoever (functionality wise).

It is possible that the squid regex engine is smart enough to identify that the (.+\.)? is useless, and it may optimize it out.... but, I doubt it.

So, you can remove all the (.+\.)? structures.

I believe that you should replace them with a \b which is a 'word break'. This will match things like http://avg.com/... and http://www.avg.com/... but it will not match http://cravg.com/.... I believe this is what you want.

It would likely be better to also anchor the regex to the beginning of the URL. If you can assume that it is an http or https URL then I would absolutely recommend that you include it as part of the regex. It will anchor the query well.

I will assume that the protocol could be https as well as http, in which case the match would be:

refresh_pattern -i ^https?//.*\bavg\.com/.*?\.(cab|exe|dll|ms[i|u|f]|asf|wm[v|a]|dat|zip|ctf|bin|gz)$

This change can be applied to all the refresh_patterns.   (replace (.+\.)? with ^https?//.*\b)


Small part of an INI parser I know there are probably thousands of INI parser implementations in C.

Nevertheless I want to implement one to learn the art of parsing grammar.

I just wrote a small piece of the INI parser and wanted to ask if I am on the right track:

void parse_line(const char *str, char **name) {
    size_t length = strlen(str);
    bool found_section = false;

    for (size_t pos = 0; pos < length; ++pos) {
        char current = str[pos];
        bool is_last_ch = (pos + 1) >= length;

        //dbg("Processing %c", current);

        if (current == '[') {
            size_t updated_pos = pos + 1;
            ssize_t match_pos = -1;

            if (is_last_ch) {
                dbg("Reached end of line without closing ] (1)!");

                return;
            }

            if (found_section) {
                dbg("Already found a section!");

                return;
            }

            //dbg("Found a [ at position %zu, looking for ] now", pos);

            for (size_t x = updated_pos; x < length; ++x) {
                if (str[x] == ';') {
                    break;
                } else if (str[x] == ']') {
                    match_pos = x;

                    break;
                }
            }

            if (match_pos == -1) {
                dbg("Reached end of line without closing ] (2)!");

                return;
            }

            //dbg("Found matching ] at position %zd", match_pos);

            assert(match_pos >= updated_pos);

            size_t length_of_section = match_pos - updated_pos;

            //dbg("Length of name is %zu", length_of_section);

            char *section_name = malloc(length_of_section + 1);
            assert(section_name);

            for (size_t x = 0; x < length_of_section; ++x) {
                size_t i = x + updated_pos;

                section_name[x] = str[i];
            }

            section_name[length_of_section] = 0;

            // utilize `section_name` in some way...
            //dbg("Section name is: %s", section_name);
            *name = section_name;
            // and then free() it somewhere

            pos += length_of_section + 1;

            assert(pos < length);

            found_section = true;
        } else if (current == ']') {
            dbg("Found unmatched ] at position %zu", pos);

            return;
        } else if (current == ';') {
            //dbg("End of line, bye!");

            return;
        } else {
            dbg("Unexpected character %c", current);

            return;
        }
    }
}

At the moment it only parses name of sections (like [name]).

I want to focus on detecting malformed grammar:

[section] comment -> invalid
comment [section] -> invalid

etc.

Example:

parse_line("[name]", ...); // gives "name"
parse_line("[name];Comment", ...); // gives "name"
parse_line(";[name];Comment", ...); // gives (null)
parse_line("[name[]", ...); // works... but should not (i already have a fix for this in mind -> check for "[" in section name fetching ; but is this good?)

Is the approach to ''wait'' for a character and then process from there on a robust way to parse things like INI files?

Here are some things that may help you improve your program.

Use a state machine

The approach you currently have uses bool variables keeping track things such as whether a section has been found, whether a character is the last one and weaves the interpretation int a long series of nested if statements.  That approach may work for small, simple grammars such as this, but it's not going to scale well as you move on to a larger set of things your program can parse.

For that reason, I'd recommend a more data-driven approach in which you write out a state machine and have the code execute according to the data within it rather than via a code-driven approach.  This makes things more compact and easier to read.

An example of such an implementation for a parser is in this answer.

Consider using regular expressions

I've given this advice before.  It's a lot easier in C++ because there's library support within the standard, but since it appears you're using POSIX-based libraries, you may already have a regular expression library available to you.

Consider using compiler tools

Another approach to consider is to use standard tools such as lex and yacc (or flex and bison) to create a much more readable and maintainable version of the parser.  These tools have a steep learning curve, but once you've mastered them, you'll be better able to see and understand how to write unambiguous parsers from scratch as well.  IMHO, this is time well spent.

Don't comment out debug statements

Just as with assert, I'd suggest that creating a version of dbg that either expands to nothing (for production code) or to a routine that prints debugging messages (for development) may be very useful.  It's much easier than commenting and uncommenting such debugging statements during the debugging process.

Don't leak memory

There is, potentially, a malloc within the code but no corresponding free.  I assume that's because it's intended that's because the caller is supposed to free it if it's allocated, but the problem is that the routine doesn't set name to NULL if it doesn't happen to allocate memory, making it incumbent on the caller to also pre-set the pointer to NULL.  This would be easier to use if, instead, the pointer were a return value that would either be NULL or a pointer that the caller is expected to free.

Avoid ssize_t if possible

Because you're initializing match_pos with -1, I see why you've chosen to make it of type ssize_t (which is signed) versus size_t which might not be signed.  However, this limits portability and there's no reason you couldn't use another sentinel value instead, such as SIZE_MAX


RX fluent subscribing to observables I have three observables:

var itemsManipulationStarted =
                Observable.FromEvent<ManipulationStartedEventArgs>(Items, "ManipulationStarted");

var pageManipulationDelta = Observable.FromEvent<ManipulationDeltaEventArgs>(this, "ManipulationDelta");

var pageManipulationCompleted =
                Observable.FromEvent<ManipulationCompletedEventArgs>(this, "ManipulationCompleted");

They are always called in that sequense: itemsManipulationStarted -> pageManipulationDelta -> pageManipulationCompleted

After every call to any observable I need to make some actions:

itemsManipulationStarted.Subscribe(e => Items.IsHitTestVisible = false);

pageManipulationDelta.SkipUntil(itemsManipulationStarted)
                     .Subscribe(e => e.EventArgs.Complete());

pageManipulationCompleted.SkipUntil(pageManipulationDelta)
                         .Subscribe(e => Items.IsHitTestVisible = true);

Is it clear or is there another way to write some "fluent" subscribing?

If you're on C# 5.0, you can use await with IObservables to write code like this:

await itemsManipulationStarted.FirstAsync();
Items.IsHitTestVisible = false;

var e = await pageManipulationDelta.FirstAsync();
e.EventArgs.Complete();

await pageManipulationCompleted.FirstAsync();
Items.IsHitTestVisible = true;

Though I'm not completely sure this will do what you want, because of timing.


Simple Comonads in Scala? trait Comonad[M[_]] {
   // map
   def >>[A,B](a: M[A])(f: A => B): M[B]

   // extract | coeta
   def counit[A](a:M[A]): A

   // coflatten | comu
   def cojoin[A](a: M[A]): M[M[A]]
}

object Comonad {

  implicit def listComonad[A]: Comonad[List]
  =
    new Comonad[List] {

      def counit[A](lsa: List[A])
      =
        lsa match { case List(a) => a }

      def cojoin[A](lsa:List[A]): List[List[A]]
      =
        List(lsa)

      def >>[A,B](lsa: List[A])(f: A => B): List[B]
      =
        lsa map f
  }

}

So yeah I'm looking at this and I don't have that correct feeling...

Anyone mind correcting this and maybe offerring one or two other simple comonads?

I believe, what's bothering you is a non-total definition of counit, right? (for cojoin one possible variation is lsa.tails) Indeed, a List does not have a valid comonad instance specifically bacause of that. It does have a valid semicomonad instance though.

Things that have a valid comonad instance are, for example: Identity, NonEmptyList, Zipper, Tuple. Here's a reddit question with more examples of comonads.


LeetCode 552: Student Attendance Record II I'm posting my code for a LeetCode problem copied here. If you have time and would like to review, please do so. Thank you!
Problem

Given a positive integer n, return the number of all possible
attendance records with length n, which will be regarded as
rewardable. The answer may be very large, return it after mod 109 + 7.
A student attendance record is a string that only contains the
following three characters:

'A' : Absent.
'L' : Late.
'P' : Present.

A record is regarded as rewardable if it doesn't contain more than one
'A' (absent) or more than two continuous 'L' (late).
Example 1:

Input: n = 2
Output: 8

Explanation:
There are 8 records with length 2 will be regarded as rewardable:

"PP" , "AP", "PA", "LP", "PL", "AL", "LA", "LL"

Only "AA" won't be regarded as rewardable owing to more than one absent times.

Note: The value of n won't exceed 100,000.

Accepted Code
class Solution {
public:
    static int checkRecord(int edges) {
        if (edges == 0) {
            return 1;
        }

        // possible candidates
        uint32_t adj_matrix[MATRIX_WIDTH_SQUARED] = {
            1, 1, 1, 0, 0, 0,
            1, 0, 0, 0, 0, 0,
            0, 1, 0, 0, 0, 0,
            1, 1, 1, 1, 1, 1,
            0, 0, 0, 1, 0, 0,
            0, 0, 0, 0, 1, 0,
        };

        uint32_t a[MATRIX_WIDTH_SQUARED];
        uint32_t b[MATRIX_WIDTH_SQUARED];
        uint32_t* accumulated = a;
        uint32_t* temp = b;
        uint32_t* adj_matrix_pointer = adj_matrix;

        std::memset(a, 0, sizeof(a));

        for (unsigned int node_x = 0; node_x < MATRIX_WIDTH_SQUARED; node_x += MATRIX_WIDTH + 1) {
            a[node_x] = 1;
        }

        // Square
        while (edges > 1) {
            if (edges & 1) {
                matrix_multiply(accumulated, adj_matrix_pointer, temp);
                std::swap(temp, accumulated);
            }

            matrix_multiply(adj_matrix_pointer, adj_matrix_pointer, temp);
            std::swap(temp, adj_matrix_pointer);
            edges >>= 1;
        }

        matrix_multiply(adj_matrix_pointer, accumulated, temp);
        uint64_t first_col_sum = 0;

        // Sum up
        for (int node_y = 0; node_y < MATRIX_WIDTH_SQUARED; node_y += MATRIX_WIDTH) {
            first_col_sum += temp[node_y];
        }

        return (uint32_t) (first_col_sum % MODE);
    }

private:
    static constexpr int MATRIX_WIDTH = 6;
    static constexpr int MATRIX_WIDTH_SQUARED = MATRIX_WIDTH * MATRIX_WIDTH;
    static constexpr uint64_t MODE = 1000000007UL;

    static void matrix_multiply(uint32_t* a, uint32_t* b, uint32_t* c) {
        for (unsigned int node_y = 0; node_y < MATRIX_WIDTH; node_y++) {
            for (unsigned int node_x = 0; node_x < MATRIX_WIDTH; node_x++) {
                uint64_t matrix_multi = 0;

                for (unsigned int k = 0; k < MATRIX_WIDTH; k++) {
                    matrix_multi += (uint64_t) (a[node_y * MATRIX_WIDTH + k]) * (uint64_t) (b[k * MATRIX_WIDTH + node_x]);
                    matrix_multi %= MODE;
                }

                c[node_y * MATRIX_WIDTH + node_x] = matrix_multi;
            }
        }
    }
};

Reference
In LeetCode's answer template, there is a class usually named Solution with one or more public functions which we are not allowed to rename.

Problem

Discuss

Use C++ Container Classes
This code looks too much like C code and not enough like C++ Code.
In the modern day C++, the use of raw pointers such as uint32_t* accumulated = a; are frowned upon, and container classes such as std::array and iterators were developed to reduce the use of raw pointers.
The C style array b is never initialized, and the C style array a is initialized twice.
        std::memset(a, 0, sizeof(a));

        for (unsigned int node_x = 0; node_x < MATRIX_WIDTH_SQUARED; node_x += MATRIX_WIDTH + 1) {
            a[node_x] = 1;
        }

The first initialization can be accomplished by:
        uint32_t a[MATRIX_WIDTH_SQUARED] = {0};

and I would prefer to see an iterator used in the second initialization.
Use Proper Casting
Currently the code is implementing C style casting ((uint32_t) (first_col_sum % MODE)) which is not as type safe as either static_cast(VALUE_TO_BE_CAST) or dynamic_cast<NEW_TYPE>(EXPRESSION). In this case I recommend static_cast.
Static Functions
It is not clear why the checkRecord() function was changed to a static member function since the original prototype was int checkRecord(int n) in the question. There really is no need for it to be static.

By declaring a function member as static, you make it independent of any particular object of the class. A static member function can be called even if no objects of the class exist and the static functions are accessed using only the class name and the scope resolution operator.

Variable Names
The symbolic constant names are clear as are the variable names adj_matrix (adjacency would be clearer), accumulated and adj_matrix_pointer, but the variable names a, b and temp are not clear and leave someone who has to read or maintain the code guessing.
Complexity
The function checkRecord() is too complex (does too much). After the if statement there should be a call to a function to execute the rest of the code, and that function might cal sub functions. Remember that it is much easier to write correct code when you limit what a function is responsible for.
There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.


C++11 version of Windows EventWaitHandle I don't know C++ at all, but for the heck of it I tossed this together because I was talking with someone about C++ and was given the impression there is absolutely nothing quite like the windows APIs ManualReset and AutoReset EventWaitHandles, so I figured I'd see if I could create one.

I think this might work - it compiles and runs but I don't really know well enough how to make a thread to test it's multi-threading off hand in C++ and don't care to try figuring it out. Figure folks here can probably tell me if I got any of the threading primitives usage completely off or have some harrible race conditions or some such.

#include <iostream>
#include <atomic>
#include <thread>
#include <mutex>
#include <condition_variable>
#include <chrono>

using namespace std;

class EventWaitHandle {

public:
    EventWaitHandle(bool handleWillCloseAfterOneWait, bool openAtStart)
        : autoReset(handleWillCloseAfterOneWait), handleOpen(openAtStart)
    { }

    void set() {
        {
            std::lock_guard<std::mutex> handleOpenLock(handleOpenLocker);
            handleOpen = true;
        }

        handleOpenSignal.notify_one();
    }

    void reset() {
        {
            std::lock_guard<std::mutex> handleOpenLock(handleOpenLocker);
            handleOpen = false;
        }

        handleOpenSignal.notify_one();
    }

    bool wait(int millisecondsBeforeTimeout) {
        {
            std::lock_guard<std::mutex> handleOpenLock(handleOpenLocker);

            if (handleOpen) {
                handleOpen = !autoReset;
                return true;
            }

            if (millisecondsBeforeTimeout < 1) // Handle isn't open and we are waiting no time, punt..
                return false;

        }

        auto start = std::chrono::system_clock::now();
        cout << "waiting.." << endl;
        std::unique_lock<std::mutex> lockJustForThisWaiter(handleOpenLocker);

        if(handleOpenSignal.wait_until(lockJustForThisWaiter, start + std::chrono::milliseconds(millisecondsBeforeTimeout)) == std::cv_status::timeout)
            return false;

        auto timeLeftToWaitInEventOfSpuriousness = std::chrono::milliseconds(millisecondsBeforeTimeout) - (std::chrono::system_clock::now() - start);
        int millisecondsLeftToWait = 0;
        millisecondsLeftToWait = std::chrono::duration_cast<std::chrono::milliseconds>(timeLeftToWaitInEventOfSpuriousness).count();

        return wait(millisecondsLeftToWait);
    }

private:

    bool autoReset;

    std::condition_variable handleOpenSignal;
    std::atomic<bool> handleOpen;
    std::mutex handleOpenLocker;
};

That's what I want review of, this is just example usage concept:

int main(){
    EventWaitHandle waitHandle(true, true);

    bool open;

    open = waitHandle.wait(100);
    if (open)
        cout << "open" << endl;
    else
        cout << "not open" << endl;

    waitHandle.set();

    open = waitHandle.wait(100);
    if (open)
        cout << "open" << endl;
    else
        cout << "not open" << endl;

    waitHandle.reset();

    open = waitHandle.wait(100);
    if (open)
        cout << "open" << endl;
    else
        cout << "not open" << endl;

    return 0;
}

Despite the good odds of a tail-call optimisation, your use of recursion seems unidiomatic and sets off alarm bells in my head. Though they are largely unfounded in a practical sense, I'd say that's enough of a warning to just perform loop-waiting with an actual loop, instead.

You could do with some documenting comments, as it's not clear to a non-WinAPI person what this class actually does.

Some of your code lines are too long to read comfortably.

I can't spot any actual bugs though.


Testing Promises vs Lock performance I'm not sure if my lock usage is correct and safe. I wanted to know what will be best approach to deal with situation when one thread have to wait for being initialized by another so I written this test:

import java.util.concurrent.locks.ReentrantLock
import java.util.{TimerTask, Timer, Date}
import scala.concurrent.{ExecutionContext, Future, Await}
import scala.concurrent.duration.Duration

class I{
  private var i:Int = 0
  def inc() = i+=1
  def value = i
}

trait Test {
  def doSomething():Unit
  def initialize(i:I):Unit
  def result:Int
}

case class TestLocks() extends Test{
  var ref:I = null

  val lock = new ReentrantLock
  val refInitialized = lock.newCondition()

  def doSomething(){
    lock.lock()
    try {
      if(ref==null) { refInitialized.await() }
      ref.inc()
    }
    finally {
      lock.unlock()
    }
  }

  def initialize(i:I){
    lock.lock()
    try{
      ref = i
      refInitialized.signal()
    }
    finally {
      lock.unlock()
    }
  }

  def result = ref.value
}

case class TestPromises() extends Test{
  val iPromise = concurrent.promise[I]
  def doSomething(){
    val i = Await.result(iPromise.future,Duration.Inf)
    i.inc()
  }

  def initialize(i:I){
    iPromise.success(i)
  }

  def result = Await.result(iPromise.future,Duration.Inf).value
}

object TestRunner{
  def apply(test:Test):Long = {

    val initializationTimeout = 10
    val timer = new Timer()
    timer.schedule(new TimerTask {
      def run(){
        test.initialize(new I)
      }
    },initializationTimeout)

    val start = new Date().getTime

    1 to 1000000 foreach (i=>{
      test.doSomething()
    })
    val end = new Date().getTime

    val duration = end-start-initializationTimeout
//    Console.println(test+" result: "+test.result+" in: "+duration+"ms")
    duration
  }
}

object Tests {
  def run() {

    def average(l: => Long) =
    {
      val vals = 1 to 100 map(_=>{l.toDouble})
      vals.sum/vals.length
    }

    def averageAsync(l: => Long) =
    {
      import ExecutionContext.Implicits.global
      val futs = 1 to 100 map(_=>{Future{l.toDouble}})
      val valsFut = Future.sequence(futs)
      val vals = Await.result(valsFut,Duration.Inf)
      vals.sum/vals.length
    }

    Console.println("Locks: "+average(TestRunner(TestLocks()))+"ms")
    Console.println("Promises: "+average(TestRunner(TestPromises()))+"ms")

    Console.println("A Locks: "+averageAsync(TestRunner(TestLocks()))+"ms")
    Console.println("A Promises: "+averageAsync(TestRunner(TestPromises()))+"ms")
  }
}

which outputs:

  Locks: 28.0ms
  Promises: 68.5ms
  A Locks: 49.87ms
  A Promises: 2307.84ms

what indicates that I have to use locks in "hot zones" and now I want to ensure if those tests are correct. Or maybe I should handle this "wait if uninitialized" case in any other way?

A completely different approach to your answer is a concept called Actors.

Rather than having multiple threads, and having these threads await each other, Actors are like tiny little processes that communicate with each other by sending messages. When you're done with a piece of work, you can send a message to another actor, telling it to go and do something else.

The Actor Model as it's known is both highly performant and simple to reason about.

Scala is fortunate enough to have an incredible implementation of the Actor Model known as Akka.

Akka allows you to very simply build large and complex systems - it can even handle distribution across many machines!

Here's a really simple example from the Akka website:

case class Greeting(who: String)

class GreetingActor extends Actor with ActorLogging {
  def receive = {
    case Greeting(who) ⇒ log.info("Hello " + who)
  }
}

val system = ActorSystem("MySystem")
val greeter = system.actorOf(Props[GreetingActor], name = "greeter")
greeter ! Greeting("Charlie Parker")

How easy is that? :-)

I strongly recommend using the Actor Model instead of using conventional threads and locks. Scala provides us with an incredibly powerful toolkit, I encourage you to use it!


Project Euler no. 17: Counting letters to write the numbers from 1 to 1000 I'm very new to programming and am admittedly embarrassed sharing my code for critique. This code works and produces the correct answer to the Project Euler Problem 17, which asks how many letters there are when the numbers from one to one thousand are written out in words (excluding spaces and punctuation).

I was hoping I could get some critique on how to make it less ugly and more streamlined, or even approach the problem from a totally new angle so I can better learn from this solution.

# the final list that holds the string lengths
addList = []

# dictionary holding integer:corresponding word pairs
numbersDict = {
0:"zero",
1:"one",
2:"two",
3:"three",
4:"four",
5:"five",
6:"six",
7:"seven",
8:"eight",
9:"nine",
10:"ten",
11:"eleven",
12:"twelve",
13:"thirteen",
14:"fourteen",
15:"fifteen",
16:"sixteen",
17:"seventeen",
18:"eighteen",
19:"nineteen",
20:"twenty",
30:"thirty",
40:"forty",
50:"fifty",
60:"sixty",
70:"seventy",
80:"eighty",
90:"ninety"
}

### There has to be an easier way to do all this below ###

def numberLetters(num):

    letters = ""

    if 0 < num <= 20:
        letters += numbersDict[num]

    if 21 <= num <= 99:
        a,b = divmod(num, 10)
        if b == 0:
            letters += numbersDict[a*10]
        else:
            letters += numbersDict[a*10] + numbersDict[b]

    if 100 <= num <= 999:
        if num % 100 == 0:
            letters += numbersDict[int(num / 100)] + "hundred"
        else:
            digit = int(num / 100)
            num = num - digit * 100
            if 0 < num <= 20:
                letters += numbersDict[digit] + "hundredand" + numbersDict[num]
            if 21 <= num <= 99:
                a,b = divmod(num, 10)
                if b == 0:
                    letters += numbersDict[digit] + "hundredand" + numbersDict[a*10]
                else:
                    letters += numbersDict[digit] + "hundredand" + numbersDict[a*10] + numbersDict[b]
    if num == 1000:
        letters += "onethousand"

    return letters

for i in range(1,1001):
    addList.append(len(numberLetters(i)))
print(sum(addList))

Aside from the repetitive logic in the code to handle numbers greater than a hundred, the solution is not bad.  There are many observations that could lead to improvements, though.

You don't need to build the strings themselves; you just need to sum their lengths.  In particular, since strings are immutable, += is relatively costly.  (Your solution is still fast enough, though.)
The challenge never requires you to treat 0 as "zero".  You can therefore simplify some logic by mapping 0 to an empty string.
The ranges 0 < num <= 20, 21 <= num <= 99, 100 <= num <= 999, and num == 1000 are mutually exclusive.
You can reduce repetition by using recursion.

Solution

numbers_dict = {
    n: len(word) for (n, word) in {
        0: "",
        1: "one",
        2: "two",
        3: "three",
        4: "four",
        5: "five",
        6: "six",
        7: "seven",
        8: "eight",
        9: "nine",
        10: "ten",
        11: "eleven",
        # ...
        90: "ninety",
    }.iteritems()
}

def num_length(n):
    if n <= 20:
        return numbers_dict[n]
    elif n < 100:
        ones = divmod(n, 10)
        return numbers_dict[10 * tens] + num_length(rest)
    elif n < 1000:
        hundreds, rest = divmod(n, 100)
        return num_length(hundreds) + len("hundred") + (
            len("and") + num_length(rest) if rest else 0
        )
    else:
        thousands, rest = divmod(n, 1000)
        return num_length(thousands) + len("thousand") + num_length(rest)

print(sum(num_length(n) for n in range(1, 1001)))

You could also use numbers_dict for memoization:

def num_length(n):
    if n in numbers_dict:
        return numbers_dict[n]
    elif n < 100:
        tens, rest = divmod(n, 10)
        numbers_dict[n] = numbers_dict[10 * tens] + num_length(rest)
        return numbers_dict[n]
    elif n < 1000:
        hundreds, rest = divmod(n, 100)
        return num_length(hundreds) + len("hundred") + (
            len("and") + num_length(rest) if rest else 0
        )
    else:
        thousands, rest = divmod(n, 1000)
        return num_length(thousands) + len("thousand") + num_length(rest)


Summing a sequence of fractions to a limit n Your method should accept a real number as a parameter representing a limit, and should add and print terms of the sequence until the sum of terms meets or exceeds that limit. For example, if your method is passed 2.0, print terms until the sum of those terms is at >= 2.0. You should round your answer to 3 digits past the decimal point.

    The following is the output from the call sequenceSum(2.0);
    1 + 1/2 + 1/3 + 1/4 = 2.083

  If your method is passed a value less than 1.0, no output should be produced. You must match the output format shown exactly; note the spaces and pluses separating neighboring terms.

I'm wondering if there's anything I can do for the edge case of n = 1? It kinda just seems like it's hanging out at the end, and it looks a bit strange to me. Also, looking at the types for each value, I'm wondering if I need to think more about the data types I chose to use, in terms of should I use a double for this, or an int?.

I'd use methods to break out the functionality of my code, but the prompt only allows me to submit one method for this type of problem. Any feedback is appreciated!

public static void sequenceSum(double n){
    if(n > 1.0){
        int denominator = 1;
        double accumulator = 0;
        System.out.print("1");
        while(accumulator < n) {
            accumulator += ( 1.0 / denominator);
            if(denominator > 1.0){
                System.out.print(" + " + 1 + "/" + denominator);
            }
        denominator++;
        }
    System.out.printf(" = %1.3f", accumulator);
    } else if(n == 1.0){
        System.out.print("1 = 1.000");
    }
}

I'm wondering if there's anything I can do for the edge case of n = 1?

Yes, I think you are considering it to much (see below: you can simply remove some branching)

  Also, looking at the types for each value, I'm wondering if I need to think more about the data types I chose to use, in terms of should I use a double for this, or an int?.

I think your current choice is fine: int for the denominator and double for the accumulator.

Following your program (my comments adress the previous block of code):

public static void sequenceSum(double n){
    if(n > 1.0){

I think you can simply remove this first condition (already well managed in the while loop)

        int denominator = 1;
        double accumulator = 0;
        System.out.print("1");
        while(accumulator < n) {
            accumulator += ( 1.0 / denominator);
            if(denominator > 1.0){

Your denominator is an int: you should compare it with 1 (and not 1.0)

                System.out.print(" + " + 1 + "/" + denominator);

You should integrate the 1 into the string: " + 1/".

            }
        denominator++;
        }
    System.out.printf(" = %1.3f", accumulator);
    } else if(n == 1.0){

This branch can simply be replaced with a condition at the very beginning to check if n < 1.0 and immediately return if it is the case.

        System.out.print("1 = 1.000");
    }
}

Miscellaneous

Instead of printing each time, you could append to a StringBuffer

You have good variable names, except for n. Maybe sumLimit is more appropriate? (or accumulatorLimit).

Simplifying > 1 conditions

You could start your denominator at 2 and your accumulator at 1.0, to get rid of the if condition inside your while loop (if n is 1.0, then the while loop will simply not be executed at all).


Filtering a phone number I am learning Scala still and refactoring some old code.  I was hoping the community here could give me pointers to fix this code up since there are some issues I have with it visually, but don't know if there's an elegant solution for my vision.

class PhoneNumber(phonenumber:String) {
  def number: String = {
    phonenumber.filter(_.isDigit) match {
      case x if x.startsWith("1") && x.length == 11 => x.substring(1)
      case x if x.length != 10 => "0000000000"
      case x => x
    }
  }

  def areaCode: String = number.take(3)

  override def toString: String =
    "(" + areaCode + ") " + number.substring(3,6) + "-" + number.takeRight(4)
}

I want to refactor my case statement as much as possible because right now I think it looks kind of strange.  One thing I am particularly not fond of is the case x => x, but I am wondering what others think about my approach in general and what I might do to make this code better visually and functionally.

One of the main changes you'll probably want to make is to change number and areaCode from functions to values. As your code is right now, every time you use number it is having to recompute your top code block. The same goes for areaCode.

At first glance I don't see a reasonable way to eliminate the conditional statements that you step though to calculate number. Because of that, there isn't a great way to utilize pattern matching. So IMO you may as well take advantage of the fact that conditional statements are expressions in Scala.

Another small tweak I made in calculating number was to change "0000000000" to "0" * 10. Its not so hard to count out ten zeros, but if you needed a larger amount of them you now know about this other method.

I added class fields for the other subsections of a typical phone number, e.g. exchangeCode and stationNumber.

Finally I utilized string interpolation for your toString method as to my mind it makes the expression easier to mentally parse.

class PhoneNumber(phoneNumber: String) {
  val number = {
    val tmpNumb = phoneNumber.filter(_.isDigit)
    if (tmpNumb.startsWith("1") && tmpNumb.length == 11) tmpNumb.drop(1)
    else if (tmpNumb.length != 10) "0" * 10
    else tmpNumb
  }

  val areaCode      = number.take(3)
  val exchangeCode  = number.drop(3).take(3)
  val stationNumber = number.drop(6).take(4)

  override def toString = s"($areaCode) $exchangeCode-$stationNumber"
}