Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tranjy cr1 #3

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open

Tranjy cr1 #3

wants to merge 3 commits into from

Conversation

jtran549
Copy link
Collaborator

Analysis
This application is used to document time spent on different projects with different clients. The client side page features input options for a Client, a Project, and the time spent on said project. Along with the inputs, it appears there is a display feature that is meant to display a list of projects and time spent on each project.

Was the program available in UC Github on time?
Yes

Did the program compile?
Yes

Is the program easy for you, as an outsider, to understand?
Yes, as far as navigating the program, it was incredibly easy and well labeled. I had no confusion as to what was what. I only had a minor gripe with the styling but it appeared to be fixed after I fixed the css referencing

CSS explanation
I wasn't sure how much text I could write on the commit for this change so I'm going to explain the change more in depth here. The reference to the css file wasn't working because you were still in the directory for the html files, and the css file was in a different directory so the reference wouldn't work.
image
Notice how if you want to get from the index.html file to the style.css file, you'd have to navigate to the templates directory, then out into into the resources directory, and then you are able to specify the path to the css file. In the change I made, i had "../" twice before I specified the path to the css file, which just tells the program to move one directory up

Three technical concepts learned

  • You can use flex in CSS to make elements stay relative to each other when the browser window is adjusted
    image
    image

  • You can use a static keyword followed by brackets to specify a function that runs only once when the class is called
    image

  • You can tie a thyme leaf attribute to a backend function in the controller
    image

…ages

The reasoning for this is just for easier organization and separation of different java files
Created an interface for the service doc. The reason for this is because the professor recommends using interfaces and inheritance for the service classes, for the interface itself I only specified a Get All method
I went ahead and fixed the stylesheed reference in the head of all the html files, the way you were doing it before, the css file wasn't getting referenced correctly.
@@ -1,4 +1,4 @@
package com.timelogr.enterprise;
package com.timelogr.enterprise.dto;

public class client {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as you are in here, make this UpperCamelCase as well. Should be Client, not client.

@@ -1,4 +1,4 @@
package com.timelogr.enterprise;
package com.timelogr.enterprise.dto;

public class employee {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto above. Make this Employee, not employee.

@@ -1,14 +1,13 @@
package com.timelogr.enterprise;
package com.timelogr.enterprise.service;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to put this in a Service package.

@discospiff
Copy link

Several good suggestions here that reduce technical debt. I recommend merging these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants