Dependency injection with reading a file I implemented this code, looking at a dependency injection program as an example. I wanted to confirm with you all whether this implementation is the right way of doing the same.

namespace TestAsyncProjectWithDI
    {
            public interface IReadFileTypes
            {
                    void ReadUsingFileSteamAsync();
            }
            public class ReadfileFromSystem:   IReadFileTypes
            {
                private readonly StreamReader _reader =null;

                public ReadfileFromSystem(StreamReader reader)
                {
                    _reader = reader;
                }
                public async void ReadUsingFileSteamAsync()
                {
                    await Task.Run(()=> Console.WriteLine("Reading the content of File: {0}", _reader.ReadToEnd()));
                    Console.ReadLine();
                }
          }

        public class Builder
        {
                public  IReadFileTypes ReadFileTypes { get; set; }
                private string path = "C:\\Brackets\\Projects\\index2.html";

                public Builder()
                {
                    ReadFileTypes = new ReadfileFromSystem(new StreamReader(new FileStream(path, FileMode.Open, FileAccess.Read)));
                }
        }

        public class Program
        {
                private readonly IReadFileTypes _readFileTypes = null;

                public Program(IReadFileTypes readFileTypes)
                {
                    this._readFileTypes = readFileTypes;
                }
                public void FunctionInvoke()
                {
                    _readFileTypes.ReadUsingFileSteamAsync();
                }
        }

        //starting point of the class
        public class Default
        {
            static void Main(string[] args)
            {
                var build = new Builder();
                var obj = new Program(build.ReadFileTypes);
                obj.FunctionInvoke();
                Console.ReadLine();
            }
        }
    }

You have implemented dependency injection correctly. IReadFileTypes is injected into Program. In terms of the correctness of your example however, pretty much everything else is completely wrong! Don't worry though, we all have to start somewhere, so I will offer some tips. These tips might help you with this example, but if you want to improve, you should probably do some personal training/studying. Pluralsight is a really nice place to start. They have some beginner C# courses that you can do with a free trial. Anyway, on with the code review!

Streams implement IDisposable, and should always be disposed of when used. Typically this is done with using block.

Builder is not a builder, it is a factory. It's job is to create instances of IReadFileTypes. This shouldn't be done in it's constructor because then you need a new factory, each time you want a new IReadFileTypes.

async void should almost always be avoided. Try to return Tasks from async methods so they can be awaited by the caller. A Task should be returned from ReadUsingFileSteamAsync() and have it's Wait() method called.

The point of an interface is to abstract away the implementation details from the client. IReadFileTypes has the method ReadUsingFileSteamAsync() which publicises exactly what the implementation is. This is called a leaky abstraction. When writing an interface, think about what other implementations might look like. One thing that you do know about all implementations is that they are going to need a file path. An alternative interface might then look something like this

public interface IFilePrinter
{
    Task PrintFileAsync(string filePath);
}

You could imagine this interface being implemented by e.g. ColoredFilePrinter, VerboseFilePrinter e.t.c.

ReadfileFromSystem is a bit nuts! You have wrapped _reader.ReadToEnd() in a Task.Run() and awaited that. Instead you could just await _reader.ReadToEndAsync().

Putting all of this together, your example could look something like

public class Default
{
    static void Main()
    {
        var printerFactory = new FilePrinterFactory();
        var filePrinter = printerFactory.CreateDefaultPrinter();

        var program = new Program(filePrinter);
        program.DoStuff();

        Console.ReadLine();
    }
}

public interface IFilePrinter
{
    Task PintFileAsync(string filePath);
}

internal class DefaultFilePrinter : IFilePrinter
{
    public async Task PintFileAsync(string filePath)
    {
        using (var reader = File.OpenText(filePath))
        {
            var fileContent = await reader.ReadToEndAsync();
            Console.WriteLine("File content: " + fileContent);
        }
    }
}

public class FilePrinterFactory
{
    public IFilePrinter CreateDefaultPrinter()
    {
        return new DefaultFilePrinter();
    }

    public IFilePrinter CreateSpellCheckingPrinter(string language)
    {
        throw new NotImplementedException();
    }
}

public class Program
{
    private const string FilePath = @"C:\Brackets\Projects\index2.html";
    private readonly IFilePrinter _filePrinter;

    public Program(IFilePrinter filePrinter)
    {
        // I don't care what kind of printer I have, I just know that I need to print a file!
        _filePrinter = filePrinter;
    }

    public void DoStuff()
    {
        _filePrinter.PintFileAsync(FilePath).Wait();

        // TODO: Do more stuff
    }
}

PS: In case you are interested, you can do the whole thing in 1 line of code

Console.WriteLine("File content: " + File.ReadAllText(@"C:\Brackets\Projects\index2.html"));


C++ Heap implementation I'm on my way of understanding basic data structures and sorting algorithms through implementing them in c++. Post is followed of my implementation of Vector. I decided to weld the heap behaviour right into Vector class, although I'm not sure if it's a good solution/practice.
The heapSort method is not yet implemented. I'm not figuring out how I can build it based on already existing methods.
As at my office heavily criticising is an accepted practice, I'm asking for
rather hard, destructive critics and review.

#include "Vector.hpp"

Vector::Vector(int size):size(size) {
    array = new int[size]();
}

Vector::Vector(int vector[], int sizeT) {
    this->array = new int[sizeT]();
    this->size = sizeT;
    for(int i = 0; i < sizeT; ++i) {
        this->array[i] = vector[i];
    }
}

Vector::Vector(Vector* v1) {
    this->array = v1->array;
    this->size = v1->size;
}

Vector::~Vector() {
    delete[] array;
}

int Vector::length() {
    return this->size;
}

int Vector::getAt(int idx) {
    if(idx >= this->size) {
        throw "out of range";
    }
    return this->array[idx];
}

int& Vector::operator[](const int index){
    if(index >= this->size) {
        throw "out of range";
    }

    return this->array[index];
}

void Vector::deteleAt(int idx) {
    for(int i = idx; i < this->length() - 1; ++i) {
        this->array[i] = this->array[i+1];
    }
    --this->size;
}

//HEAP--------PRIORITY QUEUE
int Vector::parent(int idx) {
    if(idx >= 0 && idx <= 2) {
        return this->array[0];
    }

    return this->array[idx/2];
}

bool Vector::hasLeftChild(int idx) {
    return this->leftChild(idx) < this->size;
}

bool Vector::hasRightChild(int idx) {
    return this->rightChild(idx) < this->size;
}

void Vector::buildMaxHeap() {
    for(int i = this->size / 2; i >= 0; i--) {
        this->maxHeapify(i);
    }
}

void Vector::maxHeapify(int idx) {
    if(!this->hasRightChild(idx) && !this->hasLeftChild(idx)) {
        return;
    }

    int l = this->leftChild(idx), r = this->rightChild(idx);
    int largest = idx;

    if(l < this->size && this->array[l] > this->array[idx]) {
        largest = l;
    }

    if(r < this->size && this->array[r] > this->array[largest]) {
        largest = r;
    }

    if(largest != idx) {
        this->swap(idx, largest);
        this->maxHeapify(largest);
    }
}

void Vector::heapSort() {
    for(int i = this->heapSize; i >= 1; i--) {
        this->swap(0, i);
        this->heapSize--;
        this->maxHeapify(0);
    }
}

void Vector::swap(int firstIdx, int secondIdx) {
    int rotator = this->array[firstIdx];
    this->array[firstIdx] = this->array[secondIdx];
    this->array[secondIdx] = rotator;
}

int Vector::leftChild(int idx) {
    idx = idx * 2;
    return idx;
}

int Vector::rightChild(int idx) {
    idx = (idx * 2) + 1;

    return idx;
}

void Vector::print(){
    for(int i = 0; i < this->size; ++i) {
        std::cout<<this->array[i]<<" ";
    }
    std::cout<<"\n";
}

int Vector::popFront(){
    int firstElem = this->array[0];
    this->deteleAt(0);
    return firstElem;
}

I'm asking for rather hard, destructive critics and review.

Sure.

Interface.

I don't see the interface (ie class definition). So it becomes harder to review.

Things I don't see

I don't see any move semantics for your class.

Normally (since C++11) I would expect containers to implement move semantics. i.e. I would expect to see the following methods:

 Vector(Vector&& move) nothrow
 Vector& operator=(Vector&& move) nothrow
 void swap(Vector& other) nothrow

Also I don't see your code implementing the rule of three. This means that your objects will not work correctly if there are any copies made (and C++ makes copies all the time).

To implement the rule of three you need to implement the following three methods:

Vector(Vector const&);
Vector& operator=(Vector const&);
~Vector();

Code Review

This is a dangerous interface. you are asking the user of your class to know and manually set the size. An important thing in C++ is to try and define interfaces that can not be used incorrectly.

Vector::Vector(int vector[], int sizeT) {

I prefer the iterator version of the interface. Yes it is still just as dangerous but the utility functions add in C++14 will help you correctly identify the dangers and potentially generate compiler errors.

template<typename T>
Vector::Vector(I begin, I end) {

Now when you use it.

int data[23];
Vector   v(std::begin(data), std::end(data)); // will correctly size the vector
                                              // even if the size is changed.

void test(int* data, std::size_t s)
{
    Vector v(data, data + s); // still works (you just need to add).
}

int*     data = new data[23];
Vector   v(std::begin(data), std::end(data));  // will fail to compile
                                               // For the dangerous
                                               // situations it fails to compile.
                                               // which makes you look.

Use of this-> is discouraged

I consider the use of this-> dangerous as it hides errors. It also shows that you are being lazy in naming your local/member variables.

I know you are going to ask "how does it hide errors?".

The only reason that you need to use this-> is when you have shadowed member variables. You then use this-> to differentiate the member variables from the local variables. But it requires that everybody correctly identify the member/local variable. Any mistake will still silently compile. So this type of error can not be detected by the compiler.

On the other hand if you don't allow shadowed variables (and you can make the compiler detect shadowed variables and fail your compilation) now you have no situation where dropping the this-> (or forgetting) it will NOT cause an error. Since it does not matter whether the this-> is there or not why not just drop it.

This is a funny Constructor.

Not even sure why you would have this interface.

Vector::Vector(Vector* v1) {

You normally have a copy constructor which looks like this:

Vector::Vector(Vector const& v1) {

But an interface that takes pointers is very rare (and for this type of situation is practically never done). You are leaking ownership information. Who ownes the pointer you pass? (Ownership semantics is very important in C++ as the owner of the object is the person responsible for deleting it. A pointer has no ownership information).

Error:

Additionally this interface is broken.

Vector::Vector(Vector* v1) {
    this->array = v1->array;
    this->size = v1->size;
}

Consider this situation:

{
    Vector   a(5);
    Vector   b(&a);   // This is your copy (but the same would apply if
                      // you naively converted it to a standard copy
                      // constructor)
}

In the above case both object point at the same dynamically created object. Not a problem in itself. BUT they both think they own the object and thus will both delete it in there destructor this is illegal.

Const methods

A method that does not change the state of the object should be marked as const.

int Vector::length() {
    return this->size;
}

This is both an indication to the compiler and also allows the object to be used in a context where you only have a const reference (quite common).

Exceptions

Yes you can throw anything. But normally you would throw an exception object that is derived from std::runtime_error.

In your code you are throwing an object of type char const*.

int Vector::getAt(int idx) {
    if(idx >= this->size) {
        throw "out of range";
    }
    return this->array[idx];
}

I would do it like this:

int Vector::getAt(int idx) {
    if(idx >= this->size) {
        throw std::out_of_range("Vector::at()");
    }
    return this->array[idx];
}

Don't do checks for the user.

Don't do uneeded checks for the user. Rather provide methods so that users can perform their own checks to validate there calls. You can provide a checked version of the interface as well.

The std::vector has two techniques for access elemements (just like you). operator[]() and at(). The only difference between the two is that at() validates the index and operator[]() does not.

int& Vector::operator[](const int index){

    // Would not expect this check.
    if(index >= this->size) {
        throw "Vector::out of range";
    }

    return this->array[index];
}

The reason is that in most use cases you don't need to validate the range because you have already validated that your index is within the required range:

for(int loop = 0;loop < v.size(); ++loop) {
    // No need to check the range of `loop`
    // You have guranteed it is correct by the context it is used in.
    // Thus we use the unchecked range operator.
    std::cout << v[loop] << "\n";
}

Do check the range where it is important.

Here I would check the range.

void Vector::deteleAt(int idx) {
    for(int i = idx; i < this->length() - 1; ++i) {
        this->array[i] = this->array[i+1];
    }
    --this->size;
}

The line --this->size; is unchecked. If I delete a non existent element then it still reduces the size. Probably not what you want.

Looks Fishy

This is a very standard pattern for parent and children in a heap.

//HEAP--------PRIORITY QUEUE
int Vector::parent(int idx) {
    if(idx >= 0 && idx <= 2) {
        return this->array[0];
    }

    return this->array[idx/2];
}

The fact that you are special casing 1 and 2 makes me suspicious that there is underlying error you are trying to correct for.

Should not the parent be:

parent = (child-1)/2    //   0   1   2
leftC  = parent*2 + 1   //   1   3   5
rightC = parent*2 + 2   //   2   4   6

Use standard algorithms where you can

void Vector::swap(int firstIdx, int secondIdx) {
    int rotator = this->array[firstIdx];
    this->array[firstIdx] = this->array[secondIdx];
    this->array[secondIdx] = rotator;
}

There is a std::swap() this will swap two elements in the most efficient way possible. For integers your swap is probably the most efficient. But if you change what is held by your vector to some other complex object this may no longer be the most efficient technique (think move semantics).

Printing

When having a printing function, what you are actually doing is writing to a stream. So why not provide the opportunity to use different streams. So add a parameter that allows you to specify the stream (this can default to std::cout.

void Vector::print(){
    for(int i = 0; i < this->size; ++i) {
        std::cout<<this->array[i]<<" ";
    }
    std::cout<<"\n";
}

Also the default way of printing something in C++ is to use the output operator operator<< so you may as welll write that so that it uses your print function.

std::ostream& operator<<(std::ostream& s, Vector const& output)
{
     output.print(s);
     return s;
}


Checking authorization by role Just wondering if my

else {return false;}

statements are superfluous... do I just need one return true; here?

function has_access() {
    if ( is_user_logged_in() ) {
        $role = get_current_user_role();
        $admins = array(
            'Administrator',
            'Agent',
            'Contributor',
        );

        if (in_array($role, $admins)) {
            return true;
        } else {
            return false;
        }
    } else {
        return false;
    }
}

I'd replace the conditions with guard clauses. (Flattening Arrow Code)

function has_access() {
    if ( !is_user_logged_in() ) {
        return false;
    }
    $role = get_current_user_role();
    $admins = array(
        'Administrator',
        'Agent',
        'Contributor',
    );

    if (in_array($role, $admins)) {
        return true;
    }
    return false;
}


Creating a tuple from a CSV file I have written code that reads in a CSV file and creates a tuple from all the items in a group. The group ID is in column 1 of the table and the item name is in column 2. The actual datafile is ~500M rows.

Is there any way to make this code more efficient?

Input file:

"CustID"|"Event"
1|Alpha
1|Beta
1|AlphaWord
1|Delta
2|Beta
2|Charlie
2|CharlieSay

Code:

def sequencer(myfile):
    import csv
    counter = 1
    seq = []
    sequence = []
    with open(myfile, 'rb') as csvfile:
        fileread = csv.reader(csvfile, delimiter='|', quotechar='"')
        next(fileread) ## skip header
        for row in fileread:
            #if counter == 5:
            #    break
            if 'word' in row[1] or 'say' in row[1]: ##if event has either word or say anywhere in the text then ignore (eg: even ignore adword or afdjklSAYwer)
                continue
            if int(row[0]) == counter:
                seq.extend([row[1]])
            else:
                sequence.append(seq)
                seq = [row[1]]
                counter = counter+1
        sequence.append(seq)
    return sequence

Output:

An array which is a list of lists, where each list is the sequence of events in the order of the text file for each customer ID.

The problem is that a list made from the rows of a file with ~500M rows will have millions of entries, and that’s going to really hamper Python. As long as you’re using a list, I don’t think there’s any magic you can do here to magically fix that problem.

So you should ask: do I need a list? You may be able to make to do with a generator, which is much faster and more memory efficient (because it only computes one element at a time, not precomputing everything).

For a generator, I’m assuming that the customer IDs are monotonically increasing. If not, you really do have to go through the entire file to be sure you’ve got everything for each group. But your existing code seems to assume that, so I’ll assume I can as well.

Here’s a slight reworking of your code to use generators:

import csv

def sequencer(myfile):
    """
    Reads customer IDs and events from a CSV file.  Yields events for
    successive groups.
    """
    with open(myfile, 'rb') as csvfile:
        reader = csv.reader(csvfile, delimiter='|', quotechar='"')
        next(reader) ## skip header

        current_cust_id = 1
        seq = []
        for row in reader:
            cust_id, event = row

            if ('word' in event) or ('say' in event):
                continue

            while int(cust_id) != current_cust_id:
                yield seq
                seq = []
                current_cust_id += 1
            seq.append(event)

        yield seq

I’ve done away with the massive list (sequence), and we just yield the list of per-customer events whenever the customer ID changes.

A few other comments on your code:

Put your imports at the top of your file; don’t hide them inside a function.
There isn’t much in the way of documentation. Your function should at least have a docstring, and some more comments would be nice. For example, tell me why we’re skipping lines with “Say” or “Word”. I can read that the code is doing it, but I can’t read your mind to find out why.
You were assuming that customer IDs increase by 1 every time, and that there’s never going to be a gap. What happens if you run your code against:

"CustID"|"Event"
1|Alpha
1|Beta
1|AlphaWord
1|Delta
2|Beta
2|Charlie
2|CharlieSay
5|Echo
5|Delta

Maybe you know that will be the case; I wasn’t sure, so my code is defensive against this possibility.
Rather than doing seq.extend([row[1]]), it’s much cleaner to do seq.append(row[1]). It saves you creating an additional list.


Minimal Python plugin mechanism Is there a better way to have a minimal Python plugin mechanism than the following?

(This was inspired from this post.)

import sys
import pkgutil
import os.path
import plugins
pluginsPath = os.path.dirname(plugins.__file__)
pluginModules = [name for _, name, _ in pkgutil.iter_modules([pluginsPath])]
dictOfPlugins = {}
for plugin in pluginModules:
    thePluginModuleName = "plugins."+plugin
    result = __import__(thePluginModuleName)
    dictOfPlugins[plugin] = sys.modules[thePluginModuleName]

(This does assume that all your plugins are in a directory called "plugins" and thus are modules in that package, and that that directory has a blank __init__.py file in it.)

If you wanted to look for a particular plugin, the name would be a key in the dictOfPlugins, and the module object itself would be the value. Assuming you knew what the interface to your Python plugin modules would be, this would seem to do the job. Is there a better way to do this?

Check out how Django apps work:

Plugins are genuine Python packages, so the standard tools can be used to install them
Plugins that are actually used must be in the PYTHONPATH and listed (only the package name) in the configuration.
There are scripts to help create, test, ... plugins
The plugin structure is extensible in all possible ways.
Plugins are reloaded when they change in development mode.

I'm not saying that you should use Django if you're not doing web development, but you can inspire your plugin system on it.


Quadratic equation solver I started Python 2-3 months ago, but I really haven't done much since then, just little things. Yesterday, I wrote this simple quadratic equation solver. You can give \$a, b\$ and \$c\$ any value, but they must all be integers.

I really want to know what you think about it.

from math import sqrt

while True:
        try:
                a = int(input("\nEnter a : "))
                b = int(input("Enter b : "))
                c = int(input("Enter c : "))
        except:
                print("You can only enter integers")
                continue
        if a > 0:
            sgn_a = ""
            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"
        else:
            sgn_a = "-"
            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"

        if a == 0:
            part_a = ""
        elif a == 1:
            part_a = sgn_a+"x"+str(chr(0x00B2))
        else:
            part_a = sgn_a+str(abs(a))+"x"+str(chr(0x00B2))

        if b == 0:
            part_b = ""
        elif b == 1:
            if a != 0:
                part_b = sgn_b+"x"
            else:
                part_b = "x"
        else:
            if a != 0:
                part_b = sgn_b+str(abs(b))+"x"
            else:
                part_b = str(abs(b))+"x"

        if c == 0:
            if a == 0 and b == 0:
                part_c = "0"
            else:
                part_c = ""
        elif a == 0 and b == 0:
            part_c = str(abs(c))
        else:
            part_c = sgn_c+str(abs(c))

        print("\nYour equation is %s%s%s = 0\n" %(part_a,part_b,part_c))
        if a != 0 and b != 0 and c != 0:
                D = b**2-(4*a*c)
                if D > 0:
                        x_1 = ((-b+sqrt(D))/2*a)
                        x_2 = ((-b-sqrt(D))/2*a)
                        print("X1 = %f\nX2 = %f" %(x_1,x_2))
                elif D == 0:
                        x = -b/(2*a)
                        print("X1 = X2 = %d" %x)
                elif D < 0:
                        x_1 = ((-b+sqrt(-D))/2*a)
                        x_2 = ((-b-sqrt(-D))/2*a)
                        print("X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i"))
        elif a == 0 and b != 0 and c != 0:
                x = -c/b
                if c % b != 0:
                        print("X = %d/%d or %f" %(-c,b,x))
                else:
                        print("X = %d" %x)
        elif a != 0 and b == 0 and c != 0:
                if -c > 0:
                        x_1 = sqrt(-c/a)
                        x_2 = -x_1
                        print("X1 = %f\nX2 = %f" %(x_1,x_2))
                else:
                        x_1 = sqrt(c/a)
                        x_2 = -x_1
                        print("X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i"))
        elif a != 0 and b != 0 and c == 0:
                x_2 = -b/a
                if b % a != 0:
                        print("X1 = 0\nX2 = %d/%d or %f" %(-b,a,x_2))
                else:
                        print("X1 = 0\nX2 = %d" %x_2)
        elif a == 0 and b != 0 and c == 0:
                        print("X = 0")
        elif a == 0 and b == 0:
                if c != 0:
                        print(c,"!= 0")
                else:
                        print("0 = 0. DUH!")

Do not do use bare except

Your execpt catches all exceptions including KeyboardInterrupt. Once the program is launched, there is no way (as far as I can see) to stop it.

If you are interested in failed conversions, you should catch ValueError.

Bug (solutions)

Using these values :

        a = int("0") # int(input("\nEnter a : "))
        b = int("2") # int(input("Enter b : "))
        c = int("3") # int(input("Enter c : "))

I got

Your equation is 2x+3 = 0
X = -3/2 or -2.000000

Another bug (pretty-printing)

With :

        a = int("0") # int(input("\nEnter a : "))
        b = int("-1") # int(input("Enter b : "))
        c = int("-1") # int(input("Enter c : "))

I get :

Your equation is 1x-1 = 0
X = -1

Yet another bug (no solutions)

With :

        a = int("1") # int(input("\nEnter a : "))
        b = int("0") # int(input("Enter b : "))
        c = int("0") # int(input("Enter c : "))

No solution is printed.

Organisation

If you want to make things easier to read/maintain/fix, you should split the logic into multiple independant entities like functions for instance. Once could handle the logic to get the input from the user, one to generate the pretty-printed equations, one to look for solution, etc.

Also, you should try to write automatic tests. I'll let you find online how to write unit tests in Python.

Finally, it is a good habit to define functions/classes in your file but to write code that actually calls it behind a if __name__ == "__main__": test so that it does what you want when you use your file as a script but you can also import it to enjoy the functions and stuff without having any side-effects.

Useless conversions

chr returns a string, there is no need to convert it to string afterward.

Do not repeat yourself

You have :

            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"

in two different places. There is no need for this, just do this away from the if a > 0: test.

Keep it simple

When looking for a solution, you are trying to handle all scenarios in a pretty akward way. The best place to start with is to answer the simple question : "is a different from 0 ?". If a is 0, next question is "is b different from 0?".

Trying to group the different cases makes things easier to read. Also, it is easier to ensure you've handled all interesting cases.

Suggestion

When you pretty print the equations, 1 is a special case, -1 should probably be considered just as special.

My final version of the code is below. I have mostly moved code from one place to another and I haven't fixed any issue because it is more interesting for your to do it. Also, I have a few ideas to make the pretty-printing function much more concise but it is a bit more advanced and I feel like you should learn one step at a time.

from math import sqrt

def get_input():
    while True:
        try:
            a = int("1") # int(input("\nEnter a : "))
            b = int("0") # int(input("Enter b : "))
            c = int("0") # int(input("Enter c : "))
            return a, b, c
        except ValueError:
                print("You can only enter integers")

def get_pretty_eq(a, b, c):
    if a > 0:
        sgn_a = ""
    else:
        sgn_a = "-"
    if a == 0:
        part_a = ""
    elif a == 1:
        part_a = sgn_a+"x"+chr(0x00B2)
    else:
        part_a = sgn_a+str(abs(a))+"x"+chr(0x00B2)

    if b > 0:
        sgn_b = "+"
    else:
        sgn_b = "-"
    if b == 0:
        part_b = ""
    elif b == 1:
        if a != 0:
            part_b = sgn_b+"x"
        else:
            part_b = "x"
    else:
        if a != 0:
            part_b = sgn_b+str(abs(b))+"x"
        else:
            part_b = str(abs(b))+"x"

    if c > 0:
        sgn_c = "+"
    else:
        sgn_c = "-"
    if c == 0:
        if a == 0 and b == 0:
            part_c = "0"
        else:
            part_c = ""
    elif a == 0 and b == 0:
        part_c = str(abs(c))
    else:
        part_c = sgn_c+str(abs(c))
    return part_a + part_b + part_c + " = 0"

def get_solution_as_string(a, b, c):
    if a != 0:
        if b != 0:
            if c != 0:
                D = b**2-(4*a*c)
                if D > 0:
                        x_1 = ((-b+sqrt(D))/2*a)
                        x_2 = ((-b-sqrt(D))/2*a)
                        return "X1 = %f\nX2 = %f" %(x_1,x_2)
                elif D == 0:
                        x = -b/(2*a)
                        return "X1 = X2 = %d" %x
                elif D < 0:
                        x_1 = ((-b+sqrt(-D))/2*a)
                        x_2 = ((-b-sqrt(-D))/2*a)
                        return "X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i")
            else:
                x_2 = -b/a
                if b % a != 0:
                        return "X1 = 0\nX2 = %d/%d or %f" %(-b,a,x_2)
                else:
                        return "X1 = 0\nX2 = %d" %x_2
        else:
            if c != 0:
                if -c > 0:
                        x_1 = sqrt(-c/a)
                        x_2 = -x_1
                        return "X1 = %f\nX2 = %f" %(x_1,x_2)
                else:
                        x_1 = sqrt(c/a)
                        x_2 = -x_1
                        return "X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i")
            # this lack of "else" case is very suspicious
    else:
        if b != 0:
            if c != 0:
                x = -c/b
                if c % b != 0:
                    return "X = %d/%d or %f" %(-c,b,x)
                else:
                    return "X = %d" %x
            else:
                return "X = 0"
        else:
            if c != 0:
                return c,"!= 0"
            else:
                return "0 = 0. DUH!"

if __name__ == "__main__":
    a, b, c = get_input()
    print("\nYour equation is %s\n" % get_pretty_eq(a, b, c))
    print(get_solution_as_string(a, b, c))


strReadLine function (read line by line from a string) I'm new to C language and want to read a string line by line, I made this function and want to know what do you think about it, can I rely on it a production environment ?

/**
 * this function is not thread safe,
 */
char* strReadLine(char* text)
{

    static char* text2;
    static int start = 0;
    static int length = 0;
    int startTemp;
    int lineLength = 0;
    char* line = (char*) 0;

    startTemp = start;
    //not thread safe but at least make an error
    if(start != 0 && text2 != text){
        fprintf(stderr, "thread safe guerd fired by strReadLine function\n");
        exit(EXIT_FAILURE);
    }
    text2 = text;
    //thread safty end

    //if new string
    if(length == 0 || start == 0) length = strlen(text);
    if(!length) return (char*) 0;//the string is 0 length

    for(int x = startTemp; x < length; x++){
        start++;
        lineLength++;
        if(  text[x] == '\n'){
            line = malloc(lineLength);//not lineLength+1 because I will remove the \n with \0
            strncpy(line, text + (startTemp) , lineLength);
            line[lineLength - 1] = '\0';
            break;
        }
    }

    //if end of file && not the last line
    if(start == length && !line){
        start = 0;
        line = (char*) 0;
    }

    return line;
}

I have tested it with this text file

line1
line2
line3

line5
سطر6
line7

(line 4 is empty and line 6 contains non Latin characters stored in more than 1 byte in UTF-8 encoding)

like this, and it worked as expected

//char* input;this points to the string that is read from the file(I omitted the reading code)
char* line;
int x = 0;
while( (line = strReadLine(input)) ){
    x++;
    printf("%s\n",line);
    if(x == 300){printf("emergency break\n");break;}
}

x = 0;
while( (line = strReadLine(input)) ){
    x++;
    printf("%s\n",line);
    if(x == 300){printf("emergency break\n");break;}
}

Code format

Your use of spaces, braces and parantheses is unconventional in some places.

For example:

while( (line = strReadLine(input)) ){
    x++;
    printf("%s\n",line);
    if(x == 300){printf("emergency break\n");break;}
}

Conventionally this would look more like this:

while (line = strReadLine(input)) {
    x++;
    printf("%s\n", line);
    if (x == 300) {
        printf("emergency break\n");
        break;
    }
}

You could try using your IDE's auto-format feature and see how it arranges the code. Also you might want to have a look at some C coding conventions that can be found online, to see how people commonly format their C code in a readable way.

Thread safety

It seems like you are trying to guard the function against multiple threads executing it at the same time. However, the function's purpose is to read a string line by line, not to write to it. So as long as the function is only reading from its input, there should be no need to prevent multiple concurrent executions.

If multiple threads really did execute it at the same time, you could still get a race condition between checking the variables and overwriting it, and thus not get the error message when you would expect it.

Abusing booleanness

When checking whether an integer has the value 0, it is more readable if you compare it to 0, as you do in the first of the following two lines. It reads more intuitively, because a length cannot be true or false, as the second line implies, even though the compiler can handle both. Then you also wouldn't need the comment explaining what !length is supposed to mean. if (length == 0) says it explicitly.

if(length == 0 || start == 0) length = strlen(text);
if(!length) return (char*) 0;//the string is 0 length


Output JavaScript object into HTML table of key-value pairs using jQuery This function is meant to take an object, loop through the key-values and generate/build a HTML table at a particular ID:

function insertBasicTable(basic_data, insert_id) {
  /* Insert the key/value pairs into a table element on the DOM.

  basic_data = object containing key/value pairs
  insert_id  = the id on the DOM to insert the table into

  Intentionally not including closing tags in the jquery requests;
  http://stackoverflow.com/a/14737115/2355035 */

  table_id = '#' + insert_id;
  var array_length = Object.keys(basic_data).length;
  var array_obj_names = Object.keys(basic_data);

  // create the table header
  $(table_id).empty();
  $(table_id).append('
');
  $(table_id).find('thead:last').append('Tag');
  $(table_id).find('thead:last').append('Data');

  // begin the table body and iterate through key/value pairs
  $(table_id).append('
');
  for (var i = 0; i < array_length; i++) {
    var attr_name = array_obj_names[i];
    var tag       = '' + array_obj_names[i];
    var data      = '' + basic_data[attr_name];

    $(table_id).find('tbody:last').append('
' + tag + data);
  }
}

This function is then used, along with a number of D3 scripts, to populate elements on the DOM:

linePlot.render(someObject.modules.qual.quintiles);
boxPlot.render(someObject.modules.qual.quintiles);
insertBasicTable(someObject.modules.basic, 'basic-stats-table');
// etc, etc...

Any comments about how this is being achieved? Am I overlooking either a basic tenant of how JS "should" be used (this is a huge side-effect for instance, should I be returning a formatted string then placing that string into the DOM for instance) or over-complicating things by jQuery'ing a bunch of times?

Let's start with your version:

1) Unnecessary optimization.

var array_length = Object.keys(basic_data).length;
var array_obj_names = Object.keys(basic_data);

This gives you nothing in terms of performance. It would be optimized away by the browser.

2) Chain-calling

$(table_id).empty();
$(table_id).append('
');

It is possible to do chain-calls like: $('#elem').doA().doB().doC()

3) Misunderstanding calls

$(table_id).find('thead:last').append('Tag');

.append() does exactly that: appending after the last element.

4) NuStyleLoopz

  for (var i = 0; i < array_length; i++) {
    var attr_name = array_obj_names[i];
    var tag       = '' + array_obj_names[i];
    var data      = '' + basic_data[attr_name];

You could iterate over the keys directly via Object.keys() with

Array.prototype.forEach()
Array.prototype.map()
Array.protoype.reduce() / Array.prototype.reduceRight().

Here a slighty pimped version:

makeTag=function(openTag, closeTag){
            return function(content){
                return openTag+content+closeTag;
            };
        };
tHead=makeTag("
","
");
tBody=makeTag("
","
");
td=makeTag("","	");
tr=makeTag("
","
");

function insertBasicTable(data,id){
    $('#'+id).html(
        tHead(
            tr(
                td("Tag")+
                td("Data")
            )
        )+
        tBody(
            Object.keys(data).reduce(function(o,n){
                return o+tr(
                    td(n)+""+
                    td(data[n]+"")
                );
            },"")
        )
    );
};

insertBasicTable({"key1":"value1","key2":"value2"},"mytable");

Here is the fiddle to play with.

To make HTML-generation less painfull and a bit more JS-stylish I define what is called a higher order function makeTag: simply a function returning a function. A call to var tHead=makeTag("
","
"); generates an anonymous function (a function without name), which returns every time it's called "
"+ parameter +"
". The same goes for the other tags. This allows me to do something like a DSL:

    tHead(
        tr(
            td("Tag")+
            td("Data")
        )
    )

That is not only handy, but cool B-)
Besides it is nearly as readable as html and not so noisy like the typical string concatenation and the risk of typos is really low.


Check if a postcode is in a list I'm looking for improvement / advice / reviews on how I can improve this code. I feel like there is a better way or more efficient way of doing this and I am over looking it.

The Plugin I made is very simple:

It stores a list of Postcodes that the admin inputs into a textarea - 1 Per Line.
Plugin creates a text input on the front end that allows their customers to search if their postcode is in the list with Ajax.

The Plugin has a few options to allow the admin to customize the look and functionality of the plugin. Below is the function that takes the postcode and searches the list:

function sapc_ajax_check() {
    $checker_defaults = get_option( 'sapc_checker_settings_options' );
    $found = 0; $msg = ''; $passed = 0;
    if (!empty( $_POST ) && isset($_POST['action']) && strcmp($_POST['action'], 'sapc_ajax_check') === 0) {
            if(isset($_POST['pc'])){
                if(isset($_POST['verify-int']) && $_POST['verify-int'] == 'on'){
                    if(is_numeric($_POST['pc']) && is_int((int)$_POST['pc'])){
                        $passed = 1;
                    }
                }elseif($checker_defaults['verify-int'] == 'on'){
                    $passed = 1;
                }else{$passed = 1;}
                if($passed === 1){
                    $temp_l = $checker_defaults['postcodes'];
                    $postcode_l = explode(PHP_EOL, $temp_l);
                    if(is_array($postcode_l) && !empty($postcode_l)){
                        foreach($postcode_l as $i=>$temp_p){
                            if( strpos($temp_p, ':') !== false) {
                                $v = explode(':', $temp_p);
                                if(strcmp($v[0], $_POST['pc']) === 0){
                                    $found = 1;
                                    $msg .= $v[1] .', ';
                                }
                            }else{
                                if(strcmp($temp_p, $_POST['pc']) === 0){
                                    $found = 1;
                                    $msg .= $temp_p .', ';
                                }
                            }
                        }
                        if($msg != ''){
                            $msg = substr($msg, 0, strlen($msg) - 2);
                        }
                    }else{$msg ='Error: Try Again';}
                }else{$msg ='Error: Invalid Postcode';}
            }else{$msg ='Error: No Postcode';}
    }else{$msg = 'Error: No Data';}

    if($found == 0){
        echo json_encode(array('Error', 'Postcode Not Found'), JSON_FORCE_OBJECT);
    }else{
       echo json_encode(array('Success', $msg), JSON_FORCE_OBJECT);
    }
    die();
}

There are 2 sets of variables.

First set is the default settings from the admin options page, this helps with placing widgets and allows admins to set their own defaults. Postcodes can only be set here.
Second set is from the widget instance which allows admins to personalize each widget if need be.

I'll explain some of the variables.

$_POST['pc'] = Postcode from User

$_POST['verify-int'] = Option to check if postcode is all integers - Passed from Postcode Widget

$checker_defaults['postcodes'] = list of postcodes

Postcode List accepts the following formats:

Postcode:Surbubr
Postcode

(1 per line)

What can I do to make this function more efficient and more cleaner? If you need anything else please let me know I think I've covered everything.

Inside of a function call, avoid echoing.  By hardcoding echoes, you prevent the "silent" usage of the function.  It may be necessary in the future to present the output in more than one format, so use a return inside the function declaration and perform the echo on the function call.

Pay close attention to psr-2's guidelines on control structures.  They will help you (and future developers of your code) to read your code.  Always imagine that the next person to read your code will have a headache, a hatchet, and your home address; don't set them off.

Empty on $_POST is an imprecise way of checking for expected submission data.

strcmp() provides greater specificity than your condition logic requires.   For your logic, just check if the input is identical to the string without a function call.

Condense conditionals within the same block that have the same outcome.  Multiple conditions lead to $passed = 1 so they can be consolidated. I didn't really bother to understand the conditional logic behind $passed = 1 but it should certainly be refined.

Refine your validation check on $_POST['pc'].  You are checking if is_numeric(), that's fine. Then checking if the value that is cast as (int) is an integer -- um, at this point of course it is, it has no choice.  Better yet, why not just make a single check with ctype_digit()?  You might also like to check that the strlen() is valid (only you will know if/how to design this for your region).  If you want to check the quality and length of the postcode value, perhaps it would be more sensible to use preg_match() where you can design robust/flexible validation with a single function call (again, only you can determine this).

$temp_l and $temp_p are poor variable naming choices.  As a new dev to your script, I don't instantly know what they contains (I can venture a guess, but don't ask devs to do this).  Try to practice a more literal naming convention.  Furthermore, try to avoid declaring single-use variables ($temp_l).  Often, fewer variables will lead to fewer typos/mistakes, concise code, and improved readability.  When data needs some explaining, use commenting. *notes: 1. I have read some cases where declaring a variable prior to a foreach loop can improve performance 2. Some devs don't like to see functions fed to a foreach loop, I can respect this and I don't typically do this in my own projects.

There is no use in checking if the return value from explode() is an array.  It returns an array by design, so you can remove that check.  Even if you explode a empty string with PHP_EOL, you will not get a true evaluation from empty(), so that check is pointless to write.  At the end of the day, if you try to use foreach() on an empty array, it simply won't iterate -- no worries.

If you have no intentions of using $i in your foreach loop, don't bother to declare it.  I don't like single-use variables; I super don't like no-use variables.

How to get the substring before the first occurrence of a character without explode()? strstr() with a true third parameter.  Otherwise, explode has to create an array enroute to delivering the string that you need.  My suggested snippet will attempt to extract the substring before the first colon, if there is no colon the full string will be used.

By storing qualifying matches as an array, you can avoid having to trim any trailing delimiters from your output string.  In fact, return the data without delimiters as an array so that you can easily adjust the way that your qualifying values are delimited.

Strictly speaking, having zero qualifying results from a postcode search doesn't mean that there was an "Error", so just have your function calling script accommodate for "Successful" yet "Empty" results.

I can't imagine a benefit from JSON_FORCE_OBJECT.

die() in nearly every scenario should be avoided.

Suggested Code Overhaul:

function sapc_ajax_check() {
    $checker_defaults = get_option('sapc_checker_settings_options');

    if (!isset($_POST['action']) ||  $_POST['action'] !== 'sapc_ajax_check') {
        $errors[] = 'Missing required submission data';
    }
    if (!isset($_POST['pc'])) {
        $errors[] = 'Missing postcode value';
    }
    if (isset($_POST['verify-int']) && $_POST['verify-int'] === 'on' && ctype_digit($_POST['pc'])) {
        $errors[] = 'Invalid postcode value';
    }
    if (isset($errors)) {
        return json_encode(['Error', $errors]);
    }

    $result = [];
    foreach (explode(PHP_EOL, $checker_defaults['postcodes']) as $postcode) {
        $before_colon = strstr($postcode, ':', true);
        $postcode = ($before_colon === false ? $postcode : $before_colon);
        if ($postcode === $_POST['pc'])) {
            $result[] = $postcode;
        }
    }
    return json_encode(['Success', $result]);
}

Much cleaner right?


Iterate over an Enum subgroup I'm looking for a way to get a subset of a given enum to avoid repetition and use same enum type while comparing.

I found this question Enum subset or subgroup in C# and like the idea of using static class. Whoever, It doesn't allow to iterate over all elements in the class. So I added GetValues method.

Edit:

Here is my full code:

    public partial class Devices : Form
        {
            public Devices()
            {
                InitializeComponent();

                // Here, I need to iterate over StatusEnum items
                foreach (var item in Enum.GetValues(typeof(StatusEnum)))
                {
                    checkedListBoxParent.Items.Add(item);
                }

                // and iterate over ValidStatusEnum items
                foreach (var item in ValidStatusEnum.GetValues())
                {
                    checkedListBoxChild.Items.Add(item);
                }
            }

            private void On_MouseUp(object sender, MouseEventArgs e)
            {
                // Here, I need to compare StatusEnum and ValidStatusEnum
                if (Equals(checkedListBoxParent.CheckedItems.Cast<StatusEnum>().ToList(),
                    checkedListBoxChild.CheckedItems.Cast<StatusEnum>().ToList()))
                {
                    MessageBox.Show("Parent and child are similar");
                }
            }

            private bool Equals(List<StatusEnum> list1, List<StatusEnum> list2)
            {
                if (list1.Count != list2.Count)
                    return false;
                for (int i = 0; i < list1.Count; i++)
                {
                    if (list1[i] != list2[i])
                        return false;
                }
                return true;
            }

            private bool Equals(StatusEnum status, StatusEnum validStatus)
            {
                return status == validStatus;
            }
        }

        public enum StatusEnum { Unknown, Known, NotAvailable, Available };

        internal static class ValidStatusEnum
        {
            public const StatusEnum Known = StatusEnum.Known;
            public const StatusEnum Available = StatusEnum.Available;

            public static StatusEnum[] GetValues()
            {
                return new StatusEnum[] { Known, Available };
            }
        }
}

So basically, in my winforms application, I have two CheckedListBoxes (Parent and Child) populated from StatusEnum Enum and ValidStatusEnum static class.
So, I need to iterate over enums to populate Checkboxes and I need to compare StatusEnum and ValidStatusEnum to display a message to user in case I have the same selection in both checkboxes.

Any thoughts about the design and the code?

This answer assumes there's no way the different "groups" would actually belong in different enum types. Otherwise the only sensible answer is "if you have different groups of related values, put them in different enum types". Status is a fairly broad name (enum types shouldn't have "Enum" in their names) that doesn't really says anything - if you have an Availability group with NotAvailable and Available possible values, then have an Availability enum with these values.

That's [very] little more than a hard-coded Enum.GetValues helper...

Define your enum vertically:

public enum Status
{
    Unknown,
    Known,
    NotAvailable,
    Available,
}

Then it's much easier to add attributes, e.g. a [Description]:

public enum Status
{
    [Description("Device is unknown.")]
    Unknown,
    [Description("Device is known.")]
    Known,
    [Description("Device is not available.")]
    NotAvailable,
    [Description("Device is available.")]
    Available,
}

Or whatever. If you have enum values that semantically related and need to regroup them, you can use flags for that. Use powers of 2 to specify explicit underlying values for each enum; you can use bit-shifting to simplify that:

[Flags]
public enum Status
{
    Unknown = 1 << 0,
    Known = 1 << 1,
    NotAvailable = 1 << 2,
    Available = 1 << 3,
}

And then you can define the "groupings" as you please:

[Flags]
public enum Status
{
    Unknown = 1 << 0 | Knowledge | Negative,
    Known = 1 << 1 | Knowledge | Positive,
    NotAvailable = 1 << 2 | Availability | Negative,
    Available = 1 << 3 | Availability | Positive,
    Knowledge = 1 << 20,
    Availability = 1 << 21,
    Positive = 1 << 22,
    Negative = 1 << 23,
}

And now when you're faced with a Status value, you can do this:

var isPositive = value.HasFlag(Status.Positive);

And you can get all Positive enum values rather easily:

var positiveStatusCodes = Enum.GetValues(typeof(Status))
                              .Cast<Status>()
                              .Where(status => status.HasFlag(Status.Positive))
                              .ToArray();

That way when you need to add enum values, just give them the proper flags and your "get all positive status codes" code doesn't need to change at all.


Dynamic array challenge It solves the problem listed here. How can I make this more efficient?

import java.io.*;
import java.util.*;
import java.text.*;
import java.math.*;
import java.util.regex.*;

public class Solution {

    public static int getIndex(int x, int lastAns,Integer size) {
        return ((x ^ lastAns) % size);
    }

public static void main(String[] args) {
    Scanner in = new Scanner(System.in);
    int n = in.nextInt();
    int q = in.nextInt();

    LinkedList<Integer>[] sequenceList = new LinkedList[n];
    int lastAns = 0;
    while(q-- != 0) {
        int queryType = in.nextInt();
        int x = in.nextInt();
        int y = in.nextInt();
        int index = getIndex(x, lastAns, n);
        if(queryType == 1) {
            if(sequenceList[index] == null) sequenceList[index] = new LinkedList<Integer>();
            sequenceList[index].add(y);
        } else if(queryType == 2) {
            lastAns = sequenceList[index].get(y % sequenceList[index].size());
            System.out.println(lastAns);
        }
    }
  }
}

When instancing generic collections, it' best to use the diamond operator to let the compiler figure out the generic arguments. This lets you define in a single place (type definition, not constructor) for easier future modification.

sequenceList[index] = new LinkedList<>();
When using an Object, always declare its type as the highest class in its type hierarchy (up to interface if possible).

You're not using LinkedList's specific methods, so:

List<Integer>[] sequenceList = new List[n];

Javadoc!
Use proper Objects:

Make Solution a real instance created one in main (no real job should be performed in main). Then you can make lastAns and n fields of Solution because in the problem statement it is made to be a state of the Solution, so you don't need to pass those around every time
You could Wrap your Lists in a Sequence internal class of Solution. Internal means it will be able to access the lastAns field of the Solution instance etc.

It's a bit weird to lazily initialize the sequences. You don't win much, but you put an ugly if in your code. If there is no risk having a huge but sparse array, I would drop this.
Don't one-line ifs, and always use brackets
I'm usually against calling variables x, y etc. On this case it's defined in the problem so it's ok.

Updated code:

public class Solution {
    private int n;
    private int q;
    private int lastAns = 0;
    public Solution(int size, int numberOfQuestions) {
        n = size;
        q = numberOfQuestions;
    }
    public int getIndex(int x) {
        return (x ^ lastAns) % n;
    }
    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        Solution solution = new Solution(in.nextInt(), in.nextInt());
        solution.solve(in);
    }
    public void solve(Scanner in) {
        List<Integer>[] sequenceList = new List[n];
        while(q-- != 0) {
            int queryType = in.nextInt();
            int x = in.nextInt();
            int y = in.nextInt();
            int index = getIndex(x);
            if(queryType == 1) {
                if(sequenceList[index] == null) {
                    sequenceList[index] = new LinkedList<Integer>();
                }
                sequenceList[index].add(y);
            } else if(queryType == 2) {
                lastAns = sequenceList[index].get(y % sequenceList[index].size());
                System.out.println(lastAns);
            }
        }
    }
}


Make each element equal to its neighbouring element which is maximum in a matrix I have a matrix and I want to make each of its element equal to the maximum of its neighbouring element and each time I increase number of counts till all the elements in the matrix becomes equal to the maximum element of the initial matrix. Note that this increase in element will happen simultaneously for each element.

For example:

1 2 2 1
1 1 2 1
2 2 3 4

1st time:

2 2 2 2
2 3 4 4
2 3 4 4

2nd time:

3 4 4 4
3 4 4 4
3 4 4 4

3rd time:

4 4 4 4
4 4 4 4
4 4 4 4

So, answer would be 3.

What I've tried:

#include<bits/stdc++.h>

using namespace std;
int main()
{
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    int t,i,j,k,n,m;
    cin>>t;
    while(t--)
    {
        cin>>n>>m;
        long int a[n+2][m+2],maxi[n][m],ma=0,counti=0;
        int flag[n][m];
        for(i=0;i<n+2;i++)
        {
            for(j=0;j<m+2;j++)
            {
                if(i==0 || j==0 || i==n+1 || j==m+1)
                {
                    a[i][j]=0;
                    continue;
                }
                cin>>a[i][j];
                flag[i][j]=0;
                if(a[i][j]>ma)
                    ma=a[i][j];
            }
        }
        for(i=1;i<n+1;i++)
        {
            for(j=1;j<m+1;j++)
            {
                int myints[] = {a[i][j+1],a[i][j-1],a[i-1][j],a[i+1][j],a[i+1][j+1],a[i+1][j-1],a[i-1][j+1],a[i-1][j-1]};
                maxi[i-1][j-1]= *max_element(myints,myints+8);
                if(maxi[i-1][j-1]<a[i][j])
                    maxi[i-1][j-1]=a[i][j];
                if(a[i][j]==ma)
                    counti++;
            }
        }
        if(counti==m*n)
        {
            cout<<"0"<<endl;
            continue;
        }
        int time=0;
        while(counti!=m*n)
        {
            counti=0;
            for(i=1;i<n+1;i++)
            {
                for(j=1;j<m+1;j++)
                {
                    a[i][j]=maxi[i-1][j-1];
                    if(a[i][j]==ma)
                    {
                        counti++;
                    }
                }
            }
            for(i=1;i<n+1;i++)
            {
                for(j=1;j<m+1;j++)
                {
                    int myints[] = {a[i][j+1],a[i][j-1],a[i-1][j],a[i+1][j],a[i+1][j+1],a[i+1][j-1],a[i-1][j+1],a[i-1][j-1]};
                    maxi[i][j]= *max_element(myints,myints+8);
                }
            }
            time++;
        }
        cout<<time<<endl;
    }
}

The variables I have used are badly used and make you uneasy to read the code but I used them for my comfort so please don't point out minor mistakes.

Also, I know that using namespace std and including bits/stdc++.h are also bad practices, so please avoid them and focus on the problem here.

The problem is my time complexity to traverse is going too high. So, I want the answer with least time complexity as possible. Any pseudocode or logic explanation with reduced complexity would be sufficient.

please avoid them and focus on the problem here.

  The problem is my time complexity to traverse is going too high.

No, your variable naming and abuse of C++ features is also a problem. For the most part your code is structured OK. It's moderately understandable, but fixing those things could make it really easily understandable. You do need to think about these things, not just so others can read your code, but so your future self can understand it, too.

Naming

Your variable names are not clear at all. I'm all for 1-letter variable names for loop control variables like i and j. But you've gone too far for anyone who wants to understand your code. It's not hard to change t to numMatrixes or if you really need short names, numMats. (Also, you didn't really describe that there was more than one matrix in your description. Did I understand that correctly?) n and m should be numRows and numColumns (or numCols).

Your actual matrixes should have better names, too. a should be paddedMatrix. maxi is better than your other names. ma should be maxEntry. counti is also ambiguous. What is it counting? It looks to me like it's counting the number of values that match the maximum value, so maybe call it matchCount or numMatches?

