Prime generator for SPOJ I have written code to solve the prime generator problem. I am given an input which contains in the first line the number of test cases, and for each test case, I am given a range and I have to print the list of prime numbers in that range. However, it doesn't pass and says that time limit has exceeded. I tried optimizing the code as best as I could, but I couldn't find any further optimization.

import java.util.LinkedHashSet;
import java.util.Scanner;
import java.util.Set;

class Primes {

private static Set<Long> primes = new LinkedHashSet<Long>();
private static long biggestPrime = 0;

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

    for (int i = 0; i < numberOfTC; i++) {

        long a = scanner.nextInt();
        long b = scanner.nextInt();
        long startFrom = a;

        if (a >= biggestPrime) {
            startFrom = biggestPrime;
        }
        if (biggestPrime == 0) {
            startFrom = 2;
        }
        for (long j = startFrom; j <= b; j++) {
            if (isPrime(j) && j >=a)  {
                System.out.println(j);
            }
        }
        System.out.println();
    }
    scanner.close();

}

private static boolean isPrime(long x) {
    if (x < biggestPrime) {
        if (primes.contains(x)) {
            return true;
        }
        return false;
    }
    for (Long prime : primes)
    {
        if (prime*prime > x ) {
            break;
        }
        if (x % prime == 0) {
            return false;
        }
    }
    if (biggestPrime < x) {
        primes.add(x);
        biggestPrime = x;
    }
    return true;
}
}

The essence of the algorithm used by a sieve is removing the factors of the number. eg. first we remove the factors of 2 ie. 4,6, 8,10... , and then factors of 3 ie. 9 12, 15... so on and so forth.

But applying the same logic for bigger constraints would result in TLE. Thus segment sieve comes into picture.

The segment sieve also applies the same logic BUT on the segment. Thus, Segment Sieve works only if the segment length is within manageable constraints.

Consider a segment given by [m,n]. We remove all the factors which are greater than m but less than n. To check whether a number between m and n is a factor, we use a boolean array(is_factor)...whereis_factor[i] denotes whether the number which at a offset of i from lower bound of the segment(m). Now the factors are eliminated by considered all the numbers which are less than the sqrt(n).

Let's consider an example to illustrate my point.

Suppose m=1000 and n=1100 So we will remove all the factors of x( where x ranges from 2 to sqrt(1000)).

Therefore, When x=2---> 1000,1002,1004,....1100 get crossed..(corresponding is_factor gets marked).

When x=3 --> 1002,1005,1008,....1098 get crossed.. And the process continues.

So the ones which  remain unmarked are the primes in the interval [m,n].
This link illustrates a C++ program for solving the problem Prime 1 SPOJ
. Hope this helps mate :)


Generic Adapter with search and sort functionality I have tried to create a GenericAdapter that can help you search, sort automatically without writing a lot of code.

But in order to use this you have to follow some rules. How can I make it better and where am I not using a better approach.

rules for using GenericAdapter.

extends your business model with BaseModel
extends your viewHolder with BaseViewHolder
your adapter must extends GenericAdapter

where

BaseModel.class

public abstract class BaseModel<T> implements Comparator<T>
{
   public abstract String getAttributeToSearch();
   ///public abstract int whatYouWantToCompare();
}

BaseViewHolder.class

public abstract class BaseViewHolder<T extends BaseModel> extends RecyclerView.ViewHolder
{

  public BaseViewHolder(View itemView) {
    super(itemView);
  }

  public abstract void bindData(T data);
}

GenericBaseAdapter.class

public abstract class GenericBaseAdapter<T extends BaseModel, U extends BaseViewHolder> extends RecyclerView.Adapter<U> implements Filterable {

    private Context mContext;
    private List<T> itemList;
    private List<T> origList;

    public GenericBaseAdapter(Context mContext, List<T> itemList) {
        this.mContext = mContext;
        this.itemList = itemList;
        this.origList = itemList;
    }

    @Override
    public void onBindViewHolder(U holder, int position) {
        holder.bindData(itemList.get(position));
    }

    @Override
    public int getItemCount() {
        return itemList.size();
    }

    public T getItem(int position) {
        return itemList.get(position);
    }

    public void setItems(List<T> itemList) {
        this.itemList = itemList;
        this.origList = itemList;
        notifyDataSetChanged();
    }

    @Override
    public Filter getFilter() {
        return new Filter() {
            @Override
            protected FilterResults performFiltering(CharSequence constraint) {
                final FilterResults oReturn = new FilterResults();
                final ArrayList<T> results = new ArrayList<>();
                if (origList == null)
                    origList = new ArrayList<>(itemList);
                if (constraint != null && constraint.length() > 0) {
                    if (origList != null && origList.size() > 0) {
                        for (final T cd : origList) {
                            if (cd.getAttributeToSearch().toLowerCase()
                                    .contains(constraint.toString().toLowerCase()))
                                results.add(cd);
                        }
                    }
                    oReturn.values = results;
                    oReturn.count = results.size();//newly Aded by ZA
                } else {
                    oReturn.values = origList;
                    oReturn.count = origList.size();//newly added by ZA
                }
                return oReturn;
            }

            @SuppressWarnings("unchecked")
            @Override
            protected void publishResults(final CharSequence constraint,
                                          FilterResults results) {
                itemList = new ArrayList<>((ArrayList<T>) results.values);
                // FIXME: 8/16/2017 implement Comparable with sort below
                ///Collections.sort(itemList);
                notifyDataSetChanged();
            }
        };
    }

    public void sortItems(T obje) {
        Collections.sort(itemList, obje);
        notifyDataSetChanged();
    }
}

for sorting use

mAdapter.sortItems(productList.get(0));

for searching..

mAdapter.getFilter().filter("any string to search");

Here are some suggestions take what you like from it:

You can make BaseModel an interface. This gives the added benefit that your models do not need to extend your abstract class.
In your getFilter method in a code review I'd ask you to put curly braces around your first if.

if (origList == null) {
    origList = new ArrayList<>(itemList);
}
You can instantiate your filter once as a member of the class, rather than instantiating a new one every time the function getFilter is called.
Use collections.unmodifiablelist to protect yourself from unwanted mutations of the original list, when creating or assigning a new list. This is just a precautionary suggestion that has performance implications. Your not mutating when you should not be.


Occurrences (frequency count) - exercise 3 ch. 3 in "ansi common lisp" Looking for hints on style and clarity re. below. While I'm not saying speed is irrelevant, it's less of a priority than style & avoiding bad habits at this point. Focus is on clean use of fundamentals of the language.

Summary of task:

  Take a list and return a list indicating the number of times each (eql)
    element appears, sorted from most common element to least common."

Example run with required output:

> (occurrences '(a b a d a c d c a))
((A . 4) (C . 2) (D . 2) (B . 1))

My solution:

(defun occurrences (lst)
  "Takes a list and returns a list indicating the number of times each (eql)
   element appears, sorted from most common element to least common."
  (occurrences2 lst (mapcar #'(lambda (x) (cons x 0))
                            (remove-dups lst))))

(defun occurrences2 (lst res)
  "Update res, an alist which already contains all needed entries with all
   values set to zero, with frequencies of occurrence as found in lst"
  (if (null lst)
      res ; todo: add a sort here on cdr of each element
      (occurrences2 (cdr lst)
                    (mapcar #'(lambda (x)
                            (if (eql (car x) (car lst))
                                (cons (car x) (+ (cdr x) 1))
                                x))
                         res))))

(defun remove-dups (lst)
  (if (null lst)
      nil
      (adjoin (car lst) (remove-dups (cdr lst)))))

Use primitive functions and operators as much as possible

You are defining remove-dups, and in this function you use the primitive adjoin, which adds an element to a list if not already present. But in Common Lisp the primitive function remove-duplicates is also available, that returns a list without duplicates.

Instead of (+ expression 1) use the primitive 1+ function: (1+ expression).

To iterate over a list there are convenient primitives iterative constructs, like dolist and loop (see below).

Alists

If you want to use alists, it can be more clean to use the primitive operators already defined on them, like acons, pairlis, assoc. For instance, the program could be simplified in this way:

(defun occurrences (lst)
  (let* ((elements (remove-duplicates lst))
         (alist (pairlis elements (make-list (length elements) :initial-element 0))))
    (loop for x in lst
          do (incf (cdr (assoc x alist))))
    alist))

Note that the counters inside the alist are incremented with incf; in fact another suggestion of mine is “don’t by shy to use modifying primitives”, when you are modifying things locally to some function and are sure no undesirable side-effects arise.

Hash tables

Another suggestion is: use the data structures for the task at hand. In Common Lisp there are hash tables, which are ideal for problems like yours. For instance:

(defun occurrences (lst)
  (let ((table (make-hash-table)))
    (loop for x in lst
          do (incf (gethash x table 0)))
    (loop for k being the hash-key of table
          using (hash-value v)
          collect (cons k v))))

This is of course the most efficient solution of all, since it is of O(n), and it scans only once the input list. At the end you could sort the elements returned by their car, and remember always that all the primitive functions that need to do comparisons use by default eql but can be called with an extra keyword parameter :test to chose another comparison predicate, as for instance equal to compare complex values as lists. This applies to remove-duplicates, assoc and make-hash-table.


Merging multiple sorted arrays into one sorted array I would like to merge multiple arrays that are already sorted, into one sorted array.

My arrays look like this :

{
  "arrays": [
    [
      {
        "type": "tag",
        "position": "start",
        "text": true,
        "offset": 1280,
      }
      {
        "type": "tag",
        "position": "end",
        "text": true,
        "offset": 1350,
      }
    ],
    [
      {
        "type": "tag",
        "position": "start",
        "text": false,
        "offset": 1121,
      },
      {
        "type": "tag",
        "position": "end",
        "text": true,
        "offset": 1300,
      }
    ],
    ],
...
}

My current implementation looks like this :

function getMinFromArrays(arrays, state) {
    let minIndex = -1;
    for (let i = 0, l = arrays.length; i < l; i++) {
        if (state[i] >= arrays[i].length) {
            continue;
        }
        if (minIndex === -1) {
            minIndex = i;
        }
        if (arrays[i][state[i]].offset < arrays[minIndex][state[minIndex]].offset) {
            minIndex = i;
        }
    }
    if (minIndex === -1) {
        throw new Error("minIndex negative");
    }
    return minIndex;
}

