Matrix class with overloaded `operator[]` I have implemented a Matrix class that allocates memory using unique_ptr. How to make it more diverse and optimized in terms of memory usage and efficiency?

//
// Created by Mayukh Sarkar on 26/12/19.
//

#ifndef DP2_MATRIX_H
#define DP2_MATRIX_H

#include <cstdio>
#include <memory>
#include <algorithm>

template <typename T>
class Matrix {
private:
    size_t row{};
    size_t col{};
    std::unique_ptr<T[]> data;
public:
    explicit Matrix(size_t row, size_t col, T def) {
        // Create a matrix of row X col and initialize
        // each element of the matrix with def
        this->row = row;
        this->col = col;
        this->data = std::make_unique<T[]>(this->row * this->col);
        std::fill_n(data.get(), row*col, def);
    }

    // Overload the [] operator for the 2d array like access
    T* operator[](int r) {
        if (r >= row) {
            throw std::out_of_range("Can not access out of bound element!");
        }
        return &this->data[r*col];
    }

    // Overload the << operator for console logging
    friend std::ostream& operator<< (std::ostream& os, Matrix &mObj) {
        auto shape = mObj.shape();
        for(int i=0; i< shape.first; i++) {
            for(int j=0; j<shape.second; j++) {
                os << mObj[i][j] << " ";
            }
            os << "\n";
        }
        return os;
    }

    // Set all the values of the Matrix
    void setValues(T value) {
        std::fill_n(data.get(), row*col, value);
    }

    // Get row and col values
    std::pair<size_t, size_t> shape() const {
        return std::make_pair(this->row, this->col);
    }
};

#endif //DP2_MATRIX_H

I want to specifically optimize the []operator overloading because I feel it is not good to handle pointers directly. Any suggestions?

Pragma once

Instead of

#ifndef DP2_MATRIX_H
#define DP2_MATRIX_H

consider using

#pragma once

Once is non-standard, but supported by all compilers supporting
C++14 and reduces compilation times.

Includes

On MSVC, #include <stdexcept> is needed for the out_of_range
error. I also think your include lines look tidier if you sort them in
alphabetical order. :)

Bounds checking

Matrix<int>(10, 10, 0)[10][0]

Is an error that will be detected by your bounds checking, but

Matrix<int>(10, 10, 0)[0][10]

is the same type of error, but won't be detected. Consider following
the STL convention described in this Stackoverflow
answer. That
is, you remove bounds checking from operator[] but add a method at
which does bounds checking for both dimensions.

Const correctness

You should add const declared variants of the accessors. You can
also add it to the parameters, but imho, that is not as useful.

Result:

#pragma once

#include <algorithm>
#include <cstdio>
#include <memory>
#include <stdexcept>

template <typename T>
class Matrix {
private:
    size_t row{};
    size_t col{};
    std::unique_ptr<T[]> data;
    void boundscheck(int r, int c) const {
        if (!(0 <= r && r < row && 0 <= c && c < col)) {
            throw std::out_of_range("Can not access out of bound element!");
        }
    }
public:
    // Set all the values of the Matrix
    void setValues(T value) {
        std::fill_n(data.get(), row*col, value);
    }
    explicit Matrix(size_t row, size_t col, T def) {
        // Create a matrix of row X col and initialize
        // each element of the matrix with def
        this->row = row;
        this->col = col;
        this->data = std::make_unique<T[]>(this->row * this->col);
        setValues(def);
    }
    // Overload the [] operator for the 2d array like access
    T* operator[](int r) {
        return &this->data[r * col];
    }
    const T* operator[](int r) const {
        return &this->data[r * col];
    }
    T& at(int r, int c) {
        boundscheck(r, c);
        return this->data[r * col + c];
    }
    const T& at(int r, int c) const {
        boundscheck(r, c);
        return this->data[r * col + c];
    }
    // Overload the << operator for console logging
    friend std::ostream& operator<< (std::ostream& os, Matrix &mObj) {
        auto shape = mObj.shape();
        for(int i=0; i< shape.first; i++) {
            for(int j=0; j<shape.second; j++) {
                os << mObj[i][j] << " ";
            }
            os << "\n";
        }
        return os;
    }
    // Get row and col values
    std::pair<size_t, size_t> shape() const {
        return std::make_pair(this->row, this->col);
    }
};

I don't think it is possible to make a general matrix class consume less memory or run quicker than that. Compilers generate pretty good code for the accessor functions. You probably have to use a different data type such as a sparse matrix to improve performance much.


Implement C++11 multipart sending without copying to a single buffer I am working on a C++11 project and am migrating the underlying communications library used.

The original library allowed sending messages in multiple parts using a SNDMORE flag:

socket_->send(str_1, SNDMORE);
socket_->send(str_2, SNDMORE);
socket_->send(str_3); // The last message

and receiving them in multiple parts as well:

socket_->recv(&msg);
... // do stuff
while(msg.more()) {
    socket_->recv(msg);
... // do stuff
}

The new library does not have this functionality, so I ended up creating a struct that merges upto 3 strings and then merges and sends, receives and splits them as required.

/**
*
* Created helper methods to merge 3 strings into 1 and reverse
* This is a helper to group 3 messages in a single string.
*
**/

struct MultipartSplitMerge {
   #define PADDING 8
   static std::string merge(std::string *s1, std::string *s2, std::string *s3) {
       std::string size_1 = std::to_string((*s1).size()); std::string size_2 = std::to_string((*s2).size());
       std::string padded_size_1 = std::string(PADDING - size_1.length(), '0') + size_1;
       std::string padded_size_2 = std::string(PADDING - size_2.length(), '0') + size_2;
       std::string ret_str(padded_size_1 + padded_size_2 + *s1 + *s2 + *s3);
       return ret_str; // Add the size of each piece
   }

   static void split(std::string *s, std::string *o1, std::string *o2, std::string *o3) {
       unsigned int size_1 = std::stoi((*s).substr (0, PADDING));
       unsigned int size_2 = std::stoi((*s).substr (PADDING, PADDING));
       *o1 = (*s).substr(2 * PADDING, size_1);
       *o2 = (*s).substr(2 * PADDING + size_1, size_2);
       *o3 = (*s).substr(2 * PADDING + size_1 + size_2);
   }
};

Using this splitter/merger, I now send as follows:

std::string nullstr(""); // I use this to leave one of the parts empty
socket_->send(MultipartSplitMerge::merge(&h_array, &msg, &nullstr));

The receive code now works like this:

std::string combined, temp_1, temp_2, temp_3;
socket_->receive(&combined);
MultipartSplitMerge::split(&combined, &temp_1, &temp_2, &temp_3);

I'd like to avoid the unnecessary copies if possible, and also general guidelines on improving this code.

Here's the REPL to play with (or go to source):

<script src="//repl.it/embed/MFW8/0.js"></script>

Currently, we assume that there are 3 or less splits in every communication.

Use namespace if you want a namespace

You use your struct only as a way to have merge and split not in the global namespace (and inlined). That's not necessary. We can just use a namespace:

namespace MultipartSplitMerge {
    ...
}

Don't use define for compile-time constants

You define PADDING, but you never undefine it. It's a common word in code, so it's possible that you accidentally break something. Furthermore, it's not in the same namespace as merge and split, as the preprocessor does not care about namespaces.

Instead, use a static constexpr size_t:

static constexpr size_t PADDING = 8;

This has the nice side-effect that PADDING has now a type. The compiler can now warn you when you try to compare it to other, signed types.

Prefer references instead of pointers

Not only are they easier to use, they also are harder to get wrong.

Use const on call-by-reference/-pointer arguments that are not changed

You never change s1, s2 and s3, yet you didn't use const. We could have changed their values accidentally. A const will prevent this:

inline std::string merge(const std::string& s1, const std::string& s2, const std::string& s3) {
    const std::string size_1 = std::to_string(s1.size());
    const std::string size_2 = std::to_string(s2.size());
    const std::string padded_size_1 = std::string(PADDING - size_1.length(), '0') + size_1;
    const std::string padded_size_2 = std::string(PADDING - size_2.length(), '0') + size_2;
    const std::string ret_str(padded_size_1 + padded_size_2 + s1 + s2 + s3);
    return ret_str; // Add the size of each piece
}

inline void split(const std::string& s, std::string &o1, std::string &o2, std::string &o3) {
    const unsigned int size_1 = std::stoi(s.substr(0, PADDING));
    const unsigned int size_2 = std::stoi(s.substr(PADDING, PADDING));
    o1 = s.substr(2 * PADDING, size_1);
    o2 = s.substr(2 * PADDING + size_1, size_2);
    o3 = s.substr(2 * PADDING + size_1 + size_2);
}

Note that all this inline annotations are only necessary if your functions will be in a header.

Don't repeat yourself

You pad your string, but it's a) hard to get and b) repetitive. Write a function to get rid of the duplicate logic:

inline std::string pad(size_t size, char padding, const std::string& original) {
    assert(size >= original.size());
    return std::string(size - original.size(), padding) + original;
}

inline std::string pad_size(const std::string& original) {
    return pad(PADDING, '0', std::to_string(original.size()));
}

If you find an optimization for pad or pad_size, merge gets optimized for free. And it gets a lot shorter:

inline std::string merge(const std::string& s1, const std::string& s2, const std::string& s3) {
    return pad_size(s1) + pad_size(s2) + s1 + s2 + s3;
}

All code at once

Now let us inspect the reviewed and somewhat rewritten code:

namespace MultipartSplitMerge {
static constexpr size_t FIXED_FIELD_LENGTH = 8;

// Optional `detail' namespace; shows a user that `pad' and `pad_size'
// are implementation details. Only important in a header-only
// library
namespace detail {
inline std::string pad(size_t size, char padding, const std::string &original) {
    assert(size >= original.size());
    return std::string(size - original.size(), padding) + original;
}

inline std::string pad_size(const std::string & original) {
    return pad(FIXED_FIELD_LENGTH, '0', std::to_string(original.size()));
}
} // namespace detail

inline std::string merge(const std::string& s1, const std::string& s2, const std::string& s3) {
    return detail::pad_size(s1) + detail::pad_size(s2) + s1 + s2 + s3;
}

inline void split(const std::string& s, std::string& o1, std::string& o2, std::string& o3) {
    const unsigned int size_1 = std::stoi(s.substr(0, FIXED_FIELD_LENGTH));
    const unsigned int size_2 = std::stoi(s.substr(FIXED_FIELD_LENGTH, FIXED_FIELD_LENGTH));
    o1 = s.substr(2 * FIXED_FIELD_LENGTH, size_1);
    o2 = s.substr(2 * FIXED_FIELD_LENGTH + size_1, size_2);
    o3 = s.substr(2 * FIXED_FIELD_LENGTH + size_1 + size_2);
}
} // namespace MultipartSplitMerge

At this point I wouldn't change the code anymore, unless I see via profiling that split or merge is a bottle-neck.

If you really notice that merge is a bottle-neck, you can use sprintf, but I guess that the compiler should optimize above code.


Java reflection and static classes I try to intercept some OpenGL calls for testing my rendering classes. Reflection is used for replacing the OpenGL backend.

I feel this class is badly written and I need advices for refactoring it.

package fr.meuns.opengl;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.nio.FloatBuffer;

public class GL
{
  private static class MethodSignature
  {
    String name;
    Class< ? >[] parameters;

    public MethodSignature( String name, Class< ? >... parameters )
    {
      this.name = name;
      this.parameters = parameters;
    }
  }

  private final static MethodSignature[] METHOD_SIGNATURES = new MethodSignature[] {
    new MethodSignature( "glGenBuffers" ),
    new MethodSignature( "glDeleteBuffers", int.class ),
    new MethodSignature( "glBindBuffer", int.class, int.class ),
    new MethodSignature( "glBufferData", int.class, FloatBuffer.class, int.class ),
    new MethodSignature( "glGetBufferSubData", int.class, long.class, FloatBuffer.class )
    ...
  };

  private final static String[] CONSTANT_NAMES = new String[] {
    "GL_ARRAY_BUFFER",
    "GL_ELEMENT_ARRAY_BUFFER",
    "GL_STATIC_DRAW"
    ...
  };

  public static int GL_ARRAY_BUFFER;
  public static int GL_ELEMENT_ARRAY_BUFFER;
  public static int GL_STATIC_DRAW;
  ...

  public static Method _glGenBuffers;
  public static Method _glDeleteBuffers;
  public static Method _glBindBuffer;
  public static Method _glBufferData;
  public static Method _glGetBufferSubData;
  ...