Whitespace

This code is very dense and difficult for a human to parse. I recommend using far more whitespace in your code. For example, I'd put spaces before and after every operator. That means =, ==, +, -, >>, etc.

Also, you should add some empty lines between blocks of functionality from time to time. For example, between the for loops, the if statements, andwhile loops. Even better would be to…

Use Functions

I see 3 things happening in the outer while loop. You're:

Reading in the input values for the array (while finding the maximum element)
Replacing each element in the array with the maximum of its neighbors
Finding the number of times you need to do that until you've filled the array with the maximum values

Each of those things should be a separate function.

Careful with continue

The continue keyword is essentially a goto. As such, it can make code flow harder to understand than it needs to be. I don't think your use of it adds anything, so I'd remove it in favor of using else clauses in your ifs. For example in the first inner loop, I'd change it to look like this:

            if(i == 0 || j == 0 || i == n + 1 || j == m + 1)
            {
                a[i][j] = 0;
            }
            else
            {
                cin >> a[i][j];
                flag[i][j] = 0;
                if(a[i][j] > ma)
                {
                    ma = a[i][j];
                }
            }

It does the exact same thing, but is much clearer. Now I don't have to think, "does the continue apply to the inner or outer loop?" I just automatically understand what happens when the code runs.

I'd do the same thing with the other continue.

