Implementation of perceptron, inspired by Rashka I am trying to learn the basics about neural networks by coding from scratch the perceptron model.

Since I am not a programmer and would like to improve my coding skills I would like to get your comments about this code. What do you think about it (I tried to respect the different steps: forward and backward for backpropagation)? I suppose it is quite horrific for a Python programmer and would be pleased to gain some insight about some of my evidently bad practices ;)

Here is my work (inspired by Rashka book):

class Perceptron(object):
    def __init__(self, eta=0.01, epochs=50):
        self.eta = eta
        self.epochs = epochs

    def fit(self, X, Y):
        self.w_ = np.zeros(X.shape[1])
        self.b_=0
        self.cost_ = []
        for _ in range(self.epochs):
            errors=0
            for x_i,y_i in zip(X,Y):
                y_hat=self.forward(x_i)
                gradL_W,gradL_b=self.backward(y_i,y_hat,x_i)
                self.w_,self.b_=self.update(gradL_W,gradL_b)
                errors+=(y_hat!=y_i)*1
            self.cost_.append(errors)
        return self

    def net_input(self, x):
        """Calculate net input"""
        return x.dot(self.w_) + self.b_

    def activation(self,z):
        heaviside= lambda x: (1, -1)[x<0]
        return heaviside(z)

    def forward(self,x):
        s= self.net_input(x)
        y_hat=self.activation(s)
        return y_hat

    def backward(self,y,y_hat,x):
        error=-(y-y_hat)
        gradL_b=error*1
        gradL_W=error*x
        return gradL_W,gradL_b

    def update(self,gradL_W,gradL_b):
        self.w_=self.w_-self.eta*gradL_W
        self.b_=self.b_-self.eta*gradL_b
        return self.w_,self.b_

    def predict(self,X):
        s=X.dot(self.w_)+self.b_
        y_hat=list(map(lambda x: (1, -1)[x<0],s))
        return y_hat

Style

Python comes with an official Style Guide often just called PEP8, and especially as a beginnger it's a good starting point to get going. Of course coding style is often a matter of choice, however, there are aspects you should definitely follow.

Whitespace
As you know, Python's code structure is build upon indentation, so some parts are already language-defined. The style guide also has recommendations on how to use whitespace within statements and expressions. For example, , should always be followed be a single space character, while = is preceded and followed by a single space when used in assignments (there should be no whitespace around = if used as keyword arguments to functions such as foo(batz='bar')). So you would go from

gradL_W,gradL_b=self.backward(y_i,y_hat,x_i)
self.w_,self.b_=self.update(gradL_W,gradL_b)

to

gradL_W, gradL_b = self.backward(y_i, y_hat, x_i)
self.w_, self.b_ = self.update(gradL_W, gradL_b)

which does look way cleaner IMHO.

Variable names
It's always good practice to use descriptive variable names. Simply writing self.weights instead of self.w_ wont hurt that much. Apart from that, the Style Guide also recommends to use a single leading underscore for non-public variable names. Note, this is only a convention, since Python has no "real" private member variables as you may know from other languages.

Documentation
There is a single function in your perceptron class that has a) documentation and b) also follows the docstring-style from the Style Guide. You should apply that to the other functions and maybe even to the class as well. Your current project might be simple, but if projects become more complex, good documentation will save you a lot of headache. Following the official docstring style also has the nice benefit that Python's help(...) function as well as most IDEs will easily find it.

The code

Apart from the stylistic issues mentioned above, there are some more direct code-focused aspects you should work on.

Code duplication
There is some duplicated code in your implementation of the perceptron model. E.g., you implement the scalar product at the input of the perceptron twice! The first time at net_input, which is likely where it's supposed to go, and the second time in predict. predict also reimplements pretty much the whole point of activation. It's pretty straightforward to implement predict using net_input and activation:

def predict(self, X):
    s = self.net_input(X)
    y_hat = list(map(self.activation, s))
    return y_hat

If you use Python for a while you will also learn to use list comprehensions more often than the list(map(...)) construct, simply because it's more flexible and readable. Rewriting the calculation of y_hat as list comprehension would look like y_hat = [self.activation(i) for i in s]. After you have arrived at

def predict(self, X):
    s = self.net_input(X)
    y_hat = [self.activation(i) for i in s]
    return y_hat

you may also find the similarity between predict and forward quite striking. forward can now be boiled down to simply

def forward(self, x):
    return self.predict(x)[0]

Getting the "first" result of the return value using [0] is mainly to ensure that the new version just returns a single number as it was before. Otherwise it would be a list with a single element.

Explicit is better than implicit

(Taken from the Zen of Python)

Using code like

heaviside = lambda x: (1, -1)[x < 0]
return heaviside(z)

might be clever, but clever does not equal good or readable most of the time. The same thing can also be expressed as return -1 if z < 0 else 1, which is almost a verbatim translation of the mathematical definition of the step function as presented by Rashka. If you don't like the inline-if, you may also write it as "normal" if condition:

if z < 0:
    return -1
else:
    return 1

gradL_b = error * 1 is another instance of cleverness. This time, I am not even sure what you are actually trying to accomplish here. Depending on the type of your labels y, error will either be of type int or maybe also float, and multiplying it with 1 will not change that.

errors += (y_hat != y_i) * 1 is more obvious, but could also be expressed explicitly as errors += int(y_hat != y_i). Python should even be able to do errors += y_hat != y_i without changing the end result.

Return values
I'm not 100% sure what you are trying to accomplish with some of your function return values. For example, I cannot see the point in fit returning self. updateis another instance of that topic. I can see you're using it as self.w_, self.b_ = self.update(gradL_W, gradL_b), but there is not a real reason to do this, since update already modifies the weights internally.

You will likely have to think about that topic as you further progress in your journey into the depths of neural networks (pun intended). Often, the multilayer perceptron is  one of the next logical steps in this process. At this stage, you will need the gradients of previous layers to be able to propagate errors through the network. But time will tell1.

Until then: Happy coding!

1You will also probably soon find out that modelling single neurons is not sufficient (and computationally efficient) for what there is to come. Depending on your learning materials, matrix operations will pop up more and more often. This is the time where the NumPy library will help you shine.


Functions to count vowels and consonants in strings I am coming to a problem where I am  writing a function that accepts a pointer to a C-string as its argument. The function should count the number of vowels appearing in the string and return that number. Then, I am writing another function that accepts a pointer to a C-string as its argument. This function should count the number of consonants appearing in the string and return that number.

Am I violating any of the following restrictions?

No global variables
No labels or go-to statements
No infinite loops, examples include: for(;;), while(1), while(true), do{//code}while(1);
No break statements to exit loops

Here is my code:

#include<iostream>

using namespace std;

//Function prototypes
int Vowel_count( char *str);

int Consonant_count(char *str);

 int main()

    {

     char String[30];

     char choice;

     int Nvowels,Nconsonant;

     //Input a string

     cout<<"Enter a string:";

     cin.getline(String ,30);

     do

     {

     //Displaying Menu

     cout<<"\nMENU"<<endl;

     cout<<" (A) Count the number of vowels in the string"<<endl;

     cout<<" (B) Count the number of Consonants in the string"<<endl;

     cout<<" (C) Count both the vowels and consonants in the string"<<endl;

     cout<<" (D) Enter another string"<<endl;

     cout<<" (E) Exit program"<<endl;

     //Inputting choice

      cout<<"Enter choice";

     cin>>choice;

     switch(choice)

     {

     case 'A':
     //Function call to get number of Vowels

               Nvowels=Vowel_count(String);
                cout<<"Number of vowels is:"<<Nvowels<<endl;

              break;

     case 'B':
     //Function call to get number of consonants

                Nconsonant=Consonant_count(String);

                //Outputting number of Consonants

               cout<<"Number of Consonants is:"<<Nconsonant<<endl;

                 break;

     case 'C':
     //Function call to get number of Vowels

                Nvowels=Vowel_count(String);

                //Function call to get number of consonants

                Nconsonant=Consonant_count(String);

              //Outputting Both Vowels and Consonants

              cout<<"Number of vowels is:"<<Nvowels<<endl;

                   cout<<"Number of Consonants is:"<<Nconsonant<<endl;

                 break;

     case 'D': //Inputting another string

                 cout<<"Enter another String:";

                 fflush(stdin);

                 cin.getline(String ,30);

                 break;

     case 'E': exit(0);

                  break;

     }//End of Switch

     }while(choice !='E');//End Do while

    //Pause system for a while

     system("pause");

}//End main

//Function Definitions

int Vowel_count( char *str)

{

   int count=0;//Local variable

   //Checks for all characters in C-string

   while(*str!='\0')

   {

    if(*str=='a'||*str=='e'||*str=='i'||*str=='o'||*str=='u')

         count++;

         str++;

   }//End While

//Returns count value to function call

   return count;

}//End of Vowel_count

int Consonant_count(char *str)

{

int count=0;

    //Checks for all characters in C-string

     while(*str!='\0')

     { //Checks for consonants

     if(*str!='a'&&*str!='e'&&*str!='i'&&*str!='o'&&*str!='u')

          count++;

        str++;

     }//End While

     //Returns count value to function call

     return count;

}

char String[30];
    char choice;
    int Nvowels,Nconsonant;

Don't define a variable before it is needed. Doing so increases the cognitive load on readers that trace through your code and keep mental records of every variable, their possible values, and whether they've even been set. This artifact from older languages also encourages lazy reuse of variables beyond their original intent. Define variables as you need them and keep the scope of each variable as narrow as required.

Use descriptive names. The larger the scope in which a variable exists, the more important it is to give that variable a name that indicates to the reader what it represents. String \$\rightarrow\$ input_sentence. Nvowels \$\rightarrow\$ vowel_count.

    cin.getline(String ,30);

If a user inputs 30 or more characters, std::istream::getline will set the failbit. If end-of-file is reached before the delimiter is encountered, std::istream::getline will set the eofbit. You should check if std::cin is in a readable state and reset the state if it's not, otherwise you'll have an infinite loop when an error is encountered.

    cout<<"\nMENU"<<endl;
    cout<<" (A) Count the number of vowels in the string"<<endl;
    cout<<" (B) Count the number of Consonants in the string"<<endl;
    cout<<" (C) Count both the vowels and consonants in the string"<<endl;
    cout<<" (D) Enter another string"<<endl;
    cout<<" (E) Exit program"<<endl;

Whitespace is great for readability as they help separate operators and language constructs.

Avoid std::endl. If you need to explicitly flush the cache, then explicitly stream the manipulator std::flush and comment why a flush is necessary. If you simply need a end-of-line character, prefer '\n' or "\n".

Edward covered string concatenation via the compiler, but I'd like to also point out that C++11 introduced raw string literals.

    std::cout << R"(
MENU
 (A) Count the number of vowels in the string
 (B) Count the number of Consonants in the string
 (C) Count both the vowels and consonants in the string
 (D) Enter another string
 (E) Exit program
Enter Choice
");

    cin>>choice;

As with std::istream::getline, std::istream::operator>> needs to be checked to ensure a read was successful. If extraction failed, you should probably notify the user and attempt to recover.

    case 'A':
    //Function call to get number of Vowels
    case 'B':
    //Function call to get number of consonants
    //Outputting number of Consonants
    case 'C':
    //Function call to get number of Vowels

Keep comments crisp. Everything I need to know as far as what is being done is already there in the form of code to be compiled. Comments should be reserved for explaining why the code was written the way it was (the intent). Adding verbosity slows down understanding.

To assist with the readability, consider mapping the character inputs to enumerations. You can read up on enum serialization, but just to give you an idea of how it can help readability:

enum class Selected {
    count_vowels = 'A',
    count_consonants,
    count_both,
    reset_sentence,
    exit
};

std::istream& operator>>(std::istream& in, Selected op) {
    // exercise for the reader.
}

int main() {
    // ...
        Selected user_choice;
        std::cout << "Enter choice: ";
        std::cin >> user_choice;

        switch (user_choice) {
        case Selected::count_vowels:
            // ...
        case Selected::count_consonants:
            // ...
        case Selected::count_both:
            // ...
        case Selected::reset_sentence:
            // ...
        case Selected::exit:
            // ...
        case default:
            // Handle user-input error (out of range, end of file)

            fflush(stdin);

Using std::fflush on an input stream is undefined behavior. If you were trying to reset the buffer of std::cin, see std::cin::ignore.

        case 'E':
            exit(0);
            break;
        }
    } while (choice != 'E');

    system("pause");

Once the the program encounters exit(0), all of the remaining code (break; \$\rightarrow\$ while (choice != 'E') \$\rightarrow\$ system("pause");) is unreachable. If you intend to immediately return from a function (or in this case exit the program), you don't need to loop until choice is \$E\$. This is one of those times where while (true) is actually appropriate if you didn't have the restriction.

system("pause") is slow, platform-dependent, and insecure.  Consider re-evaluating your development environment, from having permanent access to a console or utilizing the breakpoint of a debugger.

int Consonant_count(char *str) {
    // ...
        if (*str != 'a' && *str != 'e' && *str != 'i' && *str != 'o' && *str != 'u')
              count++;
            str++;

Prefer scoping your single-line statement blocks with braces. Adding code in a hurry

if (condition)
    statement;
  statementIntendedToBeScoped???

or commenting

if (condition)
    // statement
statementThatIsNowScopedToCondition

can lead to errors. Scoping with braces addresses this issue.

if (condition) {
    statement
}
statementDefinitelyNotScoped

if (condition) {
    // statement
}
statementDefinitelyNotScoped

if (*str != 'a' && *str != 'e' && *str != 'i' && *str != 'o' && *str != 'u')

Keep in mind the various possible values a type can represent. A character (char) can represent the set of consonants and vowels (A-Z, a-z). It can also represent digits, punctuation, control characters, whitespace, etc. The conditional in Consonant_count is returning true or false if the current character is any ASCII character (not any alphabetic character) that is not any of aeiou. Clearly control structures, punctuation, and whitespaces are not consonants.

In both of your functions, how do you handle uppercase vowel inputs?

Rather than check each individual vowel, consider a lookup technique.  You could create a table (one for vowel, one for consonant) that maps each character to true or false.  There is also a bit mapping alternative that builds on the lookup table approach, where you exploit the layout of the ascii table to do very fast lookups via bit shifts and masks.

bool is_vowel(char ch) {
    auto lowercase_pos = (ch | 0x20) - 96;
    if (static_cast<unsigned char>(lowercase_pos) > 31) {
       return false;
    }
    constexpr std::uint32_t vowel_mask = (1 << 1) | (1 << 5) | (1 << 9) | (1 << 15) | (1 << 21);
    return (vowel_mask & (std::uint32_t{1} << lowercase_pos)) != 0;
}

specific_count(range)
    count = 0
    for each element in the range
        if element is what we're looking for
            count = count + 1
    return count

Learn the various <algorithm>s.  You'll notice that both your functions have the same structure as the pseudocode above.  It turns out, this is a common algorithm in C++ (std::count_if).  You should be aware of this library as you learn more about functions and specifically lambdas (objects that behave like functions and can be passed around into functions as arguments).


Finding similar items in two arrays with multithreading in C++ I'm exploring multithreading for some code that I want to speed up in C++. I wrote a simple program to facilitate my understanding. The program takes in two arrays of equal length, the value of the length itself, and a threshold. It outputs all indices i (in any order) where abs(array_1[i]-array_2[i]) < threshold. Here is the code:

#include <mutex>
#include <vector>
#include <thread>
#include <iostream>

std::mutex output_mutex;

void process_element(const uint8_t* input_1, const uint8_t* input_2, const size_t i, const uint8_t threshold, std::vector<size_t>& output){
    uint8_t element_1 = input_1[i];
    uint8_t element_2 = input_2[i];
    uint8_t distance = (element_1 > element_2) ?
            element_1 - element_2 :
            element_2 - element_1;
    if(distance < threshold){
        output_mutex.lock();
        output.push_back(i);
        output_mutex.unlock();
    }
}

std::vector<size_t> process(const uint8_t* input_1, const uint8_t* input_2, const size_t length, const uint8_t threshold){
    std::vector<size_t> output;
    std::vector<std::thread> threads;
    for (size_t i = 0; i < length; ++i) {
        std::thread t([&](){
            process_element(input_1,input_2,i,threshold,output);
        });
        threads.push_back(std::move(t));
    }
    for(auto&& thread : threads){
        thread.join();
    }
    return output;
}

int main(){
    uint8_t input_1[10] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    uint8_t input_2[10] = {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
    std::vector<size_t> output = process(input_1,input_2,10, 7);
    for(auto&& element : output){
        std::cout << int(element) << ' ';
    }
}

It seems to do fine when I run it.

Am I applying multithreading correctly?

Are there any mistakes or things I can improve in my code?

Is there a better/more efficient way to compute the distance in process_element than the way I coded it?

Lastly, I didn't care about the order of the output indices when writing the code, but I couldn't help but notice that the reported values always seem to be in order. Is this just an artifact of the fact that process_element is very simple and takes almost no time at all, or will the output in general be ordered according to the order in which I run the threads?

The cost of starting up a thread is generally considered expensive. For your example of only 10 items, you won't ever see it because the application quits before you notice it started. Generally for something like this, you'd want to start up a thread to process some large portion of the array rather than one thread per element of the array. At 10 elements, it's no big deal. But once you have thousands of elements, you don't want thousands of threads running. In fact, many operating systems will limit the number of threads you can start. According to @user673679, you can get the number of concurrent threads by calling std::thread::hardware_concurrency.

I recommend limiting the number of threads to something like the number of available CPU cores on the machine. Then split the input arrays into that many parts and have each thread work on one part of the array. So if you have 8 cores, split the work into 8 parts and have each thread work on 1/8th of the array.

I ran your code with Thread Sanitizer and it's telling me you have a data race. Here's what it says:

WARNING: ThreadSanitizer: data race (pid=7975)
  Read of size 8 at 0x7ffeefbff2f0 by thread T4:
    #0 process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0::operator()() const main.cpp:34 (CPlusPlusTester:x86_64+0x100006b4a)
    #1 decltype(std::__1::forward<process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0>(fp)()) std::__1::__invoke<process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0>(process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0&&) type_traits:4339 (CPlusPlusTester:x86_64+0x100006a00)
    #2 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0>&, std::__1::__tuple_indices<>) thread:342 (CPlusPlusTester:x86_64+0x100006868)
    #3 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0> >(void*) thread:352 (CPlusPlusTester:x86_64+0x1000059f9)

  Previous write of size 8 at 0x7ffeefbff2f0 by main thread:
    #0 process(unsigned char const*, unsigned char const*, unsigned long, unsigned char) main.cpp:32 (CPlusPlusTester:x86_64+0x1000018e2)
    #1 main main.cpp:47 (CPlusPlusTester:x86_64+0x100001fa1)

  Location is stack of main thread.

  Thread T4 (tid=669186, running) created by main thread at:
    #0 pthread_create <null>:2673600 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a17d)
    #1 std::__1::__libcpp_thread_create(_opaque_pthread_t**, void* (*)(void*), void*) __threading_support:328 (CPlusPlusTester:x86_64+0x10000593e)
    #2 std::__1::thread::thread<process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0, void>(process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0&&) thread:368 (CPlusPlusTester:x86_64+0x1000055e7)
    #3 std::__1::thread::thread<process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0, void>(process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0&&) thread:360 (CPlusPlusTester:x86_64+0x100001ad8)
    #4 process(unsigned char const*, unsigned char const*, unsigned long, unsigned char) main.cpp:33 (CPlusPlusTester:x86_64+0x10000189f)
    #5 main main.cpp:47 (CPlusPlusTester:x86_64+0x100001fa1)

SUMMARY: ThreadSanitizer: data race main.cpp:34 in process(unsigned char const*, unsigned char const*, unsigned long, unsigned char)::$_0::operator()() const

Line 34 is the call to process_element(). @pschill pointed out:

  The lambda captures all variables by reference, including i. So if the thread starts a little bit too late, the for loop already incremented i and the call to process_element recieves the wrong value. A solution would be changing the lambda capture from [&] to [&, i], so that i is captured by value.

You ask:

  Is this just an artifact of the fact that process_element is very simple and takes almost no time at all, or will the output in general be ordered according to the order in which I run the threads?

I think it's just because there's so little work being done that the threads end up executing in order. One is probably done before the next is even started.


Ensuring if an object is Assignable from a class type I am trying to ensure type in the below method. How to reduce its cognitive complexity to below 15 from the current 27? It looks more readable as it is? Is there a way to break this down to make it more readable?
Shifting some of the checks to separate method will be one way but is there a better way?
private Object ensureType(Class<?> expectedClass, Object value) {
    if (value == null)
        return null;
    Class<?> valueClass = value.getClass();
    if (expectedClass.isAssignableFrom(valueClass))
        return value;

    if (expectedClass.isAssignableFrom(String.class)) {
        return value.toString();
    }
    if (expectedClass.isAssignableFrom(Date.class)) {
        if (Number.class.isAssignableFrom(valueClass)) {
            return new Date(((Number) value).intValue());
        }
        try {
            return formatter.parse(value.toString());
        } catch (ParseException e) {
            throw new ClassCastException("argument " + value + " of type " + valueClass + " cannot be converted to "
                    + expectedClass + ":" + e);
        }
    }
    if (expectedClass.isAssignableFrom(Integer.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).intValue();
        return Integer.parseInt(value.toString());
    }
    if (expectedClass.isAssignableFrom(Double.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).doubleValue();
        return Double.parseDouble(value.toString());
    }
    if (expectedClass.isAssignableFrom(Float.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).floatValue();
        return Float.parseFloat(value.toString());
    }
    if (expectedClass.isAssignableFrom(Short.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).shortValue();
        return Short.parseShort(value.toString());
    }
    if (expectedClass.isAssignableFrom(Byte.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).byteValue();
        return Byte.parseByte(value.toString());
    }
    if (expectedClass.isAssignableFrom(Long.class)) {
        if (Number.class.isAssignableFrom(valueClass))
            return ((Number) value).longValue();
        return Long.parseLong(value.toString());
    }
    if (expectedClass.isAssignableFrom(Boolean.class)) {
        return Boolean.parseBoolean(value.toString());
    }

    throw new ClassCastException(
            "argument " + value + " of type " + valueClass + " cannot be converted to " + expectedClass);
}

PS: I am moving this question from Stackoverflow to CR as it was suggested in the comments.

private Object ensureType(Class<?> expectedClass, Object value) {

Your names could be slightly better, given that this function does not only ensure something, but also converts it.
private Object convert(Class<?> targetClass, Object value) {

    throw new ClassCastException(
            "argument " + value + " of type " + valueClass + " cannot be converted to " + expectedClass);

Given that the function converts, I find the ClassCastException inappropriate. An IllegalStateException or IllegalArgumentException might be better fitting.

What you're doing here is complex, and there is not really an easier way to do it either. You have n classes that you want to map to m classes, so the complexity will always be there in one way or another. The best thing you can do is make it more readable. For example with extracting the converting parts into functions:
    // ...

    if (targetClass.isAssignableFrom(String.class)) {
        return convertToString(value);
    }

    if (targetClass.isAssignableFrom(Date.class)) {
        return convertToDate(value);
    }

    if (targetClass.isAssignableFrom(Integer.class)) {
        return convertToInteger(value);
    }

    // ...

That is as readable as it gets, in my opinion.
If you like classes, you can always extract the conversion into a single class for each mapping, and then iterate over that:
public interface Converter {
    public Class<?> getTargetType();
    public Object convert(Object value);
}

// ...

List<Converter> converters = new ArrayList<>();
converters.add(new StringConverter());
converters.add(new DateConverter());
converters.add(new IntegerConverter());
// ...

// ...

for (Converter converter : converters) {
    if (targetClass.isAssignableFrom(converter.getTargetType())) {
        return converter.convert(value);
    }
}

This, however, has the downside to split everything over many classes. A possible remedy to that might be to use single functions in the same class, like this:
HashMap<Class<?>, Function<Object, Object>> converters = new HashMap<>();
converters.put(String.class, this::convertToString);
converters.put(Date.class, this::convertToDate);
converters.put(Integer.class, this::convertToInteger);
// ...

// ...

for (Entry<Class<?>, Function<Object, Object>> converterEntry : converters.entrySet()) {
    if (targetClass.isAssignableFrom(converterEntry.getKey())) {
        return converterEntry.getValue().apply(value);
    }
}

That might be one of the easier readable and maintainable variants.
There is another, completely insane option, actually, which I feel to need to point out. You can use the above setup, with a method for every conversion, and reflection to build this list dynamically. Something along these lines:
private Integer convertToInteger(Object value);
private Date convertToDate(Object value);
private String convertToString(Object value);

// Pseudo-Code follows:

for (Method method : getClass().getDeclaredMethods()) {
    if (method.getName().startsWith("convert")) {
        if (targetValue matches method return type
                && value matches first method parameter) {
            return method.invoke(this, value);
        }
    }
}

But that should be an absolute last resort, as it is not quite easy to maintain without knowing what you've got at your hands there.


Simple matrix class C++14 I created a simple 4x4 matrix class (column-major). I would like it to be efficient and to use C++14's full capabilities. Can I improve it?

#include <array>

class mat4
{
public:
    constexpr mat4() noexcept : matrix() {}

    constexpr mat4(const std::array<float, 16> &m) noexcept : matrix(m) {}
    constexpr mat4(std::array<float, 16> &&m) noexcept : matrix(std::move(m)) {}

    constexpr mat4(const mat4 &other) noexcept : matrix(other.matrix) {}
    constexpr mat4(mat4 &&other) noexcept : matrix(std::move(other.matrix)) {}

    constexpr bool operator==(const mat4 &other) const noexcept
    {
        for (size_t i = 0; i < 16; ++i) {
            if ((*this)[i] != other[i]) {
                return false;
            }
        }

        return true;
    }

    constexpr bool operator!=(const mat4 &other) const noexcept
    {
        return !(this->operator==(other));
    }

    constexpr mat4& operator+=(const mat4 &other) noexcept
    {
        for (size_t i = 0; i < 16; ++i) {
            (*this)[i] += other[i];
        }
        return *this;
    }

    constexpr mat4& operator*=(float scalar) noexcept
    {
        for (size_t i = 0; i < 16; ++i) {
            (*this)[i] *= scalar;
        }
        return *this;
    }

    mat4& operator=(mat4 other) noexcept
    {
        std::swap(this->matrix, other.matrix);
        return *this;
    }

    constexpr float& operator[](size_t index) { return const_cast<float&>(static_cast<const std::array<float, 16>&>(matrix)[index]); }
    constexpr float operator[](size_t index) const { return matrix[index]; }

    void print() const noexcept
    {
        printf("\n");
        printf("[%.2f][%.2f][%.2f][%.2f]\n", matrix[0], matrix[4], matrix[8], matrix[12]);
        printf("[%.2f][%.2f][%.2f][%.2f]\n", matrix[1], matrix[5], matrix[9], matrix[13]);
        printf("[%.2f][%.2f][%.2f][%.2f]\n", matrix[2], matrix[6], matrix[10], matrix[14]);
        printf("[%.2f][%.2f][%.2f][%.2f]\n", matrix[3], matrix[7], matrix[11], matrix[15]);
    }

private:
    std::array<float, 16> matrix;
};

constexpr const mat4 operator+(mat4 lhs, const mat4 &rhs) noexcept
{
    lhs += rhs;
    return lhs;
}

constexpr const mat4 operator*(const mat4 &lhs, const mat4 &rhs) noexcept
{
    mat4 result;

    for (size_t i = 0; i < 4; ++i)
    {
        for (size_t j = 0; j < 4; ++j)
        {
            for (size_t k = 0; k < 4; ++k) {
                result[i + 4 * j] += lhs[i + 4 * k] * rhs[k + 4 * j];
            }
        }
    }

    return result;
}

constexpr const mat4 operator*(mat4 lhs, float scalar) noexcept
{
    lhs *= scalar;
    return lhs;
}

You can test it in this live demo here.

I'm not sure how I feel about you hardcoding that this should be a 4x4 matrix of floats, and that it should be column major. It seems straightforward enough to make a templated class like so

template <typename T, size_t numRows, size_t numCols>
class Matrix {
...
};

using mat4 = Matrix<float, 4, 4>;

It might also be worthwhile to have a boolean template parameter for whether or not it should be row or column major; in my experience many applications assume something is row major, and interfacing with your application might yield some unexpected issues.

If you do so, then I think adding a constructor to switch row/column major would be worthwhile as well.

I personally like using operator() instead of operator[] for getting a value at a given row and column; I feel like in general it shouldn't be up to the user to manually calculate the 1d index.  Something along the lines of this:

constexpr const T& operator()(size_t row, size_t col) const noexcept
{
    return matrix[col * numCols + row];
}

constexpr T& operator()(size_t row, size_t col) noexcept
{
    return matrix[col * numCols + row];
}

I also generally assume that at() (or similar) will be provided if indexing (or operator()-like access) is provided.

Don't use a member function like print(); instead implement operator<< so people can use it with their own streams as desired.  I see you're using printf - why? I've definitely used it before in my programs, but I usually think of it as a code-smell. If you want to add printf support that's fine, but I would argue that streams should be prioritized.

Your operator== is unnecessarily complicated - std::array implements it.

I don't like using this-> to refer to member functions; just use them.  Then your operator!= becomes return !operator==(other).

operator+= and operator*= can be done using std::transform.

You should implement iterators for your matrix as well. Whether you make them row-major or column-major shouldn't matter, but be consistent (assuming you make it possible to switch between orientations). No matter the orientation, the iteration order should be the same.

You should add some improved matrix multiplication algorithms (or, ideally, use a library that does it for you). With some SFINAE wizardry you can probably make a vectorized version, a tiled version, etc.  Probably only necessary if you actually make it possible to use larger matrices, although vectorization will probably still be a bit faster even for a small one.


An elegant way to find diff of two list of objects in Java This is the simplest possible implementation.

The two lists will contain at max, 3-5 elements, so the size of lists is not a matter of concern.

Any recommendation on how can I make this pretty?

private ListsHolder findDiffOfLists(List<MyObject> objsFromDB, List<MyObjRequest> objsFromRequest) {
    ListsHolder holder = this.new ListsHolder(); //An object holding two lists

    for(MyObjRequest req_obj : objsFromRequest){
        boolean isAdded = false;
        for(MyObject db_obj : objsFromDB){
            if(isSameObj(db_obj, req_obj)){
                db_obj.setVal(req_obj.getVal()); //Set new value
                holder.matchingObjList.add(db_obj);
                isAdded = true;
                break;
            }
        }

        if(!isAdded){
            holder.newObjList.add(req_obj); //If element does not exist
        }
    }

    return holder;
}

private boolean isSameObj(MyObject db_obj, MyObjRequest req_obj) {
       return req_obj.getMatch().equals(db_obj.getMatch());
}

I would create this method:

private MyObject findMatch(List<MyObject> objsFromDB, MyObjRequest req_obj){
    for(MyObject db_obj : objsFromDB){
        if(isSameObj(db_obj, req_obj)){
            return db_obj;
        }
    }
    return null;
}

and refactor accordingly.

Beyond that the only suggestion I would have is perhaps some better variable names (at least use a single naming style; if you don't already have one, use this: http://www.oracle.com/technetwork/java/codeconventions-135099.html#367).


Loading KML files into leaflet Javascript map This is working code for input KML files into leaflet map

// Load kml file
fetch('lotA.kml')
      .then( res => res.text() )
      .then( kmltext => {

            // Create new kml overlay
            parser = new DOMParser();
            kml = parser.parseFromString(kmltext,"text/xml");

            console.log(kml)

            const track = new L.KML(kml)
            map.addLayer(track)

            // Adjust map to show the kml
            const bounds = track.getBounds()
            map.fitBounds( bounds )

      })

fetch('lotB.kml')
      .then( res => res.text() )
      .then( kmltext => {

            // Create new kml overlay
            parser = new DOMParser();
            kml = parser.parseFromString(kmltext,"text/xml");

            console.log(kml)

            const track = new L.KML(kml)
            map.addLayer(track)

            // Adjust map to show the kml
            const bounds = track.getBounds()
            map.fitBounds( bounds )

      })

fetch('Schematic.kml')
      .then( res => res.text() )
      .then( kmltext => {

            // Create new kml overlay
            parser = new DOMParser();
            kml = parser.parseFromString(kmltext,"text/xml");

            console.log(kml)

            const track = new L.KML(kml)
            map.addLayer(track)

            // Adjust map to show the kml
            const bounds = track.getBounds()
            map.fitBounds( bounds )

      })

fetch('LotC.kml')
      .then( res => res.text() )
      .then( kmltext => {

            // Create new kml overlay
            parser = new DOMParser();
            kml = parser.parseFromString(kmltext,"text/xml");

            console.log(kml)

            const track = new L.KML(kml)
            map.addLayer(track)

            // Adjust map to show the kml
            const bounds = track.getBounds()
            map.fitBounds( bounds )

      })

fetch('LotD.kml')
      .then( res => res.text() )
      .then( kmltext => {

            // Create new kml overlay
            parser = new DOMParser();
            kml = parser.parseFromString(kmltext,"text/xml");

            console.log(kml)

            const track = new L.KML(kml)
            map.addLayer(track)

            // Adjust map to show the kml
            const bounds = track.getBounds()
            map.fitBounds( bounds )

      })

Is there a way to write this code shorter? I would like to use it for multiple KML layers.

This code is quite repetitive, and doesn’t adhere to the Don't Repeat Yourself principle.

One way to DRY it out is to abstract the latter promise callback to a separate function:

const addTrackAndBoundsFromKml = kmltext => {

     // Create new kml overlay
     const parser = new DOMParser();
     kml = parser.parseFromString(kmltext,"text/xml");

     const track = new L.KML(kml);
     map.addLayer(track);

     // Adjust map to show the kml
     map.fitBounds( track.getBounds() );

};

Note that const was added before the assignment of parser to avoid a global variable from being created, and bounds was eliminated because it was only used once. Also, semi-colons were added- while they are only required after a handful of statements, it could lead to errors if somehow whitespace got removed. It is a good habit to default to terminating lines with them.

Then use (a reference to) that function whenever it is needed:

// Load kml file
fetch('lotA.kml')
  .then( res => res.text() )
  .then( addTrackAndBoundsFromKml );

fetch('lotB.kml')
  .then( res => res.text() )
  .then( addTrackAndBoundsFromKml );

//etc...

This way if a change needed to happen in that function that parses the KML, adds the track layer and fits the bounds, it could be done in one place instead of each occurrence.

The file names could also be stored in an array and iterated over:

const files = ['lotA.kml', 'lotB.kml', 'Schematic.kml', 'lotC.kml', 'lotD.kml'];
for ( const file of files ) {
    fetch(file)
      .then( res => res.text() )
      .then( addTrackAndBoundsFromKml );
}

It may not be useful but you could consider using Promise.all() though the need to call the asynchronous .text() method on each result might make that really complicated.

You could also consider using the ecmascript-8 feature async functions with the await operator on the promises, as long as the browser requirements are sufficient.


Delete order's details' line without changing page using AJAX Following a teacher's JavaScript code I intend to delete order's lines with AJAX.

I have tried the code below and it allows the user to delete line by line but for example for the last line it won't delete it.

Function to display order's details:

    public static function showOrderDetails($id){
    $res=DB::execute_sql('select id,idbebida, unidades, pvp from lineaspedido where idpedido=?', array($id));

    if($res){
        $res -> setFetchMode(PDO::FETCH_NAMED);
        $first=true;
        echo '

';
        foreach ($res as $table_row) {
            if($first){ //HEADERS

                foreach ($table_row as $row_index => $row_data) { // titulos campos
                    if($first){ //TO NOT DISPLAY ID
                        $first=false;
                        continue;
                    }
                    echo "$row_index	";
                }

            }
            echo '
';

            $first=true;
            foreach ($table_row as $row_data) { // DATA
                if ($first){
                    $idLinea=$row_data;
                    $first=false;
                    echo "
";
                } else{
                    echo "$row_data	";
                }
            }

            echo "<button onclick=\"deleteLineaPedido($idLinea )\">Borrar</button>	";
           echo "
";
        }
        echo '
';
    }
}

PHP OBJECT to manage JSON:

    <?php
$res = new stdClass();
$res->deleted=false; //Attribute DELETED default false
$res->message=''; //Error message
try{
    $datoscrudos = file_get_contents("php://input"); //READ
    $datos = json_decode($datoscrudos);
    $db = new PDO("sqlite:./datos.db");
    $db->exec('PRAGMA foreign_keys = ON;'); //KEYS
    $sql=$db->prepare('delete FROM lineaspedido WHERE idpedido=?;');
    if($sql){
        $sql->execute(array($datos->id));
        if($sql->rowCount()>0){ //NUMBER OF ROWS AFFECTED
           $res->deleted=true; //WHATEVER have been deleted is refreshed
        }
    }
}catch(Exception $e){
   $res->message=$e->getMessage();

header('Content-type: application/json');
echo json_encode($res);

JS function to delete:

function deleteLineaPedido(idLinea){
if(!confirm("Desea borrar la linea de pedido: '"+idLinea+"'")){
    return;
}
var ajax= new XMLHttpRequest();
ajax.onreadystatechange=function(){
    if(this.readyState == 4 && this.status == 200) {
        var res= JSON.parse(this.responseText); //RESULT in JSON
        if(res.deleted === true){
            var fila=document.querySelector('#fila'+idLinea); //We choose the first and unique row which has whatever ID needed.
            fila.parentNode.removeChild(fila); //We delete the previous row firstly going to its parent node and then deattaching it.
        }
    }
};
ajax.open("post","borrar_linea_Pedido.php",true);
ajax.setRequestHeader("Content-Type","application/json;charset=UTF-8");
ajax.send(JSON.stringify({"id":idLinea})); //Formato {id:identificador de registro a borrar}

Is there a simpler way to achieve this?

PD: The table used 'lieanspedidos' has the following structure:

id          Numeric identifier of every row of order's details
idpedido    Numeric order identifier
idbebida    Numeric drink identifier
unidades    How many drinks have been inserted into that order's row.
PVP         Each drink's price per unit.

PHP

Variable names, excess variables, logic

When iterating through the rows in showOrderDetails(), the variable $row_index is slightly misleading because that should contain the column name, not an index (typically an integer value):

foreach ($table_row as $row_index => $row_data) { // titulos campos

A better name for that key would be something like $column_name:

foreach ($table_row as $column_name => $column_value) { // titulos campos

But the name $row_index would be appropriate for the key on the outer foreach:

foreach ($res as $row_index => $table_row) {

Then instead of creating an extra variable called $first, you could utilize $row_index and $column_name to conditionally display the rows and cells.

The first one:

if($first){ //HEADERS

could then be updated to:

if($row_index === 0){ //HEADERS

And then the second one, in the nested loop:

if($first){ //TO NOT DISPLAY ID

Could be changed to:

if($column_name === 'id'){ //TO NOT DISPLAY ID

And the second inner loop could be updated as well:

foreach ($table_row as $row_data) { // DATA
      if ($first){
          continue;

Could be rewritten to add the row if the column name is id:

foreach ($table_row as $column_name => $column_value) { // DATA
    if ($column_name === 'id'){
        $idLinea=$column_value;

And then $first could be eliminated entirely.

Alternatively, the condition in the first nested loop could be negated and then the continue could be eliminated:

if($column_name !== 'id'){ //DO NOT DISPLAY ID
    echo "<th>$column_name</th>";
}

See that all together in this playground example (not connected with Javascript/AJAX requests).

JS

The line to look up the element to remove by id attribute:

var fila=document.querySelector('#fila'+idLinea); //We choose the first and unique row which has whatever ID needed.

Could be changed to use document.getElementById()

  var fila=document.getElementById('fila'+idLinea); //We choose the first and unique row which has whatever ID needed.

There are a few discussions about which to use:

this Sitepoint forum
this SO question and its answers (and related posts)

The general consensus is that getElementById() is quicker than querySelector(). For a small page like this it would likely not be noticeable.


Promise for request/response with timeout Below is a piece of code that uses a promise for sending a request to a server, and to wait for an answer. Under normal conditions the server will always respond immediately,

But of course you don't want your code to be stuck forever if a response is missed (e.g. a network issue). So for that reason, it needs a timeout as well.

I usually write this as follows. But it takes quiet a lot of code. Any sugestions ?

new Promise( (resolve, reject) => {
  // the promise is resolved when a predefined response is received.
  const listener = (msg) => {
    const { command } = msg;
    if (command === "pong") resolve();
  };

  try {
    this.addListener(listener);

    // write the command
    this.sendMessage( { command: "ping" });

    // there should be an answer within x seconds
    await new Promise(() => setTimeout(reject, timeout));
  }
  finally {
    this.removeListener(listener);
  }
}

I have been using this as a pattern for nodejs, angular and react code.

await is only applicable in async functions. The executor function passed to Promise constructor at the code at the question is not defined as async.

The second Promise constructor is not necessary. resolve and reject defined at the single Promise executor can be used.

Include .catch() or function at second parameter of .then() chained to Promise constructor to avoid Uncaught in Promise error and handle potential error within Promise constructor function body.

new Promise((resolve, reject) => {
  try {
    const complete = () => !clearTimeout(time)
      && this.removeListener(listener);
    const listener = ({command}) => command === "pong"
      && !complete() && resolve();
    this.addListener(listener);
    this.sendMessage({ command: "ping" });
    const time = setTimeout(() => !complete() && reject(), timeout);
  } catch (e) {
      throw e
  }
})
// handle resolved `Promise`, handle error
.then(() => {}, e => console.error(e));


"Function Buffer" to avoid triggering an event handler too frequently If some big event handler is triggered too frequently, it will cause computer lagging.

In my example $(window).resize(), I want the function to be called after the user has completed his resize, instead of firing every time you resize by a single pixel.

So, I made this 'Function Buffer':

function FtnBuff(){
    var count=0;
    var delay = 50;
    var ftnList=[];

    this.add = function(ftn){
        if(isFunction(ftn)){
            ftnList.push(ftn);
        }else{
            return false;
        }
    }
    this.start = function(){
        count ++;
        setTimeout(checkbuff, delay);
    }
    this.setDelay = function(num){
        if(!isNaN(num)){
            delay = num;
        }
    }
    function checkbuff(){
        if(count==1){
            runFtn();
        }
        count--;
    }
    function runFtn(){
        for(var key in ftnList){
            ftnList[key]();
        }
    }
    function isFunction(obj) {
        var getType = {};
        return obj && getType.toString.call(obj) === '[object Function]';
    }
    return this;
}

Use case:

ftnbuff = new FtnBuff()
ftnbuff.add(aa)
ftnbuff.add(function(){
    bb('long time no C.')
})
$(window).resize(ftnbuff.start)

function aa(){
    console.log('This is aa.');
}
function bb(customText){
    console.log('This is bb.'+customText);
}

As mentioned in the comments, this is usually called "debouncing."

A simpler and easier-to-use approach would be something like this:

function debounce(fn, delay) {
    var timeout;

    return function () {
        var context = this,
            args = arguments;

        clearTimeout(timeout);
        timeout = setTimeout(function () {
            fn.apply(context, args);
        }, delay || 250);
    };
}

Use case:

$(window).resize(debounce(someFunction, 1000));

With that out of the way, let's review the code.

Broken features

The timeout is never cleared if the function is called multiple times within the delay period. Unless I'm missing something, all this is going to accomplish is delaying the function from running, but it will still run as many times as it is called. This doesn't seem to work "as advertised."

Missing features

Most functions take arguments. Many functions rely on a this binding. These buffered functions don't allow passing arguments or preserve the this binding. These are features you will probably need at some point. Incidentally, with this implementation it would be very difficult to make changes allowing arguments to your buffered functions, because FtnBuff does not have a...

Single responsibility

If you're going to debounce functions, there's no reason you can't debounce them one at a time. Your code incurs a lot of complexity from maintaining a list of functions to call (which also causes problems with passing arguments).

Let's take a step back. If you wanted to have one function handle an event, you'd do this:

addEventListener('resize', someFunction);

If you needed to have two functions handle an event (and didn't want to register them as separate listeners), you might do something like this:

addEventListener('resize', function () {
    someFunction.apply(this, arguments);
    anotherFunction.apply(this, arguments);
});

There's no reason why a debounce function should handle calling a series of functions; that's not its job. Its job is just to debounce. Calling a series of functions is already a core feature of the language; just write a function that calls the other functions and debounce that.

TLDR: Don't try to handle multiple functions, it's too limiting and not necessary.

Typechecking

You may be more familiar with strongly-typed languages; they have built-in type checking. So you think, "hmm, how can I ensure that things are the right type in this dynamically-typed language?" And you end up writing things like if (isFunction(fn)), or if (!isNaN(n)), or whatever.

This is not a road you want to go down. It's going to cause unnecessary code bloat, and it doesn't really solve anything. Imagine passing a non-function or a non-number to one of those functions. If you weren't doing these checks, things would likely fail in some way that would indicate what the problem was. Since you are doing these checks, things won't fail, but nothing useful will happen either. This is arguably worse -- I'd rather have something fail and be able to track down the problem then have nothing happen at all, and get stuck wondering why.

What you could do instead is throw errors when these checks fail. This would be better than what you have now, but again, not really necessary. It's up to you to document your API (think annotations) and it's up to the person using it to use it properly. If they're passing a string in for a delay parameter that's clearly documented as taking a number, that's user error.

TLDR: Don't write twice as much code to handle dumb user errors, document your API instead.


SMS sending through Clickatell I have to implement a method in which i'm passing two arguments as Mobile number and second one is Message to be sent.

I'm using clickatell for the first time and its successfully sending the messages but is it the right approach of coding for it.

Its the test code what i have posted can i implement this in my production code.

I have cleared the Key out and replaced with stars.

As its a paid service should i go for clickatell or is there any others options to send sms i can try out.

Please review the code.

public class Sms {

    public final String APIKEY = "*************************";
    //private String mobile;

    public void sms_generation(String mobile, String Message) {

        System.out.println("\n\nSTARTING WITH TESTING REST:");

        // Create New object (Assign auth straight away.):
        ClickatellRest click = new ClickatellRest(APIKEY);

        // We cannot test for auth, so lets start with balance:
        System.out.println("TESTING GET BALANCE");
        try {
            double response = click.getBalance();
            System.out.println(response);
        } catch (UnknownHostException e) {
            System.out.println("Host could not be found");
        } catch (Exception e) {
            System.out.println(e.getMessage());
        }

        // Assuming the auth was successful, lets send one message, to one
        // recipient:
        System.out.println("\nTESTING SEND SINGLE MESSAGE");
        try {
            ClickatellRest.Message response = click.sendMessage(mobile, Message);
            System.out.println(response);
            if (response.error != null) {
                System.out.println(response.error);
            } else {

                ClickatellRest.Message msg = click
                        .getMessageStatus(response.message_id);
                System.out.println("ID:" + msg.message_id);
                System.out.println("Status:" + msg.status);
                System.out.println("Status Description:" + msg.statusString);
                System.out.println("Charge:" + msg.charge);
                System.out.println("\nTESTING STOP MESSAGE");
                ClickatellRest.Message msgStatus = click
                        .stopMessage(response.message_id);
                System.out.println("ID:" + msgStatus.message_id);
                System.out.println("Status:" + msgStatus.status);
                System.out.println("Status Description:"
                        + msgStatus.statusString);
            }
        } catch (UnknownHostException e) {
            System.out.println("Host could not be found");
        } catch (Exception e) {
            System.out.println(e.getMessage());
        }

    }

}

1 Use Logger, do not print all your app info, in production it will mess up server logs. You can use different log level (debug, info). For example use info level in prod, and debug level in dev environment.

2  Do no catch all exceptions in service layers, you can throw them up to controller (if you use spring mvc) and catch all that there.

https://spring.io/blog/2013/11/01/exception-handling-in-spring-mvc

3 Try keep your code clean to read and unsderstand.

public void sendMessage(String mobile, String Message) {
        ClickatellRest click = new ClickatellRest(APIKEY);
        ClickatellRest.Message response = click.sendMessage(mobile, Message);
        log.debug(response);
        if (response.error == null) {
            ClickatellRest.Message msg = click.getMessageStatus(response.message_id);
            log.debug(msg);
            ClickatellRest.Message msgStatus = click.stopMessage(response.message_id);
            log.debug(msgStatus);
        } else {
            log.error(response.err);
        }
    }

4 If you can, override toString() method in ClickatellRest.Message class if you often log this object.


MontyPython (Fizzbuzz) This is an iterative review. The previous iteration can be found here.

I've now been programming in python for all of 6 hours.
This is probably my last iteration of FizzBuzz.

Thoughts?

FizzBuzz.py

from utilities import ask_something
from utilities import ask_yes_no

def print_fizz_buzz_output(start_num, end_num, pair_list):
    for num in range(start_num, end_num + 1):
        divisor_text = [text for divisor, text in pair_list if num % divisor == 0]
        if divisor_text:
            print(''.join(divisor_text))
        else:
            print(num)

def ask_divisor_text_pairs():
    loop = True
    while loop:
        divisor = ask_something('Divisor? ', int,
                                'Invalid input for divisor. Divisor must be a whole number. Please try again.')
        text = ask_something('Text? ', str)
        yield (divisor, text)

        loop = (True if 'y' == ask_yes_no('Input another Divisor (y/n)? ') else False)

def ask_iteration_range() -> tuple:
    start_num = ask_something('Start Number? ', int)
    while True:
        end_num = ask_something('End Number? ', int)
        if end_num >= start_num:
            break
        else:
            print('Invalid input for end number. Must be a whole number greater than or equal to the start number.'
                  ' Please try again.')
    return start_num, end_num

if __name__ == '__main__':
    print_fizz_buzz_output(*ask_iteration_range(), list(ask_divisor_text_pairs()))

utilities.py

def ask_something(prompt: str, convert, error_message='Invalid input.'):
    while True:
        value = input(prompt)
        try:
            return convert(value)
        except ValueError:
            print(error_message)

def ask_yes_no(prompt: str, allow_upper_case=True) -> str:
    # returns 'y' or 'n'
    while True:
        value = ask_something(prompt, str)
        if value == 'y' or (value == 'Y' and allow_upper_case):
            return 'y'
        elif value == 'n' or (value == 'N' and allow_upper_case):
            return 'n'
        else:
            print('Invalid Input. Please input "y" or "n"')

Example Input/Output

Start Number? 1
End Number? 15
Divisor? 3
Text? Monty
Input another Divisor (y/n)? y
Divisor? 5
Text? Python
Input another Divisor (y/n)? n
1
2
Monty
4
Python
Monty
7
8
Monty
Python
11
Monty
13
14
MontyPython

The code is good!

I however do have a couple of niggles:

You should use commas to import multiple functions from a module.
This changes the imports at the top to a single import:

from utilities import ask_something, ask_yes_no

== results in a boolean, no need to make on yourself.

>>> ('y' == 'y', 'y' == 'n')
(True, False)

And so you can simplify the reassignment of the loop flag.
If I already can't read half of a string, you may as well keep it all on one line.
Whilst PEP8 says that we should limit the line length to 79 characters,
formatting large chunks of words within this requirement become an eye saw to me.
And so I'd just leave them on one line.

I also don't really like the verbose messages:

  Invalid input for divisor. Divisor must be a whole number. Please try again.

The message "The input must be a whole number." is enough.
Alternately you could just re-ask the question,
which implies there's been an error.


C++ Keyboard Controller I've written a Controller for the keyboard so that I can map certain keys to callbacks within a game. A key can only call one callback, but a callback can be called by many keys.

Currently, a Controller has two std::maps - one for when the key(s) is pressed/held, and one for when the key(s) are released. These maps are SDL_Scancode to std::string (Which I may change later to an enum rather than a string? But strings are easier to read so maybe not.) The Controller also has a ControllerContext object, which is just a wrapper around two std::maps of std::string to std::function<void(void)>: one map for callbacks to happen when a key is pressed, another for callbacks to happen when a key is released.

I feel like some kind of change could be made to make the code more readable, and I am unsure of if I should have the context stored in a shared_ptr, or if I should just keep a raw pointer within the Controller.

This is how it would work:

//An event union provided by SDL
//https://www.libsdl.org/release/SDL-1.2.15/docs/html/sdlevent.html
SDL_Event e;
//A controller
Controller controller_;
ControllerContext context_;

//Build the context
controller_.SetContext(&context);

//Poll all events in the event queue, feeding them to the structure e
while(SDL_PollEvent(&e))
{
   switch(e.type)
       case SDL_KEYDOWN:
           controller_.OnKeyPress(e.key);
           break;
       case SDL_KEYUP:
           controller_.OnKeyRelease(e.key);
           break;
       default:
           break;
}

keyboardcontroller.h

#ifndef SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_
#define SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_

#include <map>
#include <SDL.h>
#include <iostream>
#include <exception>
#include <string>
#include <memory>

struct ControllerContext;

class Controller
{
public:
    Controller();

    int OnKeyPress(const SDL_Event &e);

    int OnKeyRelease(const SDL_Event &e);

    void SetContext(std::shared_ptr<ControllerContext> context_);

protected:
    std::map<SDL_Scancode, std::string> keypress_action_map_;
    std::map<SDL_Scancode, std::string> keyrelease_action_map_;
    std::shared_ptr<ControllerContext> controller_context_;
};

#endif

keyboardcontroller.cpp

#include "KeyboardController.h"
#include "ControllerContext.h"

Controller::Controller()
{
    //Some test initializations
    keypress_action_map_[SDL_SCANCODE_LEFT] = "Left";
    keypress_action_map_[SDL_SCANCODE_DOWN] = "Down";
    keypress_action_map_[SDL_SCANCODE_RIGHT] = "Right";
    keypress_action_map_[SDL_SCANCODE_UP] = "Up";

    keyrelease_action_map_[SDL_SCANCODE_LEFT] = "Left";
    keyrelease_action_map_[SDL_SCANCODE_DOWN] = "Down";
    keyrelease_action_map_[SDL_SCANCODE_RIGHT] = "Right";
    keyrelease_action_map_[SDL_SCANCODE_UP] = "Up";
}

int Controller::OnKeyPress(const SDL_Event& e)
{
    try
    {
        //std::cout << keypress_action_map_.at(e.key.keysym.scancode) << std::endl;
        controller_context_->keypress_events_.at(keypress_action_map_.at(e.key.keysym.scancode))();
        return 1;
    }
    catch (std::exception ex)
    {
        return 0;
    }
}

int Controller::OnKeyRelease(const SDL_Event& e)
{
    try
    {
        std::cout << keyrelease_action_map_.at(e.key.keysym.scancode) << std::endl;
        return 1;
    }
    catch (std::exception ex)
    {
        return 0;
    }
}

void Controller::SetContext(std::shared_ptr<ControllerContext> context_)
{
    controller_context_ = context_;
}

ControllerContext.h

#ifndef SHMUPPY_HEADER_CONTROLLER_CONTEXT_H
#define SHMUPPY_HEADER_CONTROLLER_CONTEXT_H

#include <map>
#include <functional>

struct ControllerContext
{
    std::map<std::string, std::function<void(void)>> keypress_events_;
    std::map<std::string, std::function<void(void)>> keyrelease_events_;
};

#endif

Edit: Tested implementations below.

As jason says, returning either 0 or 1, looks like boolean. Change int to bool and return true or false.

Drop the std::string layer. It doesn't add anything over enum. And enum are just as readable and there is no way typos can appear with enums.

I don't have any experience with SDL myself, but I've written a keyboard -> action controller in an attempt at a game.

I'd also like to drop the std::map in favor of a std::vector. The trick is to use continuous enums and use those as indices for the std::vector.

Something like this:

enum class KeyboardEvents : byte
{
    ON_RELEASE,
    ON_PRESS,

    ORDINAL
};

typedef std::function<void(void)> keyboardFunc_t;

std::vector<keyboardFunc_t> keyboardEvents(KeyboardEvents::ORDINAL);

void bindControls()
{
    auto onReleaseLambda = [&]()
    {
        // Do stuff
    };
    auto onPressLambda = [&]()
    {
        // Do stuff
    };

    // Bind with lambdas
    keyboardEvents[KeyboardEvents::ON_RELEASE] = onReleaseLambda;
    keyboardEvents[KeyboardEvents::ON_PRESS] = onPressLambda;

    // Alternatively, use std::bind
    keyboardEvents[KeyboardEvents::ON_RELEASE] = std::bind(&ClassName::onRelease, classInstancePtr);
    keyboardEvents[KeyboardEvents::ON_PRESS] = std::bind(&ClassName::onPress, classInstancePtr);
}

I just wrote this on the top off my head. I'm not sure if it'll compile, but you get the idea.

I would also prefer lambdas in favor of std::bind.

As for storing std::shared_ptr<ControllerContext> controller_context_; in a raw or shared_ptr, I'd say neither. But DO use std::unique_ptr. Using a unique_ptr will make sure your controller class is not default copyable, which is a good thing when dealing with limited resources.

Like this std::unique_ptr<ControllerContext> controller_context_;

Edit: From here and on:

Note, don't look too closely on main.cpp, it was mainly copied from somewhere to get SDL2 running.

ControllerContext.h:

#ifndef SHMUPPY_HEADER_CONTROLLER_CONTEXT_H
#define SHMUPPY_HEADER_CONTROLLER_CONTEXT_H

#include <vector>
#include <functional>

struct ControllerContext
{

    ControllerContext()
        : keyPressEvents(SDL_NUM_SCANCODES)
        , keyReleaseEvents(SDL_NUM_SCANCODES)
    {
        keyPressEvents.resize(SDL_NUM_SCANCODES);
        keyReleaseEvents.resize(SDL_NUM_SCANCODES);
    }

    typedef std::function<void(void)>   KeyboardFunc_t;
    typedef std::vector<KeyboardFunc_t> KeyEvents;

    KeyEvents   keyPressEvents;
    KeyEvents   keyReleaseEvents;
};

typedef std::unique_ptr<ControllerContext>  ControllerContext_ptr;

#endif

keyboardcontroller.h:

#ifndef SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_
#define SHMUPPY_HEADER_KEYBOARDCONTROLLER_H_

#include <map>
#include <SDL.h>
#include <iostream>
#include <exception>
#include <string>
#include <memory>

#include "ControllerContext.h"

class Controller
{
public:
    Controller();

    bool OnKeyPress(const SDL_Event &e);

    bool OnKeyRelease(const SDL_Event &e);

    // Clear by setting empty func
    void SetReleaseCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func);
    void SetPressCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func);

    // Clear both press and release
    void ClearCallback(SDL_Scancode code);

protected:
    ControllerContext_ptr controller_context_;
};

#endif

keyboardcontroller.cpp:

#include "KeyboardController.h"
#include "ControllerContext.h"

Controller::Controller()
    : controller_context_(new ControllerContext())
{
    //Some test initializations
    auto leftPress = [&]()
    {
        // Left press lambda
        int debug=0;
    };
    auto leftRelease = [&]()
    {
        // Left release lambda
        int debug=0;
    };

    SetPressCallback(SDL_SCANCODE_LEFT, leftPress);
    SetReleaseCallback(SDL_SCANCODE_LEFT, leftRelease);
}

bool Controller::OnKeyPress(const SDL_Event &e)
{
    const auto & scancode = e.key.keysym.scancode;

    const auto & pressFunc = controller_context_->keyPressEvents[scancode];

    if (pressFunc)
    {
        // Call callback if valid
        pressFunc();
        return true;
    }

    return false;
}

bool Controller::OnKeyRelease(const SDL_Event &e)
{
    const auto & scancode = e.key.keysym.scancode;

    const auto & pressFunc = controller_context_->keyReleaseEvents[scancode];

    if (pressFunc)
    {
        // Call callback if valid
        pressFunc();

        return true;
    }

    return false;
}

void Controller::SetReleaseCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func)
{
    controller_context_->keyReleaseEvents[code] = func;
}

void Controller::SetPressCallback(SDL_Scancode code, const ControllerContext::KeyboardFunc_t & func)
{
    controller_context_->keyPressEvents[code] = func;
}

void Controller::ClearCallback(SDL_Scancode code)
{
    controller_context_->keyPressEvents[code] = ControllerContext::KeyboardFunc_t();
    controller_context_->keyReleaseEvents[code] = ControllerContext::KeyboardFunc_t();
}

main.cpp:

#include <SDL.h>
#include <iostream>

class TestMain
{
    public:
        void handlePressW()
        {
            // W press
            int debug=0;
        }
        void handleReleaseW()
        {
            // W release
            int debug=0;
        }
};

#include "keyboardcontroller.h"

int _stdcall WinMain(struct HINSTANCE__ *hInstance, struct HINSTANCE__ *hPrevInstance, char *lpszCmdLine, int nCmdShow)
{
    if (SDL_Init(SDL_INIT_EVERYTHING) < 0)
    {
        /* Handle problem */
        fprintf(stderr, "%s\n", SDL_GetError());
    }
    SDL_Window* window = SDL_CreateWindow("Window caption", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 852, 480, 0);
    if (window == NULL)
    {
        /* Handle problem */
        fprintf(stderr, "%s\n", SDL_GetError());
        SDL_Quit();
    }
    SDL_Renderer* renderer = SDL_CreateRenderer(window, -1, 0);
    if (renderer == NULL)
    {
        /* Handle problem */
        fprintf(stderr, "%s\n", SDL_GetError());
        SDL_Quit();
    }

    Controller keycontroller;

    // Bind testmain methods
    TestMain testmain;
    keycontroller.SetPressCallback(SDL_SCANCODE_W, std::bind(&TestMain::handlePressW, testmain));
    keycontroller.SetReleaseCallback(SDL_SCANCODE_W, std::bind(&TestMain::handleReleaseW, testmain));

    bool running = true;
    while (running)
    {
        /* Clear the buffer of color, setting it to black */
        SDL_RenderClear(renderer);

        /* Draw the buffer into the window */
        SDL_RenderPresent(renderer);

        /* Handle input and events: */
        SDL_Event sdl_event;
        while (SDL_PollEvent(&sdl_event) > 0)
        {
            switch (sdl_event.type)
            {
                case(SDL_KEYDOWN):
                {
                    keycontroller.OnKeyPress(sdl_event);
                    break;
                }

                case(SDL_KEYUP):
                {
                    keycontroller.OnKeyRelease(sdl_event);
                    break;
                }

                case(SDL_QUIT):
                    running = false;
                    break;
            }

        }

        /* Run your logic. For example: */
    }

    SDL_Quit();

    return 0;
}


Spring Security Web Configurer Adapter guys.
The code below is spring security web adapter. I do not like configure(HttpSecurity) method that generates security confirmation policy.
Any ideas to do it more readable and clear?

/**
* Spring security configuration
*
* @author Eugene Ustimenko
* @date Nov 5, 2014
*/
@Configuration
@EnableWebSecurity
public class SecurityConfiguration extends WebSecurityConfigurerAdapter {

@Autowired
@Qualifier ("loginService")
private ILoginService loginService;

@Override
protected void configure (AuthenticationManagerBuilder auth)
  throws Exception {

auth.userDetailsService(loginService).passwordEncoder(passwordEncoder());
}

@Override
protected void configure (HttpSecurity http) throws Exception {

final RequestMatcher csrfRequestMatcher = new RequestMatcher() {

  private RegexRequestMatcher requestMatcher = new RegexRequestMatcher("/login/*", null);

  @Override
  public boolean matches (HttpServletRequest request) {

    return requestMatcher.matches(request);
  }
};
http.csrf()
    .requireCsrfProtectionMatcher(csrfRequestMatcher)
    .and().authorizeRequests().antMatchers("/admin/**").access("hasRole('ADMIN')")
    .and()
    .formLogin().loginPage("/login").failureUrl("/login?error")
    .usernameParameter("username").passwordParameter("password")
    .and()
    .logout().logoutSuccessUrl("/")
    .and()
    .csrf()
    .and()
    .exceptionHandling().accessDeniedPage("/403");
}

@Bean
public PasswordEncoder passwordEncoder () {

  final PasswordEncoder encoder = new BCryptPasswordEncoder();
  return encoder;
}

@Bean (name = "auth")
@Override
public AuthenticationManager authenticationManagerBean () throws Exception {

  return super.authenticationManagerBean();
}
}

You don't need to build this as one line. You could try splitting it out into multiple lines and add some comments to describe what you are doing.

I think the below example does what you want.

I have removed the anonymous RequestMatcher class. RegexRequestMatcher implements the RequestMatcher interface.
I have removed the second csrf() method call as it isn't needed.

@Override
protected void configure(final HttpSecurity http) throws Exception {

    // Enable csrf for login form
    http.csrf().requireCsrfProtectionMatcher(new RegexRequestMatcher("/login/*", null));
    // Configure login page
    http.formLogin().loginPage("/login").failureUrl("/login?error").usernameParameter("username").passwordParameter("password");
    // Configure logout redirect
    http.logout().logoutSuccessUrl("/");
    // Ensure admin pages have correct role
    http.authorizeRequests().antMatchers("/admin/**").hasRole("ADMIN");
    // Configure access denied exception redirect
    http.exceptionHandling().accessDeniedPage("/403");

}


Handling multiple try-catches in a for loop Good day! I would like to know if this is possible to be refactored into a cleaner code.
The reason I'm doing it like this is to catch the column that the error appeared on and then potentially output the column name and value for the client to view later on.
// Make the name act as the key and the value equates to ID
// $towers['tower1'] = 1;
$towers = Tower::pluck('id', 'name')->toArray();
$departments = Department::pluck('id', 'name')->toArray();

foreach($data as $key => $row) {
    try {
        // $row['tower'] = 'Hello World Tower'
        // Try to see if $row['tower'] is present in the $towers data source
        $data[$key]['tower'] = $towers[$row['tower']];
    } catch (\Exception $e) {
        // Exception occurs because index is not available
        $error_holder[$key]['tower_error'] = $e->getMessage();
    }

    // Same logic as above
    try {
        $data[$key]['department'] = $departments[$row['department']];
    } catch (\Exception $e) {
        $error_holder[$key]['department_error'] = $e->getMessage();
    }
}

You need anything here but a try catch.
foreach($data as $key => $row) {
    // $row['tower'] = 'Hello World Tower'
    // Try to see if $row['tower'] is present in the $towers data source
    if (isset($towers[$row['tower']])) {
        $data[$key]['tower'] = $towers[$row['tower']];
    } else {
        $error_holder[$key]['tower_error'] = "Tower not found for $key";
    }

    if (isset($departments[$row['department']])) {
        $data[$key]['department'] = $departments[$row['department']];
    } else {
        $error_holder[$key]['tower_error'] = "Department not found for $key";
    }
}

Although you can reduce the duplication by storing sources into array and looping over it but honestly it makes the code so muddled that I don't see the benefit.
foreach (compact('tower','department') as $name => $column)
    foreach($data as $key => $row) {
        if (isset($column[$row[$name]])) {
            $data[$key][$name] = $column[$row[$name]];
        } else {
            $error_holder[$key][$name.'_error'] = "$name not found for $key";
        }
    }
}


Pulling JSON using jQuery and manipulating the Front-end My JavaScript is fairly new and want to learn new ways to create clean, fast code. Can you have a look at this code and guide me on how to optimise this code to improve my skills? I have provided the HTML even though I am not able to edit the actual html as its generated but lets pretend I could..

What I am doing is pulling data from a Product JSON file and manipulating the data to be shown in the website.

function Details(el){
    var sku = $(el).attr("data");
    $.getJSON("/micro/"+sku, function(json) {
        if (json.price!=null) {
          $(el+' .p_img img').attr({src:json.images[2].url, alt:json.images[2].altText});
          $(el+' .p_img_lrg').attr("src",json.images[1].url);
          $(el+' .p_name').text(json.name);
          $(el+' .p_url').attr("href",json.url);
          $(el+' .p_p').text("£"+Number(json.p.value).toFixed(2));
          $(el+' .p_pu').text(json.pu);
          $(el+' .p_b_name').text(json.categories[0].name);
          $(el+' .p_b_url').attr("href",json.categories[0].url);
        if (json.oldPrice != null) {
          var pWas = ' .p_was',
              pSave = ' .p_save';
          $(el+pWas).text("was £"+json.old.value);
          $(el+pSave).text("save £"+(json.old.value-json.value));
          $(el+pSave).hide();
          $(el+pWas).css('margin-left',0).css('padding-left',0).addClass('clr').removeClass('left').css('float','none');
        }else{
          $(el+pSave,el+pWas).hide();
        }
    }

    if (json.stockLevel==0) {
      $(el+' form.add').attr({'action':'/notification','method':'post'}).html('<input type=\"hidden\" name=\"productCodePost\" value=\"'+sku+'\"><input type="submit" value="Out">');
    } else {
      $(el+' form.add').attr({'action':'/add','method':'post'}).html('<input type=\"hidden\" name=\"productCodePost\" value=\"'+sku+'\"><input type=\"hidden\" name=\"maxOrderQuantity\" value=\"\"><input type="submit" value="Add">');
    }
});
}

HTML:

        #
        test

                    now
                    &pound;

        <form class="add_to_cart_form">&nbsp;</form>

View All  Products

Functions in JavaScript, by convention, begin with a lowercase letter, unless they're constuctors. So Details() should be details()

You've got some missing indentation, which makes the code harder to read. In general your code could probably benefit from a bit more whitespace to help legibility.

You also have some funky-looking strings, like: '<input type=\"hidden\" name=\"productCodePost\" value=\"'
You use single quotes around the string, but still escape the double-quotes inside the string. This is unnecessary: A single-quoted string can contain double-quotes just fine, and a double-quoted string can contain single-quotes (like so 'double-quotes (") work fine here' or "single-quotes (') work fine here"). You only need to escape if your string contains the same type of quote that you're using to delimit the string.
In fact, you're already doing that in your code. The rest of the line I used as example looks like this '\"><input type="submit" value="Out">' and works just fine with out any backslashes.

You can also use jQuery to build those elements for you, i.e.

form.append($("<input>").attr({
  type:  hidden,
  name:  productCodePost,
  value: sku
});

which gives you a more structured and readable source code, instead of giant strings.

There's also a lot of repetition - or semi-repetition in this case: The el+"...". I'd advice something like what Matt suggested which is to store the result of $(el) once, and from there using .find() to locate the individual elements you want to update.

For the actual element updating there isn't that much that can be improved, since there's no discernible commonalities between, say, element IDs and JSON keys (in fact, the JSON seems very messy!)

There are some minor things you could do like define a renderAmount function to make sure you're always printing prices the same way:

function renderAmount(amount) {
  return "£" + Number(amount).toFixed(2);
}

Right now, you only use the .toFixed() in one place, although you have 3 different places where you need to write amounts.

Similarly, you could add a function to update images, since you do it at least twice. Not a great savings, but it helps break up the code.

function updateImage(element, info) {
  element.attr({
    src: json.url,
    alt: json.altText
  });
}

// ...

updateImage(container.find(".p_img img"), json.images[2]);

Lastly, what's going on the element that shows the savings? If there's no old price, it's just hidden, which makes sense. But if there is an old price, you update the .p_save element... and then hide it anyway?


Showing animated company KPI stats I am creating a web page in which I have 4 different numbers which show companies stats. I am using javascript to dynamically show these numbers from 0 to n (where 0 is the starting point of counter and n is the end, I have n=74, n=86, n=285, n =97). I am using javascript to start the counter from 0 to n, everything is working fine, as I am just a beginner in programming so I just want to know how experienced programmers think of this code. is this a good approach for this problem? Can the algorithm efficiency be improved?

//I am invoking this function on scroll but here I have provided the button

function letsCount(){
  var num = document.getElementsByClassName('number');
  var count = 0;
  var id = setInterval(myFunction,1);
  var val = document.getElementById('value');
  function myFunction(){
    if(count == 286 ){ //stop the counter at this value
      clearInterval(id);
    }
    else if(count < 76) { //First (Smallest) value of n
      num[0].innerHTML = count;
      num[1].innerHTML = count;
      num[2].innerHTML = count;
      num[3].innerHTML = count;
      count++;
    }
    else if(count < 87){ // second value of n
      num[1].innerHTML = count;
      num[2].innerHTML = count;
      num[3].innerHTML = count;
      count++;
    }
    else if(count < 98){ // third value of n
      num[2].innerHTML = count;
      num[3].innerHTML = count;
      count++;
    }
    else if(count < 286){ // fourth value of n
      num[2].innerHTML = count;
      count++;
    }
  }
}
button {
   position: absolute;
   top: 30px;
   left: auto;
   right: 30px;
   border: none;
   background-color: black;
   color: white;
   padding: 10px;
   cursor: pointer;
  }
button:hover {
   background-color: crimson;
}

   0

Styles Created

   0

Awards Won

   0

Stores Worldwide

   0

Varities Desinged

 <button onclick = "letsCount()">START</button>

Keep the global scope clean and data/code segregation

First I will say your code is good for a beginner but you are missing some concepts and techniques that will help you improve your code and improve the client experience.

Some important concepts

Use requestAnimationFrame for animations, and know the difference between animation and one off content changes.
Data belongs in the page. Be it the text, images, or the instructions/values for the javascript, the place for data is on the page.
Keep the global namespace clean. With a little effort you can completely avoid adding any variables to the window namespace / global scope.
Don't put code in the HTML It is tempting to put code in the page to start an animation <button onclick="startAnim()" value="start">. All good when it's a simple page but a source of headache, bugs, and unhappy clients when its get complicated
When parts of your code starts to look repetitive it's a good sign that there is a better way.
Remove event handlers when you are done. Event handlers use up resources.
Always keep in mind your page shares the computer with dozens of others on the client's machine. Do your best to reduce the total load on their device.

The javascript

Always start your code with "use strict" it helps you avoid some very common bugs and bad coding styles and runs your code significantly faster.

"use strict"; // is valid as the first line of a script tag or the first
              // line inside a function. Put anywhere else and it has no effect
window.addEventListener("load",function(){  // starting your code on the load
                                           // event ensures that all the DOM
                                           // tree is populated and ready

try to define your event handlers as named functions because you need a function referance to remove the event if you need to.

    function startButtonClick(){  // the start button event

We only want this to play once. So remove the event from the button. When events are called this references the element that triggered the event

        this.removeEventListener("click",startButtonClick); // remove event

The spread operator is a short cut way of turning a array like list into an array. Be wary as IE 11 is still popular and does not support many ES6 features. If you use things like the spread operator you should consider using babel.js so that your code can run on legacy browsers.

        var numberEl = [...document.getElementsByClassName('number')];

The numbers moving up is an animation. It will be control by requestAnimationFrame that will call the animate function at 60fps if possible, but that is not a guarantee. To ensure you get consistent animation on all devices use the supplied time the requestAnimationFrame callback supplies to time your animation.

You were using a interval of 1ms.The rule of thumb is 60fps or 16ms between display frames, updating content any higher than that rate will simply not be seen and thus will just waste resources.

        var startTime;  // holds that animation start time
        function animateNumbers(time){ // time is the first argument of the
                                       // requestAnimationFrame callback

This countSpeed controls how fast the numbers will increase per second. The animation will play at the same speed on all devices.

            const countSpeed = 120; // count speed in increments per second
            if(startTime === undefined){// if there is no start time then this must
                                        // be first frame so make it the start time
                startTime = time;
            }
            var seconds = (time - startTime) / 1000; //seconds from animation start
            var count = countSpeed * seconds; // get the current count
            done = true; // flag to indicate the animation is complete
            numberEl.forEach(element => {
                var max = Number(element.dataset.maxValue); // properties that you
                                                // get from the  page are strings.
                                                // If you know it's a  number let
                                                // Javascript know

If the count is below max for any element flag the animation as not done.

                if(count < max){
                    done = false;
                }

Try to avoid setting text content with innerHTML as it can cause all sorts of problems that are avoided by telling the DOM it is only text you are adding

                element.textContent = Math.min(max,Math.floor(count));
            }
            if(!done){  // continue animation until done
                requestAnimationFrame(animateNumbers);
            }
        }

When ever you are animating anything on the page were you define every frame use requestAnimationFrame it ensures that the animation is in sync with the hardware and any other page rendering. It also prevents updated content from being presented to early or quicker than can actualy be seen. Smooth animation is not about speed, but about being in sync and limiting the amount of work the computer must do.

        requestAnimationFrame(animateNumbers);
    }

Get the button element and add the event listener

    document.querySelector("#startButton").addEventListener("click", startButtonClick);
})

The HTML, data lives here

Placing the values for the counters inside the javascript code makes it hard to manage. The javascript is a constant, the page contains the data and is unique per view/client

The formal way to add data/attributes to elements is via the data- prefix though this is not enforced and any named value pair you add to an element can be accessed, data- ensures your data is safe from conflict or misinterpreted by the DOM.

data-max-value is not a valid javascript variable name so you can not easily acces the data using that name you need to use the javascript version created by the DOM. In HTML5 the DOM creates an attribute called dataset that you use to access the variable with. Replace data- with dataset. (that's a dot not a full stop)  then the max, then replacing any subsequent '-' by removing the - and capitalising the first letter following, thus data-max-value becomes dataset.maxValue see the code above as an example.

   0

Styles Created

   0

Awards Won

   0

Stores Worldwide

   0

Varities Desinged

Avoid placing javascrpt inside the HTML content. It makes it much harder to manage your code. Use an unquie Id to identify elements for direct or queried access from within your javascript.

<button id = "startButton" >START</button>

Put it all together

"use strict";
window.addEventListener("load",function(){
    function startButtonClick(){
        this.removeEventListener("click",startButtonClick); // remove event
        var numberEl = [...document.getElementsByClassName('number')];
        var startTime;
        function animateNumbers(time){
            const countSpeed = 120;
            if(startTime === undefined){
                startTime = time;
            }
            var seconds = (time - startTime) / 1000; // get the seconds since the animation start
            var count = countSpeed * seconds; // get the current count
            var done = true;
            numberEl.forEach(element => {
                var max = Number(element.dataset.maxValue);
                if(count < max) {
                    done = false;
                }
                element.textContent = Math.min(max,Math.floor(count));
            });
            if(!done){
                requestAnimationFrame(animateNumbers);
            }
        }
        requestAnimationFrame(animateNumbers);
    }
    document.querySelector("#startButton").addEventListener("click", startButtonClick);
});
button {
   position: absolute;
   top: 30px;
   left: auto;
   right: 30px;
   border: none;
   background-color: black;
   color: white;
   padding: 10px;
   cursor: pointer;
}
button:hover {
   background-color: crimson;
}

   0

Styles Created

   0

Awards Won

   0

Stores Worldwide

   0

Varities Desinged

<button id = "startButton" >START</button>


Implementation of enumerate I'm trying to implement the enumerate built-in using iterators. So far it works but for unordered objects, such as sets or dictionaries, I'm not sure if this implementation is considered valid, although it works.

class myEnumerate:
    def __init__(self, iterable):
        self.iterable = list(iterable)
        self.index = -1

    def __iter__(self):
        return self

    def __next__(self):
        self.index += 1
        if self.index >= len(self.iterable):
            raise StopIteration
        return self.index, self.iterable.__getitem__(self.index)

if __name__ == '__main__':
    for (i, ch) in myEnumerate("Python"):
        print(i, ch)

    # Output:
    # 0 P
    # 1 y
    # 2 t
    # 3 h
    # 4 o
    # 5 n

    for (i, ch) in myEnumerate(set(["a", "b", "c"])):
        print(i, ch)

    # Output: (order might differ)
    # 0 a
    # 1 b
    # 2 c

What do you guys think?

I think this is mistake:

self.iterable = list(iterable)

It requires loading the entire iterable into memory. Being able to avoid this and work with large collections efficiently is one of the big benefits of iterators.
It won't work on non-ending iterators like itertools.count()

If instead, you make an iterator from the iterable, you can avoid both those problems and simplify your code by depending on the iterator's own StopIteration exception:

class myEnumerate:
    def __init__(self, iterable):
        self.iterable = iter(iterable)
        self.index = -1

    def __iter__(self):
        return self

    def __next__(self):
        self.index += 1
        return self.index, next(self.iterable)

// this now works:

from itertools import count, islice
c = count()
for (i, ch) in islice(myEnumerate(count()), 1, 10):
    print(i, ch)


Pretty print uptime given seconds and omit unnecessary strings Given an integer number of seconds, return a string which represents in human readable format the number of days/hours/minutes/seconds that are equivalent.

from datetime import datetime, timedelta

def GetTime(sec):
    d = datetime(1,1,1) + timedelta(seconds=sec)
    days = (d.day-1,'d',' ') if (d.day-1 > 0) else ('','','')
    hours = (d.hour, 'h', ' ') if (d.hour > 0) else ('', '', '')
    mins = (d.minute, 'm', ' ') if (d.minute > 0) else ('', '', '')
    secs = (d.second, 's', ' ') if (d.second > 0) else ('', '', '')
    time_format = days + hours + mins + secs
    format_str = '{}'*len(time_format)

    return format_str.format(*time_format)

I think there might be a better way to do the formatting to omit values that are zero instead of combining tuples and exploding them in the format string.

I feel like the datetime module isn't really doing much to help you.  By using it, you avoid performing arithmetic and hard-coding constants 86400, 3600, and 60.  However, in exchange, you end up hard-coding the accessors .day, .hour, .minute, and .second, in such a way that you have to write similar code four times, with no easy way to generalize them.  Attempts at generalization are further complicated by the fact that you need d.day-1, due to the way that you have abused datetime addition as a trick to convert the timedelta object into a datetime object.

Your output includes a trailing space, which I consider to be an annoyance.  One way to avoid that would be to use ' '.join(…).  On the other hand, if sec is 0, then the function would return an empty string, which is also unintuitive.  I would prefer 0s as output in that special case.

In my opinion, you would be no worse off doing all of the arithmetic and formatting yourself, without the use of the datetime module.

def format_seconds(sec):
    def repeatedly(f, xs, n):
        for x in xs:
            m, n = f(n, x)
            yield m
    return ' '.join(
        '{}{}'.format(n, unit)
        for n, unit in zip(
            repeatedly(divmod, [86400, 3600, 60, 1], sec),
            'dhms'
        )
        if n
    ) or '0s'