  public static int glGenBuffers()
  {
    try
    {
      return (int)_glGenBuffers.invoke( null );
    }
    catch( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
    {
      e.printStackTrace();
      return 0;
    }
  }

  public static void glDeleteBuffers( int buffer )
  {
    try
    {
      _glDeleteBuffers.invoke( null, buffer );
    }
    catch( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
    {
      e.printStackTrace();
    }
  }

  public static void glBindBuffer( int target, int buffer )
  {
    try
    {
      _glBindBuffer.invoke( null, target, buffer );
    }
    catch( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
    {
      e.printStackTrace();
    }
  }

  public static void glBufferData( int target, FloatBuffer data, int usage )
  {
    try
    {
      _glBufferData.invoke( null, target, data, usage );
    }
    catch( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
    {
      e.printStackTrace();
    }
  }

  public static void glGetBufferSubData( int target, long offset, FloatBuffer data )
  {
    try
    {
      _glGetBufferSubData.invoke( null, target, offset, data );
    }
    catch( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
    {
      e.printStackTrace();
    }
  }

  public ...

  public static void useClass( Class< ? > glClass )
  {
    for( String name : CONSTANT_NAMES )
      try
      {
        GL.class
          .getField( name )
          .set( null, glClass.getField( name ).get( null ) );
      }
      catch( IllegalArgumentException | IllegalAccessException | NoSuchFieldException | SecurityException e1 )
      {
        e1.printStackTrace();
      }

    for( MethodSignature signature : METHOD_SIGNATURES )
      try
      {
        GL.class
          .getField( "_" + signature.name )
          .set( null, glClass.getDeclaredMethod( signature.name, signature.parameters ) );
      }
      catch( IllegalArgumentException | IllegalAccessException | NoSuchFieldException | SecurityException | NoSuchMethodException e )
      {
        e.printStackTrace();
      }
  }
}

Initialization for rendering using LWJGL :

import org.lwjgl.opengl.GL11;
import org.lwjgl.opengl.GL15;
import org.lwjgl.opengl.GL20;
...

GL.useClass( GL11.class );
GL.useClass( GL15.class );
GL.useClass( GL20.class );
...

Initialization for testing :

import fr.meuns.opengl.GLMock;
...

GL.useClass( GLMock.class );
...

public class GL

Your class name should really be a more descriptive noun.

private static class MethodSignature

You actually have no need for this private static class since Reflection gives you direct access to methods and method signatures, as you'll see in my modified version of your code below.

String name;
Class<?>[] parameters;

When you create a class, you should always explicitly declare whether a field is public, protected, or private.

private final static MethodSignature[] METHOD_SIGNATURES = new MethodSignature[] { //...
private final static String[] CONSTANT_NAMES = new String[] { //...

Again, you don't need either of these since you have Reflection, which gives you access to methods and fields.

public static int GL_ARRAY_BUFFER;
public static int GL_ELEMENT_ARRAY_BUFFER;
public static int GL_STATIC_DRAW;

public static Method _glGenBuffers;
public static Method _glDeleteBuffers;
public static Method _glBindBuffer;
public static Method _glBufferData;
public static Method _glGetBufferSubData;

These are all very odd variable names.  In general, NAMES_IN_CAPS are constants (private static final).  The beginning underscore is also a paradigm that isn't really Java-esque (and to my knowledge in other languages it is generally used for private-like fields).

public static int glGenBuffers() {

Your method names are also oddly named for Java.  In general, Java method names should be actions and verb phrases.  (For example, getGLGenBuffers())

catch(IllegalAccessException | IllegalArgumentException | InvocationTargetException e)

No need for these lengthy catch declarations since they all do the same thing and all you're doing is printing the stack trace.  catch(Exception e) will do and make your code more sensible.

public static void useClass(Class<?> glClass)

This entire method will be refactored below.  But basically you don't need two for() loops, since you're just looping through the class's fields.  This also eliminates the need for the overarching constants for the field names and the method signatures, as you'll see.

I cut your code length by about half by doing all of the above, and I think it's generally more readable and understandable this way as well.

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.nio.FloatBuffer;

public class OpenGLInterceptor {

    public static int GL_ARRAY_BUFFER;
    public static int GL_ELEMENT_ARRAY_BUFFER;
    public static int GL_STATIC_DRAW;
    //...

    public static Method _glGenBuffers;
    public static Method _glDeleteBuffers;
    public static Method _glBindBuffer;
    public static Method _glBufferData;
    public static Method _glGetBufferSubData;
    //...

    public static int invokeGenBuffers() {
        try {
            return (Integer)_glGenBuffers.invoke(null);
        }
        catch(Exception e) {
            e.printStackTrace();
            return 0;
        }
    }

    public static void invokeDeleteBuffers(int buffer) {
        try {
            _glDeleteBuffers.invoke(null, buffer);
        }
        catch(Exception e){
            e.printStackTrace();
        }
    }

    public static void invokeBindBuffer(int target, int buffer) {
        try {
          _glBindBuffer.invoke( null, target, buffer );
        }
        catch(Exception e) {
          e.printStackTrace();
        }
    }

    public static void invokeBufferData(int target, FloatBuffer data, int usage) {
        try {
            _glBufferData.invoke(null, target, data, usage);
        }
        catch(Exception e) {
            e.printStackTrace();
        }
    }

    public static void invokeGetBufferSubData(int target, long offset, FloatBuffer data) {
        try {
            _glGetBufferSubData.invoke( null, target, offset, data );
        }
        catch(Exception e) {
            e.printStackTrace();
        }
    }

    public static void useClass(Class<?> glClass) {
        for(Field field : GL.class.getDeclaredFields()) {
            try {
                if(field.getName().contains("_")) {
                    field.set(null, glClass.getDeclaredMethod(field.getName(), ((Method)field.get(null)).getParameterTypes()));
                }
                else {
                    field.set(null, glClass.getField(field.getName()).get(null));
                }
            }
            catch(Exception e) {
                e.printStackTrace();
            }
        }
    }

}


Four-lines script to make an immediate DB and document root zip backup What do you think of my script to make an immediate DB and document root zip backup? Any flaw you might find. Please inform me of it:

#!/bin/bash
mysqldump -u root -p --all-databases | zip ${drt}/db-$(date +\%F-\%T).zip
zip -r all_zipped-$(date +\%F-\%T).zip ${drt}/ -x "*/cache/*"
rm -rf  ${drt}/db-$(date +\%F-\%T).zip

It is important to me to have the backup in document root because of comfortability. It's a small server environment (with external automatic backups) and I think it'll be redundant to have a backup dir just for once-in-a-while immediate backups I do with that nice script above.

Bug?

It looks to me that the name of the zip file used in the first and the last command were intended to be the same, that is the ${drt}/db-$(date +\%F-\%T).zip part:

mysqldump ... | zip ${drt}/db-$(date +\%F-\%T).zip
...
rm -rf  ${drt}/db-$(date +\%F-\%T).zip

They will only be the same if the mysqldump and rm commands run within the same second.

Don't repeat yourself

If a non-trivial command is expected to return the same value every time you run it, then extract it to a variable.
Not only to save unnecessary processing power,
but also to keep the non-trivial logic in one place,
so that it's easy to change if ever needed, in one place.

Double-quote command arguments

It's a good rule of thumb to double-quote command arguments when they contain variables,
to protect yourself from unintended word splitting and glob expansions.

Useless flags

In rm -rf  ${drt}/db-$(date +\%F-\%T).zip,
since we're deleting a file, not a directory,
the -r flag is useless.
Avoid useless flags.

Alternative solution

I suggest to write like this:

#!/bin/bash

date=$(date +\%F-\%T)

mysqldump -u root -p --all-databases | zip "${drt}/db-$date.zip"
zip -r "all_zipped-$date.zip" "${drt}"/ -x "*/cache/*"
rm -f "${drt}/db-$date.zip"


Looping through lines of floats/int I'm new to programming and started with some tutorials about C++ on YouTube. Now I was wondering how I should continue solving below. It does work but already it has been like a lot of code to write. And I have more linetypes to define. So hope someone can point out a better way

std::string filepath = "path/to/file.ext";
std::ifstream stream(filepath);
std::string line;
int i = 1; //For debug purpose
while (getline(stream, line))
{
    if (line!= "" && line.at(0) == '0')
    {
        std::cout << "Line "<< i << " type 0: " << line << std::endl;
    }
    else if (line != "" && line.at(0) == '1')
    {
        std::cout <<  "Line " << i << " type 1: " << line << std::endl;
    }
    else if (line != "" && line.at(0) == '2')
    {
        std::cout << "Line " << i << " type 2: " << line << std::endl;
    }
    else if (line != "" && line.at(0) == '3')
    {
        std::cout << "Line " << i << " type 3: " << line << std::endl;
    }
    else if (line != "" && line.at(0) == '4')
    {
        //Example string: 4 16 -40 0 -20 -40 24 -20 40 24 -20 40 0 -20
        std::cout << "Line " << i << " type 4: " << line << std::endl;
        std::stringstream iss{ LDrawLine };

        float lineType{}, color{}, x1{}, y1{}, z1{}, x2{}, y2{}, z2{}, x3{}, y3{}, z3{}, x4{}, y4{}, z4{};
        iss >> lineType >> color >> x1 >> y1 >> z1 >> x2 >> y2 >> z2 >> x3 >> y3 >> z3 >> x4 >> y4 >> z4;
        //What I could do is make a line like below for every coordinate. The color and the lineType need to be handled different
        //The positions is passed in to the function. Afterwords i need to draw the quads.
        (*positions).push_back(  0.0f); //so else i need to write this code 16 times for this linetype and still have some other line types to go
    }
    else if (line != "" && line.at(0) == '5')
    {
        std::cout << "Line " << i << " type 5: " << line << std::endl;
    }
    else
    {
        std::cout << "Line " << i << " No type" << std::endl;
    }
    i++;
}

If anyone could give me some advice on how to handle this or a tutorial which can help me on this. The code does what it needs to do. I only want it more elegant and why less to type.

Break some of the functionality into functions. For instance, notice that all line types except for 4 behave similarly. So how about you do:

bool isSimpleType(char c) {
    return (c >= '0' && c <= '3') || (c == '5');
}

With this, you can now simplify your main loop:

for(int i = 1; std::getline(stream, line); ++i)
{
    if (line == "")
        continue;

    const char type = line.front();

    if (isSimpleType(type)) {
        std::cout << "Line " << i << " type " << type << ": " << line << "\n";
    }
    else if (type == '4') {
        // Omitted to reduce clutter
    }
    else {
        std::cout << "Line " << i << " No type" << std::endl;
    }
}

Finally, let us proceed along similar lines to simplify the case of '4'. I'll let you take over from here, but read the inputs to a vector and pass that onto a function that is responsible for verifying if the line is OK and constructing a suitable object from the data then.

Also, instead of relying on "naked values", you could also consider using a class enum Type { ... } to define the different line types. I recommend you read more about enums for this.


Rendering elements (which can change numbers) using jQuery Currently when the user selects some tags in my app I generate them in a function, like this:

// Show tags that are currently used in search
var htmlNodes = '
';

// If no pinned nodes, reset not found nodes
if (graphFactory.existPinnedNodes() == false) {
  graphFactory.updateNotFoundNodes();
}

// Add not found nodes as tags (gray)
for (let x = 0; x < graphFactory.getNotFoundNodes().length; x++) {
  htmlNodes = htmlNodes + '
' + graphFactory.getNotFoundNodes()[x] + '
';
}

// Add pinned nodes as tags (blue)
for (let q = 0; q < graphFactory.getPinnedNodes().length; q++) {
  htmlNodes = htmlNodes + '
' + graphFactory.getPinnedNodes()[q] + '
';
}

htmlNodes = htmlNodes + '

';
$('#pinnednodeslist').html(htmlNodes);

Basically I create a
 of class tags, add some li elements with a specific tag each, and then render it into the #pinnednodeslist element.

As you can see, the number of
 elements might change, depending on the number of getPinnedNodes or getNotFoundNodes, so I wonder if I have to reconstruct this html every time or if there is a more efficient way to do that...

Maybe a better way is that I don't have to mix so much HTML with my code?

For instance, I was thinking to leave all the elements there and just make them invisible, simply adding a class to show them and the html content to display within... But maybe you know of some other better way to do that?

First off: It's invalid HTML to have a div directly inside an ul element. Any special reason you need the

? Modern CSS techniques usually don't need additional elements like that.

You shouldn't repeatedly call the getter methods in a loop like that. If they do more work than just returning an array, then it's wasted performance.

let notFoundNodes = graphFactory.getNotFoundNodes();
for (let x = 0; x < notFoundNodes.length; x++) {
  htmlNodes = htmlNodes + '
' + notFoundNodes[x] + '
';
}

Or, considering you are using let which is part of ECMAScript 2015, you could alternatively use a for ... of loop:

for (let notFoundNode of graphFactory.getNotFoundNodes()) {
  htmlNodes = htmlNodes + '
' + notFoundNode + '
';
}

Concatenating strings in loops are also quite a wasteful operation, because they cannot be optimized by the runtime environment. A better way is to collect the strings in an array, and then joining them together at the end:

var htmlNodes = ['
'];

// ...

for (let notFoundNode of graphFactory.getNotFoundNodes()) {
  htmlNodes.push('
' + notFoundNode + '
');
}

// ...

htmlNodes.push('
');
$('#pinnednodeslist').html(htmlNode.join(''));

In order to avoid mixing the HTML you could use a template engine. There are literally dozens out there, with many pros and cons, which can make the choice a bit overwhelming. For an example I'll use Handlebars.js:

The template is placed inside a script element somewhere in your HTML:

Once during page initialization you compile the template:

var source = document.getElementById("tags-template").innerHTML;
var tagsTemplate = Handlebars.compile(source);

And then later you can repeatedly use the compiled template to render the data:

var context = {
   notFoundNodes: graphFactory.getNotFoundNodes(),
   pinnedNodes: graphFactory.getPinnedNodes()
};
var html = tagsTemplate(context);
$('#pinnednodeslist').html(html);

EDIT: BTW, nothing in this (or your original code) really requires jQuery. The last line could simply replaced with:

document.getElementById('pinnednodeslist').innerHtml = html;

The next step could be to use more advanced library such as React, Vue.js or Angular. These are frameworks that not only render the data with a template engine, but also can (among other things) react to changes in the data and re-render automatically when even needed. This however requires learning a bit of a different way of programming.

Finally, if the data displayed is coming from the server (via AJAX for example), then you could consider having the server render the HTML instead of sending the raw data.


Remove all adjacent duplicates For this problem, I came up with this ugly solution of appending the characters to the output and in case there is adjacent duplicate deleting from the output. Considering StringBuilder.deleteCharAt(i) is O(N), performance is O(N) + O(N) = O(N).

Please critique this, if this is a right way.

public static String removeDuplicate(String s) {
StringBuilder builder = new StringBuilder();
char lastchar = '\0';
for (int i = 0; i < s.length(); i++) {
  String str = builder.toString();
  if (!str.equals("")
      && (str.charAt(str.length() - 1) == s.charAt(i))) {
    builder.deleteCharAt(str.length() - 1);
  } else if (s.charAt(i) != lastchar)
    builder.append(s.charAt(i));
  lastchar = s.charAt(i);
}
return builder.toString();
}

Sample input outputs:

  Input:  azxxzy
  Output: ay

  Input: caaabbbaacdddd
  Output: Empty String

  Input: acaaabbbacdddd
  Output: acac

General
Your code formatting is all over the place. You should use consistent and structured indentation. See the Java Code-Style guide for, well, guidance.
if I take your code, and re-format it (Eclipse, CtrlA and CtrlshiftF), it looks like:

public static String removeDuplicate(String s) {
    StringBuilder builder = new StringBuilder();
    char lastchar = '\0';
    for (int i = 0; i < s.length(); i++) {
        String str = builder.toString();
        if (!str.equals("") && (str.charAt(str.length() - 1) == s.charAt(i))) {
            builder.deleteCharAt(str.length() - 1);
        } else if (s.charAt(i) != lastchar)
            builder.append(s.charAt(i));
        lastchar = s.charAt(i);
    }
    return builder.toString();
}

At least this allows us to see what you are doing.
Algorithm
A nice algorithm will take data from the input string, and add it to the output if it should be added. A system where you do multiple conversions, add things, and remove things, is complicated, and hard to follow.
Additionally, the null character \0 is actually a valid character in Java, so you may have (an extremely rare) bug.
I do not like that your code is removing all the duplicate values, it seems more practical to remove all but one of the duplicates, but you do not explain why it is supposed to be this way.
More Information required
The above, in itself, is a review, but, if you update your question with the requested details:

recursion yes/no/why?
dedup all, or all-but-one?

then I can add suggestions as to how to solve this in a more efficient way.


Limited memory priority queue I'm trying to implement a highly efficient limited memory priority queue. The interface is the same as a std::priority_queue.

Can anyone suggest any performance improvements on my attempt?

limited_queue.h

#include <array>
#include <algorithm>

template<typename T, std::size_t N>
class LimitedPriorityQueue
{
public:
    LimitedPriorityQueue() {}

    void push(T item)
    {
        if (next != items.end()) {
            *next = item;
            ++next;
            std::push_heap(items.begin(), next);
        } else {
            std::sort_heap(items.begin(), items.end());
            if (items.front() < item) {
                items[0] = item;
            }
            std::make_heap(items.begin(), items.end());
        }
    }

    T top()
    {
        return items.front();
    }

    void pop()
    {
        std::pop_heap(items.begin(), next);
        --next;
    }

    bool empty()
    {
        return size() == 0;
    }

    std::size_t size()
    {
        return next - items.begin();
    }

private:
    std::array<T, N> items;
    T* next = items.begin();
};

main.cpp

#include "limited_queue.h"

int main()
{
    LimitedPriorityQueue<int, 10> queue;

    for (int i = 20; i >= 0; --i) {
        queue.push(i);
    }

    while (!queue.empty()) {
        std::cout << queue.top() << std::endl;
        queue.pop();
    }
}

Also, I wasn't quite telling the truth when I said the interface is the same as a std::priority_queue; I'm missing an emplace. I'm not sure if this is possible using an underlying std::array?

Overall, it looks great. The code is clear and easy to follow, you leveraged the standard library well, and performance should be decent decent. There are however, a few things that could be improved.

It might be worth templating the comparison. All that you would have to change is passing an extra argument to make_heap, pop_head and push_heap. There's really not much of a reason to not template the comparator unless I'm missing something.

top() should return a const reference. No reason to make the user copy it unless they want.

You've allowed for move semantics by taking a value rather than const reference in push, but you haven't taken advantage of it the whole way through. Your assignments should be move assignments since you know you're done with the values. That lets you avoid a potentially expensive copy assignment.

Speaking of movement.... You are correct that can't meaningfully implement an emplace with an std::array. Since the elements are default constructed, there's no blank slot to construct in place in to. The best you could do would be a move assignment into an already constructed element which is the same performance as using push with move semantics.

What you could do, however, is use std::aligned_storage and emplace into that. It's basically a tool for getting unitialized memory that's ripe for having something constructed into it. From there, you can implement emplace just like you would in a non-stack allocated situation.

Unfortunately your other logic would get a bit more complicated, but not much. Only construction and destruction (i.e. push and pop) would change, and they would only need placement new and manual-destruction.

There's a pretty good example of what I'm talking about at on cppreference's std::aligned_storage page, though it unfortunately does not have an emplace example.

One last, highly subjective thing: I would probably name it BoundedPriorityQueue instead of LimitedPriorityQueue. Limited sounds like limited functionality to me, whereas bounded sounds like having a maximum size (like bounded buffer).

Also, it might be worth throwing in a bit of documentation. It's pretty obvious from the name and the template params, but someone could wonder what happens when it's full (example thrown, or chucking out the lowest element).


Make sure the user enters a valid floating-point number in C I would like to know if this is a good way to make sure the user enters a valid number.

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

void clear_stream(FILE *in)
{
   int ch;
   clearerr(in);

   do
      ch = getc(in);
   while (ch != '\n' && ch != EOF);

   clearerr(in);
}

float retreive_input()
{
    float input = 0;
    fflush(stdout);
    while (scanf_s("%f", &input) != 1)
    {
        clear_stream(stdin);
        printf("Invalid number. Please try again: ");
        fflush(stdout);
    }

    return input;
}

int main()
{
    float input = 0;
    printf("ENTER A NUMBER: ");
    input = retreive_input();
    printf("YOUR INPUT: %f", input);

    getchar();
    return 0;
}

@Justin advice is solid.  Read a line.

scanf_s("%f", &input) offers no advantage over the more portable scanf("%f", &input)
scanf_s/scanf("%f", &input) allows the user to enter multiple lines before the number - so a blank line of input is not detected.
scanf_s/scanf("%f", &input) does not detect trailing non-numeric input.
while (scanf_s("%f", &input) != 1) does not cope with non-numeric entries - they remain in stdin.
Overflow is UB with scanf() and friends.

Instead separate the task: input, parsing.

#define MAXLINE_SIZE 100
char buf[MAXLINE_SIZE];
if (fgets(buf, sizeof buf, stdin) == NULL) Handle_EOF();

size_t len = strlen(buf);
if (len == 0 || buf[len-1] != '\n') {
  // consider the line hostile and fail it.
  // consume rest of line
  clear_stream(stdin);
  return FAIL;
}

Parse using strtod() - Simple example.  Additional considerations if over/underflow detection needed.

errno = 0;
char *endptr;
float y = (float) strtod(buf, &endptr);

if (buf == endptr) Handle_NoConversion();
// consume trailing spaces
while (isspace((unsigned char) *endptr) endptr++;
if (*endptr) Handle_TrailingJunk().

// Good to go;
return y;

Good use of fflush(stdout); before input and after the re-prompt that may lack a \n.  I would not hard code in the re-prompt value, but pass it in.

Good use of int ch; when testing the return value of getc().

Using %f to print floating point is problematic should the float be small (output only 0.000000) or large (many to hundreds of digits).  Consider %.*e


Thread-safe message queue without mutex in C I'm trying to write a message queue implementation
so my threads can exchange data.
To keep things simple, the queue is only responsible for
sending pointers to memory. Only one thread may send messages,
and only one thread may read messages.

The struct mq_t represents a message queue.

mq_t.messages is a circular buffer of pointers.
mq_t.send is the index of the last sent message.
mq_t.recv is the index of the next message to be received.

I'd like to know some things:

Thread Safety: I didn't use a mutex or lock, but I think the code is quite thread-safe. Is this true?
In the code I use things like while (isempty(queue));. Is there a better way to handle such things, like suspending the thread until a memory change occurs?
Cross Platform: does my code rely on some non-cross platform code or undefined behaviour?
Are there other things wrong with my code (except the highly generic function names)?

#include <stdlib.h>
#include <pthread.h>
#include <stdio.h>

#define null ((void*)0)
#define SIZE 2

typedef struct {
    int send;
    int recv;
    void * messages[SIZE];
} mq_t;

inline int isfull(mq_t * queue) {
    return (queue->send+1)%SIZE == queue->recv;
}

inline int isempty(mq_t * queue) {
    return queue->send == queue->recv;
}

mq_t * create(void) {
    mq_t * queue = (mq_t*)malloc(sizeof(mq_t));
    queue->send = 0;
    queue->recv = 0;
}

void send(mq_t * queue, void * message) {
    while (isfull(queue));
    int next = (queue->send+1) % SIZE;
    queue->messages[next] = message;
    queue->send = next;
}

void * recv(mq_t * queue) {
    while (isempty(queue));
    void * message = queue->messages[queue->send];
    queue->recv = (queue->recv+1) % SIZE;
    return message;
}

void destroy(mq_t * queue) {
    while (!isempty(queue));
    free(queue);
}

Test:

void * sender(void * queue) {
    send(queue, "hello, ");
    send(queue, "world");
    send(queue, "how are you?");
    printf("sent data\n");
}

void * recver(void * queue) {
    sleep(1);
    printf("received '%s'\n", recv(queue));
    printf("received '%s'\n", recv(queue));
    printf("received '%s'\n", recv(queue));
}

int main() {
    mq_t * queue = create();

    pthread_t s, r;
    pthread_create(&s, null, sender, (void*)queue);
    pthread_create(&r, null, recver, (void*)queue);
    pthread_join(s, null);
    pthread_join(r, null);

    return 0;
}

Output:

received 'hello, '
received 'world'
received 'how are you?'
sent data

With a size of 2, your 'queue' can hold only one item, so it is not really a queue.  If
you increase the size to 8 (say) you now really have a queue, but your tests don't
work.

You seem to define send and recv as follows:

send indicates the 'slot' holding the most recently entered data (queue not empty).
recv indicates the 'slot' before the oldest data in the queue (queue not empty).
if send == recv  the queue is empty.

So with both send and recv at zero, when the first data is written it goes to slot 1 and send is incremented to 1; when the 2nd data is written it goes to slot 2 and send is incremented to 2.  If you now read from the queue, in recv you use the send counter to extract the data instead of using recv + 1.  Hence the queue fails.

A more logical arrangement might be as follows with the counters renamed in
and out (naming them the same as the access functions is confusing):

in indicates the next 'slot' to be written by send (queue not empty).
out indicates the 'slot' containing the next data to be read by recv (queue not empty).
if in == out the queue is empty.

So with both send and recv at zero, when the first data is sent, it goes into slot 0 (to which in and out already point) and in is incremented to 1.  The first read now reads from out at 0 - which is correct.

On thread safety, it is very difficult to see/analyse such things.  You would be
better to assume that it isn't (it certainly wouldn't be if there were more
than 2 threads) and use a mutex.  As your wait loops need to be rewritten
(they just waste CPU cycles needlessly) and you are using Posix threads, use a
Posix mutex and a "condition variable" to synchronise.

Here's an example, but there are many more on the web: http://www.ibm.com/developerworks/library/l-posix3/

Some minor coding points:

Add const to pointer parameters that do not change.
Return a value from create
Don't cast the return from malloc.  Casting is necessary in C++ not C
(where void* can be assigned to any pointer).  In C, adding a cast can cause
problems if you don't have the header for malloc included, as the compiler
will assume that malloc returns an int.  If int and pointer have
different sizes this is a problem.
null is not really needed.  You can use either 0 or NULL.
the _t type suffix is apparently reserved by Posix.


Calculate difference between two times I just wrote this script that calculates the time difference between two times. It's going to be used in a larger program that will store the returned value in a database.

I'm mainly wondering if I've used docstrings, comments and try/raise/except correctly since it's my first time really using them.

def time_difference(time_start, time_end):
    '''Calculate the difference between two times on the same date.

    Args:
        time_start: the time to use as a starting point
        time_end: the time to use as an end point

    Returns:
        the difference between time_start and time_end. For example:

        >>> time_difference(1500, 1600)
        60

        >>> time_difference(1100, 1310)
        130

    Raises:
        TypeError: if time_start and time_end is not a string
        ValueError: if time_start and time_end is below 0 or above 2400
    '''

    try:
        if not isinstance(time_start, str):
            raise TypeError('time_start has to be a string!')
        elif not isinstance(time_end, str):
            raise TypeError('time_end has to be a string!')
        elif int(time_start) > 2400 or int(time_end) < 0:
            raise ValueError('time_start nor time_end can be below 0 or above 2400!')
        elif int(time_start) > int(time_end):
            raise ValueError('time_start cannot be higher than time_end!')
    except (TypeError, ValueError):
        raise

    # Convert time_start and time_end to necessary variables
    # for the calculation.
    hours_begin = time_start[0] + time_start[1]
    minutes_begin = time_start[2] + time_start[3]
    hours_end = time_end[0] + time_end[1]
    minutes_end = time_end[2] + time_end[3]

    # Calculates the difference.
    minutes = ((int(hours_end) - int(hours_begin)) * 60 + \
        int(minutes_end) - int(minutes_begin))

    return(minutes)

Both the function name and the docstring are clear and informative. Good job there! However, there are a couple of typos in the examples. The function requires strings as input but the examples use ints. For example time_difference(1100, 1310) should be time_difference('1100', '1310'). A tool like doctest can help you check for mistakes like this automatically.

Generally people don't check the types of arguments in python. You are right to document the types expected by the function but after that I would just use the objects as if they are the right type and let python throw an error if they aren't. I say this because type checking often gets in the way of duck typing and reduces the flexibility and generality of your code. For example, in python 2.x, isinstance(time_start, str) returns False for unicode strings (isinstance(time_start, basestring) is a better alternative).

The try/except is unnecessary as you are simply reraising every exception you catch.

Rather than trying to parse the strings yourself (which is quite error prone as AdHominem pointed out), you can use the datetime library instead.

If we apply these changes the result looks like:

from datetime import datetime

def time_difference(time_start, time_end):
    '''Calculate the difference between two times on the same date.

    Args:
        time_start: the time to use as a starting point
        time_end: the time to use as an end point

    Returns:
        the difference between time_start and time_end. For example:

        >>> time_difference('1500', '1600')
        60

        >>> time_difference('1100', '1310')
        130
    '''

    start = datetime.strptime(time_start, "%H%M")
    end = datetime.strptime(time_end, "%H%M")
    difference = end - start
    minutes = difference.total_seconds() / 60
    return int(minutes)


JavaScript Tic Tac Toe Win Checker is Wet New to coding/Javascript. Writing a basic Tic Tac Toe game.

I'm writing the game to be dynamic, so it won't always be a three-by-three grid, but rather any size grid larger than 1. I have a completely functioning game and win checker function, but the win checker is very, very wet. I've tried several way to dry it up, but can't really find a good pattern that doesn't cause bugs or issues.

The winChecker function below receives a 2D array that could resemble something like (for a 3x3 grid):

const squares = [
  [null, null, null],
  [null, null, null],
  [null, null, null]
];

The winChecker function looks like:

const winChecker = squares => {
  for (let column = 0; column < squares.length; column++) {
    const horizontalCase = squares[column][0];
    const verticalCase = squares[0][column];
    const negativeCase = squares[0][0];
    const positiveCase = squares[0][squares.length - 1];
    if (horizontalCase !== null) {
      for (let row = 0; row < squares.length; row++) {
        if (horizontalCase !== squares[column][row]) {
          break;
        } else {
          if (row === squares.length - 1) {
            return true;
          }
        }
      }
    }
    if (verticalCase !== null) {
      for (let row = 0; row < squares.length; row++) {
        if (verticalCase !== squares[row][column]) {
          break;
        } else {
          if (row === squares.length - 1) {
            return true;
          }
        }
      }
    }
    if (negativeCase !== null) {
      for (let row = 0; row < squares.length; row++) {
        if (negativeCase !== squares[row][row]) {
          break;
        } else {
          if (row === squares.length - 1) {
            return true;
          }
        }
      }
    }
    if (positiveCase !== null) {
      for (let row = 0; row < squares.length; row++) {
        if (positiveCase !== squares[row][squares.length - 1 - row]) {
          break;
        } else {
          if (row === squares.length - 1) {
            return true;
          }
        }
      }
    }
  }
  return false;

The function above actually works, it's just real wet. Any help in drying it up would be lovely.

I think the nicest thing to do is to split your task into three bits, essentially you want to see if a row, column or diagonal has all the entries the same.

A row, column or diagonal can all be thought of just as a (1D) array.

In fact, these things are all the 'lines' (pictorially) that you can find in a 2D array (which span the whole array).

Now given a 2D array A, there's several ways to define a line, for instance the line which corresponds to the second row we can say is the pairs (i, j) such that i === 1 (and j can be anything). Similarly the left diagonal is the pairs (i, j) such that i === j.

Ok so I want to think of a line as a function, for instance I will think of the second row as the function

(i, j) => i === 1

OK, now given a line (which we represent as a function), I want to extract the elements of A which lie on that line, so I should traverse A, and for each element (i, j) I check if that element is in my line (i.e. if my function returns true for it) and if so include it in my result.

How to do this? Well I /want/ to be able to loop through a 2D array gracefully, sadly we can't do that in JS, so I'll just look at each row in turn, determine which elements of that row are in my line and keep them, something like.

function getSectionOf2DArray(arr, lineFunc) {
    return arr.map((row, i) => row.filter((_, j) => lineFunc(i, j)));
}

So now I have a 2D array still, the element at position i of the array is an array consisting of the things in row i which are in my line. If I flatten it I'll have the array of things in my line. I'm not sure if you're using node or are in the browser, so I'll write a flatten to be safe.

function getSectionOf2DArray(arr, sectionFunc) {
    return Array.prototype.concat.apply(
        [],
        arr.map((row, i) => row.filter((_, j) => sectionFunc(i, j)))
    );
}

Nearly there now, let me define the lines of a 2D array.

First the rows, well given a 2D array A, these are just the things (i, j) => i === k for k up to the length of the array.

const rowLines = array =>
    Array(array.length)
        .fill(0)
        .map((_, k) => (i, j) => i === k);

Now the columns, similar story just with a slight reversal, now it's (i, j) => j === k

const colLines = array =>
    Array(array.length)
        .fill(0)
        .map((_, k) => (i, j) => j === k);

Finally the diagonals, the left diagonal (i.e. starting in the top left) is (i, j) => i === j, while the right diagonal is (i, j) => i + j === A.length - 1

const leftDiag = array => (i, j) => i === j;
const rightDiag = array => (i, j) => i + j === array.length - 1;

(I have made leftDiag continue to be a function defined in terms of array just for consistency, it will make our life nicer later).

Great, so now if we do

const ourLines = array =>
    rowLines(array)
        .concat(colLines(array))
        .concat(leftDiag(array))
        .concat(rightDiag(array));

Then ourLines is a function which takes an array and returns an array of lines (aka functions).

Now we just need to define a function which says whether an 'evaluated' (as in I have the actual elements rather than the function) line corresponds to a win, this is easy, it's just if all the elements are the same and not equal to undefined. If all the elements are the same then when we turn the array into a set it will have just one element

const win = evalLine => new Set(evalLine).size === 1 && evalLine [0] !== undefined;

OK, now we should just go through our lines, get the actual array of elements in A they correspond to, and check if any is a win.

const winResults = array =>
    ourLines(array).map(line => win(getSectionOf2DArray(array, line)));

const isThereAWin = array => winResults(array).includes(true);

In summary,

const winChecker = squares => {
    const getSectionOf2DArray = (arr, sectionFunc) =>
        Array.prototype.concat.apply(
            [],
            arr.map((row, i) => row.filter((_, j) => sectionFunc(i, j)))
        );

    const rowLines = array => Array(array.length).fill(0).map((_, k) => (i, j) => i === k);

    const colLines = array => Array(array.length).fill(0).map((_, k) => (i, j) => j === k);

    const leftDiag = array => (i, j) => i === j;
    const rightDiag = array => (i, j) => i + j === array.length - 1;

    const ourLines = array =>
        rowLines(array)
           .concat(colLines(array))
           .concat(leftDiag(array))
           .concat(rightDiag(array));

    const win = evalLine => new Set(evalLine).size === 1 && evalLine[0] !== undefined;

    const winResults = array => ourLines(array).map(line => win(getSectionOf2DArray(array, line)));

    const isThereAWin = array => winResults(array).includes(true);

    return isThereAWin(squares);
};

I hope you agree this pattern is nice. There are ways to write more concise things for your specific use case but this is simple and extendable.


Inheritance selfmade or reinventing wheel Background

I like to code JavaScript in a typical style with "classes" and inheritance. My goal was to create an easy way to make inheritance available and keep classes that inherits from others clean. Also I like to use the module pattern and uses its look and feel.

My solution

I hope the code is self-explanatory. I'm using it in all my newer projects and it seems to work fine.

 if(typeof(SimpleJSLib)==='undefined'){
     var SimpleJSLib = {}; // define namespace
 }
 SimpleJSLib.BaseObject = function(){
     // returns an object you wether can inherit from or instantiate it
     var createInheritObject = function(inheritFunctions){
         return {
             // instantiate the current class
             // iterates through all functions which the class inherits from and calls them
             construct : function(){
                 var me = {}, _protected = {};
                 for(var i = 0; i < inheritFunctions.length; i++){
                     me = inheritFunctions[i].call(me, me, _protected);
                     if(typeof(me)==='undefined'){
                         throw 'Inherit function did not return "me"';
                     }
                 }
                 // if a constructor is defined we call them and deliver the arguments this function was called with
                 if(typeof(_protected.construct)!=='undefined'){
                     _protected.construct.call(me, arguments);
                 }
                 return me;
             },
             // clones the pointers to the functions you inherit from
             // and returns another object you can instantiate or inherit from
             inherit : function(inheritFunction){
                 var _inheritFunctions = inheritFunctions.slice(0); // clone
                 _inheritFunctions.push(inheritFunction);
                 return createInheritObject.call(window, _inheritFunctions);
             }
         };
     };

     return {
         // allows you to inherit directly from BaseObject
         inherit : function(inheritFunction){
             // this is the first call of the function, so the the inheritFunctions array just contains one function
             return createInheritObject.call(window, [inheritFunction]);
         }
     }
 }();

Questions

Is there any good argument why I shouldn't use this code snippet?
How about performance? Is there anything I could make better?

Examples

Just a few simple ones.

var MyClass = SimpleJSLib.BaseObject.inherit(function(me, _protected){
    // protected
    _protected.myValue = 'value';
    _protected.doSomething = function(){
        console.log('something')
    };
    // public
    me.getMyValue = function(){
        return _protected.myValue;
    }
    return me;
});
var MySubClass = MyClass.inherit(function(me, _protected){
    me.anotherFunction = function(){
        _protected.doSomething();
    }
    return me;
});
var myClassInstance = MyClass.construct();
console.log(myClassInstance.getMyValue());
var mySubClassInstance = MySubClass.construct();
console.log(mySubClassInstance.anotherFunction());

Using the constructor

var MyClass = SimpleJSLib.BaseObject.inherit(function(me, _protected){
    _protected.valueOne = null;
    _protected.valueTwo = null;
    _protected.construct =  function(parameters){
        _protected.valueOne = parameters[0];
        _protected.valueTwo = parameters[1];
    };
    return me;
});
var myClassInstance = MyClass.construct('valueOne', 'valueTwo');

Singelton

var Singelton = SimpleJSLib.BaseObject.inherit(function(me, _protected){
    return me;
}).construct(); // mention the construct!

From some staring at your code:

Do not declare var in an if block, it gives the wrong impression. I would just declare it on top, something like this:

//Define namespace
var SimpleJSLib;
SimpleJSLib = SimpleJSLib || {};

As far as naming, since SimpleJSLib is not a constructor, it should not start with capital S. The name JSLib is not very informative beyond what language it is writen in..
I know that for some _ in _protected means "private", but I agree with Crockford that this convention should be avoided
Now a big one: you broke support for instanceof operator, that's just wrong
Furthermore, you kind of broke support for prototype, you are also not using hasOwnProperty anywhere when you construct objects
And, JS has a perfectly valid constructor, but you decided to have your own construct
Finally, this will be messy once you want to start serializing with JSON

Honestly, read this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Introduction_to_Object-Oriented_JavaScript and abandon your code. I would not want to maintain code that relies on this.


number with the most prime divisors I've done a program and it works fine as I want to but I feel it could be better and more advanced so I want some master advice on my program which is going to take many numbers and return the number with the most prime divisors and the number of its divisors in front of the number like this 678 3 as the output
code :
list_of_inputs = []
for elements in range(5):
    list_of_inputs.append(int(input()))
list_of_inputs = list(set(list_of_inputs))

def _check_prime_sample_(n):
    if n < 2:
        return False
    for i in range(2, n):
        if n % i == 0:
            return False
    return True

def find_integer_with_most_divisors(input_list):
    from operator import itemgetter
    lis = []
    dic = {}
    for elements in input_list:
        sub_lis = []
        for items in range(1, elements):
            if elements % items == 0 and _check_prime_sample_(items) == True:
                sub_lis.append(items)
        lis.append(len(sub_lis))
        dic[elements] = len(sub_lis)
    tup = [(k, v) for k, v in dic.items()]
    sup = max(tup, key=itemgetter(1, 0))

    print('{} {}'.format((sup)[0], (sup)[1]))

find_integer_with_most_divisors(list_of_inputs)

In this example, your code is used as the starting point. I moved the logic
around a bit -- notably, separating things into other functions -- and
suggested various other changes for you to consider.
# Except for special reasons, put all imports at the top.
import math
import sys

# Put almost all code inside functions.

def main(args):

    # When feasible, use variable names that sound like ordinary English (eg,
    # inputs) rather than awkward computer-science-like names (eg,
    # list_of_inputs). Similarly, if a variable holds one thing, use a singular
    # variable name (eg, num, book, or x). And if the variable holds a
    # collection of things, use plural (eg, nums, books, or xs).

    # Also, list/set/dict comprehensions can often make code easier to
    # understand, especially when they are fairly short.

    # For scripts like this, it's often useful to allow the possibility for
    # command-line arguments.

    if args:
        nums = [int(a) for a in args]
    else:
        # Provide users at least some minimal guidance.
        prompt = 'Integer: '
        nums = [int(input(prompt)) for _ in range(5)]

    # Computing the number with the most divisors is pure mathematical
    # algorithm: keep it that way. The function should do its work and return a
    # value. Do the printing elsewhere -- for example, in main().

    # By default, print() takes a list of values and prints them
    # separated by spaces. If that's all you need, there's
    # no reason to use a format string.

    num, n_divisors = find_integer_with_most_divisors(nums)
    print(num, n_divisors)

# If a function returns True/False, intuitive names often take the
# form of is_X, has_X, contains_X, and so forth.

def is_prime(n):
    if n < 2:
        return False
    # You don't need to check beyond the square root of N.
    for i in range(2, int(math.sqrt(n)) + 1):
        if n % i == 0:
            return False
    return True

# Your version of this function used some good techniques, but it was more
# complex and harder to understand than necessary, mostly because the function
# was doing too many jobs at the same time: computing divisors for each number,
# finding the number with the most divisors, and reporting to the user. Keeping
# things separate often increases flexibly and code clarity.

# Let's start by moving the divisor computation to its own function. Again, we
# only need to check up to square root. There are faster ways to compute
# divisors, but this example prioritizes simplicity.

def divisors(n):
    divs = {1, n}
    for i in range(2, int(math.sqrt(n)) + 1):
        if n % i == 0:
            divs.add(i)
            divs.add(n // i)
    return sorted(divs)

# Now the program's primary function because very easy to write and understand.
# Also, we continue with the theme of selecting variable names that approximate
# ordinary English-language usage.

def find_integer_with_most_divisors(nums):
    counts = []
    for num in nums:
        divs = divisors(num)
        primes = [d for d in divs if is_prime(d)]
        # If you put the count first in the tuple, functions like
        # sort(), max(), min() will work exactly as desired.
        counts.append((len(primes), num))
    count, num = max(counts)
    return (num, count)

# Invoke main() with any command-line arguments, if the code was executed as a
# script. If the code is imported, main() won't be called. This device makes
# your script easier to test.

if __name__ == '__main__':
    main(sys.argv[1:])


Optimising a custom tree data structure I have the following 'custom' tree data structure.  This concept was to be thought of as, if given ("abb", 20) and ("abc", 30), then

Tree['a']['b']['b'] == 20

Tree['a']['b']['c'] == 30

Or

a
 \
  b
 / \
b   c

With b == 30, and c == 20.

Therefore taking up very limited space, and a very fast search.
However, as depth increases, the number of bytes for a fully explored tree was exponential. In the above case, 4 * 256 bytes were required for valid indexing to occur.  This was not good.

It now works in the following way; for each branch, compare its value against the n character; i.e. introducing a worst case O(256) character comparison for each depth. This is not ideal, and what I want to attempt to change.

The other question was (and still is), what is it? I couldn't quite classify it as anything but a Tree; nor relate it to any std:: containers for comparison. It is very good at what it does though, out-performing a std::map with small strings and very large strings (sizes ~20, ~500 respectively); past 600, the data structure begins to get too heavy.

Obviously, this code is a bit sketchy in how it handles the templates at the moment.  A number of things could go wrong, but my interest (other than classification) is on improving this data structure.

template <class K, class V>
class Tree {
    unsigned int depth;
    Tree* parent;

    std::vector<Tree> branches;
public:
    K key;
    V val;

    int n_trees;
    Tree(K key = 0, int depth = 0, Tree* parent = 0) : depth(depth), parent(parent), key(key) {
        val = 0;
        n_trees = 0;
        inc();
    }

    void inc() {
        if (parent) parent->inc();
        n_trees++;
    }

    void add(const K* search, const unsigned int length, const V value) {
        const int index = search[depth];

        if (length <= depth) {
            val = value;
            return;
        }

        for (unsigned int i = 0; i < branches.size(); ++i) {
            if (branches[i].key == index) {
                branches[i].add(search, length, value);
                return;
            }
        }
        branches.push_back(
            Tree(index, depth + 1, this)
        );
        branches.back().add(search, length, value);
    }
    V find(const K* search, const unsigned int length) {
        const int index = search[depth];

        if (length <= depth) {
            return val;
        }

        for (unsigned int i = 0; i < branches.size(); ++i) {
            if (branches[i].key == index) {
                return branches[i].find(search,length);
            }
        }
        return 0; // won't work for all types lol
    }
};

Why do you have m_parent? I don't think there are any reasonable operations on a trie that require a parent pointer in each node, and removing it can reduce the space used a lot.

I don't love using a depth argument in add. I'd use the following instead:

void add(const std::string& key, const T& value) {
  return add(key.data(), key.size(), value);
}
void add(const char *key, const T& value) {
  return add(key, std::strlen(key), value);
}
void add(const char* key, std::size_t len, const T& value);  // impl below

I suggest that you keep the children sorted by m_key. Write a comparison functor like so:

struct KeyLessThan {
  bool operator()(const PrefixTree& l, const PrefixTree& r) const {
    return l.m_key < r.m_key;
  }
  bool operator()(const PrefixTree& tree, char c) const {
    return l.m_key < c;
  }
};

Now you can rewrite add like this (using the signature above):

void PrefixTree::add(const char* key, std::size_t len, const T& value) {
  assert(len >= 0);
  if (len == 0) {
    m_value = value;
    return;
  }
  auto it = std::lower_bound(begin(m_branches), end(m_branches),
                             *key, KeyLessThan());
  if (it == end || it->m_key != *key) {
    it = m_branches.emplace(it, *key);
  }
  it->add(key + 1, len - 1, value);
}

The size of m_branches is bounded to be quite small (256 elements in theory; in practice if you're doing English words only it'll be 26 or 52 at the root and much smaller in interior nodes), so this should be quite fast, and it'll make find faster.

Speaking of m_branches, m_children would be a more usual name for it.

It's not really clear that you want find to return a T*; it's odd to give out handles to internal memory in a data structure like this. I would make the signature one of

const T* find(const std::string& key) const
std::pair<bool, const T&> find(const std::string& key) const

or

std::pair<bool, T> find(const std::string& key) const

(if you have access to a C++14 implementation, even better is

std::optional<T> find(const std::string& key) const

Whichever you choose, use the same overloads as with add for const char *, etc.

It is very weird to me to see an #include that's not at the top of the file, and to see printfs in the middle of a C++ file.


How can I make this function that checks whether a number is a palindrome better? I have this code:
export function is_palindrome(num: number): boolean {
    return [...num.toString()].reverse().join("") === num.toString();
}

which checks if a number is a palindrome(i.e 9009 reversed is still 9009), I do this by using the spread operator on num.toString(), reversing the array, joining it, and then comparing it to num.toString() which works but surely there must be a better way to do this.

It looks fine - note that for a function this small there will almost always be no real impact on runtime, nor on readability. The only thing I would change is avoiding converting num into a string twice.
It is also possible to check for a palindrome without explicitly reversing the string, which I have done below.
export function is_palindrome(num: number): boolean {
    const str = num.toString();
    const len = str.length;
    for (let i = 0; i < Math.floor(len / 2); i++) {
        if (str[i] !== str[len - i - 1]) {
            return false;
        }
    }
    return true;
}


A JavaScript graph plotter for schoolchildren using paper.js I have begun coding a simple graph plotter for my schoolchildren using JavaScript. I have used the paper.js library as I would like to be able to export the graphs as svg files.
I am only a beginner to all this (just a humble maths teacher) so I would hugely welcome any thoughts you may have. The code does not run particularly quickly so I would be particularly interested in any improvements to performance.
Many thanks and best wishes.
HTML:

<input type="number" id="input_xWidth" value=14>

<input type="number" id="input_yWidth" value=4>

<input type="text" id="eqn" value='sin(x)'>

<button onclick="drawGrid()">draw grid</button>
<button onclick="exportSVG()">export</button>

<canvas id="canvas_1"></canvas>

JavaScript:
var graphScope = new paper.PaperScope();
var canvas_1 = document.getElementById('canvas_1');
graphScope.setup(canvas_1);
graphScope.activate();

const scale = 10;

function drawGrid() {

  var xWidth = document.getElementById("input_xWidth").value;
  var yWidth = document.getElementById("input_yWidth").value;

  var z;
  var zz;

  //clear the canvas
  graphScope.project.activeLayer.removeChildren()

  //draw minor gridlines
  for (z = 0; z < xWidth; z++) {
    for (zz = 1; zz < 5; zz++) {
      var myPath = new graphScope.Path();
      myPath.strokeColor = new graphScope.Color(0.9, 0.9, 0.9);
      myPath.add(new graphScope.Point(z * scale + (scale / 5) * zz, -(0)));
      myPath.add(new graphScope.Point(z * scale + (scale / 5) * zz, -(yWidth * scale)));
    }
  }

  for (z = 0; z < yWidth; z++) {
    for (zz = 1; zz < 5; zz++) {
      var myPath = new graphScope.Path();
      myPath.strokeColor = new graphScope.Color(0.9, 0.9, 0.9);
      myPath.add(new graphScope.Point(0, -(z * scale + (scale / 5) * zz)));
      myPath.add(new graphScope.Point(xWidth * scale, -(z * scale + (scale / 5) * zz)));
    }
  }

  //draw major gridlines
  for (z = 0; z <= xWidth; z++) {
    var myPath = new graphScope.Path();
    myPath.strokeColor = new graphScope.Color(0.5, 0.5, 0.5);
    myPath.add(new graphScope.Point(z * scale, -(0)));
    myPath.add(new graphScope.Point(z * scale, -(yWidth * scale)));
  }

  for (z = 0; z <= yWidth; z++) {
    var myPath = new graphScope.Path();
    myPath.strokeColor = new graphScope.Color(0.5, 0.5, 0.5);
    myPath.add(new graphScope.Point(0, -(z * scale)));
    myPath.add(new graphScope.Point(xWidth * scale, -(z * scale)));
  }

  // parse equation from input box
  const node2 = math.parse(document.getElementById("eqn").value)
  const code2 = node2.compile()
  let scope = {
    x: 3,
  }

  // trim graph to grid
  var rectangle = new graphScope.Rectangle(new graphScope.Point(0, 0), new graphScope.Point(xWidth * scale, -yWidth * scale));
  var GraphBoundary = new graphScope.Path.Rectangle(rectangle);
  var graphPath = new graphScope.Path();
  for (z = 0; z < xWidth; z += 0.001) {
    scope.x = z
    graphPath.add(new graphScope.Point(z * scale, -(20 + scale * code2.evaluate(scope))));
  }
  var NewPath = graphPath.intersect(GraphBoundary, {
    trace: false
  })
  NewPath.strokeColor = new graphScope.Color(0.1, 0.1, 0.1);
  graphPath.remove();

  //fit page to canvas bounds
  graphScope.project.activeLayer.fitBounds(graphScope.view.bounds);
}

function exportSVG() {
  var fileName = "custom.svg"
  var url = "data:image/svg+xml;utf8," + encodeURIComponent(graphScope.project.exportSVG({
    asString: true
  }));
  var link = document.createElement("a");
  link.download = fileName;
  link.href = url;
  link.click();
}

I'm not familiar with Paper so can't say anything about that. And I won't say much about the HTML, as it's obviously just a functional placeholder, except two points:

Use double quotes " consistently on all attributes.
Don't use on... attributes. Assign event listeners in the script using addEventListener.

Regarding the script:

Use let/const instead of var.
Declare the variable in loops separately in the for statement using let (e.g. don't reuse the same declared variable in all loops).
Use better variable names. I have no idea what z, zz, node2, code2, etc. mean. (Also make sure all variable names start with a lower case letter.)
Don't repeatedly create the same colors in the loops. Create them once outside the loop.


Getting the date on YYYYMMDD format I'm looking for the best way to return a date on a YYYYMMDD format, such as 20180106.

Here is how I do it:

var x = new Date();
var y = x.getFullYear().toString();
var m = (x.getMonth() + 1).toString();
var d = x.getDate().toString();
(d.length == 1) && (d = '0' + d);
(m.length == 1) && (m = '0' + m);
var yyyymmdd = y + m + d;
alert(yyyymmdd);

Is it correct enough or can I achieve the same in a better way?

The code looks good and works well. Since it provides a useful bit of work, you should convert it into a function. Then you can copy that function to another program if you need so.

function yyyymmdd() {
    var x = new Date();
    var y = x.getFullYear().toString();
    var m = (x.getMonth() + 1).toString();
    var d = x.getDate().toString();
    (d.length == 1) && (d = '0' + d);
    (m.length == 1) && (m = '0' + m);
    var yyyymmdd = y + m + d;
    return yyyymmdd;
}

To make this code a little easier to read, you should rename x to now.

You could also omit the calls to toString(), which makes the code a little shorter. Plus, you should introduce the variables mm and dd, so that you don't reassign to the d and m variables. This is a generally useful pattern, because when stepping through the code you can always look at the variable definition to see how it was computed. This is not possible for variables that change their value during execution.

The modified code looks like this:

function yyyymmdd() {
    var now = new Date();
    var y = now.getFullYear();
    var m = now.getMonth() + 1;
    var d = now.getDate();
    var mm = m < 10 ? '0' + m : m;
    var dd = d < 10 ? '0' + d : d;
    return '' + y + mm + dd;
}

Or, you could inline the last few lines:

function yyyymmdd() {
    var now = new Date();
    var y = now.getFullYear();
    var m = now.getMonth() + 1;
    var d = now.getDate();
    return '' + y + (m < 10 ? '0' : '') + m + (d < 10 ? '0' : '') + d;
}

That last variant is harder to read though, therefore I prefer the previous one.

Another possibility is to define a helper function that produces a two-digit string:

function yyyymmdd() {
    function twoDigit(n) { return (n < 10 ? '0' : '') + n; }

    var now = new Date();
    return '' + now.getFullYear() + twoDigit(now.getMonth() + 1) + twoDigit(now.getDate());
}


Sum of all primes under 2 million I have this assignment to write the optimized space and time code for finding the sum of all primes under 2 million. I'm using the following function to check if each number is prime:

int IsPrime(unsigned int number) {
    if (number <= 1) return 0;
    unsigned int i;
    for (i=2; i*i<=number; i++) {
        if (number % i == 0) return 0;
    }
    return 1;
}

And running a for loop from 1 to 2 million using a long double, and checking each number if it is prime and then adding it. Is there a more optimized method to do this?

There are dozens of possible optimizations. The most obvious -- why are you multiplying i by itself every pass in the loop?! Just calculate the loop cut off once.

And why i++?! After you test 2, there's no reason to test 4, 6, or 8. You can test 2, and then start from 3 adding 2 each time instead of one.

Also, since you need to find all the primes up to a given point, you should be using a sieve rather than testing each number.


PowerShell - fast remove a directory with 10,000+ files I am sick of File Explorer's super slow deletion speed, so I tried to write a PowerShell script to make deletion faster, and while it does its job, its speed isn't as high as what I intended it to be.
I have written this:
Function Fast-Delete {
    Param(
    [Parameter(Valuefrompipeline=$True, Mandatory=$True)] [String]$Directory0
    )
    Write-Warning "This Process Will Delete Everything In The Target Directory: $Directory0, Do You Want To Confirm Deletion?" -Warningaction Inquire
    $Directory=$Directory0
    While ((Get-Childitem -Path $Directory0 -Depth 0 -Force).Count -Ne 0) {
    If ((Get-Childitem -Path $Directory -Directory -Force -Depth 0).Count -Ne 0) {
    $Directory=(Get-Childitem -Path $Directory -Directory -Force -Depth 0).Fullname | Select-Object -Index 0
    }
    If ((Get-Childitem -Path $Directory -File -Force).Count -Ne 0) {
    (Get-Childitem -Path $Directory -File -Recurse -Force).Fullname | Foreach {Remove-Item -Path $_}
    }
    $Directory1=$Directory
    $Directory=$Directory | Split-Path -Parent
    Remove-Item -Path $Directory1
    }
    Remove-Item -Path $Directory0
}

It is significantly faster than explorer, but still isn't ideal, I have tested it, I used it to delete 208,000 files in 1,000 folders , and the folders disappear at speed of 1 per second, so it's about 208 files/s, now the next challenge should be parallelization, But this is currently really way above me, but it shouldn't be too hard, I am just not experienced enough.
Update2
I have managed to make it run in parallel with this script:
function Parallel-Delete {
    param(
    [Parameter(Valuefrompipeline=$true, Mandatory=$true, Position=0)] [array]$filelist,
    [Parameter(Valuefrompipeline=$true, Mandatory=$true, Position=1)] [int]$number
    )
    0..($filelist.count-1) | Where-Object {$_ % 16 -eq $number} | foreach {Remove-Item -Path $filelist[$_]}
}

Function Fast-Delete {
    Param(
    [Parameter(Valuefrompipeline=$True, Mandatory=$True)] [String]$Directory0
    )
    Write-Warning "This Process Will Delete Everything In The Target Directory:  $Directory0, Do You Want To Confirm Deletion?" -Warningaction Inquire
    $Directory=$Directory0
    While ((Get-Childitem -Path $Directory0 -Depth 0 -Force).Count -Ne 0) {
        If ((Get-Childitem -Path $Directory -Directory -Force -Depth 0).Count -Ne 0) {
            $Directory=(Get-Childitem -Path $Directory -Directory -Force -Depth 0).Fullname | Select-Object -Index 0
        }
        If ((Get-Childitem -Path $Directory -File -Force).Count -Ne 0) {
            If ((Get-Childitem -Path $Directory -File -Force).Count -Ge 128) {
                [array]$filelist=(Get-Childitem -Path $Directory -File -Force).Fullname
                0..15 | foreach-object {Invoke-Command -ScriptBlock { Parallel-Delete $filelist $_}}
                } else {
                (Get-Childitem -Path $Directory -File -Force).Fullname | Foreach {Remove-Item -Path $_}
            }
        }
        $Directory1=$Directory
        $Directory=$Directory | Split-Path -Parent
        Remove-Item -Path $Directory1
        }
    Remove-Item -Path $Directory0 -erroraction silentlycontinue
}
$Directory0=Read-Host "Please input target directory to be deleted"
Fast-Delete $Directory0

But it still isn't ideal, it isn't 15 times faster as expected, what did I miss?
Edit: simplified the creation of parallel processes.

May be try this:
#solution 1 - Remove content of dir
remove-item "C:\temp\tmp2\*" -Recurse -Force

#Solution 2 - Remove dir and recreate dir
remove-item "C:\temp\tmp2\" -Recurse -Force
New-Item "C:\temp\tmp2\" -ItemType Directory

#Solution 3 - Loop into dir and remove all with intercept error
Get-ChildItem "C:\temp\tmp2\" -Recurse | Remove-Item -Recurse -Force -ErrorAction SilentlyContinue


Sorting linked list with comparator in Java When I want to sort data on linked list by using collections.sort() and comparator will working after write (collections.sort (...)) and then use it again with comparator?

Is it possible to just use a comparator without collection.sort (...) first?

public class JavaApplication {
   public static void main(String[] args) {
      LinkedList lList = new LinkedList();
      lList.add("1");
      lList.add("2");
      lList.add("3");
      lList.add("4");
      lList.add("5");
      System.out.println(lList);
      lList.addFirst("0");
      System.out.println(lList);
      lList.addLast("6");
      System.out.println(lList);

      lList.addFirst("2");
      System.out.println(lList);

      lList.add(3, "10");
      System.out.println(lList);

      lList.add(4,"25");
      System.out.println(lList);

      getIterator( lList ).add( "60" );
      System.out.println(lList);

      lList.addLast("10");
      System.out.println(lList);
      lList.addLast("15");
      System.out.println(lList);

      lList.add(6,"40");
      System.out.println(lList);

      getIterator( lList ).add( "70" );
      System.out.println(lList);

      lList.addFirst("75");
      System.out.println(lList);

      Collections.sort(lList);

      Collections.sort(lList, new AComparator());
      System.out.println("LinkedList (after sorting using Comparator): "+lList);
   }

   private static ListIterator getIterator(LinkedList lList) {
       return lList.listIterator( lList.size() / 2 );

   }
}

class AComparator implements Comparator<String> {
    public int compare(String s1, String s2) {
        return s1.length() - s2.length();
    }
}

And can someone give me a suggest how to efficiency the code.

Output that I want :

Unsorted [75, 2, 0, 1, 10, 25, 60, 40, 70, 2, 3, 4, 5, 6, 10, 15]
LinkedList (after sorting using Comparator): [0, 1, 2, 2, 3, 4, 5, 6, 10, 10, 15, 25, 40, 60, 70, 75]

If you're using Java Collections in any Java version later than 5, you should be using generic typing.  It's an easy way to provide type safety without dealing with casting, multiple class-specific implementations of collections, or the other hassles that have long since been forgotten.  And with the latest Java versions, you don't even need to specify the generic type of the assigned object due to type inheritance, so adding generics support is as simple as:

LinkedList<String> lList = new LinkedList<>();

and:

private static <E> ListIterator<E> midpointIterator(LinkedList<E> lList) {
    return lList.listIterator( lList.size() / 2 );
}

Which brings me to another point: use descriptive method and object names!  Looking at the name of the getIterator method made me wonder why the objects weren't being inserted at the beginning when I ran the code.  midpointIterator (or something similar) lets me know that the ListIterator I'm getting back starts at the midpoint.

Likewise with the comparator:

class StringLengthComparator implements Comparator<String>{
    public int compare(String s1, String s2) {
        return s1.length() - s2.length();
    }
}

Now I know that it compares the length of the Strings passed to it.

On the topic of Strings: are you sure you want to use them?  Because it looks like you're trying to sort a list of numbers, in which case you should use a LinkedList<Integer> and a Comparator<Integer>, although Collections.sort(List)'s default comparator will do the job fine.  Right now you're calling Collections.sort(List), which sorts in character order, and Collections.sort(List, Comparator) using your String length comparator, which sorts by length.  This puts them in numerical order:

LinkedList before sort:     [75, 2, 0, 1, 10, 25, 60, 40, 70, 2, 3, 4, 5, 6, 10, 15]
LinkedList default sort:    [0, 1, 10, 10, 15, 2, 2, 25, 3, 4, 40, 5, 6, 60, 70, 75]
LinkedList comparator sort: [0, 1, 2, 2, 3, 4, 5, 6, 10, 10, 15, 25, 40, 60, 70, 75]

I think what you're really looking to do is something like this:

import java.util.Collections;
import java.util.LinkedList;
import java.util.ListIterator;

public class LinkedListSort {
    public static void main(String[] args) {
        LinkedList<Integer> linkedList = new LinkedList<>();
        linkedList.add(1);
        linkedList.add(2);
        linkedList.add(3);
        linkedList.add(4);
        linkedList.add(5);
        linkedList.addFirst(0);
        linkedList.add(6);
        linkedList.addFirst(2);
        linkedList.add(3, 10);
        linkedList.add(4,25);
        midpointIterator( linkedList ).add( 60 );
        linkedList.add(10);
        linkedList.add(15);
        linkedList.add(6,40);
        midpointIterator( linkedList ).add(70);
        linkedList.addFirst(75);

        System.out.println("LinkedList before sort:  " + linkedList);
        Collections.sort(linkedList);
        System.out.println("LinkedList default sort: " + linkedList);
    }

    private static <E> ListIterator<E> midpointIterator(LinkedList<E> lList) {
        return lList.listIterator( lList.size() / 2 );
    }
}


CommandBinding refresh on parameter changed I need the command I am executing on a datagrids context menu to know which column was clicked on.

XAML

<DataGrid x:Name="dataGrid" ItemsSource="{Binding Table}" ContextMenuOpening="DataGrid_ContextMenuOpening">
    <DataGrid.ContextMenu>
        <ContextMenu>
            <MenuItem Header="Add Column" Command="{Binding AddColumn}"/>
            <MenuItem Header="Remove Column" Command="{Binding RemoveColumn}"/>
        </ContextMenu>
    </DataGrid.ContextMenu>

</DataGrid>

Code behind:

private void DataGrid_ContextMenuOpening(object sender, ContextMenuEventArgs e)
{
    var current = e.OriginalSource as DependencyObject;
    while(current != null && !(current is DataGridCell))
    {
        current = VisualTreeHelper.GetParent(current);
    }
    var cell = current as DataGridCell;
    foreach (var item in dataGrid.ContextMenu.Items.OfType<MenuItem>())
    {
        item.CommandParameter = cell.Column.DisplayIndex;
        (item.Command as DelegateCommand<object>)?.RaiseCanExecuteChanged();
    }
}

I'm having to break MVVM by having the view tell the VM command that its execution has changed because the command binding doesn't seem to refresh on the parameter changing. Is there a better way to do this?

I'm having to break MVVM by having the view tell the VM command that its execution has changed because the command binding doesn't seem to refresh on the parameter changing.

The fundamental problem here seems to be that MenuItem tests whether (in effect) Command.CanExecute(CommandParameter) to update its own Enabled property when Command changes, but not when CommandParameter changes.

It seems to me that the least inelegant way of working around this (IMO) bug in WPF is to subclass MenuItem and use DependencyProperty.OverrideMetadata to register a callback on CommandParameterProperty which updates an overridden IsEnabledCore property. To really do it properly you'd want to also override metadata for CommandTargetProperty and handle RoutedCommand specially, as in CommandHelpers.CanExecuteCommandSource.

An alternative approach, which would be hackier and require more XAML but wouldn't get quite so deep into the inner workings of WPF, would be an IMultiValueConverter which takes two parameters (the command and the parameter) and returns a parameterless command. Then you could set up multibindings such that a change in the parameter causes the MenuItem's Command to change, triggering its CanExecute test.

Both of these approaches would require changes in the XAML of all the grids, but no per-grid codebehind. They would also eliminate the assumption, which IMO is one of the nastiest problems of the codebehind in the question, that all commands are instances of DelegateCommand<object>.

(The other nasty problem is that if the while loop is broken because current == null, the body of the second loop will throw a NullReferenceException).


Add index to duplicate entry in List I wrote this piece of code to append index to any duplicate entry in List<T> like in the sample below.

Input

["dupe", "not dupe", "dupe", "also not dupe"]

Output

["dupe1", "not dupe", "dupe2", "also not dupe"]

But the problem is it's O(n^2) in the second loop and I'm working on a large list (30+ items). I'm very new to C# and didn't know much built-in functions, so I wanted to know is there anything I can do to optimize this?

List<string> duplicate = new List<string>(); // For storing duplicate item
IEnumerable<IGrouping<string, string>> g = header.GroupBy(i => i);

// Find duplicate and store it in duplicate
foreach (var item in g)
    if (item.Count() > 1)
        duplicate.Add(item.Key);

// Loop through duplicate to append the index
foreach (var item in duplicate) {
    int start = 1;
    for (int i = 0; i < header.Count; i++) {
        // If header[i] is a duplicate item append index to it.
        if (string.Equals(header[i], item, StringComparison.OrdinalIgnoreCase)) {
            header[i] = string.Format("{0}{1}", header[i], start);
            start++;
        }
    }
}

If you collect the indices along with the duplicates, you don't need to loop over the list again:

var header = new List<string>();
var duplicates = header.Select((x, index) => new { x, index }).ToLookup(x => x.x, x => x.index);
foreach(var duplicate in duplicates.Where(x => x.Count() > 1)) {
  var start = 1;
  foreach(var index in duplicate) {
    header[index] = $"{duplicate.Key}{start++}";
  }
}

This uses string interpolation ($-strings) as a shorthand for string.Format.

On premature optimisation:

You seem concerned about the \$O(n^2)\$ performance of your solution. At the same time you mention your actual input will have ~30 items. Before spending too much time worrying about the efficiency of your code, did you try to find out whether your current solution is problematic? It seems to me that the list is small enough that even a less efficient solution should be quick enough.

Before trying to optimise specific parts of your code, ask yourself two things:

Do I need to optimise? Is execution time currently a problem?
Where is the efficiency problem in the code?

Before optimising, profile your program to figure out where the problems actually are. Before that, efficiencies like  \$O(n^2)\$  or \$O(n \log n)\$ are purely academic.


Multiplying two arrays I would like to ask how I can make my code more simple and effective. I know that this code can be 100% better. I am supposed to load number for each array and then take the 2 arrays (4*3 and 3*4) and multiply them into 3*3 array.

The main point is making thescanf_s for both int a and int b in one "piece of code."

#include "stdafx.h"
#include <cstdlib>
#include <iostream>
#include <cmath>
using namespace std;

int main()
{
    int a[3][4];
    int b[4][3];
    int c[3][3] = { {0, 0, 0}, {0, 0, 0}, {0, 0, 0} };

    for (int j = 0; j != 4; j++)
    {
        for (int l = 0; l != 4; l++)
        {
            cout << "Zadej a[" << j << "," << l << "]: ";
            scanf_s("%d", &a[j][l]);
        }
    }

    for (int j = 0; j != 4; j++)
    {
        for (int l = 0; l != 4; l++)
        {
            cout << "Zadej b[" << j << "," << l << "]: ";
            scanf_s("%d", &b[j][l]);
        }
    }

    for (int j = 0; j != 3; j++)
    {
        for (int l = 0; l != 3; l++)
        {
            ;
            printf(" |%d| ", c[j][l] += a[j][l] * b[j][l]);
        }
        printf("\n");
    }

    return 0;
}

Don't put everything into one big main()

If you can separate out the reading of inputs and writing of results from the actual multiplications, then it will be easier to test the multiplication code separately.

Avoid non-standard libraries

Here we have "stdafx.h" and scanf_s that aren't part of standard C++.  Ditch those and use the standard facilities (e.g. std::cin >> a[j][l]).

Include what you use

We don't seem to use <cmath> anywhere, so let's drop that.  We'll need <cstdio> for std::printf() - or switch to C++ style output using <iostream>.

Avoid using the whole std namespace

The std namespace isn't one of the few that's designed to be imported wholesale like that, and there's potential for name conflicts when moving to a new standards version.  Specify just the names you need, or just get used to writing std:: - it's intentionally very short.

Use C++ collections

It's easier to work with the C++ collection types such as std::array or std::vector than with raw (C-style) arrays (which decay to pointers when passed as function arguments).