Avoid magic numbers

You have a couple of uses of the number 8. What does it represent? Does it need to change if you change anything else in the code? It appears that you're trying to use the number of values in the myints array. If that's what you want, you should use that directly. You can write it as:

const int numMyInts = sizeof(myints) / sizeof(myints[0]);

Stop Abusing C++

You say you know that using namespace std is bad, but you do it anyway. My compiler can't even find bits/stdc++.h, so I had to include the 2 headers you actually need: algorithm and iostream.

Errors and Warnings

My compiler (llvm) won't actually compile your code. It points out the error you have in the myints array. It's only big enough to hold ints, but you're trying to copy long ints into it. This might work for small values, but not for values that exceed the size of int.

You have an unused variable k. You should remove it.

Your algorithm fails for negative values.

Performance

In order to understand what's causing your code to be slow, you should profile it. That will tell you for certain where the slowdown is. You will often be surprised by the answer.

My first guess (and it's best to test it rather than guessing, because guesses are often wrong) is that it's costing you a lot of time copying the neighboring elements into the myints array. I wonder if you could avoid the copies by using some pointer arithmetic? Finding the max of several values is simple enough that you probably don't need to call max_elements(). You could manually unroll the loop into something like this:

const long int kRowStride = numCols - 2; // Number of elements to skip to get to the right element in the next row
long int *nextInt = &a[ i - 1 ][ j - 1 ];
long int currentMax = *nextInt;
nextInt++;
currentMax = std::max(currentMax, *nextInt);
nextInt++;
currentMax = std::max(currentMax, *nextInt);
nextInt += kRowStride;
//… etc.for the remaining 2 rows

It's possible that there's some matrix operation that will do the equivalent of your search in fewer steps. I don't remember enough matrix math to know off the top of my head whether that's true or not, but often the best optimization is not to optimize your algorithm, but to find a better algorithm. So I'd explore that route, as well.


Optimization of General Purpose Makefile I wrote a makefile that would generate a simple executable from the example directory structure.
project/
  source/
  include/
  makefile

The makefile itself comes equipped with dependency tracking, and automatically generates two directories named build and dependency. Currently the makefile I made works, yet it takes anywhere from three to five seconds to build the executable on my machine. I was wondering whether my makefile source code was setup inefficiently, or am I just being impatient?
target    := driver.exe
compiler  := g++
ccflags   := -Wall -Werror -pedantic -pedantic-errors -O2 -std=c++17
src_dir   := source
build_dir := build
deps_dir  := dependency
ccfiles   := $(wildcard $(src_dir)/*.cc)
objects   := $(patsubst $(src_dir)/%.cc, $(build_dir)/%.o, $(ccfiles))

deps := $(patsubst $(build_dir)/%.o, $(deps_dir)/%.d, $(objects))
-include $(deps)
deps_flags = -MMD -MF $(@:$(build_dir)/%.o=$(deps_dir)/%.d)

$(target): $(objects)
    @$(compiler) -o $@ $^
    @echo "Successfully built "$@"!"

$(build_dir)/%.o: $(src_dir)/%.cc | setup
    @$(compiler) $(cflags) -o $@ -c $< $(deps_flags)
    @echo "Successfully built "$<" into "$@"!"

.PHONY: setup
setup:
    @mkdir -p $(build_dir) $(deps_dir)

.PHONY: clean
clean:
    rm -rf $(target) $(build_dir) $(deps_dir)

Instead of defining your own compiler variable, I would override the implicit CXX variable. This also goes for the implicit CXXFLAGS (instead of your ccflags).
This allows other users to easily change the compiler used when using a different environment than you.
You might also want to remove all the leading @ in favour of make -s


Custom indexOf() without String methods I created my own indexOf function. I was wondering if anyone could help me come up with a way to make it more efficient. I am practicing for interviews so the catch is that I cannot use any String methods. I believe the runtime of this method is O(n2) with space of O(n). Correct me if I am wrong.

Also, I want to ensure the program runs safely and correctly, the only test case I can think of it the length comparison.

public static int myIndexOf(char[] str, char[] substr) {
    int len = str.length;
    int sublen = substr.length;
    int count = 0;
            if (sublen > len) {
                return -1;
            }
    for (int i = 0; i < len - sublen + 1; i++) {
        for (int j = 0; j < sublen; j++) {
            if (str[j+i] == substr[j]) {
                count++;
                if (count == sublen) {
                    return i;
                }
            } else {
                count = 0;
                break;
            }
        }
    }
    return -1;
}

Complexity

Pedantically, the time-complexity is \$ O( m \times n ) \$, where m is str.length and n is substr.length. This matters when \$ \left| m-n \right| \$ is large.

The Space complexity is \$ O(1) \$. You do not allocate any size-based memory structures.

Safety

It all looks good. There are no threading issues, no leaks, no problems.

Correctly

Nope, I don't like the lack of neat handling for invalid inputs.... you should be null-checking, etc. Getting a raw 'NullPointerException' looks bad.

Edit: Note that Josay has pointed out that your code (and my code below) produce different behaviour to String.indexOf() when the search term is the empty-string/empty-array.

Alternative

I think your code is fine, but... I tend to use loop break/continue more than most... and, this saves a bunch of code in this case...

Also, for readability, I often introduce a limit variable when the loop-terminator can be complicated....

Consider the following loops which do not need the count variable:

int limit = len - sublen + 1;
searchloop: for (int i = 0; i < limit; i++) {
    for (int j = 0; j < sublen; j++) {
        if (str[j+i] != substr[j]) {
            continue searchloop;
        }
    }
    return i;
}
return -1;


Neo4J Rest Javascript wrapper As a toy project, I started evaluating the Neo4J graph database and its Rest interface. I'm trying to write a simple graph visualization in JavaScript. In daily business I'm a Java developer and maybe that's the reason why I'm not that happy with my current approach. In Java I would have written a Neo4J class, with inner (data) classes for Node and Relation as their id is tightly couple to the server (http://localhost:7474/db/data/node/0). Maybe they even would get a common parent class.

In JavaScript I don't get away from my object oriented mind. I need some ideas in relation to JavaScript best practice.

function Neo4J(server) {
    this.server = server;
}

function Node(connection, response) {
    this.id = connection.sanitizeId(response['self']);
    this.data = response['data'];
}

function Relation(connection, response) {
    this.id = connection.sanitizeId(response['self']);
    this.start = connection.sanitizeId(response['start']);
    this.end = connection.sanitizeId(response['end']);
    this.type = response['type'];
}

Neo4J.prototype = {
    sanitizeId: function (id) {
        return id.replace(this.server, "");
    },

    query: function (query, success) {
        return post(this.server + "/cypher", {"query": query, "params": {} }, success);
    },

    getAllNodes: function (callback) {
        var connection = this;
        this.query("MATCH n RETURN n", function (nodes) {
            $.each(nodes['data'], function (i, val) {
                callback(new Node(connection, val[0]));
            });
        });
    },

    getAllRelations: function (callback) {
        var connection = this;
        this.query("START r=rel(*) RETURN r", function (nodes) {
            $.each(nodes['data'], function (i, val) {
                callback(new Relation(connection, val[0]));
            });
        });
    },

    getNode: function (id, callback) {
        var connection = this;
        get(this.server + id + "/properties", function (data) {
            if (data == null) data = {data: {}};
            else data = {data: data};
            data['self'] = id;
            callback(new Node(connection, data));
        });
    },

    createNode: function (callback) {
        var connection = this;
        post(this.server + '/node', null, function (data) {
            callback(new Node(connection, data));
        });
    },

    setNodeProperty: function (node, key, value, callback) {
        put(this.server + node.id + '/properties/' + key, '"'+value+'"', function (data) {
            node.data[key] = value;
            callback(node);
        });
    },

    createRelation: function (start, target, type, callback) {
        var connection = this;
        post(this.server + start + '/relationships', {to: this.server + target, type: type}, function (data) {
            callback(new Relation(connection, data));
        });
    }

}

The post() and  put() are just ajax calls to the Neo4J server.

Interesting question,

The most striking thing is your repeated use of this pattern:

connection.sanitizeId(response['xxx']);

I dont believe sanitizeId should be a function of connection. Also, response is not a good name because it often is part of a response from the query and not the actual response. And, it is suggested that response.xxx makes more sense than response['xxx']. To that end, I feel that sanitizeId should be part of the response or data. Also, I would be tempted to call it simply sanitize.

function addSanitizer( data , server){
  data.sanitize = function( propertyName ){
    return this[propertyName].replace( server , "" );
  }
  return data;
}

Then you could do

function Node(response) {
    this.id   = response.sanitize( 'self' );
    this.data = response.data;
}

Or, if you agree that response might be misleading

function Node(node) {
    this.id   = node.sanitize( 'self' );
    this.data = node.data;
}

Node with be called like this:

getNode: function (id, callback) {
    var connection = this;
    get(this.server + id + "/properties", function (data) {
        if (data == null) data = {data: {}};
        else data = {data: data};
        data['self'] = id;
        callback(new Node(this.addSanitizer( data, this.server ), data));
    });
},

From a style point:

You are skipping new lines on your if statements: if (data == null) data = {data: {}};, dont do that
Your code stretches quite a bit horizontally, but not too bad, feel free to to split up some of your longer lines into something more readable, this :

createRelation: function (start, target, type, callback) {
    var connection = this;
    post(this.server + start + '/relationships', {to: this.server + target, type: type}, function (data) {
        callback(new Relation(connection, data));
    });
}

could be

createRelation: function (start, target, type, callback) {
    var connection = this,
        data = {to: this.server + target, type: type};
    post(this.server + start + '/relationships', data , function (data){
        callback(new Relation(connection, data));
    });
}

The code looks all in all fine to me. For a Java developer you did good :P


Pandas : Apply Merge operations from a column I have a data frame, which contains three columns:
import numpy as np
import pandas as pd
np.random.seed(0)
dataframe = pd.DataFrame({'operation': ['data_a', 'data_b', 'avg', 'concat', 'sum', 'take_a', 'concat'],
             'data_a': list(np.random.uniform(-1,1,[7,2])), 'data_b': list(np.random.uniform(-1,1,[7,2]))})

Column 'operation' represent merge column, so if there is 'data_a' value in Column 'operation', it means take that particular row's data_a value, if there is 'avg' operation, then take the average of 'data_a' and 'data_b' of that particular row so on.
What I am expecting in the output, a new column contains the values as per the operation column's merge functions:

What I have tried:
dataframe['new_column'] = 'dummy_values'

for i in range(len(dataframe)):

    if dataframe['operation'].iloc[i]  == 'data_a':
        dataframe['new_column'].iloc[i] = dataframe['data_a'].iloc[i]
    elif dataframe['operation'].iloc[i] == 'data_b':
        dataframe['new_column'].iloc[i] = dataframe['data_b'].iloc[i]
    elif dataframe['operation'].iloc[i] == 'avg':
        dataframe['new_column'].iloc[i] = dataframe[['data_a','data_b']].iloc[i].mean()
    elif dataframe['operation'].iloc[i] == 'sum':
        dataframe['new_column'].iloc[i] = dataframe[['data_a','data_b']].iloc[i].sum()
    elif dataframe['operation'].iloc[i] == 'concat':
        dataframe['new_column'].iloc[i] = np.concatenate([dataframe['data_a'].iloc[i], dataframe['data_b'].iloc[i]], axis=0)

That is obviously not a good way to do this, It's very slow too. I am looking for more pandas version to do this task.

Wrap the logic in a function, then .apply that function.

Consider then doing another function lookup using a simple dictionary. This keeps your functions very short and simple.

Something like:
def _op_get_a(row):
    return row.data_a

def _op_get_b(row):
    return row.data_b

def _op_avg(row):
    return (row.data_a + row.data_b) / 2.0

def _op_sum(row):
    return row.data_a + row.data_b

def _op_concat(row):
    return np.concatenate([row.data_a, row.data_b], axis=0)

def process_row(row):
    return {
        'data_a': _op_get_a,
        'data_b': _op_get_b,
        'avg': _op_avg,
        'sum': _op_sum,
        'concat': _op_concat,
    }[row.operation](row)

# The "axis=1" is critical to apply this row-wise rather than column-wise
dataframe['new_column'] = dataframe.apply(process_row, axis=1)

I haven't tested this code, but you get the idea. Supporting new row operations is as simple as writing it as a row-wise function and hooking it into the lookup dictionary. The code is written at the smallest possible unit and able to be individually documented without creating a big cluttered mess.


DAO for fetching organizations and departments through a REST API I have the following class structure:

public class CustomDAOImpl implements CustomDAO {

    public List<Organization> getOrganizations() {
        List<Organization> results = new ArrayList<Organization>();
        Client client = ClientHelper.createClient();
        Resources<OrganizationResource> response = client
                .target(restApiRoot + "organizations")
                .request(MediaType.APPLICATION_JSON)
                .get(new GenericType<Resources<OrganizationResource>>(){
                });
        List<OrganizationResource> resources = response.getContent();
        for (OrganizationResource resource : resources) {
            results.add(resource.getEntity());
        }
        return results;
    }

    public List<Department> getDepartments() {
        List<Department> results = new ArrayList<Department>();
        Client client = ClientHelper.createClient();
        Resources<DepartmentResource> response = client
                .target(restApiRoot + "departments")
                .request(MediaType.APPLICATION_JSON)
                .get(new GenericType<Resources<DepartmentResource>>(){
                });
        List<DepartmentResource> resources = response.getContent();
        for (DepartmentResource resource : resources) {
            results.add(resource.getEntity());
        }
        return results;
    }
}

The abstract class :

public abstract class ResourceSupport {
    @XmlElement(name="link")
    private List<Link> links;
}

Entities :

public class DepartmentResource extends ResourceSupport {
    @XmlElement(name="department")
    private Department entity;

    public Department getEntity(){
        return entity;
    }
}

public class OrganizationResource extends ResourceSupport {
    @XmlElement(name="organization")
    private Organization entity;

    public Organization getEntity(){
        return entity;
    }
}

I'm new to JAVA, so I am not familiar enough with generics.

How can I improve my approach to have cleaner more elegant code?

The code must compile in JAVA 7 platform.

EDIT

I tried something like this :

private <T, U> List<T> makeRequest(Class<T> clazz1, Class<U> clazz2, String path) {
    List<T> results = new ArrayList<T>();
    Client client = ClientHelper.createClient();
    Resources<U> response = client
            .target(restApiRoot + path)
            .request(MediaType.APPLICATION_JSON)
            .get(new GenericType<Resources<U>>(){
            });
    List<U> resources = response.getContent();
    ...
    return results;
}

invoke like this :

List<Organization> list = makeRequest(Organization.class, OrganizationResource.class, "organizations");

But i have now an java.lang.ClassCastExceptionjava.lang.ClassCastException:

Source : java.lang.reflect.Method cannot be cast to java.lang.Class
  at com.owlike.genson.reflect.TypeUtil.getTypes(TypeUtil.java:397)
  at com.owlike.genson.reflect.TypeUtil.match(TypeUtil.java:331)
  at com.owlike.genson.convert.BasicConvertersFactory.provide(BasicConvertersFactory.java:97)
  ...
  at com.owlike.genson.Genson.provideConverter(Genson.java:155)
  at com.owlike.genson.Genson.deserialize(Genson.java:387)
  at com.owlike.genson.ext.jaxrs.GensonJsonConverter.readFrom(GensonJsonConverter.java:122)

On the advise of a person from reddit here : Java code refactoring, in order to get rid of code duplication

I tried something like this :

private <T0, T1 extends ResourceSupport<T0>> List<T0> getResources(Class<T0> clazz1, Class<T1> clazz2, String path, GenericType<Resources<T1>> genericInstance ) {
    List<T0> results = new ArrayList();
    Client client = ClientHelper.createClient();
    Resources<T1> response = client
            .target(restApiRoot + path)
            .request(MediaType.APPLICATION_JSON)
            .get(genericInstance);
    List<T1> resources = response.getContent();
    for (T1 resource : resources) {
        results.add(resource.getEntity());
    }
    return results;
}

@XmlRootElement
@Getter
@Setter
public class OrganizationResource extends ResourceSupport<Organization> {

    @XmlElement(name="organization")
    private Organization entity;

    @Override
    public Organization getEntity() {
        return entity;
    }
}

@XmlRootElement
@Getter
@Setter
public abstract class ResourceSupport<T>  {
    @XmlElement(name="link")
    private List<Link> links;

    public abstract T getEntity();
}

Called like this :

GenericType<Resources<OrganizationResource>> genericInstance = new GenericType<Resources<OrganizationResource>>(){};
List<Organization> list = getResources(Organization.class, OrganizationResource.class, "organizations", genericInstance);

For information, the rest api response is :

{
  "links": [],
    "content": [
        {
            "organization": {
                "id": "GES",
                "code": "GES",
                "name": "GES XX1",
                "legalNumber": "654654654"
            },
            "links": [
              {
                  "rel": "self",
                  "href": "https://server:8443/api/organizations/GES"
              },
              {
                  "rel": "organizations",
                  "href": "https://server:8443/api/organizations"
              },
            ]
        },
        {
            "organization": {
                "id": "PRO",
                "code": "PRO",
                "name": "PRO XX1",
                "legalNumber": "54545646"
            },
            "links": [ ... ]
        ...
    ]
}


“Removing” reference from its ReferenceQueue Sometimes I need to ensure that some references won't be processed while examination from the associated ReferenceQueue. Generally at those moments I don't know reachability status of the referent.

The only way I've came up with:

class MyReference<V> extends SoftReference<V> {
    private boolean active = true;

    public MyReference(V referent, ReferenceQueue<? super V> queue) {
        super(referent, queue);
    }

    public void disable() {
        active = false;
    }

    public boolean isActive() {
        return active;
    }
}

// examination
Reference<? extends V> ref;
while ((ref = referenceQueue.poll()) != null) {
    @SuppressWarnings("unchecked")
    MyReference<V> myRef = (MyReference<V>) ref;
    if (myRef.isActive()) {
        // process ref
    }
}

Is this the right approach?

I believe there is a flaw in your logic that is insurmountable......

SoftReferences, once they are added to the queue, have already lost the Referent instance. I cannot think of any reason why you would want to handle an 'active' queued (the value has been GC'd) softreference in any way differently from an 'inactive' one.

Once the reference has been queued it's too late to do anything about the instance it was softly referencing.

On the other hand, if you want to 'process' a referant, and make sure it is not GC'd during the processing, then jsut keep a reference to it:

V referent = softref.get();
if (referent == null) {
    // it's already been GC'd (and at some point we will see it poll'd from the Q)
    return;
}
// do things with our referent
// since our referent varaible is a hard-reference,
//      nothing can happen to the soft reference.
....

// then clear our reference (or return from our method,
// or clear our hard reference in some other way)
referent = null;
// after this the referent can (possibly) be GC'd again

EDIT after comments:

From the Javadoc for SoftReference:

  Suppose that the garbage collector determines at a certain point in time that an object is softly reachable. At that time it may choose to clear atomically all soft references to that object and all soft references to any other softly-reachable objects from which that object is reachable through a chain of strong references. At the same time or at some later time it will enqueue those newly-cleared soft references that are registered with reference queues.

The GC will only nequeue newly cleared soft references. Further, from the SoftReference.get() method, we have:

  Returns this reference object's referent. If this reference object has been cleared, either by the program or by the garbage collector, then this method returns null

Finally, here is a third-party 'reference' on 'references' ... heh. It appears to be quite good.

EDIT 2: how to keep a value 'active' (responding to comments)

If the requirement is to prevent a particular Soft reference referent from being GC'd, the answer is relatively simple: create a hard reference and keep it.... For example:

private static final IdentityHashMap<MyType, Object> active = new .....;
private static final Object token = new Object();

private static final boolean forceActive(SoftReference<MyType> reference) {
    MyType referent = softref.get();
    if (referent == null) {
        // it's already been GC'd ... and ...
        // if the reference was created with a ReferenceQueue, we can
        // expect to 'poll' the reference from that queue at some point
        // maybe we have already done that....
        return false;
    }
    // referent is now a hard reference, we are guaranteed that `reference`
    // is not on any Queue, and that it will not be GC'd (since we have a hard reference)

    // we want to keep our referent 'active', so we need to keep a hard reference:
    // the key in the identity hash map is our hard link.
    active.put(referent, token);
    // we exit our keep-active method, but the hard reference remains.
    return true;
}

private static final boolean deActive(SoftReference<MyType> reference) {
    MyType referent = softref.get();
    if (referent == null) {
        // it's already been GC'd ... and ... that means it was not
        // in our active map (otherwise it would not have been GC'd).
        return false;
    }
    // if the map contains the referent as a key, then it will return 'token'.
    // and we have now successfully removed the hard-reference to the referent.
    return token == active.remove(referent);
}


SPOJ DIVSUM - Divisor Summation DIVSUM - Divisor Summation
  #number-theory
  Given a natural number n (1 <= n <= 500000), please output the summation of all its proper divisors.

  Definition: A proper divisor of a natural number is the divisor that is strictly less than the number.

  e.g. number 20 has 5 proper divisors: 1, 2, 4, 5, 10, and the divisor summation is: 1 + 2 + 4 + 5 + 10 = 22.

  Input

  An integer stating the number of test cases (equal to about 200000), and that many lines follow, each containing one integer between 1 and 500000 inclusive.

  Output

  One integer each line: the divisor summation of the integer given respectively.

Example

Sample Input:

  3
  2
  10
  20

Sample Output:

  1
  8
  22

code:-

import java.util.*;
import java.lang.*;
import java.io.*;

class Ideone
{
    public static void main (String[] args) throws java.lang.Exception
    {
        BufferedReader BR=new BufferedReader(new    InputStreamReader(System.in));
    int t,i,j;
    t=Integer.parseInt(BR.readLine());
    int a[]=new int [t];
    int sum[]=new int [t];
    for(i=0;i<t;i++)
    {
        a[i]=Integer.parseInt(BR.readLine());
        sum[i]=0;
        for(j=1;j<a[i];j++)
        {
            if(a[i]%j==0)
            sum[i]+=a[j];
        }

   }

    for(i=0;i<t;i++)
    {
        System.out.println(sum[i]);
    }
    }
}

I am getting time limit exceeded error for this program from SPOJ.  How do I avoid it?

There is a much better solution to your problem. Some mistakes you could have avoided -

• You use 2 loops. One would also do it.

• You iterate all the elements in the nested array. You do not need to include the element itself (as indirectly mentioned in the question). Therefore, you can iterate only a[i]/2 elements.

You can optimize this further also. Here is my code but unfortunately it is in Python. Hope you can at least understand that. It is an easy language.

for i in range(int(input())):
    x = int(input())
    sum=0
    for j in range(x/2):
        if(x%j==0):
            sum += j
    print(sum)


Mapping the differences between JavaScript objects The reason I want to do this is that the JavaScript objects are created from XML elements and if the element has changed I want to be able to detect that change in the JavaScript objects. So I wrote a function which maps out the differences between the first and second object. Can you improve on this function? (mapObjectDifferences, not the helper functions.)

<html>
<head>
<title>MAP OBJECT DIFFERENCES</title>
<script type="text/javascript">

    function isArray(object) {
        return Object.prototype.toString.apply(object) == "[object Array]";
    }

    function getType(variable) {
        var type = typeof variable;
        if (type == "object")
            if (isArray(variable))
                return "array";
        return type;
    }

    function arrayToObject(array) {
        var object = {};
        for (var i = 0; i < array.length; ++i)
            if (array[i] !== "undefined") object[i] = array[i];
        return object;
    }

    function mapObjectDifferences(outerObject, innerObject) {
        var _mapObjectDifferences = function (outerObject, innerObject, parentMap, parentName) {
            var localMap = {};
            for (var outerProp in outerObject) {
                if (outerObject.hasOwnProperty(outerProp)) {
                    var match = false;
                    var outerPropValue = outerObject[outerProp];
                    var outerType = getType(outerPropValue);
                    var result;
                    for (var innerProp in innerObject) {
                        if (innerObject.hasOwnProperty(innerProp)) {
                            var innerPropValue = innerObject[innerProp];
                            var innerType = getType(innerPropValue);
                            if (outerProp == innerProp && outerType == innerType) {
                                match = true;
                                if (outerType == "array" || outerType == "object") {
                                    if (outerType == "array") {
                                        outerPropValue = arrayToObject(outerPropValue);
                                        innerPropValue = arrayToObject(innerPropValue);
                                    }
                                    _mapObjectDifferences(outerPropValue, innerPropValue, localMap, outerProp);
                                }
                                break;
                            }
                        }
                    }
                    if (match == false) {
                        localMap[outerProp] = outerType;
                        if (parentMap)
                            parentMap[parentName] = localMap;
                    }
                    else if (parentMap) {
                        var difChild = false;
                        for (var prop in localMap)
                            if (localMap.hasOwnProperty(prop)) {
                                difChild = true;
                                break;
                            }
                        if (difChild == true)
                            parentMap[parentName] = localMap;
                    }
                }
            }
            return localMap;
        }
        return _mapObjectDifferences(outerObject, innerObject);
    }

    var o1 = {
        val: "level one",
        val2: 1,
        val3: 3,
        m: {
            s2: "this is level two",
            l1: ["a", "b", "c", 1, {a:"a"}],
            ao: [{ x: "1" }, { y: 1 }, { z: 1}]
        },
        n: {
            n1: { abc: 123 }
        }
    };
    var o2 = {
        val: "level on23e",
        val2: 1,

        m: {
            s3: "this is level two",
             l1: ["a", "b", "c"],
            ao: [{ x: 1 }, { y: 1 }, { z: "3"}]
        },
        n: {
            n1: { abc: "abc" }
        }
    }
    var result = mapObjectDifferences(o1, o2);
    debugger;
</script>
</head>
<body>
</body>
</html>

The existing suggestions are good for trimming down your current code, but, as always, the most important thing to optimize is the algorithm itself. I took a crack at your problem, as I understand it, and here's the result:

function difference(o1, o2) {
    var k, kDiff,
        diff = {};
    for (k in o1) {
        if (!o1.hasOwnProperty(k)) {
        } else if (typeof o1[k] != 'object' || typeof o2[k] != 'object') {
            if (!(k in o2) || o1[k] !== o2[k]) {
                diff[k] = o2[k];
            }
        } else if (kDiff = difference(o1[k], o2[k])) {
            diff[k] = kDiff;
        }
    }
    for (k in o2) {
        if (o2.hasOwnProperty(k) && !(k in o1)) {
            diff[k] = o2[k];
        }
    }
    for (k in diff) {
        if (diff.hasOwnProperty(k)) {
            return diff;
        }
    }
    return false;
}

Note that I didn't try to mimic your code exactly:

Arrays are compared thoroughly, not just on numbered properties.
Difference map contains new values themselves, not their types.
If there are no differences then the function returns false, not {}.


A Class to validate password strength What would be the best approach to write this class.

This class validates 3 types of passwords,

Normal
Medium
Strong

Github Link

<?php namespace azi\Rules;

use azi\Arguments;
use azi\Rules\Contracts\RuleInterface;

/**
 * Class Password
 *
 * @package azi\Rules
 */
class Password implements RuleInterface
{
    const NORMAL = 'normal';
    const MEDIUM = 'medium';
    const STRONG = 'strong';

    protected $message = null;

    /**
     * @param $field
     * @param $password
     * @param Arguments $args
     * @return mixed
     */
    public function validate($field, $password, Arguments $args)
    {
        $type = $args->getVariables();

        if ($type && $type != self::NORMAL) {
            return $this->is($type[ 0 ], $password);
        }

        return $this->isNormal($password);
    }

    /**
     * @param $strength
     * @param $password
     * @return bool
     */
    public function is($strength, $password)
    {
        $method = sprintf("is%s", ucwords($strength));
        return call_user_func([$this, $method], $password);
    }

    /**
     * @param $password
     * @return bool
     */
    public function isNormal($password)
    {
        if (strlen($password) < 8) {
            $this->message = '{field} must be at least 8 characters long';
            return false;
        }

        if (!preg_match("#[a-zA-Z]#", $password)) {
            $this->message = '{field} must contain at least one letter';
            return false;
        }

        return true;
    }

    /**
     * Checks a strong password
     *
     * @param $password
     * @return bool
     */
    public function isStrong($password)
    {
        if (!$this->isMedium($password)) {
            return false;
        }

        if (!preg_match("#[!@\#\$%^&*()_+\-=\[\]{};':\"\\|,.<>\/?]+#", $password)) {
            $this->message = '{field} must contain at least special character';
            return false;
        }

        return true;
    }

    /**
     * @param $password
     * @return bool
     */
    public function isMedium($password)
    {
        if (!$this->isNormal($password)) {
            return false;
        }

        if (!preg_match("#[a-z]#", $password)) {
            $this->message = '{field} must contain at least one lowercase letter';
            return false;
        }

        if (!preg_match("#[A-Z]#", $password)) {
            $this->message = '{field} must contain at least one uppercase letter';
            return false;
        }

        return true;
    }

    /**
     * @return mixed
     */
    public function message()
    {
        if ($this->message) {
            return $this->message;
        }

        return "{field} must be a good password";
    }
}

/**
 * @param $strength
 * @param $password
 * @return bool
 */
public function is($strength, $password)
{
    $method = sprintf("is%s", ucwords($strength));
    return call_user_func([$this, $method], $password);
}

This is not okay.

You're taking user input... (or something that I assume to be user input...) and then looking for a method in your own class, and then executing it. This whilst you know or at least can reasonably assume that you know which values the user input might contain.

So let's nuke it.

If we nuke it, then we need a new implementation here.

/**
 * @param $field
 * @param $password
 * @param Arguments $args
 * @return mixed
 */
public function validate($field, $password, Arguments $args)
{
    $type = $args->getVariables();

    if ($type && $type != self::NORMAL) {
        return $this->is($type[ 0 ], $password);
    }

    return $this->isNormal($password);
}

Well, we can use a switch. See also the documentation here: http://php.net/manual/en/control-structures.switch.php (I'm not familiar with PHP syntax, but you should understand the gist of the idea.)

/**
 * @param $field
 * @param $password
 * @param Arguments $args
 * @return mixed
 */
public function validate($field, $password, Arguments $args)
{
    $type = $args->getVariables();

    if ($type && $type != self::NORMAL) {
        switch ($type [ 0 ]) {
            case self::NORMAL:
                return $this->isNormal($password);
                break;
            case self::MEDIUM:
                return $this->isMedium($password);
                break;
            case self::STRONG:
                return $this->isStrong($password);
                break;
        }
    }

    return $this->isNormal($password);
}

Lastly, I'd say you should reorder your methods so they are ordered as "normal", "medium" and "strong" in the code as well. It reads better.