module.exports = function (arrays) {
    const totalLength = arrays.reduce(function (sum, array) {
        return sum + array.length;
    }, 0);

    const resultArray = new Array(totalLength);

    const state = arrays.map(function () { return 0; });

    let i = 0;

    while(i <= totalLength - 1) {
        const arrayIndex = getMinFromArrays(arrays, state);
        resultArray[i] = arrays[arrayIndex][state[arrayIndex]];
        state[arrayIndex]++;
        i++;
    }

    return resultArray;
};

I think It might be possible to optimize this code (sometimes, it seems to be quite slow (70ms for 10 arrays of size 5000 each).

The built-in facilities should take care of this for you.
You can just merge them with concat and then use Array's sort method, which takes an optional custom comparator (in your case, "compare by offset").  This should run in O(n log(n)):
function merge(arrays) {
  return arrays.reduce((m,x) => m.concat(x), [])
               .sort((a,b) => a.offset - b.offset);
}

NOTE: This doesn't take full advantage of the fact that the individual arrays are already sorted (which will give an O(n) solution), but it's so much simpler it's worth checking if it meets your needs before optimizing further.
UPDATE
I went ahead and did a rewrite with some guesses at micro-optimizations that might help (or might not).  Try it on your sample data and see what happens.
The code is a bit more compact than your original, but still not what I consider highly readable.  I had to sacrifice some readability to write this in an optimized, procedural way:
function merge(arrays) {

  var indexes = Array(arrays.length).fill(0);
  var result  = [];
  var minIndex, minValue, minObject; // for minByOffset method

  const totalLength    = arrays.reduce((sum, arr) => sum + arr.length, 0);
  const infiniteOffset = {offset: Infinity};
  const nextUnusedElm  = (arr,i) => arr[indexes[i]] || infiniteOffset;

  while (result.length < totalLength) {
    var candidates = arrays.map(nextUnusedElm);
    minByOffset(candidates);
    indexes[minIndex]++;
    result.push(minObject);
  }

  return result;

  function minByOffset(arr) {
    minValue = Infinity;
    arr.forEach((x,i) => {
      if (x.offset >= minValue) return;
      minIndex = i;
      minObject = x;
      minValue = x.offset;
    });
  }
}

A final optimization you could make (again, it may or may not have much of an effect) is to remove an array once all of its elements have been used up.  That is, restructure the code to avoid some unnecessary comparisons.  In your code, it would avoid:
if (state[i] >= arrays[i].length) {

In my version, it would avoid:
arr[indexes[i]] || infiniteOffset;

That is, an extra array lookup and a comparison inside the loop.  These are fast operations, so the optimization may not matter...


X86 Feature detection in preparation for Long Mode These are the three steps that need to be taken in order to successfully activate Long Mode. Error reporting is sparse as most real hardware for developing an OS is 64 bit but probably still a good idea to implement this verification procedure.

I did a little tweaking of some of the examples that can be found on the web either in whole or in part and believe 74 bytes might be as tight as it can get.

    PageZero    equ 0xb800
        ID  equ 0b100000

; AMD 24592 Rev 3.14 2007 Feature detection. Section 3.6 pg 74
; ------------------------------------------------------------

  ; A: Test if CPU supports CPUID

    pushfd          ; Save a current copy of flags

    ; Create a simplified pointer that becomes essentially SS:BX

    push    ss
    mov     bx, sp      ; BX will point to bits 23-16 of EFLAGS
    pop     ds

    ; Read flags, toggle (ID) and write back to flags.

    pushfd                  ; Get another copy of EFLAGS
    or      byte [bx], ID   ; Toggle bit 21 (ID) [bit 5 of BX]
    mov     al, [bx]        ; Save a copy
    popfd                   ; Write value back to EFLAGS

    ; Read flags again and if ID remains inverted, then processor supports CPUID.

    pushfd
    pop     dx              ; DX = Bits 15 - 0 of EFLAGS
    pop     dx              ; DX = Bits 16 - 31
    popfd                   ; Restore flags to original values
    xor     al, dl          ; Bit 4 of AL & DL is (ID)
    jz      TstExt

; As this error is very improbable, near center bottom there will be an uppercase "E"
; yellow on red, with two red flashing bars on each side.

FncErr: mov di, (22*80+39)*2 ; Point to center position of 21th row in video
        mov ax, PageZero     ; Page 0 of 80x26x16 (Mode 3)
        mov es, ax
        mov eax, 0x2e4684b1 ; This makes the desired character combination work
        stosd
        stosw

        cli
        hlt
        jmp     $ - 1       ; Just hang.

  ; B: Are extended functions supported

TstExt:
    mov     eax, 0x80000000
    push    eax
    cpuid           ; Extended function limit
    pop     edx
    cmp     eax, edx
    jb      FncErr

  ; C: Does processor support Long Mode.

    mov     al, 1       ; Set EAX = 0x80000001
    cpuid
    bt      edx, 29     ; 64 bit available if bit is on.
    jnc     FncErr

Documentation for this flag reads:

  If a software procedure can set and clear this flag, the processor executing the procedure supports the CPUID instruction

This clearly means that the flag does not have a defined value. It could be OFF or ON at any time!

Where your comment says that you "Toggle bit 21 (ID)", the instruction actually sets the bit. I don't think that would be too useful since you don't use the pre-existing value of this bit.
When you exclusive or XOR the registers AL and DL, you should prudently mask away the 7 remaining bits.

All the forementioned boils down to the fact that the final test (notice the typo in the comment; should be bit 5)

xor     al, dl          ; Bit 4 of AL & DL is (ID)
jz      TstExt

will always check out fine.

Although your method to get a DS:BX pointer is quite ingenious, I prefer using [bp-2] as you can see in below code.

    mov     bp, sp
    pushfd
    mov     al, [bp-2]        ;Pre-existing value
    xor     byte [bp-2], ID   ;Toggle bit 21
    popfd
    pushfd
    mov     ah, [bp-2]        ;Hopefully changed value
    popfd
    and     ax, (ID<<8)+ID    ;Mask off unimportant bits
    cmp     al, ah
    jne     TstExt

FncErr:

I omitted preserving the ID bit as per definition this should not be necessary.

FncErr: mov di, (22*80+39)*2 ; Point to center position of 21th row in video

The address points to the 23rd row.

cmp     eax, edx   <<< EDX=0x80000000
jb      FncErr

Are you sure the service can't report EAX=0x80000000 ?
Perhaps it could be better to bail out with:

jbe     FncErr

Now you're certain that leaf 0x80000001 exists.

Reaching leaf 0x80000001 through mov al, 1 will continue to work for as long as the manufacturer doesn't create some high numbered leafs (0x80000100 and up). You have to ask yourself how future-proof that you want your program to be!


Hackerrank problem -"Another Prime Problem" I came across this issue in the problem titled: Another Prime Problem. Here's my solution with JavaScript which passed test case-1, but for other test cases it led to timeout.
function processData(input) {
       input=input.split('\n');
       input.shift();
       for(var i=0;i<input.length;i++){
          values(input[i]);
       }
    }
    function values(num){
       var sum=0;
       num=num.split(' ');
       for(var i=num[0];i<=num[1];i++){
           for(var j=2;j<=i;j++){
               if(i%j==0 && isprime(j)){
                   sum+=j;
               }
           }
       }
       console.log(sum)
    }
    function isprime(val){
       let flag=1;
       for(var i=2;i<val;i++){
            if(val%i==0){
               flag=0;
            }
       }
       if(flag==1){
          return true;
       }
       else{
           return false;
       }
    }

What's the issue in this code that leads to timeout?
The above program has a very bad time complexity, I guess due to multiple loops and functions, but not being much experienced with algorithms this is only solution I can think of right now. Any help would be appreciated.
Additional info:
Problem statement in a nutshell: find the sum of prime factors of each number in the range [X,Y].
Input Format:
The first line contains T denoting the total number of testcases.
First line of each testcase contains two integers X and Y.
Constraints:
1 ≤ T ≤ 10000
2 ≤ X ≤ Y ≤ 10^5
Output Format:
Sum of prime factors of each number in the range [X, Y].
Currently my code calculates the sum of primes (i know this cause I'm able to pass first test case) but the remainder of test cases lead to Timeout.

An immediate problem is that is_prime() is expensive, and you call it too many times. Prepare a sufficiently large list of primes in advance (and use a sieve for that).

This will give you a certain performance boost, but I am positive it will not be enough to avoid timeouts. The real problem is with the algorithm: you try to factorize the numbers, and the factorization is hard.

Instead of bruteforcing, do the math homework: how many numbers in the range have a prime \$p\$ as a factor? Hint: if there are N of them, they'd contribute N * p to the answer.


Project Euler #4: Largest palindrome product How can I make my code for Project Euler #4 more efficient and cleaner?

Project Euler #4: Largest palindrome product:

  A palindromic number reads the same both ways. The largest palindrome
  made from the product of two 2-digit numbers is 9009 = 91 × 99.

  Find the largest palindrome made from the product of two 3-digit
  numbers.

public class LargestPalindromeProduct {
    public static void main(String[] args) {

        int product = 0;
        int largest = 0;
        for (int first = 1; first < 1000; first++) {
            for(int second = 1; second < 1000; second++) {
                product = first * second;
                if ((product / 100000) % 10 == product % 10 && product / 10000 % 10 == product / 10 % 10 && product / 1000 % 10 == product / 100 % 10) {
                    //System.out.println(product);
                    if (product > largest) {
                        largest = product;

                    }
                }

            }
        }
        System.out.println(largest);
    }
}

For increased performance I think it might make sense to enhance your loop:

Start with 999 each and count downwards, as the largest product will most likely involve bigger numbers. This allows you to implement a strategy for pruning and return earlier (see other answers and comments for details).
Count only in the range of 100-999, since 1-99 aren't 3-digit numbers.

For a cleaner code you should refactor your if clause into a separate method. Something like

private boolean isPalindrome(int product) {
    return ((product / 100000) % 10 == product % 10
         && (product / 10000) % 10 == (product / 10) % 10
         && (product / 1000) % 10 == (product / 100) % 10);
}

A cleaner code would for me also involve consistent formatting and use of brackets, e.g.

(product / 100000) % 10 vs product / 10000 % 10
for (int first = 1 vs for(int second = 1


Splitting a list by indexes I am trying to build an efficient function for splitting a list of any size by any given number of indices. This method works and it took me a few hours to get it right (I hate how easy it is to get things wrong when using indexes).

Am I over-thinking this? Can you help me minimize the code? I am just not seeing it. ALSO, is there any way to simplify the input of so many variables such as in this situation?

def lindexsplit(List,*lindex):

    index = list(lindex)

    index.sort()

    templist1 = []
    templist2 = []
    templist3 = []

    breakcounter = 0
    itemcounter = 0
    finalcounter = 0

    numberofbreaks = len(index)
    totalitems = len(List)

    lastindexval = index[(len(index)-1)]
    finalcounttrigger = (totalitems-(lastindexval+1))

    for item in List:

        itemcounter += 1

        indexofitem = itemcounter - 1

        nextbreakindex = index[breakcounter]

        #Less than the last cut
        if breakcounter <= numberofbreaks:

            if indexofitem < nextbreakindex:

                templist1.append(item)

            elif breakcounter < (numberofbreaks - 1):

                templist1.append(item)

                templist2.append(templist1)

                templist1 = []

                breakcounter +=1

            else:

                if indexofitem <= lastindexval and indexofitem <= totalitems:

                    templist1.append(item)

                    templist2.append(templist1)

                    templist1 = []

                else:

                    if indexofitem >= lastindexval and indexofitem < totalitems + 1:

                        finalcounter += 1

                        templist3.append(item)

                        if finalcounter == finalcounttrigger:

                            templist2.append(templist3)

    return templist2

Example:

mylist = [1,2,3,4,5,6,7,8,9,0,10,11,12,13,14,15]
lindexsplit(mylist,3,5,8)
return [[1, 2, 3, 4], [5, 6], [7, 8, 9], [0, 10, 11, 12, 13, 14, 15]]

There is a lot simpler way to do this. You can use list slicing and the zip function.

List slicing essentially cuts up a given list into sections. The general form is list[start:stop:step]. The start section of a slice designates the first index of the list we want to included in our slice. The stop section designates the first index of the list we want excluded in our slice. The step section defines how many indices we are moving as well as in which direction (based on whether it is positive or negative). An example:

>>> x = [1, 2, 3, 4]
>>> x[1:3]
[2, 3]
>>> x[2:]
[3, 4]
>>> x[0:4]
[1, 2, 3, 4]
>>> x[0:4:1]
[1, 2, 3, 4]
>>> x[0:4:2]
[1, 3]
>>> x[0:4:3]
[1, 4]
>>> x[0:4:4]
[1]
>>> x[0:4:5]
[1]

The zip function takes sequences and creates a zip object that contains tuples of their corresponding index elements:

>>> for pair in zip([1, 2, 3], ['a', 'b', 'c']):
...    print(pair)
(1, 'a')
(2, 'b')
(3, 'c')

You can combine these two strategies to simplify your function. Here is my version of your lindexsplit function:

def lindexsplit(some_list, *args):
    # Checks to see if any extra arguments were passed. If so,
    # prepend the 0th index and append the final index of the
    # passed list. This saves from having to check for the beginning
    # and end of args in the for-loop. Also, increment each value in
    # args to get the desired behavior.
    if args:
        args = (0,) + tuple(data+1 for data in args) + (len(some_list)+1,)

    # For a little more brevity, here is the list comprehension of the following
    # statements:
    #    return [some_list[start:end] for start, end in zip(args, args[1:])]
    my_list = []
    for start, end in zip(args, args[1:]):
        my_list.append(some_list[start:end])
    return my_list


Django query_set filtering in the template Can I make my template syntax simpler? I'm hoping to eliminate the if and maybe also the for block.

This worked in the shell but I can't figure out the template syntax.

recipes[0].recipephotos_set.get(type=3).url

model.py

class Recipe(models.Model):
    ....

class RecipePhotos(models.Model):
    PHOTO_TYPES = (
    ('3', 'Sub Featured Photo: 278x209'),
    ('2', 'Featured Photo: 605x317'),
    ('1', 'Recipe Photo 500x358'),
    )
    recipe = models.ForeignKey(Recipe)
    url = models.URLField(max_length=128,verify_exists=True)
    type =  models.CharField("Type", max_length=1, choices=PHOTO_TYPES)

view.py

recipes = Recipe.objects.filter(recipephotos__type=3)

template.html

{% for recipe in recipes %}

      {% for i in recipe.recipephotos_set.all %}
          {% if i.type == '3' %}
              {{ i.url }}
          {% endif %}
      {% endfor %}

  {{ recipe.recipe_name }}

{% empty %}

I'll refer you to a Stack Overflow post that pretty much nails the answer.

I assume that you want to display all recipes that have "Sub Featured Photos".

The call recipes = Recipe.objects.filter(recipephotos__type=3) will give you a queryset of recipes that have at least one photos with type 3. So far so good. Note, that this code is in the views.py file and not in the template. Like the StackOverflow post mentioned, you should put the filtering code in your view or model.

Personally I'd prefer writing a function for your model class:

class Recipe(models.Model):
    (...)
    def get_subfeature_photos(self):
        return self.recipephotos_set.filter(type="3")

And access it in the template like this:

{% for recipe in recipes %}
      {% for photo in recipe.get_subfeature_photos %}
            {{ photo.url }}
      {% endfor %}
      (...)
{% endfor %}

Please note that the function is using filter() for multiple items instead of get(), which only ever returns one item and throws a MultipleObjectsReturned exception if there are more.


Writing PHP PDO bindParameters Is there a better way to write this?

$query = $db->getConnection()->prepare("INSERT INTO `users` (first_name, last_name, email, username, password, unique_id, date_joined) VALUES(:fname, :lname, :email, :username, :pass, :uniqueid, :date_joined)");
$query->bindParam(':fname', $first_name, PDO::PARAM_STR);
$query->bindParam(':lname', $last_name, PDO::PARAM_STR);
$query->bindParam(':email', $email, PDO::PARAM_STR);
$query->bindParam(':username', $username, PDO::PARAM_STR);
$query->bindParam(':pass', $passHash, PDO::PARAM_STR);
$query->bindParam(':uniqueid', $emailHash, PDO::PARAM_STR);
$query->bindParam(':date_joined', $date, PDO::PARAM_STR);
$query->execute();

Since you have PDO::PARAM_STR everywhere, you can just pass all the parameters to the execute:

$query = $db->getConnection()->prepare("INSERT INTO `users` (first_name, last_name, email, username, password, unique_id, date_joined) VALUES(:fname, :lname, :email, :username, :pass, :uniqueid, :date_joined)");
$query->execute(array(
    ':fname' => $first_name,
    ':lname' => $last_name,
    ':email' => $email,
    ':username' => $username,
    ':pass' => $passHash,
    ':uniqueid' => $emailHash,
    ':date_joined' => $date
));

  What about if one wasn't a string? Would the "best" practice be to follow this and just bindParam as usual on the variable that isn't PDO::PARAM_STR?

You can have a custom version of the execute which could work like this:

myCustomExecute($query, array(
    ':fname' => [$first_name, PDO::PARAM_STR]
    ':lname' => [$last_name, PDO::PARAM_STR],
    ':age' => [$age, PDO::PARAM_INT]
));

Inside the myCustomExecute you just loop over the array and do bindValue for each array item (note, it is better to use bindValue instead of bindParam, see for example, this).

But I feel like you want something bigger than this to avoid writing a boilerplate code. In this case your are looking for an ORM.

The practical solution is to search for existing solution and choose the one you like.

Less practical way would be to write something simple around PDO.

The final syntax would be something like this:

// ActiveRecord pattern
$model = new Person();
$person->first_name = 'John';
$person->age = 22;
$person->save();

// OR, in the case of DataMapper pattern, you have separate
// object for your model and another one which handles
// the database operation
$table = new PersonMapper();
$person = new Person();
$person->first_name = 'John';
$person->age = 22;
$table->save($person);

The record class or mapper class usually have some kind of metadata (you define it manually or they get this metadata directly from the database), so they know the type of each filed, as well as some other limitations (like uniqueness checks). And usually you are able to specify some own validation rules to check before the data is saved to the database.

This removes a lot of the boiler plate, because with plain PDO you have to repeat your fields for times - twice in the SQL (first_name -> :fname) and then twice in the bindValue or execute calls.


Java Implementation of linked list add(i, x) method I'm currently trying to brush up on ADT implementations, specifically an implementation for a Linked List (I'm using Java 5 to do this).

Is this implementation I've written for add(i, x) correct and efficient?

public void add(int i, Object x) {

    // Possible Cases:
    //
    //     1. The list is non-empty, but the requested index is out of
    //        range (it must be from 0 to size(), inclusive)
    //
    //     2. The list itself is empty, which will only work if i = 0
    //
    // This implementation of add(i, x) relies on finding the node just before
    // the requested index i.

    // These will be used to traverse the list
    Node currentNode = head;
    int indexCounter = 0;

    // This will be used to mark the node before the requested index node
    int targetIndex = i - 1;

    // This is the new node to be inserted in the list
    Node newNode = new Node(x);

    if (currentNode != null) {

        while (indexCounter < targetIndex && currentNode.getNext() != null) {

            indexCounter++;
            currentNode = currentNode.getNext();
        }

        if (indexCounter == targetIndex) {

            newNode.setNext(currentNode.getNext());
            currentNode.setNext(newNode);

        } else if (i == 0) {

            newNode.setNext(head);
            head = newNode;
        }

    } else if (i == 0) {

        head = newNode;
    }
}

First: Java-1.5 and a List of Object, not a generic List? But a CS-degree and intermediate level?

Second: Your comment complicates 2 cases: List is empty or not - if not ... - well, if you don't distinguish the cases, from:

// 1. The list is non-empty, but the requested index is out
// of range (it must be from 0 to size (), inclusive)

to:

// it must be from 0 to size (), inclusive.

Now look at condition 2:

// 2. The list itself is empty, which will only work if i = 0

If the size is 0, and i=0, then i is in the range of 0 to 0 inclusive, isn't it? So it is just one condition, and a much shorter condition.

However, the code seems right, and can't be simplified like the condition*), and it isn't that easy, even if it looks easy in the end. Several days seems a lot, but several hours can vanish like nothing, expecially, if you get a wrong start. I remember John Bentley, who wrote, that a simple binary search wasn't correctly implemented by most of the professional developers he interviewed. By none of them, if I remember correctly ('Programming Pearls').

*) This claim isn't proved. Disprove it, friends!

I would expect an IndexOutOfBoundsException if the index is negative or too big. A size-Method would be useful for that.


Function to validate a GUID, an EntityState, and some data In the beginning of my methods that handle the calls from outside, I run this method to make sure that everything's OK, or let the caller know what's wrong:

private static void Validate(Guid userId, EntityState state, object data)
{
  if (userId == Guid.Empty)
    throw new Exception("Token unrecognized!");
  if (state == EntityState.Detached)
    throw new Exception("Action unrecognized!");
  if (data == null)
    throw new Exception("Data unrecognized!");
}

I was happy with the method until R# remarked that I'm declaring the variables without using them, except for pre-condition check. Well, it's true that I do. It's kind of the whole idea.

It bothers me that the tool remarks on it, because it feels like smarter people than me have put in the message there. However, turning it off isn't an option because that'd turn off even the checks for actually unnecessarily declared variables.

Should I refactor the code? How can I improve it?

It makes sense to use these arguments and throw ArgumentException or ArgumentNullException instead of just Exception:

private static void Validate(Guid userId, EntityState state, object data)
{
    if (userId == Guid.Empty)
        throw new ArgumentException("Token unrecognized!", nameof(userId));
    if (state == EntityState.Detached)
        throw new ArgumentException("Action unrecognized!", nameof(state));
    if (data == null)
        throw new ArgumentNullException(nameof(data), "Data unrecognized!");
}

Since the nameof() is a C#6 feature, in the previous versions of C# you should use argument name string instead: "userId", "state", "data".


3 versions of SQL query to find students with simultaneous classes This is a toy example where we have 3 tables:

Student(snum, sname)
Enrolled(snum, cname)
Class(cname, meets_at)

and we want to find the sname of the students that are enrolled in at least
two classes that meet at the same time.

Here are three attempts, which one is best and why ?

select sname, (count(distinct cname) - count(distinct meets_at))
from (select st.*, cl.*
      from Student st, Class cl, Enrolled en
      where st.snum = en.snum
      and cl.cname = en.cname)
group by sname
having count(distinct cname) - count(distinct meets_at) >= 1;

select S.sname
from Student S
where S.snum in (
                select E.snum
                from Enrolled E, Class C
                where E.cname = C.cname
                group by E.snum, C.meets_at
                having count(E.cname) >= 2
                );

select S.sname
from Student S
where S.snum in ( select E1.snum
                  from Enrolled E1, Enrolled E2, Class C1, Class C2
                  where E1.snum = E2.snum and E1.cname <> E2.cname and
                  E1.cname = C1.cname and E2.cname = C2.cname
                  and C1.meets_at = C2.meets_at);

They are all messy.   And please don't use old style joins.

Two is the least messy as it goes at it more directly.

But you can do it with joins

select st.sname, cl.meets_at
from Student st
join Enrolled en
  on en.snum = st.snum
join Class cl
  on cl.cname = en.cname
group by st.sname, cl.meets_at
having count(*) > 1

That will return more than one row per sname if they have more than one class that has overlapping times.  But that is the most direct approach to the logic (in my opinion).

This would be a clean way to go at single sname

select st.sname
from Student st
where exists ( select 1
               from Enrolled en
               join Class cl
                 on cl.cname = en.cname
               where en.snun = st.snum
               group by cl.meets_at
               having count(*) > 1
             )

another - this might be the best efficiency

select st.sname
from Student st
join  ( select cl.snum
        from Enrolled en
        join Class cl
          on cl.cname = en.cname
        group by cl.snum
        having count(*) > 1
      ) cl2
 on st.snum = cl.snum


Passing Node objects into insert routine of linked list I've created a linked list class in Python, but I'm not sure how to enforce only passing Node objects into the insert routine.

# linked list in Python

class Node:
    """linked list node"""
    def __init__(self, data):
        self.next = None
        self.data = data
    def insert(self, n):
        n.next = self.next
        self.next = n
        return n
    def showall(self):
        print self.data
        if self.next != None:
            self.next.showall()

ll = Node("one")
ll.insert(Node("two")).insert(Node("three"))
ll.showall()

You could delegate the construction of nodes to the insert function :

#!/usr/bin/python

# linked list in Python

class Node:
    """linked list node"""
    def __init__(self, data):
        self.next = None
        self.data = data
    def insert(self, data):
        n = Node(data)
        n.next = self.next
        self.next = n
        return n
    def showall(self):
        print self.data
        if self.next != None:
            self.next.showall()

ll = Node("one")
ll.insert("two").insert("three")
ll.showall()

You could make this more concise by adding a default argument to your __init__() :

class Node:
    """linked list node"""
    def __init__(self, data, next=None):
        self.next = next
        self.data = data
    def insert(self, data):
        self.next = Node(data, self.next)
        return self.next
    def showall(self):
        print self.data
        if self.next != None:
            self.next.showall()

ll = Node("one")
ll.insert("two").insert("three")
ll.showall()

Additional point : from PEP 8 :

  Comparisons to singletons like None should always be done with is or
  is not, never the equality operators.


Reading a Bitmap from disk on a separate thread on Android I want to defer the reading of a bitmap to another thread. I'm mainly concerned about concurrency issues since I'm kind of green on that subject, so I would like to know if this code has any potential flaws.

public class SomeActivity extends Activity {

    Bitmap threadBitmap;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        new BitmapTask().execute();
        Button button = (Button) findViewById(R.id.button1);
        button.setOnClickListener(new OnClickListener() {

            @Override
            public void onClick(View v) {
                if(threadBitmap != null) {
                    // Assume bitmap has loaded and do something with it here
                }
            }

        });
    }

    public class BitmapTask extends AsyncTask<Void, Void, Bitmap> {

        @Override
        protected Bitmap doInBackground(Void... params) {
            Bitmap bitmap = null;
            InputStream in = null;

            try {
                in = getAssets().open("mybitmap.png");
                bitmap = BitmapFactory.decodeStream(in);
            } catch (IOException e) {
                e.printStackTrace();
            } finally {
                try {
                    if(in != null) {
                        in.close();
                    }
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }

            return bitmap;
        }

        @Override
        protected void onPostExecute(Bitmap result) {
            threadBitmap = result;
        }

    }

}

I would implement a progress bar to inform the user of the image loading status to eliminate confusion to what it's going on.

Also, you could deactivate the button for as long as the thread is running. And reactivate it in onPostExecute().


Find the longest string without repeating a letter I've written some code to scan large files and search for long strings that you can write without repeating a letter. I'm self-taught in python and I'm wondering what I've missed.

import sys
letters ={} #letters is a dictionary
currentResult=""
ideas=[]
with open(sys.argv[1]) as f:
  while True:
    c = f.read(1)
    if not c:
      print "End of file"
      break
    if c in letters:
        letters={}
        temp=currentResult.rsplit(' ',1)[0]
        ideas.append(temp.rstrip('?:!.,;'))
        currentResult=""
    currentResult+=c
    if not c.isspace():
        letters[c]=True
ideas=sorted(set(ideas))
ideas.sort(lambda x,y: cmp(len(x), len(y)))
for x in ideas:
    print x.strip()

I do believe the algorithm in the pervious answer and in the OP does not do what the OP wanted. It test for the longest string starting with the line's first character, and then repeat from the first non repeating string. For example if the string is

abcaefgb

The algorithm given will return

abc

while the right answer should be

bcaefg

I believe the following algorithm will find the right string. Note that if there are several equal length longest strings it will return the first

def scantillrepeat(line):
    found = ''
    for char in line:
        if not char in found:
            found = found + char
        else:
            break
    return found

def findlongest(f):
    for line in f:
        longestfound = ''
        longestfoundlen = 0
        for k in range(len(line)):
            candidate = scantillrepeat(line[k:])
            if len(candidate) > longestfoundlen:
                longestfound = candidate
                longestfoundlen = len(candidate)
    return longestfound

To test

In [39]: f = ['abcaefgb']

In [40]: findlongest(f)
Out[40]: 'bcaefg'

Note that my version does not allow joining lines separated by newline \n. This should be altered if needed.


Validating and saving a user profile, using fat model vs. view code I heard that fat models are good idea and I should avoid operations on database in my views so I do something like this below. Can you tell me if it's good idea and which version is better? Please also review the rest of the code by the way.

views.py

class CandidateCreateProfileView(AnonymousRequiredMixin, FormView):
    form_class = CandidateCreateProfileForm
    template_name = 'users/candidate_create_profile.html'
    success_url = reverse_lazy('account_login')

    def dispatch(self, request, *args, **kwargs):
        activation_key = self.kwargs['activation_key']
        self.user = get_object_or_404(User, activation_key=activation_key)

        if self.user.key_expires < timezone.now():
            return render_to_response('users/candidate_confirm_expired.html')
        if self.user.is_active:
            return HttpResponseRedirect(reverse_lazy('account_login'))

        return super(CandidateCreateProfileView, self).dispatch(
            request, *args, **kwargs)

    def get_initial(self):
        initial = super(CandidateCreateProfileView, self).get_initial()
        initial['name'] = self.user.name
        initial['mobile_phone'] = self.user.mobile_phone
        return initial

    def form_valid(self, form):
        self.user.name = form.cleaned_data['name']
        self.user.mobile_phone = form.cleaned_data['mobile_phone']
        self.user.notification_type = form.cleaned_data['notification_type']
        self.user.set_password(form.cleaned_data['password'])
        self.user.is_active = True
        self.user.save()

        return super(CandidateCreateProfileView, self).form_valid(form)

vs.

models.py

class User(models.Model):
    name = models.CharField(max_length=50)
    ...
    def create_candidate_profile(self, name, mobile_phone, notification_type, password):
        self.name = name
        self.mobile_phone= mobile_phone
        self.notification_type=notification_type
        self.set_password(password)
        self.is_active = True
        self.save()

and then in my CandidateCreateProfileView view:

def form_valid(self, form):
    self.user.create_candidate_profile(
        name = form.cleaned_data['name'],
        mobile_phone = form.cleaned_data['mobile_phone'],
        notification_type = form.cleaned_data['notification_type'],
        password = form.cleaned_data['password']
    )
    return super(CandidateCreateProfileView, self).form_valid(form)

Yes, using MVC frameworks, or in case of Django modified term MTV (Model-Template-View) you will often hear the expression "Fat models, thin controllers". I do think it is a good idea and the business logic should be put into the model.

In your case the method create_candidate_profile is performing a database operation. More important it changes the database and persists data to it.

Data persistence should be done in the model. So, yes, I'd say the second version is definitely the way to go.

In future you might want to add another possibility for creating candidate profile, maybe through a custom command. Then you can make use of this model method:

User.create_candidate_profile(
    name, mobile_phone, notification_type, password
)

The only line in the code I'd slightly change is the following:

activation_key = self.kwargs['activation_key']

should become:

activation_key = self.kwargs.get('activation_key', 'deafult value')

That is just for the case that self.kwargs doesn't have the key activation_key, so you can pass some default value, maybe an empty string ''.


Connecting to RabbitMQ I would like a review for this connection to RabbitMQ. I just developed it and seems to be working well, but I would like another set of eyes on it before putting this on the sever.

package models

import com.rabbitmq.client.Connection
import com.rabbitmq.client.ConnectionFactory
import com.rabbitmq.client.ConnectionFactory
import com.rabbitmq.client.MessageProperties

import anorm.SQL
import anorm.sqlToSimple
import anorm.toParameterValue
import play.api.Play.current

object RabbitMQConnection {
  private var connection: Connection = null
  def getConnection(ss:Connection): Connection = {
    println(ss+"  connection <<<<<<<<<<<<<<<<<<<<<<<<")
    connection=ss
    connection match {
      case null => {
        val factory = new ConnectionFactory()
        println("waiting for new connection")
        factory.setHost("172.22.22.222")
        println("host setted")
        connection = factory.newConnection()
        println("connection created")
        connection
      }
      case _ =>{
        println("connection is not null")
        connection
      }
    }
  }
}

object RMQ {
  var connection = RabbitMQConnection.getConnection(null)

  def setQ(qName: String, message: String) = {
    println("ping received")
    try {
      println(connection)
      if (connection != null) {
        if (connection.isOpen()) {
          println("connection is open")
        } else {
          connection = RabbitMQConnection.getConnection(null)
          println("connection is new "+connection)
        }
        println("connetion is ready to use")
        val channel = connection.createChannel()
        channel.queueDeclare(qName, true, false, false, null) //suggestion
        channel.basicPublish("", qName, MessageProperties.PERSISTENT_TEXT_PLAIN, message.getBytes())
        println("status" + channel.close())
        println("setQ complete executed for " + qName)
        Map("result" -> "success")

      } else {
        println("connection can't established to rabbit mq for =>" + qName)
        LogFile.QLogs(qName, message)
        Map("result" -> "error")
      }
    } catch {
      case e: Exception =>
        println(e.printStackTrace())
        println("Rabbit Mq Server is Down for =>" + qName)
        LogFile.QLogs(qName, message)
        Map("result" ->"error")
    }
  }

}

Well, the first thing I would recommend is replacing all of those println statements with some kind of logging. In any kind of production environment, logging is something that needs more thought and care so that it is useful, without consuming resources unnecessarily. Or, if they're just for the original developer during development, maybe delete them.

I find the structure of this code to be rather odd. For all the trappings of OO programming, what you essentially have is two global public functions and one global public variable.

The function name setQ doesn't seem to say what you're actually doing - which appears to be creating a channel and publishing a message to it. publishMessage would make more sense, I think.

All of the code that checks the usability of the connection should be extracted into a single function, something along the lines of haveUsableConnection - the two if/else blocks are making it hard to see the code that is actually doing the work.

I question the hard coded IP address. Magic numbers and magic strings are frequently a bad idea.


NSURLSession load images on demand I'm trying to come up with a system to populate my item images on demand, this is what I have so far. One obvious bug is that an item image could be downloaded multiple time since the _image will remain nil until the first download has completed. I could work around this with a loading property.

My main issue is about priority, ideally I would like the images currently visible in the tableview to be downloaded first. ie As soon as the tableview appears it starts downloading the first 20 or so initially visible item.images, but if the users scrolls down to the bottom then items 20-100 all get queued in order which results in the now visible images 80-100 being last to download. Not really sure how to deal with this.

@interface Item : NSObject
@property (nonatomic, strong) UIImage *image;
@end;

@implementation Item

- (UIImage *)image
{
    if (!_image) {

        NSURL *url = [NSURL URLWithString:@"http://loremflickr.com/50/50/paris"];
        NSURLSessionDataTask *imageTask = [[NSURLSession sharedSession] dataTaskWithURL:url completionHandler:^(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error) {
            _image = [UIImage imageWithData:data];
            dispatch_async(dispatch_get_main_queue(), ^{
                [[NSNotificationCenter defaultCenter] postNotificationName:@"ItemUpdated" object:self];
            });

        }];
        [imageTask resume];
    }
    return _image;
}

@end

@interface TableViewController : UITableViewController
@property (nonatomic, strong) NSArray *items;
@end

@implementation TableViewController

- (void)viewDidLoad
{
    [super viewDidLoad];

    NSMutableArray *items = [[NSMutableArray alloc] initWithCapacity:100];
    for (int i=0; i<100; i++) {
        Item *item = [[Item alloc] init];
        [items addObject:item];
    }
    self.items = items;
}

- (void)viewWillAppear:(BOOL)animated
{
    [super viewWillAppear:animated];
    [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(itemUpdated:) name:@"ItemUpdated"  object:nil];
}

- (void)viewWillDisappear:(BOOL)animated
{
    [super viewWillDisappear:animated];
    [[NSNotificationCenter defaultCenter] removeObserver:self name:@"ItemUpdated"  object:nil];
}

- (void)itemUpdated:(NSNotification *)notification
{
    Item *item = (Item *)notification.object;

    NSUInteger index = [self.items indexOfObject:item];
    NSIndexPath *indexPath = [NSIndexPath indexPathForRow:index inSection:0];
    [self.tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationFade];
}

#pragma mark - Table view data source

- (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView
{
    return 1;
}

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    return self.items.count;
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    Item *item = [self.items objectAtIndex:indexPath.row];

    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"Cell" forIndexPath:indexPath];
    cell.imageView.image = item.image;
    cell.textLabel.text = [NSString stringWithFormat:@"Item %d", indexPath.row];

    return cell;
}

@end

I have a possible solution for your case, this is what I am thinking.
1) You can implement scrollViewDidEndDragging delegate and load your image for the visible cells.
for ex.

  - (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView
    {
            [self loadImagesForOnScreenRows];
    }

loadIamgesForOnScreenRows  will look like:

- (void)loadImagesForOnScreenRows
{

    NSArray *visiblePaths = [self.tracksListing indexPathsForVisibleRows];

    for (NSIndexPath *indexPath in visiblePaths){
            [[self.tableView cellForRowAtIndexPath:indexPath] bindItemObject:record];
        }
    }
}

In my case bindItemObject is for downloading the image implemented in tableViewCell class, and record contains the url for the image.

The only problem which you will encounter in this case is, when the tableView appears for the first time.
In that case I suggest you to programmatically call the

[self.tableView scrollToRowAtIndexPath:indexPath
                          atScrollPosition:UITableViewScrollPositionTop
                                  animated:YES];

and give indexpath as indexpath for row:0 and section:0

I think this can solve your problem.


Dynamically change attribute value with split() (JavaScript) What I want to do

Make my code cleaner.

What my code does

Thanks to Stack overflow, I finally wrote a code which creates option tags dynamically (OptionTime). The value of data-range attribute for Year was static. I wanted to set the years range dynamically, and I did it (See between //start and //End). However, I feel my code is lengthy. Can anyone make my code better, please?

Here is my code

// Start
/* Fetch DOM */
const selectYear = document.querySelector('select[name="Year"]');

/* Set years */
const now = new Date();
const year = now.getFullYear();
const startYear = year - 50;
const endYear = year - 18;

/* Change Year's data-range */
function overwriteYears() {
  let dataRange = selectYear.dataset.range;
  let [start, end, unit] = dataRange.split(' ');
  start = startYear;
  end = endYear;
  selectYear.setAttribute('data-range', `${start} ${end} ${unit}`);
}
overwriteYears();
// End

/* Create <option> tags */
class OptionTime extends HTMLSelectElement {
  constructor() {
    super();
  }
  connectedCallback() {
    let [start, end, unit] = this.dataset.range.split(' ');

    for (let i = start; i <= end; i++) {
      this.add(new Option(`${i}${unit}`, `${i}${unit}`));
    }
  }
};
customElements.define('option-time', OptionTime, {
  extends: 'select'
});
<select is="option-time" name="Year" data-range="%startYear% %endYear% 年">
  <option value="" disabled selected>Year</option>
</select>
<select is="option-time" name="Month" data-range="1 12 月">
  <option value="" disabled selected>Month</option>
</select>
<select is="option-time" name="Day" data-range="1 31 日">
  <option value="" disabled selected>Day</option>
</select>

I hope this helps. Made some comments in the snippet.

/* Create <option> tags */
class OptionTime extends HTMLSelectElement {
  constructor() {
    super();
    this.setDateRange();
  }

  setDateRange() {
    const now = new Date();
    const year = now.getFullYear();
    const unit = this.dataset.range.split(' ')[2];
    const startYear = year - 50; // No need to deconstruct start/end and assign years again.
    const endYear = year - 18;

    // `this` give access to HTML element directly.
    this.setAttribute('data-range', `${startYear} ${endYear} ${unit}`);

    for (let i = startYear; i <= endYear; i++) {
      this.add(new Option(`${i}${unit}`, `${i}${unit}`));
    }
  }
};

customElements.define('option-time', OptionTime, {
  extends: 'select'
});

<select is="option-time" name="Year" data-range="%startYear% %endYear% 年">
  <option value="" disabled selected>Year</option>
</select>
<select is="option-time" name="Month" data-range="1 12 月">
  <option value="" disabled selected>Month</option>
</select>
<select is="option-time" name="Day" data-range="1 31 日">
  <option value="" disabled selected>Day</option>
</select>


Sorting a 2D array on 1 dimension Inspired by a previous question, I was looking into converting a table to a sorted list (ascending) -

Resulting in this -

I used a very simple bubble-sort, but I don't know how efficient that would be with larger data sets, either in records or if I wanted to sort on more than one condition. Basically, this was an exercise in using arrays when built-in excel tools would be better.

Option Explicit

Public Sub GetBids()
    Dim i As Long
    Dim j As Long
    Dim customer As String
    Dim counter As Long
    Dim size As Long
    Dim sortedBids As Variant
    counter = 1

    Dim lastRow As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    Dim lastColumn As Long
    lastColumn = Cells(1, Columns.Count).End(xlToLeft).Column
    size = WorksheetFunction.Count(Range(Cells(2, 2), Cells(lastRow, lastColumn)))

    Dim arrayOfBids As Variant
    ReDim arrayOfBids(1 To size, 1 To 2)
    For i = 2 To lastRow
        customer = Cells(i, 1)
        For j = 2 To lastColumn
            If Not IsEmpty(Cells(i, j)) Then
            arrayOfBids(counter, 1) = Cells(i, j)
            arrayOfBids(counter, 2) = customer
            counter = counter + 1
            End If
        Next
    Next

    sortedBids = BubbleSort(arrayOfBids)
    Range(Cells(1, 6), Cells(size, 7)) = sortedBids

End Sub

Private Function BubbleSort(ByVal arrayOfBids As Variant) As Variant
    Dim temporaryArray As Variant
    Dim i As Integer
    Dim exchangeMade As Boolean
    ReDim temporaryArray(1 To 1, 1 To 2)

    Do
        exchangeMade = True
        For i = 1 To UBound(arrayOfBids) - 1
            If arrayOfBids(i, 1) > arrayOfBids(i + 1, 1) Then
                exchangeMade = False
                temporaryArray(1, 1) = arrayOfBids(i, 1)
                temporaryArray(1, 2) = arrayOfBids(i, 2)
                arrayOfBids(i, 1) = arrayOfBids(i + 1, 1)
                arrayOfBids(i, 2) = arrayOfBids(i + 1, 2)
                arrayOfBids(i + 1, 1) = temporaryArray(1, 1)
                arrayOfBids(i + 1, 2) = temporaryArray(1, 2)
            End If
        Next i
    Loop While Not (exchangeMade)

    BubbleSort = arrayOfBids
End Function

Overall, pretty good code.  There are a couple things I would personally change though.

1 - I'd start by qualifying the references to Cells and Range even if it's assumed that everything is done with the same sheet - I'd grab a reference to ActiveSheet right at the start and use that throughout. Using a Worksheet reference inside a With block is much faster than using the global objects. Outside of the performance, it's generally a good habit to get into as it avoids using an implicit reference.

2 - The call to calculate the size of the array is a bit of overkill - there really isn't a need for a function call with:

size = WorksheetFunction.Count(Range(Cells(2, 2), Cells(lastRow, lastColumn)))

You already have all the information you need to calculate it with a simple multiplication:

size = (lastRow - 1) * (lastColumn - 1)

Excel is almost certainly doing more work to count it, because a Range object doesn't have to be rectangular.

3 - You write your array by setting the .Value of the target Range to the array, but your reads are still done inside of a loop.  Not only that, but you read the same value (Cells(i, j)) from the worksheet twice here:

If Not IsEmpty(Cells(i, j)) Then
arrayOfBids(counter, 1) = Cells(i, j)

Remember, Cells(r, c) is a function call (2 actually - see #1) so you should be caching the result if the return value isn't going to change between the calls. I'd read the source Range into an array also and then iterate over that.

4 - While I'm on that section of code, the block enclosed by...

If Not IsEmpty(Cells(i, j)) Then
'...
End If

...should be indented another level.

5 - IsEmpty probably isn't the best choice (see this review) - you really just need to test that the cell isn't equal to vbNullString. Or if a zero bid is invalid, test that the cell isn't equal to 0.

6 - temporaryArray is basically being used as a tuple to store the swap values in your BubbleSort implementation.  However, when you store them in an array you carry a bunch of SafeArray overhead along with it, especially since it's curiously a two dimensional array with the first dimension fixed at 1 element. A one dimensional array would be better (the runtime performs a multiply internally every time you address a cell in a 2D array). Even better would be to create a simple type to store them in:

Private Type Tuple
    ItemOne As Variant
    ItemTwo As Variant
End Type

7 - One small advantage that a bubble sort has is that it sorts the array in place.  But..., your implementation takes the array ByVal and then just returns a copy of the sorted array when it's done. This is a huge performance hit that negates the advantage of an in-place sort. Think of it in terms of memory copies.  The call sortedBids = BubbleSort(arrayOfBids) creates a copy of sortedBids in memory and then passes it to BubbleSort. The BubbleSort function then sorts the copy in place. But the copy is just a locally scoped variable, so finally you call BubbleSort = arrayOfBids which copies it again and puts it on the stack as the return value.  Take advantage of possible the sole benefit of your algorithm and convert BubbleSort to a Sub that takes the array ByRef.

8 - Your counter i should be declared as a Long in BubbleSort.  I'd actually consider this a bug, because the variables you use to size the array you pass it are Long. Speaking of which, you also need overflow protection in GetBids(). WorksheetFunction.Count returns a Double which it has to because the total number of possible cells in a Range is 17,179,869,184 (the maximum possible return value in your code is 17,178,804,225). This will easily overflow a Long.  Granted, you won't be able to write the results if it overflows (and I shudder to think of the execution time for a bubble sort with 1,048,575 elements), but a graceful exit would be nicer than an unhandled exception.

9 - The return value of UBound(arrayOfBids) will never change inside your Do loop in BubbleSort. This value should also be cached outside of the loop - there isn't a need for repeated function calls to determine what is essentially a static value.

10 - You should swap the order of your data collection loops. Based on the sample data, the values on each row should always be increasing from left to right. This indicates that the final placement of an element in the sorted output is more strongly correlated by which column it's in instead of which row it's in. But when you're building the arrayOfBids that is eventually going to be sorted, you're building it in row order then column order.  While this doesn't make as much of a difference with a bubble sort as it would with other sort algorithms, you are basically ensuring that your input to the sort will be closer to the worst case than it will to the best case.  If you switch the loop order to this...

For j = 2 To lastColumn
    For i = 2 To lastRow
    Next
Next

...you are using this characteristic of your typical data set to make sure your sort input is closer to best case.


Cyclic "dependency detection" in JavaScript I have written some simple code to detect cyclic dependencies for a module loading system and I'd like some feedback on how I can go about improving it. The code itself finds the first cycle in the dependency graph that starts at the provided node and returns a list of all of the nodes traversed along the way.

Here's an example:

The white node is the one we are starting the traversal at. The JS representation of this graph would look like this:

{
  'A': ['B', 'C'],
  'B': [],
  'C': ['D'],
  'D': ['A']
}

The output from my function is ['A', 'C', 'D', 'A'], which is a list of the nodes traversed before finding a cycle.

Here is the code I have written to generate the array:

function findCyclicDependencies(definitions, identifier) {
  var stack = [];

  // Internal search function.
  var internalSearch = function(currentIdentifier) {

    // If we have visited this node, return whether or not it is the one we
    // are looking for.
    if (stack.indexOf(currentIdentifier) !== -1) {
      return currentIdentifier === identifier;
    }

    stack.push(currentIdentifier);

    // Check all of the child nodes to see if they contain the node we are
    // looking for.
    var found = definitions[currentIdentifier].some(internalSearch);

    // Remove the current node from the stack if it's children do not
    // contain the node we are looking for.
    if (!found) {
      stack.splice(stack.indexOf(currentIdentifier), 1);
    }

    return found;
  };

  // If there isn't a cyclic dependency then we return an empty array,
  // otherwise we return the stack.
  return internalSearch(identifier) ? stack.concat(identifier) : [];
}

And some not-so great code for doing the tests:

[
  {
    name: 'Non-cyclic',
    definitions: {
      'A': ['B', 'C'],
      'B': [],
      'C': ['D'],
      'D': []
    },
    identifier: 'A',
    expected: []
  },
  {
    name: 'Cyclic',
    definitions: {
      'A': ['B', 'C'],
      'B': [],
      'C': ['D'],
      'D': ['A']
    },
    identifier: 'A',
    expected: ['A', 'C', 'D', 'A']
  }
].forEach(function(test) {
  var result = findCyclicDependencies(test.definitions, test.identifier),
      passed = true;

  if (test.expected.length === result.length) {
    for (var i = 0; i < test.expected.length; i++) {
      if (test.expected[i] !== result[i]) {
        passed = false;
        break;
      }
    }
  } else {
    passed = false;
  }

  if (passed) {
    console.log(test.name + ': pass');
  } else {
    var message = test.name + ': fail\n' +
                  '  Got: ' + JSON.stringify(result) + '\n' +
                  '  Expected: ' + JSON.stringify(test.expected);

    console.warn(message);
  }
});

I'm not all that bothered about the quality of the test code as it's mainly for demonstration purposes to show my function works.

One thing I'm not sure about with my code (that I'm not even sure is in the scope of Code Review):

Is there any case where found is false but the stack doesn't contain currentIdentifier (i.e. stack.indexOf(currentIdentifier) === -1? I'm wondering if I need to check whether or not currentIdentifier is in the stack before trying to remove it, as trying to remove something not present ends up with the last element being removed erroneously (indexOf returns -1 and splice(-1, 1) removes the last element from the array).

HashMap of visited nodes is more efficient than array lookup

First of I'd like to acknowledge that I think you've done a fantastic job so far. Your code is thought out really well and from my tests appears to be rock solid. My biggest concern is that using Array.indexOf() isn't as efficient as storing a map of visited nodes:

var visited= {};

if (visited[node]) {
  return;
}
visited[node] = true;
definitions[node].some(internalSearch);

Not only does the map have O(1) lookup--which is nice--but it also would prevent some redundant searches. For example, take a look at the following case:

definitions = {
  'A': ['B', 'C'],
  'B': ['D'],
  'C': ['D'],
  'D': []
}

Here's a JSFiddle that shows how node D is checked twice. That doesn't seem like a lot, but the more the dependency tree's branches weave the more redundancy that's introduced:

definitions = {
  'A': ['B', 'C'],
  'B': ['D'],
  'C': ['D'],
  'D': ['E', 'F']
  'E': ['G']
  'F': ['G']
  'G': ['H', 'I'],
  'H': ['J'],
  'I': ['J'],
  'J': ['K', 'L'],
  'K': ['M'],
  'L': ['M'],
  'M': []
}

Here's the JSFiddle for this case. Note how M is checked 16 times!

Of course, your module loader may never run into such test cases, but IMO, the time it would take to implement the map is well worth the increase in your algorithm's efficiency.

Redundant comments

I'm not sure if you're just trying to help explain what your code does for code review, but comments like:

// Internal search function.
var internalSearch = function(currentIdentifier) {

don't really contribute to understanding; the code is self explanatory. Your code could be much shorter without the comments, so consider which of your comments actually adds value to the code. Anything else may just be clutter.

No faulty case

As of your concern, I'm pretty confident that currentIdentifier, will always be in the stack when you call for it to be removed. My reasoning is as follows:

Proof - First, you check to see if a node, A is on the stack:

A is on the stack:

return, the stack is unaffected

A is not yet on the stack:

A is pushed onto the stack [ ..., A]
Call internalSearch on each child, C, consider the three cases of children:

If C === A then C is found on the stack because A is on the stack:

return, the stack is unaffected

If C !== A and C is not found the stack:

Add C to the stack `[ ..., A, C, ... ]
Call internalSearch on each of C's children
Remove C from the stack [ ..., `A, ... ]
return

If C !== A and is found on the stack:

return, the stack is unaffected

Remove A from the stack [ ... ]

The only way for the stack to have the currentIdentifier removed would be if one of it's decedents were equal to it, but because you check the stack at the top of each call to internalSearch, you never deal with a duplicate node, so this will never happen.


When an object has different representations... what's the OO pattern? I've an AbstractMessage object (a hierarchy, actually). It represents a small text message and can be sent using different transport methods: HTTP, REST and a "mailer" transport. Each transport relies on an external library for executing the transport itself, injected using a DI container.

Message itself may have different representations (query string, resource string or an instance of\Swift_Message), based on the transport used. A transport should use the more appropriate representation, again injected using constructor injection.

interface TransportInterface
{
    public function executeTransport(AbstractMessage $message, array &$fails = array());
}

class HttpTransport implements TransportInterface
{
    /**
     * @var \Guzzle\Service\Client
     */
    private $client;

    /**
     * @var Converter\MessageConverterInterface
     */
    private $converter;

    public function __construct(Client $client, MessageConverterInterface $converter)
    {
        $this->client    = $client;
        $this->converter = $converter;
    }

    public function executeTransport(AbstractMessage $message, array &$fails = array())
    {
        $representation = $this->converter->convert($message);

        /* ... */
    }
}

class RestTransport implements TransportInterface
{
    /**
     * @var \Guzzle\Service\Client
     */
    private $client;

    /**
     * @var Converter\MessageConverterInterface
     */
    private $converter;

    public function __construct(Client $client, MessageConverterInterface $converter)
    {
        $this->client    = $client;
        $this->converter = $converter;
    }

    public function executeTransport(AbstractMessage $message, array &$fails = array())
    {
        $representation = $this->converter->convert($message);

        /* ... */
    }
}

class MailerTransport implements TransportInterface
{
    /**
     * @var \Swift_Mailer
     */
    private $mailer;

    /**
     * @var Converter\MessageConverterInterface
     */
    private $converter;

    public function __construct(Swift_Mailer $mailer,
        MessageConverterInterface $converter)
    {
        $this->mailer    = $mailer;
        $this->converter = $converter;
    }

    public function executeTransport(AbstractMessage $message, array &$fails = array())
    {
        $representation = $this->converter->convert($message);

        /* ... */
    }
}

The MessageConverterInterface and the actual helper may be fairly simple:

interface MessageConverterInterface
{
    /**
     * @param AbstractMessage $message
     * @return mixed
     */
    public function convert(AbstractMessage $message);
}

class MessageHelper
{
    /**
     * @var Transport\TransportInterface
     */
    private $mailer;

    public function send(AbstractMessage $message, array &$fails = array())
    {
        $this->transport->executeTransport($message, $fails);
    }

}

Question: is this a good OO pattern for the library design? Does the pattern have a name?

I would use an abstract class instead of, or in addition to, an interface. You have quite a bit of repetition that could be removed by doing so. The only problem then would be that you would have to use protected instead of private properties and methods, at least for those that you need to share between the abstract class and inheriting class, but I don't think that should make too much of a difference. Remember: You can't create an instance of an abstract class, so you have to use some other "initiator" and call that in each constructor.

abstract class AbstractTransport {

    protected
        /** @var \Guzzle\Service\Client */
        $client,

        /** @var \Swift_Mailer */
        $mailer,

        /** @var Converter\MessageConverterInterface */
        $converter
    ;

    protected function initClient( Client $client, MessageConverterInterface $converter ) {
        $this->client = $client;
        $this->converter = $converter;
    }

    protected function initMailer( Swift_Mailer $mailer, MessageConverterInterface $converter ) {
        $this->mailer = $mailer;
        $this->converter = $converter;
    }

    public function executeTransport( AbstractMessage $message, array &$fails = array() ) {
        $representation = $this->converter->convert( $message );
    }
}

You might also want to be careful with referencing. It can sometimes be hard to determine where a variable was changed if you don't know that it was assigned by reference somewhere. This is entirely your choice, just a suggestion. I would, however, like to point out that if this parameter is referenced, then setting a default value to it is kind of pointless. If the parameter has not been set, then that value can't be used and should therefore not be used at all to avoid the overhead.

I've never heard of, nor could find a library design pattern, so I can't be sure. As for the name of the pattern you are using? I honestly don't know. But it doesn't matter. Functional code is much better than any code that adheres to a design pattern just for the sake of doing so. Many times I see programmers attempting to force their code into a design pattern, and some times its not even the right pattern. Design patterns are not necessary at this stage. Code for functionality. Once you are comfortable with OOP, then you can start worrying about design patterns and how and when to apply them.

Now, as to whether this is good OOP or not; It appears to be fine, however, this code is incomplete, so it is hard to tell. The concepts seem fine, but the implementation is unsure. You are using encapsulation, inheritance, and a sort of polymorphism. Those are some of the main points of OOP, and you seem to grasp them fairly well, though you could probably stand to look into polymorphism a bit more. You are also using interfaces and dependency injection. These are also important concepts. A couple more points that might help is to ensure you are following the Single Responsibility Principle (can't be sure) and the "Don't Repeat Yourself" (DRY) principle. As I pointed out above, you could use some help with DRY.

Finally, your code samples could have easily been reduced to a single interface and class with a note that the other related classes only differed in the commented area. The other classes, except that last, are redundant, whereas that last is unnecessary in this context. If you would have submitted complete classes, this would not be the case. The second interface was slightly relevant, but was also unnecessary.

I hope this helps!

EDIT

Example implementation of above code:

class RestTransport extends AbstractTransport {
    public function __construct( $client, $converter ) {
        $this->initClient( $client, $converter );
    }
}

Per comment, here's how to do it with separate classes for each type:

//only worth it if both client and mailer implementations share more than this
abstract class AbstractTransport implements TransportInterface {
    public function executeTransport( AbstractMessage $message, array &$fails = array() ) {
        $representation = $this->converter->convert( $message );
    }
}

class Client extends AbstractTransport {
    public function __construct( Client $client, MessageConverterInterface $converter ) {
        $this->client = $client;
        $this->converter = $converter;
    }

    public function executeTransport( AbstractMessage $message, array &$fails = array() ) {
        parent::executeTransport( $message, $fails );

        /*
        assuming there will be shared aspects unique to clients
        you can use polymorphism to further extend method
        same in the mailer
        otherwise it is unnecessary to redefine here
        */
    }
}

class RestTransport extends Client {
    public function executeTransport( AbstractMessage $message, array &$fails = array() ) {
        parent::executeTransport( $message, $fails );

        /* ... */
    }
}

And you'd do pretty much the same thing with the mailer, and any of its classes. The problem with this implementation is that the parent classes, in this case "Client", can be instantiated, and by itself it doesn't do anything. This is why I was originally suggesting the abstract class.


Update any controls' Text for the specified language I would like to optimize the below code The only difference is the datatypes RadioButton, Label, and Button. Outside the method I have an loop that iterates over all the controls in the aspx-page. I have the limitation of .NET 3.5. The code is intended to set language relevant strings on the controls of a certain type in a aspx-page without having to name each control.

using System.Web.UI.WebControls;
using System.Web.UI.WebControls.WebParts;
...
public partial class MyUserControl : UserControl
...

    private static void SetTextOnControls(MyLanguageTexts keys, ControlCollection theControls)
    {
        foreach (Control control in theControls)
        {
            SetTextOnControl(keys, control);
        }
    }

    private static void SetTextOnControl(MyLanguageTexts keys, Control control)
    {

        if (control is RadioButton)
        {
            try
            {
                (control as RadioButton).Text = SPContext.Current.Web.Locale.LCID == 1033 ?
                    DataBinder.Eval(keys.en, control.ID).ToString() :
                    DataBinder.Eval(keys.sv, control.ID).ToString();
            }
            catch (Exception)
            {
                (control as RadioButton).Text = "Key not found: " + control.ID;
            }
        }
        else if (control is Label)
        {
            try
            {
                (control as Label).Text = SPContext.Current.Web.Locale.LCID == 1033 ?
                DataBinder.Eval(keys.en, control.ID).ToString() :
                DataBinder.Eval(keys.sv, control.ID).ToString();
            }
            catch (Exception)
            {
                (control as Label).Text = "Key not found: " + control.ID;
            }
        }
        else if (control is Button)
        {
            try
            {
                (control as Button).Text = SPContext.Current.Web.Locale.LCID == 1033 ?
                DataBinder.Eval(keys.en, control.ID).ToString() :
                DataBinder.Eval(keys.sv, control.ID).ToString();
            }
            catch (Exception)
            {
                (control as Button).Text = "Key not found: " + control.ID;
            }
        }

        SetTextOnControls(keys, control.Controls);
    }

Try using the dynamic keyword (.NET 4.5). Since you just updated the question with information that you are limited to 3.5, I suggest using reflection to get the Text property of the control.

var textAwareControl = (dynamic)control;
try
{
    textAwareControl.Text = SPContext.Current.Web.Locale.LCID == 1033 ?
    DataBinder.Eval(keys.en, control.ID).ToString() :
    DataBinder.Eval(keys.sv, control.ID).ToString();
}
catch (RuntimeBinderException) {
    // throw;
    // throw, or ignore, or make sure to only dispatch compatible controls to this method
}
catch (Exception)
{
    textAwareControl.Text = "Key not found: " + control.ID;
}


Setting instance data attributes to None in __init__ In Python 3.7, I frequently find myself writing something like this in __init__:
    self.foo: Optional[Foo] = None

For example:
window = Window()
# do some more setup here before going to user
path = get_path_from_user()
window.build(path)

class Window:
    def __init__(self):
        self.image: Optional[Image] = None

    def build(self, path):
        self.image = Image(path)

I justify this to myself as:

Wishing to keep __init__ light (for maximum reuseabilty) or
Being unable to complete the instance when __init__ executes.

However, I typically don't use the None value for any real meaning in the code. In this example, suppose a Window is meaningless without an image. It's just a placeholder until build is called so...
Is this an anti-pattern when doing so for either of the two reasons?
If so, what is a better way to do it?
(Your opinion is what I'm seeking)
Simply removing the line from __init__ flags a code inspection warning in PyCharm: "Instance Attribute instantiated outside __init__". This seems reasonable to me as it ensures I'm able to look in __init__ and know I'm always seeing all the data attributes.

According to PEP 526, the initializer isn't needed.  Have you tried:
class Window:
    def __init__(self):
        self.image: Optional[Image]    # <== this is just a type hint, no initialization

    def build(self, path):
        self.image = Image(path)

I don't use PyCharm, so I don't know if it would still complain.


Geeks for Geeks: Trapping Rain Water Problem - time limit exceeded Problem Statement:
Given an array arr[] of N non-negative integers representing the height of blocks. If width of each block is 1, compute how much water can be trapped between the blocks during the rainy season.

Input:
N = 6
arr[] = {3,0,0,2,0,4}

Output:
10

For complete description, please go to this link.

My Code:
// { Driver Code Starts
#include<bits/stdc++.h>

using namespace std;

 // } Driver Code Ends

// function to find the trapped water in between buildings
// arr: input array
// n: size of array
int maxValueInArray(int arr[], int start, int end){
    if(start == end) return arr[start];
    int mx = 0;
    for(int i = start; i<=end; i++){
        mx = max(arr[i], mx);
    }
    return mx;

}
int trappingWater(int arr[], int n){

    // Your code here
    int ans = 0;
    for(int i = 1; i<n-1; i++){
        int curr = arr[i];
        int maxLeft = maxValueInArray(arr,0, i-1);
        int maxRight = maxValueInArray(arr, i+1, n-1);
        if(maxLeft == curr || maxRight == curr) ans += 0; // 8, 8, 3
        else if(maxLeft >= curr && curr >= maxRight) ans += 0; //8, 5, 3
        else if(maxLeft <= curr && curr <= maxRight) ans+= 0; //2, 4, 7
        else if(maxLeft < curr && curr > maxRight) ans+= 0; //5, 7, 6
        else ans += min(maxLeft, maxRight) - arr[i];
    }
    return ans;
}

// { Driver Code Starts.

int main(){

    int t;
    //testcases
    cin >> t;

    while(t--){
        int n;

        //size of array
        cin >> n;

        int a[n];

        //adding elements to the array
        for(int i =0;i<n;i++){
            cin >> a[i];
        }

        //calling trappingWater() function
        cout << trappingWater(a, n) << endl;

    }

    return 0;
}  // } Driver Code Ends

Issue:

I'm unable to understand why I'm getting a time exceeded error in this code. Can somebody tell what's going wrong?

Your current solution is \$ O(n^2) \$ because of the nested loops.
There are a couple ways to solve this in \$ O(n) \$ time without having to use any extra arrays. One way is to go from left-to-right while keeping track of the highest wall you’ve seen so far, then do the same thing from right-to-left until you reach the last highest wall you saw when going left-to-right. It’s \$ O(n) \$, but you‘ll need to look at almost every array index twice if the wall at index 0 is the highest.
The other way is to alternate between going from left-to-right or right-to-left depending on which side had the last highest wall seen. If you do it like this then you’ll always only need to look at each index one time. You can write the function like this:
int trappingWater(int arr[], int arr_size)
{
    if (arr_size <= 0)
    {
        return 0;
    }

    int water = 0;

    int max_left = arr[0];
    int max_right = arr[arr_size - 1];

    int left_ind = 0;
    int right_ind = arr_size - 1;

    while (left_ind < right_ind - 1)
    {
        if (max_left <= max_right)
        {
            left_ind += 1;
            if (arr[left_ind] > max_left)
            {
                max_left = arr[left_ind];
            }
            else
            {
                water += max_left - arr[left_ind];
            }
        }
        else
        {
            right_ind -= 1;
            if (arr[right_ind] > max_right)
            {
                max_right = arr[right_ind];
            }
            else
            {
                water += max_right - arr[right_ind];
            }
        }
    }
    return water;
}

For your current code, it would be better if you avoided using namespace std; to avoid possible name conflicts: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice. You could also use better variable names in some places. There are some spots where you explain what the variable is in a comment right after making the variable. Instead of doing that, you could just give the variable a more complete name. Then you don’t have to check the comment if you forget what it is. Here’s what your current code would look like with those two things in mind:
int maxValueInArray(int arr[], int start, int end){
    if(start == end) return arr[start];
    int maxHeight = 0;
    for(int i = start; i<=end; i++){
        maxHeight = std::max(arr[i], maxHeight);
    }
    return maxHeight;

}

int trappingWater(int arr[], int arrSize){

    // Your code here
    int ans = 0;
    for(int i = 1; i<arrSize-1; i++){
        int currentWall = arr[i];
        int maxLeft = maxValueInArray(arr,0, i-1);
        int maxRight = maxValueInArray(arr, i+1, arrSize-1);
        if(maxLeft == currentWall || maxRight == currentWall) ans += 0; // 8, 8, 3
        else if(maxLeft >= currentWall && currentWall >= maxRight) ans += 0; //8, 5, 3
        else if(maxLeft <= currentWall && currentWall <= maxRight) ans+= 0; //2, 4, 7
        else if(maxLeft < currentWall && currentWall > maxRight) ans+= 0; //5, 7, 6
        else ans += std::min(maxLeft, maxRight) - arr[i];
    }
    return ans;
}

// { Driver Code Starts.

int main(){

    int testCases;
    std::cin >> testCases;

    while(testCases--){
        int arrSize;

        std::cin >> arrSize;

        int arr[arrSize];

        //adding elements to the array
        for(int i =0; i<arrSize; i++){
            std::cin >> arr[i];
        }

        //calling trappingWater() function
        std::cout << trappingWater(arr, arrSize) << std::endl;

    }

    return 0;
}  // } Driver Code Ends


Binary Search Tree in Ruby Binary Search Tree, with preorder traverse. What other methods should I add? How can I improve it?

class Bst

  attr_accessor :data, :left, :right

  def initialize(data)
    self.data = data
  end

  def insert(data)
    if  self.data < data
      if (self.right )
        self.right.insert(data)
      else
        self.right = Bst.new(data)
      end
    else
      if (self.left )
        self.left.insert(data)
      else
        self.left = Bst.new(data)
      end
    end
  end

  def each(&blk)
    Bst.traverse_preorder(self,&blk)
  end

  def self.traverse_preorder(node, &blk)
    return unless node
    traverse_preorder(node.left,&blk)
    blk.call(node.data)
    traverse_preorder(node.right,&blk)
  end

end

Note: Each tip assumes you followed all of the ones before.

There are a bunch of trailing whitespace characters. Get rid of 'em. They're ugly.

Bst is a terrible class name. It means nothing out-of-context, and out-of-context is where it will be. I'd recommend renaming to BinarySearchTree.

In the if statements, there's sometimes an extra space before the close-paren but not after the open; I've only ever seen both or neither. Personally, I would just get rid of the parentheses entirely -- they're unnecessary.

Prefer if obj.nil? over unless obj. It makes it much clearer that it's checking the existence of an object, rather than, say, the result of a method called obj.

Prefer @data to self.data. It functions the same but it's far more standard to directly access instance variables, unless accessing it requires some guard clauses or the like. Ditto for self.right and self.left. The exception is when you're accessing the class from outside, in which case you obviously don't have direct access to instance variables.

Put a space after (but not before) commas -- so traverse_preorder(node.left,&blk) becomes traverse_preorder(node.left, &blk).

In that method, I'd recommend changing blk to block, because it makes no sense to shorten it.

With these changes, here's what your code looks like:

class BinarySearchTree
  attr_accessor :data, :left, :right

  def initialize(data)
    @data = data
  end

  def insert(data)
    if @data < data
      if @right.nil?
        @right = BinarySearchTree.new(data)
      else
        @right.insert(data)
      end
    else
      if @left.nil?
        @left = BinarySearchTree.new(data)
      else
        @left.insert(data)
      end
    end
  end

  def each(&block)
    BinarySearchTree.traverse_preorder(&block)
  end

  def self.traverse_preorder(node, &block)
    node.left.traverse_preorder(&block)
    blk.call(node.data)
    node.right.traverse_preorder(&block)
  end
end