Get data from some SOAP resources Student http://semisalsaja.com/sekolah/student.asmx?WSDL
- studentList

Mentor http://semisalsaja.com/sekolah/mentor.asmx?WSDL
- mentorList

study http://semisalsaja.com/sekolah/study.asmx?WSDL
- studyList

I'm trying to get data from some SOAP resources (mentioned above):

class SchoolSoap {
    protected $_config = array();
    public $_soap, $_error, $_params, $_result;
    public function __construct($params = array()) {
        $this->_config = $params;
        try {
            $this->_soap = new SoapClient($this->_config['url'], array('trace' => 1, 'exception' => 1));
        } catch (Exception $e) {
            $this->_error = $e->getMessage();
        }
    }
    public function actionSoap($function) {
        try {
            $result = $this->_soap->__soapCall($function, array('parameters' => $this->_params));
            $this->_result = $result;
            return true;
        } catch (Exception $e) {
            $this->_error = $e->getMessage();
            return false;
        }
    }
    public function setParam($params) {
        $this->_params = $params;
        return $this;
    }
}

The 1st class is for calling SOAP and it will be called/ instanced from static classes.

class StudentHelper {
    public static function getStudentList($config, $params, SchoolSoap $soap = null) {
        $function = 'studentList';
        if ($soap == null) {
            $soap = new SchoolSoap($config);
        }
        $result = $soap->setParam($params)->actionSoap($function);
        if ($result == false) {
            return false;
        } else {
            return $soap->_result->$function;
        }
    }
}

class MentorHelper {
    #similar with class StudentHelper
}

class StudyHelper {
    #similar with class StudentHelper
}

The next next classes (I called Helper) are static class. I created resources Student, Mentor, Study, for every SOAP. For example, in class StudentHelper, there is a method named getStudentList that will get the student's data, and it will be called from a function (next code). class Mentor and Study are similar.

Usage

function printStudentList() {
    $config = array(
        'url' => 'http://semisalsaja.com/sekolah/student.asmx?WSDL',
        'username' => 'user123',
        'key' => 'abcdefghij'
        );
    $params = array('start' => 1, 'limit' => 20);
    return StudentHelper::getStudentList($config, $params);
}

Next is function to call Static class (example, call StudentHelper).  I passed configs and params in this function, and it will return bool false if soap failed and data of student if soap succeeded.

$student_list = printStudentList();
print_r($student_list);

Here I call parse function into variable, then print the variable.

Is my design correct with the concept of OOP in PHP? Any suggestion for me to make it better? I'm still new to learning about programming, especially OOP.

Ok, given that you're after some CR on your understanding (and usage) of OO techniques, I'll go through your code almost line by line. If my criticisms strike you as blunt, please keep in mind that my goal is to help, not to offend.

Coding standards
This has become a bit of a hang-up of mine, but I can't stress enough how important it is to write code that conforms to some form of standard. PHP, of course, does not (yet) have an official standard as such, but the unofficial PHP-FIG standard is widely accepted, and all major players (Zend, Symfony, CakePHP, ... the list is on the site) subscribe to this standard. As should you.

This entails, among other things:

property names don't start with an underscore to indicate visibility (private, protected). That's what we did in the PHP4 days. I think none of us is willing to go back to that era.
Opening braces for methods, functions and classes go on a new line
Code is indented by spaces, 4 of them, tabs are not to be used. If you use nothing but tabs, then that's an acceptable deviation from the standard, but be consistent. SVC's like Git will give you grief otherwise.
access modifiers aren't optional, they are required.
Method names are camelCased, variables/properties are either camelCased or you can use underscores. Either way: be consistent. I prefer camelCased properties.

Type-hinting all the way
I see you use the occasional type-hint. That's great! Fantastic even, but you're not using them as much as you can. You can't hint for primitive types, but you can use doc-blocks, casts and you can hint for array variables.
More on this later.

Your code, at a glance
Rather than listing my recommendations and linking to the wiki of the SOLID principles, let's get stuck in and review your actual code. I'll already change a thing or 2 to make your code more readable (ie conforming to the PHP-FIG standards):

class SchoolSoap
{
    /**
     * @var array
     */
    protected $config = array();

Stop here (yes, already). I've removed the underscore, but I wonder: why are you initializing your property to an empty array, if you have a constructor that will always re-assign that property? Just initialize the properties to null if you don't need them to have a specific default value.
I've also added a doc-block that makes it clear to everyone what this property is expected to be, what you'll assign it to, and how it'll be used. This will help your IDE provide you with useful auto-completion info, and error reporting.

    /**
     * @var \SoapClient
     */
    public $soap = null;
    /**
     * @var string|null
     */
    public $error = null;
    //add your own doc-blocks from now on :)
    public $params = null;
    public $result = null;

Now, the methods:

    /**
     * Some info about the construct, will show in auto-complete
     * with any decent IDE
     * @param array $params = array()
     * @return $this
     */
    public function __construct(array $params = array())
    {
        $this->config = $params;
        try
        {//this is non-standard, but I use allman indentation
         //that's my guilty pleasure
            $this->soap = new SoapClient(
                $this->config['url'],
                    array(
                        'trace' => 1,
                        'exception' => 1
                    )
                );
        }
        catch (Exception $e)
        {
            $this->error = $e->getMessage();
        }
    }

So, I've cleaned up a bit, but there are still quite a few issues left. For starters: you allow for the user to pass an array $params to the constructor. If he fails to do so, then that's fine, but the array defaults to an empty array. That, too, isn't a problem, but what is problematic is that you create the instance of SoapClient using $this->config as though it is an associative array and as though it has certain keys set!. You don't do any isset checks (meaning your code might emit notices).

Then, if the SoapClient could not be constructed, you catch any Exception that might have been thrown. But SoapClient doesn't throw Exception instances, it throws a specific type of exceptions: SoapFault.
Catching any higher (as in parent of a particular exception) type of exception is known as pokemon exception handling. It reads as if the code was written by somebody who didn't really know what to expect.

Not that this really matters, because your entire class revolves around this SoapClient instance being set. If it fails, then your class shouldn't be allowed to exist. It's likely that the code creating the instance contains a bug, and provided faulty data. That kind of thing should be communicated immediately: the error shouldn't be caught by you, because you can't fix it: the error should be thrown back to the user! That's Why I'd suggest you write this as a constructor:

/**
 * Simple wrapper constructor
 * @param string $wsdl
 * @param array $options = null
 * @return $this
 * @throw SoapFault
 */
public function __construct($wsdl, array $options = null)
{//make options null by default, but force wsdl to be passed explicitly
    $this->config = $options
    $this->soap = new SoapClient(
        $wsdl,
        $this->config
    );
}

We end up with less code, but again: with a decent IDE, you'll get more information about what this constructor does when you type new SchoolSoap(.
You no longer need the $this->error property, and don't have to hope that the user will check that property's value: if an error occurred, he'll get an exception thrown at him, which is as it should be!

The other methods should be easy to improve, along the same lines:

/**
 * Perform a soapCall to $function, passing $parameters
 * If no parameters are provided, $this->params will be used
 * @param string $function
 * @param array $parameters = null
 * @return bool
 * @throw SoapFault
 */
public function actionSoap($function, array $parameters = null)
{
    if ($parameters === null)
        $parameters = $this->params;
    //assign to property directly, don't use temp var... no need
    $this->result = $this->soap->__soapCall(
        $function,
        array(
            'parameters' => $parameters
        )
    );
    return true;//wouldn't return $this->result; be more usefull?
}

The setter methods are a great idea. I especially like their being fluent (chainable), nothing but formatting needs to change there:

public function setParam($params)
{
    $this->params = $params;
    return $this;
}

A wrapper is best when it exposes its object
What you have written here is, essentially, a wrapper object: it wraps an instance of SoapClient, to offer a different (possibly neater, safer and clearer) API.
However, I need a reason to use a wrapper, it should allow me to do what I want. This wrapper severely restricts my usage of the SoapClient class. What's to stop me from simply writing this:

$schoolSoap = new SoapClient($wsdl, $options);
$result = $schoolSoap->__soapCall($function, $params);

A wrapper should somehow expose some of the internals of the wrapped object, but perhaps check the arguments that the user is trying to pass, or at least offer constants that the user can use to facilitate the manipulation of the wrapped object. The blindest, slowest, but easiest way to do so is by implementing a magic __call method in your SchoolSoap class:

public function __call($method, array $args)
{
    if (method_exists($this->soap, $method))
    {//perhaps add an is_callable check here, too
        $return = call_user_func_array(
            array($this->soap, $method),
            $args
        );
        if ($return === $this->soap)
            return $this;//don't return the wrapped object
        return $return;
    }
    throw new \BadMethodCallException(
        sprintf(
            "Method '%s' does not exist in classes %s and %s",
            $method,
            __CLASS__,
            get_class($this->soap)
        )
    );
}

Of cours, a safer way is to implement certain methods of the wrapped objects in your wrapper, check the arguments and then call the wrapped object's method:

public function getLastResponse($fromObject = false)
{
    if ($fromObject === false)
        return $this->result;//as set in the `actionSoap` method
    return $this->soap->__getLastResponse();//from wrapped object
}

This is a clean, easy to maintain and safe way to expose your wrapped object, and it doesn't give me, as a user, a reason not to use your class.
Of course, if you do this, then you don't need that slow magic __call method anymore, and you can simply do away with that (as well you should).
An in-between solution is to have a private property, holding an array with methods that can be called by the user, and use those in your __call method. That saves you some method_exists calls.

Ok, I'll leave you with this final recommendation concerning the first of your classes:

Passing arrays to constructors is easy, and shortens the number of arguments your methods take. I can see why one would do this. However, if you are going to write OO code, then it can be safer/better still to pass data objects containing that same information. You can type-hint for those specific objects, and be sure that the properties you need will exist.

I could post an example here, but instead I'd invite you to check this github fork of mine, and in particular the Config class. The Config is what, logically, configures the behaviour of all classes that make API calls. By hinting for an instance of that object in the constructor, I can rest assured that things like the url, usage mode, user and API-KEY will be set, and ready to be used. I use getters and setters throughout the code, to ensure my data is correctly formatted and sanitized, too.
there are some magic getters and setters, and I thing also a magic __call method somewhere, but I'm still working on the code, to get rid of them.

If you compare my fork to the code I started off with, I think it's pretty clear that passing data objects is a big step forwards.

There are other things in your code that I will discuss further (like your use of the dreaded static keyword), so I will be adding to this answer from time to time. For now, though, I think I've given you sufficient food for thought.

Update

As promised: Back for some more reviewing-stuff. I'll start off with that one helper class you've included, and take it from there. I already mention the static keyword isn't to my liking. Why isn't it? Well, static properties/variables are variables that retain their state. That means that, if a function has a static variable, and you assign a value to that var, that value is kept in memory in between function calls.
Class properties work similarly, but that value is shared between all instances of that class. You can also access that property at without an instance, as though it were a global variable. And that's the problem with statics: they are, essentially globals in OO disguise, and we all know globals are bad.

Same goes for your method: it works like a function, and is globally accessible, like a global function. Only, static method calls are slower than function calls! Then why bother with a class, and not simply create a function?
That would, indeed be better than what you have now, but it's not in tune with your wanting to write more OO code

So let's make it object oriented, and make good use of the advantages of OOP: you can use properties to store certain things, and like, for example your SoapClient instance. At the moment, you create a new SoapClient instance every time you instantiate SchoolSoap. Wouldn't it be nice if we only had to do that once? Your static method now creates a new SchoolSoap instance every time it is called, without being passed an existing instance already, because its functionality depends on that SchoolSoap being there. What's more: it's function in your program is a specific use-case of the capabilities of SchoolSoap. So its job is the same (performing a SOAP call), only its a specific type of call.

In OO code, it wouldn't be uncommon to see StudentHelper being a child of SchoolSoap. Therefore, I suggest you go down that route, and use inheritance:

class StudentHelper extends SchoolSoap
{
    public function getStudentList($params)
    {
        $result = $this->setParam($params)
            ->actionSoap('studentList');
        if (!$result)
            return $result;
        return $result->studentList;//or $this->_result->studentList;
    }
}

All the calls you have, that deal with students go in this class, and for each group of calls, you can create a new child class of SchoolSoap.
The usage is very simple:

$student = new StudentHelper($config);//what you pass to SchoolSoap's constructor
$studentList = $student->getStudentList($params);
$student->addStudent($params);//if that call exists

All of the goodies that SchoolSoap (methods and properties) are also properties of StudentHelper now (except for private properties), same goes for methods.

Of course, now the name StudentHelper is a bit weird, because it isn't a helper anymore: it's a full-blown wrapper, too, so I'd change the name of SchoolSoap to BaseSoap, because it's your base class, and change StudentHelper to StudentSoap or something, to better reflect what the class is.

I've posted a review of a singleton class on this site a while back. In it, I go into more details on why statics are to be avoided, feel free to check it out:

Singleton design pattern


Test password for adherence to policy I have written a program to test whether a password contains upper-case, digits and a $.

#include <stdio.h>
#include <conio.h>
#include <ctype.h>
#include<string.h>

int main()
{

    char pass[20];                      //Declaring the array to store pass
    int i,flag1=0, flag2=0, flag3=0;

    printf("Please enter a password having a Upper case letter, a number and also a $ sign \n");
    scanf("%s",&pass);

        {
            for(i=0;i<=strlen(pass);i++)
            if( isupper(pass[i]) )
            flag1=1;
            else if(isdigit(pass[i]))               //Loop to check if all element in pass has a digit, alpha & '$'
            flag2=1;
            else if(pass[i]=='$')
            flag3=1;
        }
                {
                    if((flag1==1) && (flag2==1) && (flag3==1))      //All condition true
                    {
                    printf("\nNice Password!");
                    }
                    else
                        {
                        printf("\nPassword must contain a upper case letter, a number and a $ sign");  //Something missing
                        }
                }

            return 0;
}

You have introduced some weird an IMO unnecessary scope blocks here:

    {
        for(i=0;i<=strlen(pass);i++)
        if( isupper(pass[i]) )
        flag1=1;
        else if(isdigit(pass[i]))               //Loop to check if all element in pass has a digit, alpha & '$'
        flag2=1;
        else if(pass[i]=='$')
        flag3=1;
    }

and here:

            {
                if((flag1==1) && (flag2==1) && (flag3==1))      //All condition true
                {
                printf("\nNice Password!");
                }
                else
                    {
                    printf("\nPassword must contain a upper case letter, a number and a $ sign");  //Something missing
                    }
            }

The code should be reformatted like this probably:

        for(i=0;i<=strlen(pass);i++) {
            if( isupper(pass[i]) ) {
                flag1=1;
            }
            else if(isdigit(pass[i])) {               //Loop to check if all element in pass has a digit, alpha & '$'
                flag2=1;
            }
            else if(pass[i]=='$') {
                flag3=1;
            }
            if((flag1==1) && (flag2==1) && (flag3==1)) {     //All condition true
                printf("\nNice Password!");
            }
            else {
                printf("\nPassword must contain a upper case letter, a number and a $ sign");  //Something missing
            }
        }


Is this C# 'hack' a bad idea? As C# doesn't have generic specialisation like C++ template specialisation and I wanted a way to be able to do it, I came up with a little hack that goes a bit like this:

    public sealed class Specialisation<T>
    {
        private Specialisation() { }
        private static Specialisation<T> instance = new Specialisation<T>();
        public static Specialisation<T> Instance { get { return instance; } }
    }
    public interface ISpecialisable<T>
    {
        T SpecialisedMethod(Specialisation<T> t);
    }
    public class Specialised : ISpecialisable<int>, ISpecialisable<float>
    {
        public double Value;
        public int SpecialisedMethod(Specialisation<int> t) { return (int)Value; }
        public float SpecialisedMethod(Specialisation<float> t) { return (float)Value; }
    }
    public static class Program
    {
        public static void Main()
        {
           Specialised s = new Specialised();
           s.Value = 12.3456789;
           Console.WriteLine(s.SpecialisedMethod(Specialisation<int>.Instance));
           Console.WriteLine(s.SpecialisedMethod(Specialisation<float>.Instance));
           Console.ReadKey();
        }
    }

Other than this not being the 'right way' to do things and being a complete mistreatment of the method overloading system, are there any possible side effects that could happen as a result of this horrible hack?

EDIT:

An applied example of specialisation in such a manner:

    // assume Specialisation is unchanged
    public interface IConvertible<T> // Not to be confused with System.IConvertible
    {
        T Convert(Specialisation<T> t);
    }
    public class ConvertMe : IConvertible<int>, IConvertible<float>
    {
        public double Value;
        public int Convert(Specialisation<int> t) { return (int)Value; }
        public float Convert(Specialisation<float> t) { return (float)Value; }
    }
    public static class Program
    {
        public static void Main()
        {
           ConvertMe c = new ConvertMe();
           c.Value = 12.3456789;
           Console.WriteLine(c.Convert(Specialisation<int>.Instance));
           Console.WriteLine(c.Convert(Specialisation<float>.Instance));
           Console.ReadKey();
        }
    }

EDIT:
Another example I just thought of as an extension of/alternative to the IConvertible<T> example. This technique allows the normal generic method syntax to be used externally, whilst allowing the class developer to internally define specialisations.

public interface IConvertible<T>
{
    T Convert(Specialisation<T> t);
    T Convert<T>();
}
// assume Specialisation<T> is unchanged
public class ConvertMe : IConvertible<int>, IConvertible<float>
{
    public double Value;
    int IConvertible<int>.Convert(Specialisation<int> t) { return (int)Value; }
    float IConvertible<float>.Convert(Specialisation<float> t) { return (float)Value; }
    public T Convert<T>() { return ((IConvertible<T>)this).Convert(Specialisation<T>.Instance); }
}
public static class Program
{
    public static void Main()
    {
        ConvertMe c = new ConvertMe();
        c.Value = 12.3456789;
        Console.WriteLine(c.Convert<int>());
        Console.WriteLine(c.Convert<float>());
        Console.ReadKey();
    }
}

This use of specialisation could be generalised to other applications in order to provide specialised behaviour. I won't add any more examples as the ones provided are long enough as it is, and I'm sure everyone here has the skill and imagination to find other applications of this pattern.

First of all, I would question the need for such interface. It makes sense for a type to hold a value, but I think it doesn't make much sense for a type to decide to what types to convert that value, or how.

Second, if you really do need this, then I would prefer each target type to have a separate method, like ConvertToInt() (or ConvertToInt32() if you follow the naming guidelines).

You don't necessarily need an interface for this, but even if you do, you're exchanging a small inconvenience in implementation (having to write few very simple interfaces with some repetition) for a big inconvenience in usage (having to write that Specialisation<int>.Instance every time and making the code less readable). So, I think using this approach is worth it even with interfaces.

Third, if you really do need this and you want to take advantage of generics, you could use explicit interface implementation and casting to decide to what type to convert. Something like:

public interface IConvertible<T>
{
    T Convert();
}

public sealed class ConvertMe : IConvertible<int>, IConvertible<float>
{
    public double Value;
    int IConvertible<int>.Convert() { return (int)Value; }
    float IConvertible<float>.Convert() { return (float)Value; }
}

public static class Program
{
    public static void Main()
    {
       ConvertMe c = new ConvertMe();
       c.Value = 12.3456789;
       Console.WriteLine(((IConvertible<int>)c).Convert());
       Console.WriteLine(((IConvertible<float>)c).Convert());
    }
}

This results in code that is shorter and more readable (I think) than your version.

One issue with it is that if you don't make ConvertMe sealed, trying to convert to an unsupported type becomes a runtime exception, instead of a compile-time error.


Function that generates steps time series from user given values I created simple function, that generate values of series representing repeating sequence of steps in time. User can define:

step values
width of the steps
how many times the step sequence should repeat
or the size of returned series

If the size is not defined, the size of returned data should be determined by number of repeats.

So the call

steps(2, [1, 2, 3], repeat=2)

should return

[1 1 2 2 3 3 1 1 2 2 3 3]

The function follows

def steps(step_width, values, repeat=1, size=None):
    """
    This function generates steps from given values.

    **Args:**

    * `step_width` - desired width of every step (int)

    * `values` - values for steps (1d array)

    **Kwargs:**

    * `repeat` - number of step sequence repetions (int), this value is used,
      if the `size` is not defined

    * `size` - size of output data in samples (int), if the `size` is used,
      the `repeat` is ignored.

    **Returns:**

    * array of values representing desired steps (1d array)
    """
    try:
        step_width = int(step_width)
    except:
        raise ValueError('Step width must be an int.')
    try:
        repeat = int(repeat)
    except:
        raise ValueError('Repeat arg must be an int.')
    try:
        values = np.array(values)
    except:
        raise ValueError('Values must be a numpy array or similar.')
    # generate steps
    x = np.repeat(values, step_width)
    if size is None:
        # repeat according to the desired repetions
        x_full = np.tile(x, repeat)
    else:
        try:
            repeat = int(repeat)
        except:
            raise ValueError('Repeat arg must be an int.')
        # repeat till size is reached and crop the data to match size
        repeat = int(np.ceil(size / float(len(x))))
        x_full = np.tile(x, repeat)[:size]
    return x_full

I would appreciate any feedback. Especially I am not sure about effectivity of the error raising as it is implemented right now.

Your type checks are distracting, they are probably better extracted to a separate function.

def check_type_or_raise(obj, expected_type, obj_name):
    if not isinstance(obj, expected_type):
        raise TypeError(
            "'{}' must be {}, not {}".format(
                obj_name,
                expected_type.__name,
                obj.__class__.__name__)

Note that this has slightly different behaviour than your code: you would e.g. accept a step_width of 2.5 and convert it to a 2. This is typically not what you want, so I think it is better to raise for such cases.

If you don't mind the conversion, then you probably don't need to wrap your calls to int() in a try and re-raise, as you will already get a nice error, e.g.:

>>> int('abc')
ValueError: invalid literal for int() with base 10: 'abc'

I would also refactor your code to have the output creation happen in a single, common place, after some manipulation of the arguments:

def steps(step_width, values, repeat=1, size=None):
    check_type_or_raise(step_width, int, 'step_width')
    check_type_or_raise(repeat, int, 'repeat')
    values = np.asarray(values)
    if size is not None:
        check_type_or_raise(size, int, 'size')
        # This does rounded up integer division without floating point
        repeat = (size - 1) // (len(values) * step_width) + 1
    return np.tile(np.repeat(values, step_width), repeat)[:size]


Implementation of Heap Sort Is this the correct implementation of Heap Sort using Java? How can it be improved further?

import java.util.Arrays;

public class HeapSort {

    public static int heapSize;
    public static int LEFT(int i)
    {
        //returns left index of a zero index based array
        return 2*i+1;
    }

    public static int RIGHT(int i)
    {
        //returns right index of a zero based array
        return 2*i+2;
    }

    public static void BUILD_MAX_HEAP(int A[])
    {
        heapSize = A.length;//heap size initialised
        for(int i=A.length/2; i>=0;i--)//since n/2, n/2+1 ... are leaves we can start from n/2 step downwards
        {
            //call MAX_HEAPIFY on each root node starting from n/2
            MAX_HEAPIFY(A, i);
        }
    }

    public static void MAX_HEAPIFY(int A[],int i)
    {
        int l=LEFT(i);//the left element's index which is 2*i+1 (for zero based indexed array)
        int r=RIGHT(i);//right index = 2*i+2;
        int largestElementIndex = -1;//index can't be negative so initialise largest index , it will be used later
        //heapSize is the current size of the heap being worked upon
        //check if left index lies within the heap.
        //if element at left index is greater than root(A[i]) element, max heap property is violated
        //copy the index of violated element in largestElementIndex
        if(l<heapSize && A[l]>A[i]){
            largestElementIndex = l;
        }
        else //if max heap property is not violated copy the root's index in largestElementIndex
        {
            largestElementIndex=i;
        }
        //check to see the right sub tree for max heap property violation
        //here the largestElementIndex is calculated from previous step
        if(r<heapSize && A[r]>A[largestElementIndex])
        {
            largestElementIndex = r;
        }
        //if root doesn't has the largest index then swap the largest element with root element

        if(largestElementIndex!=i)
        {
            int temp = A[i];
            A[i]=A[largestElementIndex];
            A[largestElementIndex]=temp;
            //after swap, recursively call the MAX_HEAPIFY on the largest index(root element)
            MAX_HEAPIFY(A, largestElementIndex);
        }
    }

    public static void HEAP_SORT(int A[])
    {
        //max heap is built with heapSize initialised
        BUILD_MAX_HEAP(A);
        //starting from end loop through entire array
        for(int i=A.length-1;i>=0;i--)
        {
            //since heap is already heapified and maintains max heap property
            //the first element of the array is root of max heap
            //swap it with the extreme element of the array i.e. setting the largest element to the end of array
            int temp = A[0];
            A[0]=A[i];
            A[i]=temp;
            //reduce the heap window by 1
            heapSize  = heapSize-1;
            //call max heapify on the reduced heap
            MAX_HEAPIFY(A,0);
        }
    }

    public static void main(String[] args) {

        int A[] = new int[]{4,1,3,2,16,9,10,14,8,7};
        HEAP_SORT(A);
        System.out.println(Arrays.toString(A));
    }
}

As everyone has immediately pointed out, please stick with the namingConventions for methods and parameter names.

Your code is not object oriented, as Java code should be.  One sign is that you are using static functions everywhere.  Another hint is that the name of a class should always be a noun; if it's a verb then your objects are likely to be crappy.  You want to define a MaxHeap class.  Its outline should look like this:

public class MaxHeap {
    private int data[];
    private int size;

    public static void sort(int data[]) { ... }

    public MaxHeap(int data[]) { ... }
    public int removeNext() { ... }
    public int size() { return this.size; }

    private int leftChild(int i) { return 2 * i + 1; }
    private int rightChild(int i) { return 2 * i + 2; }
    private void heapify(int i) { ... }
}

Decide what operations you want to publicly support in your interface.  You'll need the constructor.  Since you will be doing removal of the root node in your HEAD_SORT() function anyway, you might as well support that in the public interface as removeNext().  A size() method is trivial to implement.  It would be nice to implement add(int datum) as well, but for now let's leave it out since it's not necessary for your immediate goal of implementing heapsort.  We should also provide sort(int[] data) as a class method, because that is the point of this exercise.  (There's no need to call it heapSort(int[] data), as "heap" is already part of the name of the class.)  Everything else should be declared private — the inner workings of the class are nobody else's business.

The heap constructor is just your BUILD_MAX_HEAP() code.  The only real correction I would make is that you can start i one element before where you did.

public MaxHeap(int data[]) {
    this.data = data;
    this.size = data.length;
    for (int i = size / 2 - 1; i >= 0; i--) {
        heapify(i);
    }
}

Now, on to the heapify() method.  There's no need to call it maxHeapify(), since it's a member of the MaxHeap class.  It can just take one parameter, since the data array is now an instance variable.  Also,

private void heapify(int i) {
    // You might as well start with this optimistic assumption
    int largestElementIndex = i;

    // The optimistic assumption leads to nice parallelism between the
    // left-child and right-child cases.
    int l = leftChild(i);
    if (l < size && data[l] > data[largestElementIndex]) {
        largestElementIndex = l;
    }
    int r = rightChild(i);
    if (r < size && data[r] > data[largestElementIndex]) {
        largestElementIndex = r;
    }

    // If heap consistency was locally violated
    if (largestElementIndex != i) {
        int swap = data[i];
        data[i] = data[largestElementIndex];
        data[largestElementIndex] = swap;

        // Recursively heapify the affected sub-tree
        heapify(largestElementIndex);
    }
}

Here's removeNext().  It returns the next largest element (the root node value) and shrinks the heap.

public int removeNext() throws NoSuchElementException {
    if (size == 0) {
        throw new NoSuchElementException();
    }
    int next = data[0];
    data[0] = data[--size];
    heapify(0);
    return next;
}

Finally, here's sort().

public static void sort(int[] data) {
    MaxHeap heap = new MaxHeap(data);
    for (int i = heap.size() - 1; i >= 0; i--) {
        int nextMax = heap.removeNext();

        // This is a rather naughty move, since heap.data is
        // aliased to data.  We do this only because we know
        // from the way the heap works that the end of the array
        // can be used to store our sorted result in place.
        data[i] = nextMax;
    }
}

One thing I'm not happy about in the above code is the fact that the MaxHeap adopts the input array rather than making a copy.  That is probably surprising behaviour to users of the class, who probably don't expect the contents of the passed-in data array to be modified.

On the other hand, it's nice to be able to implement sort() in place without allocating another array.

One cop-out would be to make the constructor private, essentially declaring that the code in this class is only usable through the sort() function.

A workaround is to implement two variants of the constructor, such that you have to explicitly ask for the array to be adopted:

/**
 * Copies the data
 */
public MaxHeap(int data[]) {
    this(data, true);
}

/**
 * If copyData is false, MaxHeap adopts the data array, and may overwrite its
 * contents; it does not make a copy!
 */
public MaxHeap(int data[], boolean copyData) {
    this.data = copyData ? Arrays.copyOf(data, data.length) : data;
    ...
}

public static void sort(int[] data) {
    MaxHeap heap = new MaxHeap(data, false);
    ...
}


Lamba sorting for results I have the List method that takes sorting parameter to sort results.  The sorting parameter value is the same as the column name.

public ActionResult List(string sorting = "Name", string sortingMethod = "ASC")
{
    var productList = productRepository.Get();

    if(!String.IsNullOrEmpty(Sorting){
        if(String.IsNullOrEmpty(SortingMethod) ||SortingMethod == "ASC"){
            switch(sorting){
                case "Name" :
                    productList = productList.OrderBy(x => x.Name); break;
                case "Price" :
                    productList = productList.OrderBy(x => x.Price); break;
                case "Category" :
                    productList = productList.OrderBy(x => x.Price); break;
                //...
            }
        }else{
            switch(sorting){
                case "Name" :
                    productList = productList.OrderByDescending(x => x.Name); break;
                case "Price" :
                    productList = productList.OrderByDescending(x => x.Price); break;
                case "Category" :
                    productList = productList.OrderByDescending(x => x.Price); break;
                //...
            }
        }
    }

    return View(productList.ToList());
}

But I believe there is a better way to make code shorter.

public ActionResult List(string sorting = "Name", string sortingMethod = "ASC")
{
    var productList = productRepository.Get();

    if(!String.IsNullOrEmpty(Sorting){
        if(String.IsNullOrEmpty(SortingMethod) ||SortingMethod == "ASC"){
            productList = productList.OrderBy();  // <== ???
        }else{
            productList = productList.OrderByDescending(); // <== ???
        }
    }

    return View(productList.ToList());
}

What about seperating your sorting from your criteria?  I'm like you, I'm sure there is another way so am interested to see other opinions. I'm not exactly sure if this would scale for multiple orderBys either but I think it might (NOTE:  this code is untested and some types are not defined)

Something along the lines of:

public ActionResult List(string sorting = "Name", string sortingMethod = "ASC")
{
   var sortDirection = GetSortDirection(sortingMethod);
   var criteria = GetSortCriteria(sorting);

   var products = GetProductsOrderBy(criteria, sortDirection);

   return View(products.ToList());
}

// I'm not actually sure of the return type here.  Will attempt to test when I
// get in front of my development pc
Func<Product, object> GetSortCriteria(string sortOn)
{
   switch(sortOn)
   {
      case "Price":
      case "Category":
         return x => x.Price;
      default:
         return x.Name;
   }
}

IEnumerable<ProductList> GetProductsOrderBy(Action sortOn, SortDirection sortDirection)
{
    var productList = productRepository.Get();

    return sortDirection == SortDirection.Ascending ?
           productList.OrderBy(sortOn) :
           productList.OrderByDescending(sortOn);
}

SortDirection GetSortDirection(string sortBy)
{
   return sortBy == "ASC" ? SortDirection.Ascending : SortDirection.Descending;
}


Convert first letter of every word to uppercase I am learning swift and I came across this problem.
Converting each start letter to capitalized form if it's lowercase.
func upperCaseFirstCharacter(str:String){
    let myArr = str.components(separatedBy: " ")
    var finalStr :String=""
    for word in myArr {
        let myStr = word.replacingCharacters(in: ...str.startIndex, with: word.first?.uppercased() ?? "")
        finalStr.append(myStr)
        if myArr.last != word{
            finalStr.append(" ")
        }
    }
    print(finalStr)
}

A few observations on improving the code snippet, but using the same basic logic:

If you split the string with components, you can use joined to put them back together.
The ...str.startIndex syntax will crash if you supply a zero length string. I would suggest using prefix and dropFirst to simplify the code.
When writing routines like this, I’d generally suggest using StringProtocol so that it works for both string and substrings. In this case, I would make it an extension of StringProtocol.
This method of capitalizing the first character of every word (regardless of part of speech) is sometimes called “start case” or “proper case”. I would suggest renaming the method accordingly.

Thus:
extension StringProtocol {
    func startcased() -> String {
        components(separatedBy: " ")
            .map { $0.prefix(1).uppercased() + $0.dropFirst() }
            .joined(separator: " ")
    }
}

let string = "this is a test"
print(string.startcased())                                  // This Is A Test

If the in the intent was to achieve “title case”, we often wouldn't capitalize certain parts of speech, e.g., articles and conjunctions. The NaturalLanguage library can be used for more refined capitalization logic:
import NaturalLanguage

extension StringProtocol {
    func titlecased() -> String {
        var capitalizeWordRanges: [Range<String.Index>] = []

        var result = String(self)

        // find words other than conjunctions and determiners

        let tagger = NLTagger(tagSchemes: [.lexicalClass])
        tagger.string = result
        let options: NLTagger.Options = [.omitWhitespace, .joinContractions]
        var isStartOfSentence = true

        tagger.enumerateTags(in: startIndex..<endIndex, unit: .word, scheme: .lexicalClass, options: options) { tag, range in
            guard let tag = tag else { return true }

            print(tag.rawValue, self[range])

            switch tag {
            case .sentenceTerminator:               // see if we're potentially starting a new sentence
                isStartOfSentence = true
                return true

            case .punctuation, .openQuote, .closeQuote, .openParenthesis, .closeParenthesis:
                return true

            case .conjunction where self[range].count < 4,
                 .determiner where self[range].count < 4,
                 .preposition where self[range].count < 4:         // only capitalize conjunctions or articles at the start of a sentence
                if isStartOfSentence { fallthrough }

            default:
                capitalizeWordRanges.append(range)            // otherwise, add this to the array of ranges to t
            }

            isStartOfSentence = false

            return true
        }

        // loop through, doing replacements; do it in reverse order because sometimes the capitalized string isn't the same length, e.g. German ß -> SS

        for range in capitalizeWordRanges.reversed() {
            result.replaceSubrange(range.lowerBound...range.lowerBound, with: result[range.lowerBound].uppercased())
        }

        return result
    }
}

let string2 = "this is a test"
print(string2.titlecased())                                 // This Is a Test

let string3 = #"The sign said, "don't feed the animals.""#
print(string3.titlecased())                                 // The Sign Said, "Don't Feed the Animals."

let string4 = "¿donde está la biblioteca?"
print(string4.titlecased())                                 // ¿Donde Está la Biblioteca?

Note, enumerating through the words solves another problem, namely situations where words may not preceded immediately by a space, but rather, perhaps punctuation, such as the quotation marks before “don't” in string3, above. Just splitting and joining by spaces will only work in the simplest of scenarios. Also, some languages have punctuation at the start of a sentence, e.g. the Spanish “¡Ay, caramba!” or “¿Donde está la biblioteca?” If you iterate through the actual words, rather than relying on spaces, you solve that problem.
This admittedly is not perfect “title case” logic. E.g. it is taking care of the capitalizing of the first character, but not shifting the rest of the word to lowercase (e.g., if the input string was entirely in uppercase). But if you are only worried about the first letter, this is probably better than the naive start/proper capitalization routine. And if you want to implement a richer title case rendition, this routine might be a good starting point.


Implementation of observer pattern for Node.js I wrote an implementation of observer pattern as part of my online battle Chess game:

const observers = Symbol();

class Observer{

    constructor(){

        if(new.target === Observer){

            throw new Error('Cannot create new Observer object. Observer is supposed to be inherited only.');
        }

        /**
         * @private
         * @type {Set}
         */
        this[observers] = new Set();
    }

    on(observer, event, callback){

        this.getObservers().add({

            observer,
            event,
            callback
        });
    }

    off(observer, event){

        const observers = this.getObservers();
        const observerEntries = this.getObservers().values();

        for(let entry of observerEntries){

            if(entry.observer === observer && entry.event === event){

                observers.delete(entry);
            }
        }
    }

    notify(event, data = {}){

        const observerEntries = this.getObservers().values();

        for(let entry of observerEntries){

            if(entry.event === event){

                entry.callback.call(entry.observer, data);
            }
        }
    }

    getObservers(){

        return this[observers];
    }
}

module.exports = Observer;

The source code for the full game is in a GitHub repository. This is an early version, without many features implemented and possibly with many bugs. I have a humble request to review my code. I'm aware of several things (backend Node code is very messy). The client-side is written in MVC and Observer patterns. I'm looking for feedback (both positive and negative) and advice/guidelines.

The name of the Observer class is confusing: an Observer contains observers?  I think that the class would make more sense if you renamed it Observable.

For the .on() and .off() methods, I think that the event parameter should come first.  That would mimic the natural English expression: "on event tell observer to do action".  It's also how it's done in other frameworks like jQuery.

I'm mildly annoyed by the excessive vertical spacing of the code.


Ruby object declared as constant I have this piece of Ruby code:

HTTPCLIENT = HTTPClient.new

def read_page(url)
  HTTPCLIENT.get(url).body
end

The HTTPClient object is being created and declared as a constant, and thus it can be accessible by methods.

I would do this by declaring it as a global variable ($) or as a class variable (@), but I'm not sure if it's actually wrong the way it is.

I don't declare objects as variables because, well, their memory contents can vary. I only use constants to declare simple types (strings, numbers) or classes and modules. But objects? I'm not sure about it.

My question is: is this a bad code or it's just me? If so, then why do you think is a bad code?

I'd propose a 3rd way: Make it a method:

def client
  @client ||= HTTPClient.new
end

def read_page(url)
  client.get(url).body
end

There are some advantages to this:

You get the same scoping as with a constant (which you wouldn't get with a $-global), as well as visibility modifiers.
It's flexible. You can return anything that fulfils the API contract. It's the Dependency Inversion Principle (the D in SOLID) of relying on an abstraction. It also allows you to stub/mock the method and return value in tests much more easily.
If you want it to be shared across instances, use object composition (e.g. PageReader.new(existing_client)); if you want different instances to have their own client, use the implementation above.

I'd agree that constants should for be kept "primitive" - or at any rate, they should be data-only. E.g. a Hash, while not "primitive" per se, used as constant lookup table is fine. But a HTTP client instance is a bit much.

A 4th option is to make it a singleton, which is more or less a "constant instance". It'd probably require you to wrap (or extend) the HTTPClient class, e.g.:

require 'singleton'

class Client
  include Singleton

  def initialize
    @client = HTTPClient.new
  end

  def get(url)
    @client.get(url)
  end
end

# ...

def read_page(url)
  Client.instance.get(url).body
end


Advent of Code 2019, Day 2 in Haskell This is my solution for Day 2 of Advent of Code 2019.

It's the eval function I want to get reviewed. Is this acceptable Haskell code? Is it even close to being idiomatic? Also, is it OK to leverage laziness this way?

module Day2 where

import Data.Vector (Vector, fromList, head, (!), (//))
import Data.List.Split (splitOn)

type Intcode = Vector Int

data Op = Add | Mult | Noop

intToOp :: Int -> Op
intToOp 1 = Add
intToOp 2 = Mult
intToOp 99 = Noop
intToOp x = error $ "invalid opCode, should not happen" ++ show x

eval :: Intcode -> Intcode
eval intcode = go intcode 0
  where
    go intcode currentIndex =
      let
        op = intToOp $ intcode ! currentIndex
        v1Pos = intcode ! (currentIndex + 1)
        v2Pos = intcode ! (currentIndex + 2)
        savePos = intcode ! (currentIndex + 3)
        nextIndex = currentIndex + 4
        v1 = intcode ! v1Pos
        v2 = intcode ! v2Pos
      in
        case op of
          Add -> go (intcode // [(savePos, (v1 + v2))]) nextIndex
          Mult -> go (intcode // [(savePos, (v1 * v2))]) nextIndex
          Noop -> intcode

part1 :: IO ()
part1 = do
  input <- readFile "../input/day2.txt"
  let intcode = fromList (read <$> splitOn "," input)
  let result = eval $ intcode // [(1, 12), (2, 2)]
  putStrLn $ show $ result ! 0

part2 :: IO ()
part2 = do
  input <- readFile "../input/day2.txt"
  let memory = fromList (read <$> splitOn "," input)
  let inputs = [(i, j) | i <- [0..99], j <- [0..99]]
  let results = fmap (\(noun, verb) -> (eval $ memory // [(1, noun), (2, verb)], noun, verb)) inputs
  let (result, noun, verb) = Prelude.head $ filter (\(res, _, _) -> (res ! 0) == 19690720) results
  putStrLn $ show $ 100 * noun + verb

slice combines some lookups. The index arithmetic can be frontloaded.

eval :: Intcode -> Intcode
eval = go [0,4..] where
  go (index:es) intcode =
    let
      [opcode, v1Pos, v2Pos, savePos] = toList $ slice index 4 intcode
      v1 = intcode ! v1Pos
      v2 = intcode ! v2Pos
    in
      case intToOp opcode of
        Add  -> go es $ intcode // [(savePos, v1 + v2)]
        Mult -> go es $ intcode // [(savePos, v1 * v2)]
        Noop -> intcode

// takes linear time on immutable vectors. Noop should be Halt. This displays the unreliability of adding data types for their suggestive names. Therefore, inline intToOp.

eval :: Intcode -> Intcode
eval = modify $ \intcode ->
  void $ runMaybeT $ for_ [0,4..] $ \index -> do
    [opcode, v1Pos, v2Pos, savePos] <- traverse (read intcode) [index..index+3]
    v1 <- read intcode v1Pos
    v2 <- read intcode v2Pos
    case opcode of
      1 -> lift $ write intcode savePos $ v1 + v2
      2 -> lift $ write intcode savePos $ v1 * v2
      99 -> empty
      _ -> error $ "invalid opCode, should not happen" ++ show opcode

Of course, all of this imperativeness doesn't feel Haskell. However, interpreting an imperative language is pretty much the one time you need not feel bad about that.


Abstract repository for entity framwork I write abstract repository

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Entity;
using System.Linq;
using System.Linq.Expressions;

namespace Framework.DataLayer
{
    public class Context<T> : IContext<T> where T : class
    {
        private readonly DbContext _dbContext;

        public Context(DbContext dbContext)
        {
            _dbContext = dbContext;
        }

        public bool TryGet(Expression<Func<T, bool>> predicate, out T entity)
        {
            entity = List(predicate).SingleOrDefault();
            return entity != null;
        }

        public T Get(Expression<Func<T, bool>> predicate)
        {
            return List(predicate).Single();
        }

        public List<T> List(Expression<Func<T, bool>> predicate = null)
        {
            IQueryable<T> result = _dbContext.Set<T>().AsQueryable();
            if (predicate != null)
                result = result.Where(predicate);
            return result.ToList();

        }

        public T Add(T t)
        {
            _dbContext.Entry(t).State = EntityState.Added;
            return t;

        }

        public void Delete(Expression<Func<T, bool>> predicate)
        {
            List(predicate).ToList().ForEach(p => { _dbContext.Entry(p).State = EntityState.Deleted; });
        }

        public void Delete(T entity)
        {
            _dbContext.Entry(entity).State = EntityState.Deleted;
        }

        public T Update(T t)
        {
            _dbContext.Entry(t).State = EntityState.Modified;
            return t;

        }

    }
}

Is this correct?

For one, it is not an abstract repository, but a generic repository. Never mind. As mentioned in a comment, generic repositories are hardly ever helpful and I would never use them. But apart from that, there's a couple of things I can say about this code:

TryGet should be absolutely safe (that's what the "Try" prefix conveys), so use FirstOrDefault() and only return an entity if exactly one is fetched.
I would not return a List, but an IEnumerable, so you retain deferred execution.
If the repository is only internally used, e.g. to supply data for services, I would return IQueryable, so you can compose queries from different repositories, do projections, includes and everything you can do with IQueryable and not with IEnumerable.
As said in another comment, it is not necessary to return anything from Add and Update, because you always modify a reference type.
I would prefer method names like MarkForDelete, because that's what actually happens.


Pandas merge a "%Y%M%D" date column with a "%H:%M:%S" time column I have a dataframe with a column composed by date object and a column composed by time object.
I have to merge the two columns.
Personally, I think it is so ugly the following solution.
Why I have to cast to str?
I crafted my solution based on this answer
#importing all the necessary libraries
import pandas as pd
import datetime

#I have only to create a Minimal Reproducible Example
time1 = datetime.time(3,45,12)
time2 = datetime.time(3,49,12)
date1 = datetime.datetime(2020, 5, 17)
date2 = datetime.datetime(2021, 5, 17)
date_dict= {"time1":[time1,time2],"date1":[date1,date2]}

df=pd.DataFrame(date_dict)

df["TimeMerge"] = pd.to_datetime(df.date1.astype(str)+' '+df.time1.astype(str))

We can let pandas handle this for us and use DataFrame.apply and datetime.datetime.combine like this:
df["TimeMerge"] = df.apply(lambda row: datetime.datetime.combine(row.date1, row.time1), axis=1)

Although the following approach is more explicit and might therefore be more readable if you're not familiar with DataFrame.apply, I would strongly recommend the first approach.
You could also manually map datetime.datetime.combine over a zip object of date1 and time1:
def combine_date_time(d_t: tuple) -> datetime.datetime:
    return datetime.datetime.combine(*d_t)

df["TimeMerge"] = pd.Series(map(combine_date_time, zip(df.date1, df.time1)))

You can also inline it as an anonymous lambda function:
df["TimeMerge"] = pd.Series(map(lambda d_t: datetime.datetime.combine(*d_t), zip(df.date1, df.time1)))

This is handy for simple operations, but I would advise against this one-liner in this case.

By the way, the answer your were looking for can also be found under the question you linked.


Dealing with a dirty table I have to deal with a really ugly looking response I get back from a particular device. The string represents a table. Each section of the string represents the form:

["id", "date_time", "path", "type","duration","count", "rate"]

Which as an example looks like:

"1,2014-4-3 10:1:10,somerandompath,some/screwy!stringwithstufflike[0,0,0,0]inIt,0,0;"

all table entries as you can see are terminated by ; and have their elements separated by ,

But some entries like in some/screwy!stringwithstufflike[0,0,0,0]inIt have commas within their data,.. and times/dates 10:1:10 aren't padded with 0's to be consistent

This is what I have so far, but it doesn't feel very optimized...

def GetEventTable(self):
        # After I split by ',' commas can be inside of [..],
        # so I combine entries until their number of starting/end braces match
        def split_entry(entry):
            splitentry = entry.split(',')
            for ii in range(len(splitentry))[::-1]:
                start_bracket = splitentry[ii].count('[')
                end_bracket = splitentry[ii].count(']')
                if not start_bracket == end_bracket:
                    splitentry[ii-1:ii+1] = [splitentry[ii-1] + ',' + splitentry[ii]]
            return splitentry
        event_lable = ["id", "date_time", "path", "type","duration","count", "rate"]
        ## This section just talks to the remote device and gets the unformatted string ###
        print "Sending: GET EVENT TABLE"
        stat, raw_data = SendRecieve(self, "GET EVENT TABLE")
        if IsError(stat):
            raise Exception("Error type '{}': {}".format(stat, raw_data))
        print raw_data
        ##################################################################################
        events = raw_data.split(';')[:-1]
        event_table = [{key:value for key,value in zip(event_lable,split_entry(entry) )} for entry in events ]
        return event_table

How can I improve the efficiency and also the readability?

It returns a list of dictionaries with keys ["id", "date_time", "path", "type","duration","count", "rate"]

Disclaimer

Because a lot of context is missing, here's the code I will review :

def split_entry(entry):
    splitentry = entry.split(',')
    for ii in range(len(splitentry))[::-1]:
        start_bracket = splitentry[ii].count('[')
        end_bracket = splitentry[ii].count(']')
        if not start_bracket == end_bracket:
            splitentry[ii-1:ii+1] = [splitentry[ii-1] + ',' + splitentry[ii]]
    return splitentry

event_lable = ["id", "date_time", "path", "type","duration","count", "rate"]
raw_data = "1,2014-4-3 10:1:10,somerandompath,some/screwy!stringwithstufflike[0,0,0,0]inIt,0,0;"
events = raw_data.split(';')[:-1]
event_table = [{key:value for key,value in zip(event_lable,split_entry(entry) )} for entry in events ]
print event_table

ii is a pretty bad name. i is twice shorter and so many times easier to understand.

Instead of creating a new list by reversing the return of range, you could just use all the potential of range (or xrange):

Instead of using not A == B, you could write a more natural A != B

Looping over an array using range(len(a)) is an anti-pattern in python. To loop on an array while keeping track of the index, the usual solution is to use enumerate(). In your case, because you are updating the array anyway, it is quite tricky to loop on it.

Final point : I had trouble understanding your list slicing stuff but as I was messing a bit with it, I obtained a result different from yours ('rate' appears in the final result so it might actually be a good thing) :

def split_entry(entry):
    splitentry = entry.split(',')
    for i in xrange(len(splitentry)-1,0,-1):
        chunk = splitentry[i]
        if chunk.count('[') != chunk.count(']'):
                splitentry[i-1] += ',' + chunk
                splitentry[i] = '0'
    return splitentry

event_lable = ["id", "date_time", "path", "type","duration","count", "rate"]
raw_data = "1,2014-4-3 10:1:10,somerandompath,some/screwy!stringwithstufflike[0,0,0,0]inIt,0,0;"
events = raw_data.split(';')[:-1]
event_table = [{key:value for key,value in zip(event_lable,split_entry(entry) )} for entry in events]
print event_table


Eight Directions Crossword I solved a problem which you can read about here, but it gives me "time limit exceeded" even though it runs in 4 seconds. I really don't understand why.

I don't think my code can be any simpler, basically it has as many functions as there are solutions I think. At the start of my code I find the first letter matching the first letter of the word I am searching for, then when it finds it, it checks in 8 directions if it can make the whole word without going out of the crossword.

program ideone;
var
kar:array[1..1000, 1..1000] of char;
konacno:int64;
i2,i3,n,xx,yy:integer;
s:string;
procedure trag(y:integer; x:integer; y1:integer; x1:integer; i:integer);
        var
        x2,y2:integer;
    begin
    x2:=x+x1;
    y2:=y+y1;
    while (x2<=n) and (x2>0) and (y2<=n) and (y2>0) do begin
        if kar[y2,x2]=s[i+1] then
            if i=length(s)-1 then
            konacno:=konacno+1
            else
            trag(y2,x2,y1,x1,i+1);

    x2:=x2+x1;
    y2:=y2+y1;
    end;
    end;
begin
readln(n,s);
delete(s,1,1);
xx:=1;
yy:=1;
for i2:=1 to n do begin
for i3:=1 to n do
read(kar[i2,i3]);
readln();
end;
konacno:=0;

for i2:=1 to n*n do begin
    if kar[yy][xx]=s[1] then begin
        trag(yy,xx,-1,-1,1);
        trag(yy,xx,-1,0,1);
        trag(yy,xx,-1,1,1);
        trag(yy,xx,0,1,1);
        trag(yy,xx,1,1,1);
        trag(yy,xx,1,0,1);
        trag(yy,xx,1,-1,1);
        trag(yy,xx,0,-1,1);
    end;
    xx:=xx+1;
    if xx>n then begin
    xx:=1;
    yy:=yy+1;
    end;

end;

writeln(konacno);

end.

If you have any tips please tell me, I am new to Pascal, so I don't know all the tricks for speeding up the program, and I don't want to use C or C++.

First you can change those non-required letters to a specific character like '.'.

Turning the 2D array into a string or array of byte. For example:

1234
5678
90ab
cdef
=>
string1: 1234
string2: 5678
string3: 90ab
string4: cdef

Remove '.' in these string so that later u can check faster.

Also doing this with other rows, column, and oblique.

Scan result in -> or <- direction only.

Related algorithm you may find here.

Moreover, you can omit some impossible cases such as:

input:

8 aaaa
aaaaoooo
oooooooo
oooooooo
oooooooo
oooooooo
oooooooo
oooooooo
oooooooo

Find those which never touch required characters like the last three rows in this case and  do not trag() with it.

I suggest using this algorithm to scan it:

For rows:

var
    ignore: array[1..1000, 1..1000] of boolean;
    counter: integer;
counter:=0;
for i:=1 to n do
    while j<=n do
    begin
        ignore[i, j]:=false;
        if kar[i, j] = '.' then
        begin
             count:=count+1;
             ignore[i, j]:=counter> length(s);
             break;
        end
        else
            counter:=0;
    end;


Scraping Premier League results I have a code that scrapes list of URL and appends dataframe as below:
from selenium import webdriver
import pandas as pd
from tabulate import tabulate
import os
os.chdir(r"C:\Users\harsh\Google Drive\sportsintel.shop\Files")
cwd = os.getcwd()
print(cwd)

browser = webdriver.Chrome()

browser.get("https://www.oddsportal.com/soccer/england/premier-league/results/")

df = pd.read_html(browser.page_source, header=0)[0]

dateList = []
gameList = []
scoreList = []
home_odds = []
draw_odds = []
away_odds = []

for row in df.itertuples():
    if not isinstance(row[1], str):
        continue
    elif ':' not in row[1]:
        date = row[1].split('-')[0]
        continue
    time = row[1]
    dateList.append(date)
    gameList.append(row[2])
    scoreList.append(row[3])
    home_odds.append(row[4])
    draw_odds.append(row[5])
    away_odds.append(row[6])

result_comp_1 = pd.DataFrame({'date': dateList,
                       'game': gameList,
                       'score': scoreList,
                       'Home': home_odds,
                       'Draw': draw_odds,
                       'Away': away_odds})

print(tabulate(result_comp_1))

browser.get("https://www.oddsportal.com/soccer/england/premier-league/results/#/page/2/")

df = pd.read_html(browser.page_source, header=0)[0]

dateList = []
gameList = []
scoreList = []
home_odds = []
draw_odds = []
away_odds = []

for row in df.itertuples():
    if not isinstance(row[1], str):
        continue
    elif ':' not in row[1]:
        date = row[1].split('-')[0]
        continue
    time = row[1]
    dateList.append(date)
    gameList.append(row[2])
    scoreList.append(row[3])
    home_odds.append(row[4])
    draw_odds.append(row[5])
    away_odds.append(row[6])

result_comp = pd.DataFrame({'date': dateList,
                       'game': gameList,
                       'score': scoreList,
                       'Home': home_odds,
                       'Draw': draw_odds,
                       'Away': away_odds})

new_df =result_comp_1.append(result_comp, ignore_index=True)

Can I make my code better to avoid redundancy?

Seems like there's one major point of redundancy, in that you're performing the same logic on the two different pages you're scraping (requesting site, parsing results, storing in dataframe).
You could eliminate the redundancy with a function that takes in the site URL and performs the logic... something like def get_matches_data(url)...


Parse RRULE icalendar entries I'm using the icalendar module to parse (google) calendar entries and the dateutil.rrule module to deal with recurring events.

Right now, I'm using the rrulestr class to parse (part of) the RRULE data and rruleset to add EXDATE entries:

from icalendar import Event
from dateutil.rrule import rrulestr, rruleset
from datetime import datetime, timedelta
from pytz import UTC
from pprint import pprint

# example google calendar entry
event_text = """BEGIN:VEVENT
SUMMARY:Test Event
DTSTART;TZID=Europe/Paris:20160806T100000
DTEND;TZID=Europe/Paris:20160806T120000
RRULE:FREQ=WEEKLY;UNTIL=20160910T000000Z
EXDATE;TZID=Europe/Paris:20160813T100000
EXDATE;TZID=Europe/Paris:20160820T100000
END:VEVENT"""

event = Event.from_ical(event_text)

rules_text = '\n'.join([line for line in event.content_lines()
                        if line.startswith('RRULE')])

rules = rruleset()

first_rule = rrulestr(rules_text,
                      dtstart=event.get('dtstart').dt)

# in some entries, tzinfo is missing
if first_rule._until and first_rule._until.tzinfo is None:
    first_rule._until = first_rule._until.replace(tzinfo=UTC)

rules.rrule(first_rule)

exdates = event.get('exdate')

if not isinstance(exdates, list):  # apparently this isn't a list when
    exdates = [exdates]            # there is only one EXDATE
for exdate in exdates:
    try:
        rules.exdate(exdate.dts[0].dt)
    except AttributeError:  # sometimes there is a None entry here
        pass

now = datetime.now(tz=UTC)
in_2_months = now + timedelta(days=60)
pprint(rules.between(now, in_2_months))

Apparently this works, but I'm wondering if there is a more compact and cleaner way of doing this?

I especially don't like the fact that the RRULE string is parsed twice.

Could the absence of tzinfo be handled differently?

Something else?

... a more compact and cleaner way of doing this? I especially don't like the fact that the RRULE string is parsed twice.

If you're going to use the rrule module's public API, then yes, your approach is the best way of doing this.


Server socket "wrap" functions This is a small socket library I made to avoid complicated C boilerplate code and several #includes for connecting sockets, bind, listening etc.

Please note that socket_create() does not support bind to specific IP addresses and that the IP parameter is ignored.

The only C++11 feature I am using is constexpr. Even though it is C++11, I decided not to wrap everything in a class, but to work with standard int.

In the future, I might add Unix domain socket in a way similar to  socket_create() and / or SSL support, but at the moment I do not need those.

sockets.h

#ifndef NET_SOCKETS_H_
#define NET_SOCKETS_H_

#include <cstdint>

namespace net{

constexpr int SOCKET_NONBLOCK   = 0x01;
constexpr int SOCKET_TCPNODELAY = 0x02;

// ===========================

constexpr int SOCKET_DEFAULTOPT = SOCKET_NONBLOCK & SOCKET_TCPNODELAY;

// ===========================

int socket_create(const char *ip, uint16_t port, uint16_t backlog = 0, int options = SOCKET_DEFAULTOPT) noexcept;

bool socket_makeNonBlocking(int fd) noexcept;
bool socket_makeTCPNoDelay(int fd) noexcept;

int socket_accept(int fd) noexcept;

void socket_close(int fd) noexcept;

bool socket_check_eagain() noexcept;

}

#endif

sockets.cc

#include "sockets.h"

#include <sys/socket.h>     // EAGAIN
#include <arpa/inet.h>      // AF_INET
#include <netinet/tcp.h>    // TCP_NODELAY
#include <fcntl.h>      // fcntl

#include <unistd.h>     // close

#include <errno.h>      // errno

namespace net{

bool socket_check_eagain() noexcept{
    return errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK;
}

bool socket_makeNonBlocking(int const fd) noexcept{
    if (fd >= 0)
        return fcntl(fd, F_SETFL, O_NONBLOCK) >= 0;

    return false;
}

bool socket_makeTCPNoDelay(int const fd) noexcept{
    int const opt = 1;
    return (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, & opt, sizeof opt) >= 0);
}

void socket_close(int const fd) noexcept{
    if (fd >= 0)
        ::close(fd);
}

int socket_accept(int const fd) noexcept{
    if (fd >= 0)
        return ::accept(fd, NULL, NULL);

    return -1;
}

int socket_create(const char *ip, uint16_t const port, uint16_t const backlog, int const options) noexcept{
    int fd = socket(AF_INET , SOCK_STREAM , 0);

    if(fd < 0)
        return -1;

    int const opt = 1;
    if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, & opt, sizeof opt) < 0){
        ::close(fd);
        return -2;
    }

    if (options & SOCKET_NONBLOCK)
    if (! socket_makeNonBlocking(fd) ){
        ::close(fd);
        return -3;
    }

    if (options & SOCKET_TCPNODELAY)
    if (! socket_makeTCPNoDelay(fd) ){
        ::close(fd);
        return -4;
    }

    struct sockaddr_in address;

    address.sin_family = AF_INET;
    address.sin_addr.s_addr = INADDR_ANY;
    address.sin_port = htons(port);

    if (bind(fd, (struct sockaddr *) & address, sizeof address) < 0){
        ::close(fd);
        return -5;
    }

    if (listen(fd, backlog ? backlog : SOMAXCONN) < 0){
        ::close(fd);
        return -6;
    }

    return fd;
}

} // namespace

NO. NO. NO. NO.

This does not do you any good none at all. In fact I think this is worse than doing nothing. All you have done is created a new layer that provides no advantages over the old and exposed a new set of error codes (that are not standard).

This is definitely NOT C++ code. You have just written another C layer that has all the same disadvantages of C and absolutely none of the benefits provided by C++.

The main thing to learn about C++ (in fact the overriding absolutely essential thing) is RAII. Resource acquisition is initialization. When you create an object it gets the resource, if it fails to get the resource during initialization you throw and the object does not exist and thus can not be used. Because the object only exists if it was correctly constructed most of the runtime checking to make sure the object is in the correct state disappears (because we know it is in the correct state because it exists).

I would expect the interface to a socket class to look like this:

Socket   webserver(8080);

while(webserver.accept(connection))
{
    InBoundConnection connection(webserver.buildConnection());

    connection.send(HttpObject(404, "Miss404 not found"));
}


Java MVC Implementation I am trying to write an MVC based application from scratch using Java 8 and Swing... I am having some difficulties with this.

I want to know if I am going the correct way about this...

My controller code is:

public class Controllerv2 {
private final Map<String, View> views = new HashMap<>();
private final Map<String, Command> commands = new HashMap<>();

private final ModelContainer model = new ModelContainer(this);

public void addView(String key, View view) {
    views.put(key, view);
}

public View getView(String key) {
    return views.get(key);
}

public void showView(String key) {
    getView(key).showForm();
}

public void addCommand(String commandString, Command cmd) {
    commands.put(commandString, cmd);
}

public void executeControllerCommand(String commandString) {
    commands.get(commandString).execute();
}
}

public abstract class View {
protected final JDialog dialog = new JDialog();
protected final GUIAction guiAction = new GUIAction();
protected final Controllerv2 controls;

Views are subclassed from this class:

public View(Controllerv2 suppliedController) {
    this.controller = suppliedController;
}
// Fires up the form
protected abstract void initComponents();

// Package up the fields into an object of type T and return
public abstract <T> T readFields();

// Self explanatory functions
public void showForm() {dialog.setVisible(true);}
public void hideForm() {dialog.setVisible(false);}

// For fields that need updating
public abstract void updateFields(String[] msg);
}

This is an example of a concrete View:

public final class VBasexLoginR extends View {
private JButton connectButton;
private JLabel connectionLabel;
private JTextField hostField;
private JLabel hostLabel;
private JLabel passwordLabel;
private JButton pingButton;
private JTextField portField;
private JLabel portLabel;
private JPasswordField pwField;
private JLabel titleLabel;
private JTextField uidField;
private JLabel userId;

public VBasexLoginR(Controllerv2 c) {
    super(c);
    initComponents();
}

@Override
protected void initComponents() {// Boilerplate to setup the form}

@Override
public BaseXCredentials readFields() {
    return new      BaseXCredentials(hostField.getText(),portField.getText(),uidField.getText(),pwField.getPassword());
}

@Override
public void updateFields(String[] msg) {
    connectionLabel.setText(msg[0]);
}
}

Am I going in the right direction with this?

Also, I think I may need different models, one for running queries on BaseX and then ones for working with the results. Thus far I have...

public class ModelContainer {
private final Map<String, Model> models = new HashMap<>();
private final Map<String, ModelCommand> commands = new HashMap<>();
private final Controllerv2 controller;

ModelContainer(Controllerv2 suppliedController) {
    this.controller = suppliedController;
}

public void addModel(String key, Model model) {
    models.put(key, model);
}

public Model getModel(String key) {
    return models.get(key);
}

public void addCommand(String key, ModelCommand cmd) {
    commands.put(key, cmd);
}

public void executeModelCommand(String key) {
    commands.get(key).execute();
}
}

Views can be got at through the getView method in controls

Is this correct or is this overkill?

EDIT -- Taking into account Mat's Mug response, changed the code to reflect the naming conventions suggested.

I'm only going to scratch the surface here, and leave more seasoned java reviewers take a stab at this one, for I've never written a line of java-8 - but some concepts transcend language barriers.

Naming in particular.

Signatures can, and should talk.

public void addView(String viewString, View view) {
    views.put(viewString, view);
}

public View getView(String viewString) {
    return views.get(viewString);
}

Take this line:

public void addView(String viewString, View view) {

The name viewString is pretty confusing. One has to look at how it's implemented to go "oh, that's a key for a hashmap!". How about this instead?

public void addView(String key, View view) {

Now there's no ambiguity whatsoever, and getView(String key) becomes crystal-clear.

Consistency is King

This is just a minor thing, but I'd prefer consistency in the choice of verbs used to describe an action. For example, if a form is a view and a form gets shown, then a view should be shown too, not opened:

public void openView(String viewString) {
    getView(viewString).showForm();
}

Would be

public void showView(String viewString) {
    getView(viewString).showForm();
}

Same with commands - if a command executes, I wouldn't want the verb run to refer to the same thing, I'd prefer it be consistent:

public void runControllerCommand(String commandString) {
    commands.get(commandString).execute();
}

Would be:

public void executeControllerCommand(String commandString) {
    commands.get(commandString).execute();
}

Single-letter identifiers

Don't. This:

ModelContainer(Controllerv2 c) {
    this.controls = c;
}

Is pretty darn confusing. Does c stand for controller, or for controls? Actually there seems to be something fishy going on here, "controls" is a nice name for a collection of controls, but you have it as ..a Controllerv2 (is there a Controllerv3? Avoid "versioning" classes like this, especially in the early development stages). Notice the puzzled expression in my face? ;)

Anyway, that c would probably be better of as controller.

Constructor arguments

I like this part - final makes your intent very clear:

private final Map<String, Model> models = new HashMap<>();
private final Map<String, ModelCommand> commands = new HashMap<>();
private final Controllerv2 controls;

ModelContainer(Controllerv2 c) {
    this.controls = c;
}

I'd push it a step further though, and provide a way to initialize the class with models and commands, like this:

private final Map<String, Model> models = new HashMap<>();
private final Map<String, ModelCommand> commands = new HashMap<>();
private final Controllerv2 controller;

ModelContainer(Controllerv2 controller, Map<String, Model> models, Map<String, ModelCommand> commands) {
    this.controller = controller;
    this.commands = commands;
    this.models = models;
}

And now the client code doesn't need to call AddCommand 20 times when there's that many commands to add.

Indentation

Your indentation is a little off, too. Whenever a code file ends like this:

}
}

You know something's gone wrong. I'd blame it on the Java-style braces but that's just the c# mug talking, just select all but the signature and last closing brace, and give that Tab button some lovin' ;)


Filter out unwanted substrings while generating all possible relevant substrings Write a Program that determines where to add periods to a decimal string so that
  the resulting string is a valid IP address. There may be more than one valid IP address
  corresponding to a string, in which case you should print all possibilities. For example, if the mangled string is "19216811" then some of the corresponding IP addresses are 192.168.1.1 and 19.216.81.1

Elements of Programming Interviews Section 6.9 See Bottom of Page 82, asks the following as a variant to the above problem:

Now suppose we need to solve the analogous problem when the number of periods is a parameter k and the
string length s is unbounded.

See attempt below, I am generating all permutations, see permute(s), of the string and then filtering later hence I end up doing unnecessary work. Can I do better?

def period_partition(s, k):
    slen = len(s)

    def is_valid_octet(s):
        # section ranges 0 - 255 and `00` or `000`, `01` are not valid but 0 is
        return slen == 1 or (s[0] != "0" and 0 <= int(s) <= 255)

    def permute(s):
        if len(s) > 0:
            for i in range(1, slen + 1):
                first, rest = s[:i], s[i:]
                for p in permute(rest):
                    yield [first] + p
        else:
            yield []

    results = set()

    for s in filter(
        lambda x: len(x) == k + 1 and all(is_valid_octet(i) for i in x), permute(s),
    ):
        results.add(".".join(s))

    return list(results)

if __name__ == "__main__":
    for args in [
        ("", 1),
        ("1234", 2),
        ("19216811", 3),
        ("192168111234", 3),
        ("192168111234", 4),
    ]:
        print(period_partition(*args))
        print()

Output:

[]

['1.23.4', '12.3.4', '1.2.34']

['1.92.168.11', '192.16.81.1', '19.216.81.1', '192.1.68.11', '192.16.8.11', '19.216.8.11', '19.2.168.11', '19.21.68.11', '192.168.1.1']

['192.168.111.234']

['192.16.81.11.234', '19.21.68.111.234', '19.2.168.111.234', '192.168.1.112.34', '192.168.11.12.34', '192.168.11.1.234', '192.168.111.23.4', '192.168.1.11.234', '192.1.68.111.234', '192.168.111.2.34', '19.216.81.112.34', '192.16.81.112.34', '19.216.81.11.234', '192.168.11.123.4', '192.16.8.111.234', '19.216.8.111.234', '1.92.168.111.234']

NB:
This is just a toy problem and its connection to valid IP addresses, IPv4 and IPv6, I think, is just to provide constraints on valid octets as periods are assigned. So 1.23.4 is an unbounded string s, "1234", after being partitioned with periods k=2

Use a generative approach

Iterating over all possible permutations and filtering out the invalid ones results in lots of wasted work.  Move the filtering operation earlier so many invalid permutations are not generated in the first place.

For example, permute() contains for i in range(1, slen + 1), but we know that an octet can only be up to 3 digits long.  Change the loop to for i in range(1, min(4, len(s)+1)). Also, if first is not a valid octet skip the recursive call (none will result in a valid address).

Something like this:

def period_partition(s, k):

    def is_valid_octet(s):
        # section ranges 0 - 255 and `00` or `000`, `01` are not valid but 0 is
        return s == '0' or not s.startswith('0') and 0 < int(s) < 256

    if s and k:
        for i in range(1, min(4, len(s)+1)):
            first, rest = s[:i], s[i:]
            if is_valid_octet(first):
                yield from (f"{first}.{p}" for p in period_partition(rest, k-1))

    elif s and is_valid_octet(s):
        yield s

testcases = [
    ("", 1),
    ("1234", 2),
    ("19216811", 3),
    ("192168111234", 3),
    ("192168111234", 4),
    ("19216811123444", 4),
    ("192168111234444", 4),
]

for s, k in testcases:
    print(f"\nperiod_partition({s!r}, {k})")

    for partition in period_partition(s, k):
        print(f"  {partition}")

Note: I modified period_partition() to be a generator yields all valid partitions.  Use list() if you need an actual list.


Loosely coupled application wide events I have a much needed rewrite of an inherited application ahead of me and have started to sketch out some possible solutions / build some prototypes for different parts of the application.

I'd like to have some feedback if this is a good approach to provide application events.

Goals:

Loosely coupled
Strictly typed / refactor friendly
Declarative
Adhere to SRP (Single responsibility principle)
Lightweight

All events are classes derived from EventBase. An IoC Container will handle lifetimes of brokers and subscribers. Constructor injection is used. All types implementing IEventSubscriber<> (where T : EventBase) will be automatically registered in the container. My DefaultEventBroker implementation then has an optional IEnumerable<IEventSubscriber<>> dependency, which the container will resolve to all visible subscribers for a given event type. Under the hood a System.Reactive.Subjects.Subject<> (Microsoft Reactive Extensions) will do the heavy lifting.

Essentially I provide two ways to subscribe to events:

Subscribe by setting a dependency on (one or more) IEventBroker<>

public class MyEventListeningComponent
{
    public MyEventListeningComponent(IEventBroker<ProgramStartedEvent> eventBroker)
    {
        eventBroker.Subscribe(e =>
        {
            Console.WriteLine("[MyEventListeningComponent] Program Started: {0}", e.StartDate.ToString());
        });
    }
}

Subscribe by implementing (one or more) IEventSubscriber<>

public class MyEventSubscriber : IEventSubscriber<ProgramStartedEvent>, IEventSubscriber<ProgramEndedEvent>
{
    public void OnNext(ProgramStartedEvent value)
    {
        Console.WriteLine("[MyEventSubscriber] OnNext: Program Started: {0}", value.StartDate.ToString());
    }

    public void OnNext(ProgramEndedEvent value)
    {
        Console.WriteLine("[MyEventSubscriber] OnNext: Program Ended: {0}", value.EndDate.ToString());
    }
}

Any more or less obvious pitfalls? Suggestions?

Side note: IEventSubscriber<> is essentially an IObserver<> without the OnCompleted and OnError methods, because I can subscribe to multiple event types by implementing IEventSubscriber<> with different generic type arguments and I wouldn't be able to get an event type context in OnCompleted and OnError (because those methods do not have generic type arguments). I'm relatively sure I won't be needing those hooks in this scenario anyway.

Edit: Events are raised (or published) via IEventBroker<>

public class MyEventRaisingComponent
{
    public MyEventRaisingComponent(IEventBroker<ProgramStartedEvent> eventBroker)
    {
        eventBroker.Publish(new ProgramStartedEvent() { StartDate = DateTime.Now });
    }
}

The code itself looks good, but I would look into providing a single way of subscribing to events, it will make the job easier for other developers reading and changing the code.

The first method of subscribing (via IEventBroker<T> injection) requires subscriber object to be created first, so it may not receive all events that you may expect it to receive. Every time you create an instance of subscriber it adds a new event subscription thus causing potential memory leaks and side effects. You might need to add Unsubscribe method to IEventBroker<T> if you prefer to leave it.

Second method (the one I would prefer) is better suited for application-wide events since it moves the responsibility of subscription to the infrastructure thus avoiding multiple subscriptions and memory leaks (comparing to first option). It is also easier to unit-test since part of responsibilities are taken away from subscriber.

Update based on question update: Publishing messages also looks good. In this case (following my suggestion to leave only second method of event subscription) I would remove Subscribe method from IEventBroker<T> declaration, and refactor it to non-generic IEventPublisher interface with generic method void Publish<T>(T eventToPublish).


Make loading icon appear only after 2 seconds On any ajax calls made that take more than 2 seconds, I want a loading icon to appear. I have css that takes care of showing the loading icon when the 'loading' class is added to the body.

And here is the relevant code:

<script>
    var loadingIconOn = false;

    $(document).on({
        ajaxStart: function () {
            loadingIconOn = true;
            setTimeout(function () {
                if(loadingIconOn == true)
                    $('body').addClass("loading");
            }, 2000);

        },
        ajaxStop: function () {
            $('body').removeClass("loading");
            loadingIconOn = false;
        }
    });
</script>

This seems to be working fine, but I'm wondering if this might get me into trouble if multiple actions are made at the same time to do different ajax operations, and is there a better way of doing this?

EDIT (Here's why I want to wait 2 seconds):

There are actions that show immediate feedback, however the ajax call takes a split second to process. I don't want the user to see the loading symbol flicker on and off in cases like this. I only want to show it when something is actually taking a long time. No reason for them to feel like the site is slow when they wouldn't otherwise notice any slow down.

The ajaxStart() handler is only called if there are no other ajax requests currently in progress, so if there is already something in progress, the handler will never be fired.

Th ajaxStop() handler will only be called when a request completed and there are no other outstanding requests. So, theoretically, if you have ajax requests being fired every second, the loading icon could remain on your screen in perpetuity.  If you are looking at building a heavily ajax-driven UI, it might not be too wise to rely on the global ajax functionality for this reason, as you have to then concern yourself with interaction of different ajax requests and how they may impact the user experience.

From a UX standpoint, I would find it odd that you only want to display the loading icon until after 2 seconds have passed.  This seems like a really long time to wait to give the user some feedback that something is happening in response to some action they have taken.

There is no reason to have your global loadingIconOn variable at all. It would be preferable instead to work with jQuery queue functionality to give explicit control over how the animations are sequenced.

For example:

$(document).on({
    ajaxStart: function () {
        $('body').delay(2000, 'loadingIcon').queue('loadingIcon', function() {
            $(this).addClass('loading').dequeue();
        });
    },
    ajaxStop: function () {
        $('body').clearQueue('loadingIcon').removeClass('loading');
    }
});

This also gives the advantage of having the function to delay then add the class specifically "scoped" to only be relevant to the body element as well as the opportunity to set up a specific queue (loadingIcon in this example), which deals only with this specific functionality.

Note that you don't need to use a custom queue like loadingicon. You could simplify this down by using the default fx queue, which would look like:

$(document).on({
    ajaxStart: function () {
        $('body').delay(2000).queue(function() {
            $(this).addClass('loading').dequeue();
        });
    },
    ajaxStop: function () {
        $('body').clearQueue().removeClass('loading');
    }
});

But, I find it best practice to keep animations related to a certain set of functionality within their own queues so as to minimize potential conflicts with effects you might add in the future. Like say you added some functionality in the future to change some of other property/class on <body>, if the effects were in their own queue, they could be applied to the element simultaneously with the icon loading effect without causing undesirable interactions (obviously other than if they impact the same CSS properties in the loading class).


JavaScript number obfuscater I got inspired to do this after seeing this repo containing a bunch of weird syntactical quirks in JS, namely this example called "It's a fail!". The example was essentially the following:

  You would not believe, but …

(![] + [])[+[]] +
 (![] + [])[+!+[]] +
 ([![]] + [][[]])[+!+[] + [+[]]] +
 (![] + [])[!+[] + !+[]];
// -> 'fail'

I found this quite interesting; however, as revealed by the explanation later on, the actual characters were merely coincidentally obtained using the fact that certain array operations return true, false and undefined, from which we can grab certain indices. The issue with using this for text obfuscation, of course, it that it's not applicable to most characters. However, the way we grab those indices is also using some array syntax, following just three rules:

// Number definitions
+[] // -> 0
+!+[] // -> 1

// Addition
+!+[] + +!+[] // -> 2
+!+[] + !+[] // -> also 2, but with one less character

// Concatenation by arrayifying
+!+[] + [+[]] // -> 10
+!+[] + [+!+[]] // -> 11

Using this, we could implement an algorithm to convert any number into such a format. I've also made some optimisations, so for example, instead of doing +!+[] + +!+[] + +!+[] + +!+[] + +!+[] + +!+[] + +!+[] + +!+[] just to do 9, we could simply do [+!+[] + [+[]] - !+[]] - making 10 by concatenating 1 and 0, then subtracting 1. The same process is done for any number > 5.

Below is my code. Any comments on readability, naming, etc. is obviously welcome, but I'm also open to any suggestions on further ways to make the output number more compact.

const obfuscateNumber = num => {
  let chars = [...num.toString()];

  let outputStr = "";
  let tokens = 0;
  chars.forEach(char => {
    let digit = parseInt(char);

    outputStr += (tokens > 0? " + " : "")
    if (digit === 0){
      outputStr += (tokens > 0? "[" : "")
                      + "+[]"
                      + (tokens > 0? "]" : "");
    } else if (digit === 1) {
      outputStr += (tokens > 0? "[" : "")
                      + "+!+[]"
                      + (tokens > 0? "]" : "");
    } else if (digit <= 5) {
      outputStr += "[";
      for (i = 0; i < digit; i++){
        outputStr += (i == 0? "" : " + ") + "!+[]";
      }
      outputStr += "]";
    } else {
      outputStr += "[+!+[] + [+[]]";
      for (i = 0; i < 10 - digit; i++){
        outputStr += " - !+[]";
      }
      outputStr += "]";
    }

    tokens++;
  });

  return outputStr;
}

Note: when I use the word 'obfuscate', I mean it in the sense that the number is unreadable to average humans. Obviously, it easily decodable, and I'm not planning to implement it into some production-grade obfuscation software therefore.

Your custom function is 36 lines of very hard to digest code.  You are performing a battery of if-elseif-else conditionals on the same variable -- for this reason, it is most appropriate to employ a switch case (even though I have a strong bias against them) as a matter of best practice.  If the goal is to obfuscate the output AND the code, I reckon you've found a winner.

I'll offer a comparison using tests 5, 11, and 987654321.

Test Results:

5 -> [!+[] + !+[] + !+[] + !+[] + !+[]]
11 -> +!+[] + [+!+[]]
987654321 -> [+!+[] + [+[]] - !+[]] + [+!+[] + [+[]] - !+[] - !+[]] + [+!+[] + [+[]] - !+[] - !+[] - !+[]] + [+!+[] + [+[]] - !+[] - !+[] - !+[] - !+[]] + [!+[] + !+[] + !+[] + !+[] + !+[]] + [!+[] + !+[] + !+[] + !+[]] + [!+[] + !+[] + !+[]] + [!+[] + !+[]] + [+!+[]]

Alternatively, you might leverage Javascript-native calls to base64 encode: btoa() and atob().  Because you are dealing purely with integers, you don't need to concern yourself with "The Unicode Problem".
To encode a number use btoa(num) and to decode it back use atob(num).

Test Results:

5 -> NQ==
11 -> MTE=
987654321 -> OTg3NjU0MzIx

Benefits include:

Future developers of your code will be able to instantly research what your process is doing.
There is no need to write a custom function
The output is far, far better compressed
The process of decrypting the generated string is just as simple as encrypting it
If you want to further obfuscate the generated strings, you can cleanly add your own "special sauce" that will offer more "entertainment" for crackers.


Implementation of Generic SQL Data Reader I am using below virtual method to read the data from SQL Data Reader like:

public IList<District> GetList()
{
    IList<District> _list = new List<District>();

    SqlConnection con = new SqlConnection(ConStr);
    try
    {
        string StoreProcedure = ConfigurationManager.AppSettings["SP"].ToString();

        SqlCommand cmd = new SqlCommand(StoreProcedure, con);
        cmd.CommandType = CommandType.StoredProcedure;
        con.Open();
        SqlDataReader rdr = cmd.ExecuteReader();
        _list = new GenericReader<District>().CreateList(rdr);
        rdr.Close();
        con.Close();
    }
    finally
    {
        IsConnectionOpenThenClose(con);
    }
    return _list;
}

District Class:

public class District
{
    public int id { get; set; }
    public string name { get; set; }
}

And GenericReader Class as:

public class GenericReader<T>
{
    public virtual List<T> CreateList(SqlDataReader reader)
    {
        var results = new List<T>();

        while (reader.Read())
        {
            var item = Activator.CreateInstance<T>();
            foreach (var property in typeof(T).GetProperties())
            {
                if (!reader.IsDBNull(reader.GetOrdinal(property.Name)))
                {
                    Type convertTo = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType;
                    property.SetValue(item, Convert.ChangeType(reader[property.Name], convertTo), null);
                }
            }
            results.Add(item);
        }
        return results;
    }
}

Is this approach is better or still, we can refactor?

GetList()

SqlConnection, SqlCommand and SqlDataReader are all implementing the IDisposable interface hence you should either call Dispose()  on that objects or enclosing them in a using block.
You should use var instead of the concrete type if the right-hand-side of an assignment makes the concrete type obvious.
E.g the line SqlConnection con = new SqlConnection(ConStr); we can see at first glance that the concrete type   is SqlConnection and therfor we should use var con = new SqlConnection(ConStr); instead.
Using abbreviations for naming things shouldn't be done because it makes reading and maintaining the code so much harder.
Underscore-prefixed variablenames are usually used for class-level variables. Method-scoped variables should be named using camelCase casing hence list would be better than _list because Sam the maintainer wouldn't wonder about it.
You return an IList<> which is good because coding against interfaces is the way to go.


Fibonacci Sequence - Faster method? Just decided to make a quickie Fibonacci Sequence via jQuery and I was wondering if anybody can think of a faster way to interpret it? I can type pretty much any number in and it returns in a fraction of a second.

Can anybody think of a quicker method to calculate a specific Fibonacci number?

http://jsfiddle.net/st9sgrcx/1/

$(function(){
 var counter = 1,
        fibo = [0,1],
        temp = 0,
        whichOne = prompt("Which fibonnaci number do you want?");
    function fib(prev, cur){
        temp = prev + cur;
        fibo.push(temp);
        counter = temp;
        temp = cur;
    }
    for(var i=0; i<whichOne; i++){
        fib(temp, counter);
    }
    alert(temp);
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>

"fibonnaci" is a misspelling.

There really isn't any need for jQuery here.

As you have already observed, there is also no need to keep the entire array, unless you wanted to modify the code to reuse the memoized results for multiple calculations.

If you want the fastest way to calculate a specific Fibonacci number, even when the number is large, then go for the closed-form formula instead:

$$
F_n = \frac{\phi^n - \psi^n}{\sqrt{5}}
$$
… where
\$\phi = \dfrac{1 + \sqrt{5}}{2}\$ and \$\psi = \dfrac{1 - \sqrt{5}}{2}\$.